Bug 126856 - TextBreakIterator's should support Latin-1 for all iterator types
Summary: TextBreakIterator's should support Latin-1 for all iterator types
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Text (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Sam Weinig
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-01-12 15:24 PST by Sam Weinig
Modified: 2014-01-21 23:38 PST (History)
11 users (show)

See Also:


Attachments
Part 1 (45.97 KB, patch)
2014-01-12 15:36 PST, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (70.60 KB, patch)
2014-01-13 10:53 PST, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (73.10 KB, patch)
2014-01-14 21:00 PST, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (38.14 KB, patch)
2014-01-15 19:45 PST, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (23.00 KB, patch)
2014-01-15 21:21 PST, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (22.97 KB, patch)
2014-01-15 21:58 PST, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (23.63 KB, patch)
2014-01-16 21:30 PST, Sam Weinig
rniwa: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Weinig 2014-01-12 15:24:30 PST
TextBreakIterator's should support Latin-1 for all iterator types so we don't need to convert to UTF-16 unnecessarily.
Comment 1 Sam Weinig 2014-01-12 15:36:35 PST
Created attachment 220987 [details]
Part 1
Comment 2 Darin Adler 2014-01-12 15:45:32 PST
Comment on attachment 220987 [details]
Part 1

View in context: https://bugs.webkit.org/attachment.cgi?id=220987&action=review

> Source/WebCore/platform/text/TextBreakIteratorICU.cpp:61
> +static TextBreakIterator* setTextForIterator(TextBreakIterator* iterator, StringView string)

Should this take a reference instead of a pointer?

> Source/WebCore/platform/text/TextBreakIteratorICU.cpp:77
> +        ubrk_setUText(reinterpret_cast<UBreakIterator*>(iterator), text, &setTextStatus);

I’d like to see a helper function so we have only one reinterpret_cast instead of many. Or maybe just transition to use UBreakIterator directly since we don’t need a non-ICU abstraction for this.

> Source/WebCore/platform/text/TextBreakIteratorICU.cpp:143
> +TextBreakIterator* wordBreakIterator(const UChar* buffer, int length)

Why not make this take a StringView too?

> Source/WebCore/platform/text/TextBreakIteratorICU.cpp:152
> +TextBreakIterator* sentenceBreakIterator(const UChar* buffer, int length)

And this.

> Source/WebCore/platform/text/TextBreakIteratorICU.cpp:161
> +TextBreakIterator* cursorMovementIterator(const UChar* buffer, int length)

And this.

> Source/WebCore/platform/text/TextBreakIteratorICU.cpp:267
> +void releaseLineBreakIterator(TextBreakIterator* iterator)

Reference?

> Source/WebCore/platform/text/TextBreakIteratorICU.cpp:356
> +// Iterator implemenation.
> +
> +int textBreakFirst(TextBreakIterator* iterator)
> +{
> +    return ubrk_first(reinterpret_cast<UBreakIterator*>(iterator));
> +}
> +
> +int textBreakLast(TextBreakIterator* iterator)
> +{
> +    return ubrk_last(reinterpret_cast<UBreakIterator*>(iterator));
> +}
> +
> +int textBreakNext(TextBreakIterator* iterator)
> +{
> +    return ubrk_next(reinterpret_cast<UBreakIterator*>(iterator));
> +}
> +
> +int textBreakPrevious(TextBreakIterator* iterator)
> +{
> +    return ubrk_previous(reinterpret_cast<UBreakIterator*>(iterator));
> +}
> +
> +int textBreakPreceding(TextBreakIterator* iterator, int pos)
> +{
> +    return ubrk_preceding(reinterpret_cast<UBreakIterator*>(iterator), pos);
> +}
> +
> +int textBreakFollowing(TextBreakIterator* iterator, int pos)
> +{
> +    return ubrk_following(reinterpret_cast<UBreakIterator*>(iterator), pos);
> +}
> +
> +int textBreakCurrent(TextBreakIterator* iterator)
> +{
> +    return ubrk_current(reinterpret_cast<UBreakIterator*>(iterator));
> +}
> +
> +bool isTextBreak(TextBreakIterator* iterator, int position)
> +{
> +    return ubrk_isBoundary(reinterpret_cast<UBreakIterator*>(iterator), position);
> +}
> +
> +bool isWordTextBreak(TextBreakIterator* iterator)
> +{
> +    int ruleStatus = ubrk_getRuleStatus(reinterpret_cast<UBreakIterator*>(iterator));
> +    return ruleStatus != UBRK_WORD_NONE;
>  }

This should all just go. It’s a wrapper that we don’t need.

> Source/WebCore/platform/text/icu/UTextProviderLatin1.cpp:46
> +    0, 0, 0,

Some nullptr maybe?

> Source/WebCore/platform/text/icu/UTextProviderLatin1.cpp:52
> +    0,
> +    0,

Some nullptr maybe?

> Source/WebCore/platform/text/icu/UTextProviderLatin1.cpp:56
> +    0, 0, 0

Some nullptr maybe?

> Source/WebCore/platform/text/icu/UTextProviderLatin1.cpp:64
> +        return 0;

nullptr

> Source/WebCore/platform/text/icu/UTextProviderLatin1.cpp:72
> +    /* Point at the same position, but with an empty buffer */

Maybe // instead of /* comment? In this whole file perhaps.

> Source/WebCore/platform/text/icu/UTextProviderLatin1.cpp:98
> +            uText->chunkOffset = (int32_t)(index - uText->chunkNativeStart);

static_cast?

> Source/WebCore/platform/text/icu/UTextProviderLatin1.cpp:109
> +            uText->chunkOffset = (int32_t)(index - uText->chunkNativeStart);

static_cast?

> Source/WebCore/platform/text/icu/UTextProviderLatin1.cpp:206
> +    uText->context = 0;

nullptr?

> Source/WebCore/platform/text/icu/UTextProviderLatin1.cpp:224
> +    text->a = (int64_t)length;

static_cast? no cast at all?

> Source/WebCore/platform/text/icu/UTextProviderLatin1.h:42
> +UText* openLatin1UTextProvider(UTextWithBuffer* utWithBuffer, const LChar* string, unsigned length, UErrorCode* status);
> +UText* openLatin1ContextAwareUTextProvider(UTextWithBuffer* utWithBuffer, const LChar* string, unsigned length, const UChar* priorContext, int priorContextLength, UErrorCode* status);

utWithBuffer and status argument names should be omitted.

> Source/WebCore/platform/text/icu/UTextProviderUTF16.cpp:43
> +    0, 0, 0,

nullptr?

> Source/WebCore/platform/text/icu/UTextProviderUTF16.cpp:48
> +    0, 0, 0, 0,

nullptr?

> Source/WebCore/platform/text/icu/UTextProviderUTF16.cpp:50
> +    0, 0, 0,

nullptr?
Comment 3 Sam Weinig 2014-01-12 16:42:11 PST
> > Source/WebCore/platform/text/TextBreakIteratorICU.cpp:77
> > +        ubrk_setUText(reinterpret_cast<UBreakIterator*>(iterator), text, &setTextStatus);
> 
> I’d like to see a helper function so we have only one reinterpret_cast instead of many. Or maybe just transition to use UBreakIterator directly since we don’t need a non-ICU abstraction for this.

I'll do that in a follow up.

> > Source/WebCore/platform/text/TextBreakIteratorICU.cpp:143
> > +TextBreakIterator* wordBreakIterator(const UChar* buffer, int length)
> 
> Why not make this take a StringView too?
> 
> > Source/WebCore/platform/text/TextBreakIteratorICU.cpp:152
> > +TextBreakIterator* sentenceBreakIterator(const UChar* buffer, int length)
> 
> And this.
> 
> > Source/WebCore/platform/text/TextBreakIteratorICU.cpp:161
> > +TextBreakIterator* cursorMovementIterator(const UChar* buffer, int length)
> 
> And this.

That is step 2.
Comment 4 Sam Weinig 2014-01-12 17:24:44 PST
Committed r161844: <http://trac.webkit.org/changeset/161844>
Comment 5 Sam Weinig 2014-01-13 10:53:54 PST
Created attachment 221063 [details]
Patch
Comment 6 WebKit Commit Bot 2014-01-13 10:56:23 PST
Attachment 221063 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/PlatformGTK.cmake', u'Source/WebCore/WebCore.vcxproj/WebCore.vcxproj', u'Source/WebCore/WebCore.vcxproj/WebCore.vcxproj.filters', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/dom/CharacterData.cpp', u'Source/WebCore/editing/TextCheckingHelper.cpp', u'Source/WebCore/editing/VisibleUnits.cpp', u'Source/WebCore/platform/graphics/StringTruncator.cpp', u'Source/WebCore/platform/graphics/mac/ComplexTextController.cpp', u'Source/WebCore/platform/text/TextAllInOne.cpp', u'Source/WebCore/platform/text/TextBoundaries.cpp', u'Source/WebCore/platform/text/TextBreakIterator.cpp', u'Source/WebCore/platform/text/TextBreakIterator.h', u'Source/WebCore/platform/text/TextBreakIteratorICU.cpp', u'Source/WebCore/rendering/RenderText.cpp', u'Source/WebCore/rendering/break_lines.h', u'Source/WebKit/ios/ChangeLog', u'Source/WebKit/ios/Misc/WebNSStringDrawing.mm', u'Source/WebKit/ios/Misc/WebUIKitSupport.mm', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/UIProcess/efl/TextCheckerEfl.cpp', '--commit-queue']" exit_code: 1
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:215:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:223:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:224:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:225:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:226:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:227:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:228:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:229:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:230:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:231:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:232:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:233:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:234:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:235:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:236:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:237:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:238:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:239:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:240:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:241:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:242:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:243:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:244:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:245:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:246:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:258:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:259:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:260:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:261:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:262:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:263:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:264:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:265:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:275:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:276:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:277:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:278:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:279:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:280:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:281:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:282:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 41 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 EFL EWS Bot 2014-01-13 10:58:16 PST
Comment on attachment 221063 [details]
Patch

Attachment 221063 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/5738164891156480
Comment 8 EFL EWS Bot 2014-01-13 11:00:41 PST
Comment on attachment 221063 [details]
Patch

Attachment 221063 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/6641037883736064
Comment 9 kov's GTK+ EWS bot 2014-01-13 13:22:06 PST
Comment on attachment 221063 [details]
Patch

Attachment 221063 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/5595110804815872
Comment 10 Sam Weinig 2014-01-14 21:00:46 PST
Created attachment 221228 [details]
Patch
Comment 11 WebKit Commit Bot 2014-01-14 21:02:43 PST
Attachment 221228 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/PlatformGTK.cmake', u'Source/WebCore/WebCore.vcxproj/WebCore.vcxproj', u'Source/WebCore/WebCore.vcxproj/WebCore.vcxproj.filters', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/dom/CharacterData.cpp', u'Source/WebCore/editing/TextCheckingHelper.cpp', u'Source/WebCore/editing/VisibleUnits.cpp', u'Source/WebCore/page/TouchAdjustment.cpp', u'Source/WebCore/platform/graphics/StringTruncator.cpp', u'Source/WebCore/platform/graphics/mac/ComplexTextController.cpp', u'Source/WebCore/platform/text/TextAllInOne.cpp', u'Source/WebCore/platform/text/TextBoundaries.cpp', u'Source/WebCore/platform/text/TextBreakIterator.cpp', u'Source/WebCore/platform/text/TextBreakIterator.h', u'Source/WebCore/platform/text/TextBreakIteratorICU.cpp', u'Source/WebCore/platform/text/enchant/TextCheckerEnchant.cpp', u'Source/WebCore/rendering/RenderText.cpp', u'Source/WebCore/rendering/break_lines.h', u'Source/WebKit/ios/ChangeLog', u'Source/WebKit/ios/Misc/WebNSStringDrawing.mm', u'Source/WebKit/ios/Misc/WebUIKitSupport.mm', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/UIProcess/efl/TextCheckerEfl.cpp', '--commit-queue']" exit_code: 1
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:215:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:223:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:224:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:225:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:226:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:227:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:228:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:229:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:230:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:231:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:232:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:233:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:234:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:235:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:236:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:237:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:238:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:239:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:240:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:241:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:242:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:243:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:244:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:245:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:246:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:258:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:259:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:260:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:261:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:262:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:263:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:264:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:265:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:275:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:276:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:277:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:278:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:279:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:280:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:281:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:282:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 41 in 18 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 12 Sam Weinig 2014-01-15 19:42:54 PST
Comment on attachment 221228 [details]
Patch

I'm going to break this up.
Comment 13 Sam Weinig 2014-01-15 19:45:03 PST
Created attachment 221324 [details]
Patch
Comment 14 WebKit Commit Bot 2014-01-15 19:47:53 PST
Attachment 221324 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/PlatformGTK.cmake', u'Source/WebCore/WebCore.vcxproj/WebCore.vcxproj', u'Source/WebCore/WebCore.vcxproj/WebCore.vcxproj.filters', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/platform/text/TextBreakIterator.cpp', u'Source/WebCore/platform/text/TextBreakIteratorICU.cpp', '--commit-queue']" exit_code: 1
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:174:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:182:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:183:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:184:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:185:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:186:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:187:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:188:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:189:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:190:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:191:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:192:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:193:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:194:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:195:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:196:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:197:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:198:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:199:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:200:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:201:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:202:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:203:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:204:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:205:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:217:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:218:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:219:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:220:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:221:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:222:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:223:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:224:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:234:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:235:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:236:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:237:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:238:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:239:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:240:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/text/TextBreakIterator.cpp:241:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 41 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 15 Sam Weinig 2014-01-15 19:55:27 PST
Committed r162109: <http://trac.webkit.org/changeset/162109>
Comment 16 Sam Weinig 2014-01-15 21:17:48 PST
One more patch to go before we can close this guy.
Comment 17 Sam Weinig 2014-01-15 21:21:03 PST
Created attachment 221329 [details]
Patch
Comment 18 EFL EWS Bot 2014-01-15 21:25:02 PST
Comment on attachment 221329 [details]
Patch

Attachment 221329 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/5492986548846592
Comment 19 EFL EWS Bot 2014-01-15 21:30:01 PST
Comment on attachment 221329 [details]
Patch

Attachment 221329 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/5889102792622080
Comment 20 Build Bot 2014-01-15 21:47:08 PST
Comment on attachment 221329 [details]
Patch

Attachment 221329 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6672203105959936
Comment 21 kov's GTK+ EWS bot 2014-01-15 21:52:33 PST
Comment on attachment 221329 [details]
Patch

Attachment 221329 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/5205691354578944
Comment 22 Sam Weinig 2014-01-15 21:58:12 PST
Created attachment 221331 [details]
Patch
Comment 23 EFL EWS Bot 2014-01-15 22:01:58 PST
Comment on attachment 221331 [details]
Patch

Attachment 221331 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/6480987403845632
Comment 24 EFL EWS Bot 2014-01-15 22:08:16 PST
Comment on attachment 221331 [details]
Patch

Attachment 221331 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/6291626053861376
Comment 25 kov's GTK+ EWS bot 2014-01-15 22:22:40 PST
Comment on attachment 221331 [details]
Patch

Attachment 221331 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/4615102749736960
Comment 26 Sam Weinig 2014-01-16 21:30:02 PST
Created attachment 221438 [details]
Patch
Comment 27 Anders Carlsson 2014-01-16 22:15:06 PST
Comment on attachment 221438 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=221438&action=review

> Source/WebCore/ChangeLog:9
> +        now unnecessary up-conversion to UTF-16 in the process.

up-conversions?
Comment 28 Sam Weinig 2014-01-16 22:18:34 PST
Committed r162184: <http://trac.webkit.org/changeset/162184>
Comment 29 Alexey Proskuryakov 2014-01-21 23:38:58 PST
This change made multiple tests crash on Windows, filed bug 127408.