120891 2013-09-06 13:13:08 -0700 Internals should always cause a layout before calling into TextIterator 2013-09-09 13:27:33 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED BlinkMergeCandidate P2 Normal --- 120685 1 rniwa rniwa ap commit-queue darin esprehn kling koivisto oldest_to_newest 925806 0 rniwa 2013-09-06 13:13:08 -0700 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. 925808 1 210787 rniwa 2013-09-06 13:15:29 -0700 Created attachment 210787 Fixes Internals 925814 2 210787 ap 2013-09-06 13:21:46 -0700 Comment on attachment 210787 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. 925816 3 210787 darin 2013-09-06 13:23:28 -0700 Comment on attachment 210787 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? 925818 4 210787 darin 2013-09-06 13:24:30 -0700 Comment on attachment 210787 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. 925821 5 210787 darin 2013-09-06 13:25:36 -0700 Comment on attachment 210787 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? 925844 6 ap 2013-09-06 13:57:27 -0700 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. 925849 7 210797 rniwa 2013-09-06 14:02:38 -0700 Created attachment 210797 Force a layout in TextIterator's constructor 926218 8 210797 commit-queue 2013-09-07 10:18:04 -0700 Comment on attachment 210797 Force a layout in TextIterator's constructor Rejecting attachment 210797 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 926872 9 rniwa 2013-09-09 13:27:33 -0700 Committed r155378: <http://trac.webkit.org/changeset/155378> 210787 2013-09-06 13:15:29 -0700 2013-09-06 14:02:34 -0700 Fixes Internals bug-120891-20130906131528.patch text/plain 2133 rniwa SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE1NTIwOCkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIwIEBACisyMDEzLTA5LTA2ICBSeW9zdWtl IE5pd2EgIDxybml3YUB3ZWJraXQub3JnPgorCisgICAgICAgIEludGVybmFscyBzaG91bGQgYWx3 YXlzIGNhdXNlIGEgbGF5b3V0IGJlZm9yZSBjYWxsaW5nIGludG8gVGV4dEl0ZXJhdG9yCisgICAg ICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMjA4OTEKKworICAg ICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBNZXJnZSBodHRwczov L2Nocm9taXVtLmdvb2dsZXNvdXJjZS5jb20vY2hyb21pdW0vYmxpbmsvKy81ZmVlNWRhN2IwNGE3 MTAxNzFjNzliZDZlODdlY2EzNTMzMTg4ZTQ1CisKKyAgICAgICAgVGV4dEl0ZXJhdG9yIGV4cGVj dHMgdGhlIGxheW91dCB0byBiZSB1cC10by1kYXRlLiBGb3JjZSB1cGRhdGluZyB0aGUgbGF5b3V0 IGluIHRoZXNlIEludGVybmFscyBtZXRob2RzCisgICAgICAgIHRvIGF2b2lkIHByb2JsZW1zIGlu IHRoZSBmdXR1cmUgc3VjaCBhcyBpbnRlcm1pdHRlbnQgdGVzdCBmYWlsdXJlcy4KKworICAgICAg ICAqIHRlc3RpbmcvSW50ZXJuYWxzLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkludGVybmFsczo6 cmFuZ2VGcm9tTG9jYXRpb25BbmRMZW5ndGgpOgorICAgICAgICAoV2ViQ29yZTo6SW50ZXJuYWxz Ojpsb2NhdGlvbkZyb21SYW5nZSk6CisgICAgICAgIChXZWJDb3JlOjpJbnRlcm5hbHM6Omxlbmd0 aEZyb21SYW5nZSk6CisKIDIwMTMtMDktMDYgIFZpdmVrIEdhbGF0YWdlICA8dml2ZWsudmdAc2Ft c3VuZy5jb20+CiAKICAgICAgICAgQ2FsbGluZyB3aW5kb3cuY2xvc2UoKSBzaG91bGQgaW5kaWNh dGUgZmFpbHVyZSB3aXRoIHdhcm5pbmcgbWVzc2FnZQpJbmRleDogU291cmNlL1dlYkNvcmUvdGVz dGluZy9JbnRlcm5hbHMuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL3Rlc3Rpbmcv SW50ZXJuYWxzLmNwcAkocmV2aXNpb24gMTU1MjA1KQorKysgU291cmNlL1dlYkNvcmUvdGVzdGlu Zy9JbnRlcm5hbHMuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0xMDMwLDYgKzEwMzAsNyBAQCBQYXNz UmVmUHRyPFJhbmdlPiBJbnRlcm5hbHM6OnJhbmdlRnJvbUxvCiAgICAgICAgIHJldHVybiAwOwog ICAgIH0KIAorICAgIHNjb3BlLT5kb2N1bWVudCgpLnVwZGF0ZUxheW91dElnbm9yZVBlbmRpbmdT dHlsZXNoZWV0cygpOwogICAgIHJldHVybiBUZXh0SXRlcmF0b3I6OnJhbmdlRnJvbUxvY2F0aW9u QW5kTGVuZ3RoKHNjb3BlLCByYW5nZUxvY2F0aW9uLCByYW5nZUxlbmd0aCk7CiB9CiAKQEAgLTEw NDAsNiArMTA0MSw4IEBAIHVuc2lnbmVkIEludGVybmFsczo6bG9jYXRpb25Gcm9tUmFuZ2UoRWwK ICAgICAgICAgcmV0dXJuIDA7CiAgICAgfQogCisgICAgc2NvcGUtPmRvY3VtZW50KCkudXBkYXRl TGF5b3V0SWdub3JlUGVuZGluZ1N0eWxlc2hlZXRzKCk7CisKICAgICBzaXplX3QgbG9jYXRpb24g PSAwOwogICAgIHNpemVfdCB1bnVzZWRMZW5ndGggPSAwOwogICAgIFRleHRJdGVyYXRvcjo6Z2V0 TG9jYXRpb25BbmRMZW5ndGhGcm9tUmFuZ2Uoc2NvcGUsIHJhbmdlLCBsb2NhdGlvbiwgdW51c2Vk TGVuZ3RoKTsKQEAgLTEwNTMsNiArMTA1Niw4IEBAIHVuc2lnbmVkIEludGVybmFsczo6bGVuZ3Ro RnJvbVJhbmdlKEVsZW0KICAgICAgICAgcmV0dXJuIDA7CiAgICAgfQogCisgICAgc2NvcGUtPmRv Y3VtZW50KCkudXBkYXRlTGF5b3V0SWdub3JlUGVuZGluZ1N0eWxlc2hlZXRzKCk7CisKICAgICBz aXplX3QgdW51c2VkTG9jYXRpb24gPSAwOwogICAgIHNpemVfdCBsZW5ndGggPSAwOwogICAgIFRl eHRJdGVyYXRvcjo6Z2V0TG9jYXRpb25BbmRMZW5ndGhGcm9tUmFuZ2Uoc2NvcGUsIHJhbmdlLCB1 bnVzZWRMb2NhdGlvbiwgbGVuZ3RoKTsK 210797 2013-09-06 14:02:38 -0700 2013-09-07 10:18:04 -0700 Force a layout in TextIterator's constructor bug-120891-20130906140237.patch text/plain 3958 rniwa SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE1NTIxMSkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDI1IEBACisyMDEzLTA5LTA2ICBSeW9zdWtl IE5pd2EgIDxybml3YUB3ZWJraXQub3JnPgorCisgICAgICAgIEludGVybmFscyBzaG91bGQgYWx3 YXlzIGNhdXNlIGEgbGF5b3V0IGJlZm9yZSBjYWxsaW5nIGludG8gVGV4dEl0ZXJhdG9yCisgICAg ICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMjA4OTEKKworICAg ICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBJbnNwaXJlZCBieSBo dHRwczovL2Nocm9taXVtLmdvb2dsZXNvdXJjZS5jb20vY2hyb21pdW0vYmxpbmsvKy81ZmVlNWRh N2IwNGE3MTAxNzFjNzliZDZlODdlY2EzNTMzMTg4ZTQ1LgorCisgICAgICAgIEZvcmNlIGEgbGF5 b3V0IGluIHRoZSBjb25zdHJ1Y3RvcnMgb2YgVGV4dEl0ZXJhdG9yLCBhbmQgU2ltcGxpZmllZEJh Y2t3YXJkc1RleHRJdGVyYXRvciBhbmQgcmVtb3ZlCisgICAgICAgIHN1cGVyZmx1b3VzIGNhbGxz IHRvIHVwZGF0ZUxheW91dCgpIGluIG90aGVyIHBsYWNlcy4KKworICAgICAgICBBcyBtdWNoIGFz IEkgaGF0ZSBmb3IgYSBjb25zdHJ1Y3RvciB0byBoYXZlIGEgc2lkZSBlZmZlY3QgbGlrZSB0aGlz LCBJIGNvdWxkbid0IHRoaW5rIG9mIGEgYmV0dGVyIHBsYWNlCisgICAgICAgIHRvIHVwZGF0ZSB0 aGUgbGF5b3V0LiBVbmZvcnR1bmF0ZWx5LCB3ZSdyZSBzbG93bHkgbW92aW5nIGF3YXkgZnJvbSBt YW51YWxseSBjcmVhdGVpbmcgVGV4dEl0ZXJhdG9yIGFuZAorICAgICAgICB3cmFwcGluZyB0aGVt IGluIGEgc3RhdGljIGZ1bmN0aW9uLgorCisgICAgICAgICogZWRpdGluZy9UZXh0SXRlcmF0b3Iu Y3BwOgorICAgICAgICAoV2ViQ29yZTo6VGV4dEl0ZXJhdG9yOjpUZXh0SXRlcmF0b3IpOgorICAg ICAgICAoV2ViQ29yZTo6U2ltcGxpZmllZEJhY2t3YXJkc1RleHRJdGVyYXRvcjo6U2ltcGxpZmll ZEJhY2t3YXJkc1RleHRJdGVyYXRvcik6CisgICAgICAgIChXZWJDb3JlOjpUZXh0SXRlcmF0b3I6 OnJhbmdlRnJvbUxvY2F0aW9uQW5kTGVuZ3RoKToKKyAgICAgICAgKFdlYkNvcmU6OmZpbmRQbGFp blRleHQpOgorCiAyMDEzLTA5LTA2ICBEYXZpZCBIeWF0dCAgPGh5YXR0QGFwcGxlLmNvbT4KIAog ICAgICAgICBBZGQgbmV3IFJlbmRlckJsb2NrRmxvdyBjbGFzcy4KSW5kZXg6IFNvdXJjZS9XZWJD b3JlL2VkaXRpbmcvVGV4dEl0ZXJhdG9yLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29y ZS9lZGl0aW5nL1RleHRJdGVyYXRvci5jcHAJKHJldmlzaW9uIDE1NTIwNSkKKysrIFNvdXJjZS9X ZWJDb3JlL2VkaXRpbmcvVGV4dEl0ZXJhdG9yLmNwcAkod29ya2luZyBjb3B5KQpAQCAtMjkzLDYg KzI5Myw4IEBAIFRleHRJdGVyYXRvcjo6VGV4dEl0ZXJhdG9yKGNvbnN0IFJhbmdlKiAKICAgICBp ZiAoIXIpCiAgICAgICAgIHJldHVybjsKIAorICAgIHItPm93bmVyRG9jdW1lbnQoKS0+dXBkYXRl TGF5b3V0SWdub3JlUGVuZGluZ1N0eWxlc2hlZXRzKCk7CisKICAgICAvLyBnZXQgYW5kIHZhbGlk YXRlIHRoZSByYW5nZSBlbmRwb2ludHMKICAgICBOb2RlKiBzdGFydENvbnRhaW5lciA9IHItPnN0 YXJ0Q29udGFpbmVyKCk7CiAgICAgaWYgKCFzdGFydENvbnRhaW5lcikKQEAgLTExMTgsNiArMTEy MCw4IEBAIFNpbXBsaWZpZWRCYWNrd2FyZHNUZXh0SXRlcmF0b3I6OlNpbXBsaWYKICAgICBpZiAo IXIpCiAgICAgICAgIHJldHVybjsKIAorICAgIHItPm93bmVyRG9jdW1lbnQoKS0+dXBkYXRlTGF5 b3V0SWdub3JlUGVuZGluZ1N0eWxlc2hlZXRzKCk7CisKICAgICBOb2RlKiBzdGFydE5vZGUgPSBy LT5zdGFydENvbnRhaW5lcigpOwogICAgIGlmICghc3RhcnROb2RlKQogICAgICAgICByZXR1cm47 CkBAIC0yNDI3LDcgKzI0MzEsNiBAQCBQYXNzUmVmUHRyPFJhbmdlPiBUZXh0SXRlcmF0b3I6OnJh bmdlRnJvCiAgICAgICAgICAgICAvLyBGSVhNRTogVGhpcyBpcyBhIHdvcmthcm91bmQgZm9yIHRo ZSBmYWN0IHRoYXQgdGhlIGVuZCBvZiBhIHJ1biBpcyBvZnRlbiBhdCB0aGUgd3JvbmcKICAgICAg ICAgICAgIC8vIHBvc2l0aW9uIGZvciBlbWl0dGVkICdcbidzLgogICAgICAgICAgICAgaWYgKGxl biA9PSAxICYmIGl0LmNoYXJhY3RlckF0KDApID09ICdcbicpIHsKLSAgICAgICAgICAgICAgICBz Y29wZS0+ZG9jdW1lbnQoKS51cGRhdGVMYXlvdXRJZ25vcmVQZW5kaW5nU3R5bGVzaGVldHMoKTsK ICAgICAgICAgICAgICAgICBpdC5hZHZhbmNlKCk7CiAgICAgICAgICAgICAgICAgaWYgKCFpdC5h dEVuZCgpKSB7CiAgICAgICAgICAgICAgICAgICAgIFJlZlB0cjxSYW5nZT4gcmFuZ2UgPSBpdC5y YW5nZSgpOwpAQCAtMjU5Myw5ICsyNTk2LDYgQEAgdHJ5QWdhaW46CiAKIFBhc3NSZWZQdHI8UmFu Z2U+IGZpbmRQbGFpblRleHQoY29uc3QgUmFuZ2UqIHJhbmdlLCBjb25zdCBTdHJpbmcmIHRhcmdl dCwgRmluZE9wdGlvbnMgb3B0aW9ucykKIHsKLSAgICAvLyBDaGFyYWN0ZXJJdGVyYXRvciByZXF1 aXJlcyByZW5kZXJlcnMgdG8gYmUgdXAtdG8tZGF0ZQotICAgIHJhbmdlLT5vd25lckRvY3VtZW50 KCktPnVwZGF0ZUxheW91dCgpOwotCiAgICAgLy8gRmlyc3QsIGZpbmQgdGhlIHRleHQuCiAgICAg c2l6ZV90IG1hdGNoU3RhcnQ7CiAgICAgc2l6ZV90IG1hdGNoTGVuZ3RoOwpJbmRleDogTGF5b3V0 VGVzdHMvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIExheW91dFRlc3RzL0NoYW5nZUxvZwkocmV2 aXNpb24gMTU1MjExKQorKysgTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTQgQEAKKzIwMTMtMDktMDYgIFJ5b3N1a2UgTml3YSAgPHJuaXdhQHdlYmtpdC5v cmc+CisKKyAgICAgICAgSW50ZXJuYWxzIHNob3VsZCBhbHdheXMgY2F1c2UgYSBsYXlvdXQgYmVm b3JlIGNhbGxpbmcgaW50byBUZXh0SXRlcmF0b3IKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtp dC5vcmcvc2hvd19idWcuY2dpP2lkPTEyMDg5MQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIFByb2dyZXNzaW9uLgorCisgICAgICAgICogcGxhdGZvcm0v bWFjL2VkaXRpbmcvaW5wdXQvY2FyZXQtcHJpbWFyeS1iaWRpLWV4cGVjdGVkLnR4dDoKKwogMjAx My0wOS0wNiAgTWlrZSBXZXN0ICA8bWt3c3RAY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFJldmFs aWRhdGlvbiBoZWFkZXIgYmxhY2tsaXN0aW5nIHNob3VsZCBiZSBjYXNlLWluc2Vuc2l0aXZlLgpJ bmRleDogTGF5b3V0VGVzdHMvcGxhdGZvcm0vbWFjL2VkaXRpbmcvaW5wdXQvY2FyZXQtcHJpbWFy eS1iaWRpLWV4cGVjdGVkLnR4dAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBMYXlvdXRUZXN0cy9wbGF0Zm9ybS9t YWMvZWRpdGluZy9pbnB1dC9jYXJldC1wcmltYXJ5LWJpZGktZXhwZWN0ZWQudHh0CShyZXZpc2lv biAxNTUyMDUpCisrKyBMYXlvdXRUZXN0cy9wbGF0Zm9ybS9tYWMvZWRpdGluZy9pbnB1dC9jYXJl dC1wcmltYXJ5LWJpZGktZXhwZWN0ZWQudHh0CSh3b3JraW5nIGNvcHkpCkBAIC0xLDQgKzEsNCBA QAotMDogMTI0LDUwOCwwLDI4CiswOiA4LDU2NCwwLDI4CiAxOiAyMSw1NjQsMCwyOAogMjogMzYs NTY0LDAsMjgKIDM6IDQ4LDU2NCwwLDI4Cg==