Summary: | ASSERT(m_state == ParsingState) fires @ www.canalplus.fr | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Geoffrey Garen <ggaren> | ||||||||
Component: | WebCore Misc. | Assignee: | Tony Gentilcore <tonyg> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abarth, ademar, ap, commit-queue, eric, tonyg | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | OS X 10.5 | ||||||||||
URL: | http://www.canalplus.fr/c-sport/c-football/italie/pid3517-videos-calcio.html | ||||||||||
Attachments: |
|
Description
Geoffrey Garen
2011-05-03 18:03:33 PDT
The ASSERT was added in http://trac.webkit.org/changeset/66841 with the rationale "It is better to ensure it is only called while parsing." There doesn't seem to be any explanation of what guarantees this invariant -- just an aspiration toward a world where this invariant was true. We'll need to look a why the ASSERT is firing to determine whether the ASSERT is incorrect or whether the caller is incorrect. I'm preparing a reduction. I'm pretty sure we don't want to remove the ASSERT. If it hits, we'd go through http://www.whatwg.org/specs/web-apps/current-work/#the-end twice. The page is doing some tricky combinations of writing into an iframe then calling document.close() which is incorrectly leading to the second prepareToStopParsing() call for the document. It's turning out to be hard to reduce. One potential fix is to change the "if (isStopped())" in prepareToStopParsing() to "if (!isParsing())", but until I get a proper reduction, I'm not sure I understand the issue fully. I'll keep working on it, but just wanted to give a quick update. Created attachment 92428 [details]
Testcase
Created attachment 92431 [details]
Patch
The code for stopping/canceling/ending parsing really needs to be beaten with a stick. (In reply to comment #8) > The code for stopping/canceling/ending parsing really needs to be beaten with a stick. I couldn't agree more. It doesn't line up with the spec well, has known and unknown bugs and several fixmes. The problem is that it is so fragile, and the stopping/canceling sometimes bleeds over into port-specific logic. :( Comment on attachment 92431 [details] Patch Rejecting attachment 92431 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-4', 'build-..." exit_code: 2 Last 500 characters of output: .... http/tests/xmlhttprequest/cross-origin-authorization.html -> failed ............................................................................................................................ http/tests/xmlhttprequest/web-apps ............... http/tests/xmlhttprequest/workers ........... http/tests/xmlviewer . http/tests/xmlviewer/dumpAsText ........... 797.38s total testing time 23459 test cases (99%) succeeded 1 test case (<1%) had incorrect layout 15 test cases (<1%) had stderr output Full output: http://queues.webkit.org/results/8571536 Created attachment 92465 [details]
Archive of layout-test-results from cr-jail-4
The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: cr-jail-4 Port: Mac Platform: Mac OS X 10.6.7
Comment on attachment 92431 [details]
Patch
That's a known flaky test. I expect it just flaked twice in a row when we tried to land. :(
The commit-queue encountered the following flaky tests while processing attachment 92431 [details]: http/tests/appcache/non-html.xhtml bug 60308 (author: ap@webkit.org) The commit-queue is continuing to process your patch. Comment on attachment 92431 [details] Patch Clearing flags on attachment: 92431 Committed r85894: <http://trac.webkit.org/changeset/85894> All reviewed patches have been landed. Closing bug. Revision r85894 cherry-picked into qtwebkit-2.2 with commit c88e89d <http://gitorious.org/webkit/qtwebkit/commit/c88e89d> |