Bug 177698 - [iOS WK2] API test EditorStateTests.ContentViewHasTextInContentEditableElement is a flaky failure
Summary: [iOS WK2] API test EditorStateTests.ContentViewHasTextInContentEditableElemen...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-09-29 19:13 PDT by Wenson Hsieh
Modified: 2017-10-03 20:14 PDT (History)
7 users (show)

See Also:


Attachments
Patch (2.92 KB, patch)
2017-09-29 19:47 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Fix some awkward ChangeLog wording (2.89 KB, patch)
2017-09-29 22:35 PDT, Wenson Hsieh
rniwa: review+
Details | Formatted Diff | Diff
Patch for landing (2.97 KB, patch)
2017-09-30 13:46 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.