http://trac.webkit.org/changeset/143664 and http://trac.webkit.org/changeset/143681 made tests fail on Mac WK1 bots: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&tests=http%2Ftests%2Fsecurity%2Ffeed-urls-from-remote.html e.g. http://build.webkit.org/results/Apple%20MountainLion%20Release%20WK1%20(Tests)/r143686%20(7135)/results.html --- /Volumes/Data/slave/mountainlion-release-tests-wk1/build/layout-test-results/http/tests/security/feed-urls-from-remote-expected.txt +++ /Volumes/Data/slave/mountainlion-release-tests-wk1/build/layout-test-results/http/tests/security/feed-urls-from-remote-actual.txt @@ -1,6 +1,7 @@ Policy delegate: attempt to load feed://127.0.0.1:8000/security/resources/feed.xml with navigation type 'link clicked' originating from HTML > #document +FAIL: Timed out waiting for notifyDone to be called < rdar://problem/5329440> REGRESSION: Clicking links with the feed:// protocol in Safari 3 does nothing This test is to see if a feed URL can be followed from a link on a page. -Test Finished. +Test not completed.
Added a failing test expectation in http://trac.webkit.org/changeset/143689.
Yeah, we saw similar for Chromium, but I assumed it seemed specific to our DRT implementation. For now I'm just going to gate the offending code with ENABLE_THREADED_PARSER. That's slightly lame, and will potentially block enabling the threaded parser for other ports. This is a result of very very fragile code in the loader subsystem. The real solution is to re-write the loader, but that's out of scope for this change. :) It's also possible that we'll find a better solution than this one as we fix our last couple failures for the threaded parser. Thanks for filing.
(In reply to comment #2) > > This is a result of very very fragile code in the loader subsystem. The real solution is to re-write the loader, but that's out of scope for this change. :) I don't think rewriting it really helps. The real problem with the loader is that we don't have a good strategy to test it.
Created attachment 189688 [details] Patch
Comment on attachment 189688 [details] Patch rs=me.
(In reply to comment #3) > (In reply to comment #2) > > > > This is a result of very very fragile code in the loader subsystem. The real solution is to re-write the loader, but that's out of scope for this change. :) > > I don't think rewriting it really helps. The real problem with the loader is that we don't have a good strategy to test it. Perhaps. :) It's a state machine which is distributed between at least 5 classes, and about 20 bools. :) I suspect it would be a lot easier to understand as a single progressive state enum.
Comment on attachment 189688 [details] Patch Rejecting attachment 189688 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-01', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 189688, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue Last 500 characters of output: -conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' 54>At revision 13692. ________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' ________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' Updating webkit projects from gyp files... Updating OpenSource fatal: read error: Connection reset by peer Died at Tools/Scripts/update-webkit line 151. Full output: http://queues.webkit.org/results/16701504
Updating working directory Failed to run "['Tools/Scripts/update-webkit', '--chromium', '--force-update']" exit_code: 2 Updating OpenSource fatal: read error: Connection reset by peer Died at Tools/Scripts/update-webkit line 151. Odd.
Comment on attachment 189688 [details] Patch Clearing flags on attachment: 189688 Committed r143695: <http://trac.webkit.org/changeset/143695>
All reviewed patches have been landed. Closing bug.
Committed r143719: <http://trac.webkit.org/changeset/143719>
(In reply to comment #11) > Committed r143719: <http://trac.webkit.org/changeset/143719> Manually rolled this out in r143719 due to crashes occurring on debug builds of AppleMac WK1, EFL and GTK. Would have done the rollout through sheriffbot, but it was failing to update the checkout. Examples: http://build.webkit.org/results/Apple%20Lion%20Debug%20WK1%20(Tests)/r143709%20(6946)/results.html http://build.webkit.org/results/Apple%20MountainLion%20Debug%20WK1%20(Tests)/r143709%20(5750)/results.html http://build.webkit.org/results/EFL%20Linux%2064-bit%20Debug%20WK2/r143713%20(9747)/results.html http://build.webkit.org/results/GTK%20Linux%2064-bit%20Debug%20WK1/r143709%20(307)/results.html Given that this only seemed to fix one test and introduced crashes in multiple tests I reckoned rolling out is best.
Yeah, Eric just missed the other half. Will fix shortly.
Created attachment 189780 [details] Patch for landing
Sorry for the trouble.
Comment on attachment 189780 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=189780&action=review > Source/WebCore/dom/Document.cpp:5781 > + // FIXME: This should always be enabled, but it seems to cause > + // http/tests/security/feed-urls-from-remote.html to timeout on Mac WK1 > + // see http://webkit.org/b/110554 and http://webkit.org/b/110401 > +#if ENABLE(THREADED_HTML_PARSER) > loader()->checkLoadComplete(); > +#endif Didn't this part need a null check instead of an ENABLE?
> Didn't this part need a null check instead of an ENABLE? That might of worked, but I wanted to keep isLoading and checkLoadComplete in sync.
Comment on attachment 189780 [details] Patch for landing Clearing flags on attachment: 189780 Committed r143753: <http://trac.webkit.org/changeset/143753>