RESOLVED FIXED Bug 209086
Color Picker crashes on touch
https://bugs.webkit.org/show_bug.cgi?id=209086
Summary Color Picker crashes on touch
Megan Gardner
Reported 2020-03-13 15:22:24 PDT
Color Picker crashes on touch
Attachments
Patch (3.66 KB, patch)
2020-03-16 10:49 PDT, Megan Gardner
wenson_hsieh: review+
Patch (3.68 KB, patch)
2020-03-16 10:57 PDT, Megan Gardner
wenson_hsieh: review+
Patch (3.69 KB, patch)
2020-03-16 11:00 PDT, Megan Gardner
no flags
Megan Gardner
Comment 1 2020-03-16 10:49:26 PDT
Wenson Hsieh
Comment 2 2020-03-16 10:54:06 PDT
Comment on attachment 393662 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393662&action=review > LayoutTests/fast/forms/color/color-input-activate-crash.html:13 > + if (!testRunner.runUIScript) Nit - spaces instead of tabs. > LayoutTests/fast/forms/color/color-input-activate-crash.html:16 > + var input = document.getElementById('colorInput'); Nit - it looks like there are a mix of spaces and tabs here. > LayoutTests/fast/forms/color/color-input-activate-crash.html:19 > + await UIHelper.activateElementAndWaitForInputSession(input); > + document.getElementById('result').innerHTML = 'PASS: Test did not crash'; > + testRunner.notifyDone(); Ditto with these lines. > LayoutTests/fast/forms/color/color-input-activate-crash.html:22 > + window.addEventListener('load', runTest, false); Ditto.
Megan Gardner
Comment 3 2020-03-16 10:57:06 PDT
Wenson Hsieh
Comment 4 2020-03-16 10:58:14 PDT
Comment on attachment 393664 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393664&action=review > LayoutTests/fast/forms/color/color-input-activate-crash.html:17 > + wait UIHelper.activateElementAndWaitForInputSession(input); wait => await?
Megan Gardner
Comment 5 2020-03-16 11:00:24 PDT
Darin Adler
Comment 6 2020-03-16 12:50:00 PDT
Comment on attachment 393666 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393666&action=review > Source/WebCore/ChangeLog:8 > + Vector sizing lost in refactor. Not perfomance sensitive code, so just expanding vector as needed. Thanks for fixing my mistake! There are really two questions about using append. One you have answered here. The other is whether the vector lives a long time -- if it did we would want to make sure it doesn’t use a lot of space. I think it does not live a long time. If the vector did live a long time, there are two ways we could deal with that: 1) Less effective: Add a call to suggestions.shrinkToFit() after the loop. 2) More effective: Leave this using uncheckedAppend and add this line of code before the loop: suggestions.reserveInitialCapacity(std::distance(dataList->suggestions().begin(), { })); I’m going to assume it does not live a long time.
WebKit Commit Bot
Comment 7 2020-03-16 13:21:08 PDT
Comment on attachment 393666 [details] Patch Clearing flags on attachment: 393666 Committed r258516: <https://trac.webkit.org/changeset/258516>
Radar WebKit Bug Importer
Comment 8 2020-03-16 15:17:39 PDT
Note You need to log in before you can comment on or make changes to this bug.