Bug 60101 - ASSERT(m_state == ParsingState) fires @ www.canalplus.fr
Summary: ASSERT(m_state == ParsingState) fires @ www.canalplus.fr
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Tony Gentilcore
URL: http://www.canalplus.fr/c-sport/c-foo...
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-03 18:03 PDT by Geoffrey Garen
Modified: 2011-05-09 13:44 PDT (History)
6 users (show)

See Also:


Attachments
Testcase (279 bytes, text/html)
2011-05-05 09:46 PDT, Tony Gentilcore
no flags Details
Patch (4.20 KB, patch)
2011-05-05 10:20 PDT, Tony Gentilcore
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from cr-jail-4 (207.93 KB, application/zip)
2011-05-05 14:24 PDT, WebKit Commit Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Geoffrey Garen 2011-05-03 18:03:33 PDT
1. Go to http://www.canalplus.fr/c-sport/c-football/italie/pid3517-videos-calcio.html
--> ASSERT
Comment 1 Geoffrey Garen 2011-05-03 18:06:08 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.
Comment 2 Adam Barth 2011-05-03 18:14:39 PDT
We'll need to look a why the ASSERT is firing to determine whether the ASSERT is incorrect or whether the caller is incorrect.
Comment 3 Alexey Proskuryakov 2011-05-04 11:21:32 PDT
See also: bug 50253.
Comment 4 Tony Gentilcore 2011-05-04 12:51:00 PDT
I'm preparing a reduction.
Comment 5 Tony Gentilcore 2011-05-05 09:11:42 PDT
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.
Comment 6 Tony Gentilcore 2011-05-05 09:46:29 PDT
Created attachment 92428 [details]
Testcase
Comment 7 Tony Gentilcore 2011-05-05 10:20:17 PDT
Created attachment 92431 [details]
Patch
Comment 8 Adam Barth 2011-05-05 10:37:32 PDT
The code for stopping/canceling/ending parsing really needs to be beaten with a stick.
Comment 9 Tony Gentilcore 2011-05-05 12:19:52 PDT
(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 10 WebKit Commit Bot 2011-05-05 14:24:03 PDT
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
Comment 11 WebKit Commit Bot 2011-05-05 14:24:07 PDT
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 12 Eric Seidel 2011-05-05 14:30:24 PDT
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. :(
Comment 13 WebKit Commit Bot 2011-05-05 15:36:50 PDT
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 14 WebKit Commit Bot 2011-05-05 15:38:20 PDT
Comment on attachment 92431 [details]
Patch

Clearing flags on attachment: 92431

Committed r85894: <http://trac.webkit.org/changeset/85894>
Comment 15 WebKit Commit Bot 2011-05-05 15:38:26 PDT
All reviewed patches have been landed.  Closing bug.
Comment 16 Ademar Reis 2011-05-09 13:43:59 PDT
Revision r85894 cherry-picked into qtwebkit-2.2 with commit c88e89d <http://gitorious.org/webkit/qtwebkit/commit/c88e89d>