Merge https://chromium.googlesource.com/chromium/blink/+/5fee5da7b04a710171c79bd6e87eca3533188e45 Calling into TextIterator without causing a layout first doesn't work and will break once we lazy attach everywhere. This fixes lots of tests that break once we lazy attach inside the parser.
Created attachment 210787 [details] Fixes Internals
Comment on attachment 210787 [details] Fixes Internals View in context: https://bugs.webkit.org/attachment.cgi?id=210787&action=review > Source/WebCore/ChangeLog:11 > + TextIterator expects the layout to be up-to-date. Force updating the layout in these Internals methods > + to avoid problems in the future such as intermittent test failures. We also call the same from WebKit, and don't update the layout. It would seem inconsistent if we only did the in Internals.
Comment on attachment 210787 [details] Fixes Internals View in context: https://bugs.webkit.org/attachment.cgi?id=210787&action=review >> Source/WebCore/ChangeLog:11 >> + to avoid problems in the future such as intermittent test failures. > > We also call the same from WebKit, and don't update the layout. It would seem inconsistent if we only did the in Internals. I’m not sure what your point is. Calling without updating layout is a bug. Why do we need to replicate that bug in our test harness? Would doing that help us make certain kinds of tests?
Comment on attachment 210787 [details] Fixes Internals View in context: https://bugs.webkit.org/attachment.cgi?id=210787&action=review >>> Source/WebCore/ChangeLog:11 >>> + to avoid problems in the future such as intermittent test failures. >> >> We also call the same from WebKit, and don't update the layout. It would seem inconsistent if we only did the in Internals. > > I’m not sure what your point is. Calling without updating layout is a bug. Why do we need to replicate that bug in our test harness? Would doing that help us make certain kinds of tests? By the way, sounds like we need to fix that WebKit bug! We should see whether we can reproduce a problem and fix the code whether we can or not.
Comment on attachment 210787 [details] Fixes Internals View in context: https://bugs.webkit.org/attachment.cgi?id=210787&action=review >>>> Source/WebCore/ChangeLog:11 >>>> + to avoid problems in the future such as intermittent test failures. >>> >>> We also call the same from WebKit, and don't update the layout. It would seem inconsistent if we only did the in Internals. >> >> I’m not sure what your point is. Calling without updating layout is a bug. Why do we need to replicate that bug in our test harness? Would doing that help us make certain kinds of tests? > > By the way, sounds like we need to fix that WebKit bug! We should see whether we can reproduce a problem and fix the code whether we can or not. Maybe Alexey is suggesting that TextIterator itself be responsible for calling updateLayout?
I don't know if/why layout needs to be manually updated (I simply don't understand how this works). But if it does, then it definitely seems that this should be done by TextIterator itself.
Created attachment 210797 [details] Force a layout in TextIterator's constructor
Comment on attachment 210797 [details] Force a layout in TextIterator's constructor Rejecting attachment 210797 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-03', 'apply-attachment', '--no-update', '--non-interactive', 210797, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: with fuzz 3. patching file Source/WebCore/editing/TextIterator.cpp Hunk #4 FAILED at 2596. 1 out of 4 hunks FAILED -- saving rejects to file Source/WebCore/editing/TextIterator.cpp.rej patching file LayoutTests/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file LayoutTests/platform/mac/editing/input/caret-primary-bidi-expected.txt Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Antti Koivisto']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Full output: http://webkit-queues.appspot.com/results/1719377
Committed r155378: <http://trac.webkit.org/changeset/155378>