379.72 KB, image/png
5.22 KB, patch
|Details | Diff | Splinter Review|
Created attachment 522633 [details] Screen shot (STR) and A page should appear after clicking Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.2a1pre) Gecko/20110328 Firefox/4.2a1pre ID:20110328125522 Loading forever on www.cleansui.com. This does not happen on Firefox3.6.x and Google Chrome 12.0.717.0 canary build Steps to Reproduce: 1. Start Minefield with new profile 2. Open URL ( http://www.cleansui.com/shop/ ) 3. Click "浄水器商品一覧"(Image Link) See Screenshot Actual Results: Loading forever Expected Results: Loading should be completed immediately.
Regression window: Works: http://hg.mozilla.org/mozilla-central/rev/83c887dff0da Mozilla/5.0 (Windows; U; Windows NT 6.1; WOW64; en-US; rv:1.9.3a5pre) Gecko/20100503 Minefield/3.7a5pre ID:20100503040502 Fails: http://hg.mozilla.org/mozilla-central/rev/3a7920df7580 Mozilla/5.0 (Windows; U; Windows NT 6.1; WOW64; en-US; rv:1.9.3a5pre) Gecko/20100503 Minefield/3.7a5pre ID:20100503105056 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=83c887dff0da&tochange=3a7920df7580 html5.parser.enable=false does _not_ help. UA spoofing(IE8, Chrome 12 and Firefox3.6) also do _not_ help.
Regression window: In case od enable HTML5 user_pref("html5.enable", true); user_pref("html5.parser.enable", true); Works: http://hg.mozilla.org/mozilla-central/rev/fd765c193770 Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20091118 Minefield/3.7a1pre ID:20091118051933 Fails: http://hg.mozilla.org/mozilla-central/rev/21972540dbf4 Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20091119 Minefield/3.7a1pre ID:20091119050202 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=fd765c193770&tochange=21972540dbf4 Regression window(TM nightly) In case od enable HTML5 user_pref("html5.enable", true); user_pref("html5.parser.enable", true); Works: http://hg.mozilla.org/tracemonkey/rev/f8188f1c00f0 Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20091118 Minefield/3.7a1pre ID:20091118053516 Fails: http://hg.mozilla.org/tracemonkey/rev/c84622414ed3 Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20091119 Minefield/3.7a1pre ID:20091119045932 Pushlog: http://hg.mozilla.org/tracemonkey/pushloghtml?fromchange=f8188f1c00f0&tochange=c84622414ed3
Findings so far: * The parser waits forever for http://www.cleansui.com/js/import.js to finish loading. * http://www.cleansui.com/js/import.js is in the prefetch list. * When the script loader adopts the preload as a real load, the request object is still in the "loading" state. * Loading http://www.cleansui.com/js/import.js alone works fine. Looks like a problem in the script preloading machinery.
Upon further investigation, it seems that the problem is that nsContentSink::StyleSheetLoaded never gets called for the <link>-based style sheet before the script even though nsHtml5TreeOpExecutor sets self (inheriting from nsContentSink) as an observer. Bouncing over to the style system in case someone working on it recognizes something interesting in the regression range. Note that the sheet has a lot of imports.
To clarify: The HTTP request for http://www.cleansui.com/js/import.js finishes but the execute blocker added for the style sheet never gets removed and, therefore, the script is never executed. And when the script is never executed, it never unblocks the HTML parser.
In local build on ubuntu10.04(from m-c repository): enabled HTML5 user_pref("html5.enable", true); user_pref("html5.parser.enable", true); build from c61a7e1570c0 : fails build from 20b6e47d16d0 : fails build from e04af661ed40 : fails build from 7cda86954b4c : works build from 7380c012628e : works build from 34bc9a8eb49e : works Regressedd by: e04af661ed40 Henri Sivonen — Bug 482919 - Add speculative parsing to the HTML5 parser. r=bnewman.
Yeah, this is a bug in the CSS loader, and a longstanding one; my reduced testcase shows the problem in Firefox 3.0 and later (but not Firefox 2). The only thing the HTML5 parser did is make it a little easier to trigger. Unfortunately, I can't attach a testcase to this bug, but writing one is very easy. You need three files: file1.css: @import url("file2.css"); file2.css: @import url("file1.css"); test.html: <!DOCTYPE html> <link rel="stylesheet" href="file1.css"> <link rel="stylesheet" href="file2.css"> <script> alert('PASS'); </script> The key is that LoadChildSheet has some recursion protection but now that I look at it carefully it's too weak. In particular, it doesn't catch the mutual recursion above. When that testcase is loaded, we kick off both the file1 and file2 loads. Then say file1 loads first; we parse it and coalese its child sheet with the file2 load that's still in progress. Now we're waiting on that load to finish.... but when file2 loads it coalesces its child sheet with the now-waiting file1 load, and then they're both waiting on each other. On the site in question, the relevant markup is this: <link rel="stylesheet" type="text/css" href="/css/style.css" /> <link href="/css/import.css" rel="stylesheet" type="text/css" media="all" /> where import.css imports style.css and style.css imports import.css. The only reason speculative parsing matters is that there's a <script> between the two <link>s, so without speculative parsing we'd load and fully parse all the descendants of style.css before starting the import.css load; when we get to the style.css imported from the import.css imported from style.css the recursion protection we have _does_ kick in, and we're ok. The reason our recursion protection fails is that it only considers the ancestor chain of parentData, and not the set of ancestor chains for the loads coalesced with parentData. I need to think about this a bit and see what the minimal amount of checking we can do to break the recursion is.
I can not reproduce anymore. Maybe the issue was fixed by the site.
I can most certainly reproduce using the steps in comment 7!
Created attachment 550093 [details] [diff] [review] Improve the recursion detection in the CSS loader to detect mutual recursion scenarios.
Comment on attachment 550093 [details] [diff] [review] Improve the recursion detection in the CSS loader to detect mutual recursion scenarios. Review of attachment 550093 [details] [diff] [review]: ----------------------------------------------------------------- r=me