Bug 177698

Summary: [iOS WK2] API test EditorStateTests.ContentViewHasTextInContentEditableElement is a flaky failure
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: WebKit2Assignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, jlewis3, joepeck, rniwa, ryanhaddad, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Fix some awkward ChangeLog wording
rniwa: review+
Patch for landing none

Description Wenson Hsieh 2017-09-29 19:13:03 PDT
This test occasionally fails on the bots:

"Expected -[WKContentView hasText] to be NO, but observed: YES (after inserting an image element)"
Comment 1 Wenson Hsieh 2017-09-29 19:47:06 PDT
Created attachment 322268 [details]
Patch
Comment 2 Wenson Hsieh 2017-09-29 22:35:59 PDT
Created attachment 322282 [details]
Fix some awkward ChangeLog wording
Comment 3 Ryosuke Niwa 2017-09-30 13:23:44 PDT
Comment on attachment 322282 [details]
Fix some awkward ChangeLog wording

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

> Source/WebCore/editing/VisibleUnits.cpp:1895
> -    Vector<UChar32> characters(maxCharacters);
> +    Vector<UChar32> characters(maxCharacters, 0);

Can we just make this an array of length 3 as in?
UChar32 maxCharacters[maxCharacters] = {0};
or at least give it an inline capacity so that it won't do a heap allocation?
Comment 4 Wenson Hsieh 2017-09-30 13:36:42 PDT
(In reply to Ryosuke Niwa from comment #3)
> Comment on attachment 322282 [details]
> Fix some awkward ChangeLog wording
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=322282&action=review
> 
> > Source/WebCore/editing/VisibleUnits.cpp:1895
> > -    Vector<UChar32> characters(maxCharacters);
> > +    Vector<UChar32> characters(maxCharacters, 0);
> 
> Can we just make this an array of length 3 as in?
> UChar32 maxCharacters[maxCharacters] = {0};
> or at least give it an inline capacity so that it won't do a heap allocation?

Sounds good! Changed `characters` into a 0-initialized UChar32 array.
Comment 5 Wenson Hsieh 2017-09-30 13:46:20 PDT
Created attachment 322299 [details]
Patch for landing
Comment 6 WebKit Commit Bot 2017-09-30 14:25:54 PDT
Comment on attachment 322299 [details]
Patch for landing

Clearing flags on attachment: 322299

Committed r222683: <http://trac.webkit.org/changeset/222683>
Comment 7 Radar WebKit Bug Importer 2017-10-02 17:04:44 PDT
<rdar://problem/34780869>
Comment 8 Alexey Proskuryakov 2017-10-03 20:14:32 PDT
Comment on attachment 322299 [details]
Patch for landing

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

> Source/WebCore/ChangeLog:23
> +        members in the PostLayoutData struct). We also turn `characters` into a UChar32 array on the stack, to avoid the
> +        heap allocations using a Vector<UChar32>.

I think that adding inline capacity to the vector would be a better fix for the latter part.