Last Comment Bug 645998 - Loading forever on www.cleansui.com
: Loading forever on www.cleansui.com
Status: RESOLVED FIXED
: regression
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: All All
: P1 normal (vote)
: mozilla8
Assigned To: Boris Zbarsky [:bz] (Out June 25-July 6)
:
Mentors:
http://www.cleansui.com/shop/product/...
Depends on:
Blocks: 482919
  Show dependency treegraph
 
Reported: 2011-03-29 03:16 PDT by Alice0775 White
Modified: 2011-08-08 10:34 PDT (History)
7 users (show)
bzbarsky: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Screen shot (STR) and A page should appear after clicking (379.72 KB, image/png)
2011-03-29 03:16 PDT, Alice0775 White
no flags Details
Improve the recursion detection in the CSS loader to detect mutual recursion scenarios. (5.22 KB, patch)
2011-08-02 08:52 PDT, Boris Zbarsky [:bz] (Out June 25-July 6)
jonas: review+
Details | Diff | Review

Description Alice0775 White 2011-03-29 03:16:41 PDT
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.
Comment 1 Alice0775 White 2011-03-29 03:32:03 PDT
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.
Comment 2 Alice0775 White 2011-03-29 03:44:23 PDT
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
Comment 3 Henri Sivonen (:hsivonen) 2011-03-29 04:31:55 PDT
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.
Comment 4 Henri Sivonen (:hsivonen) 2011-03-29 05:02:45 PDT
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.
Comment 5 Henri Sivonen (:hsivonen) 2011-03-29 05:06:07 PDT
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.
Comment 6 Alice0775 White 2011-03-29 18:10:10 PDT
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.
Comment 7 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-03-30 23:14:42 PDT
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.
Comment 8 Alice0775 White 2011-04-14 04:26:18 PDT
I can not reproduce anymore.

Maybe the issue was fixed by the site.
Comment 9 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-04-14 06:15:11 PDT
I can most certainly reproduce using the steps in comment 7!
Comment 10 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-08-02 08:52:04 PDT
Created attachment 550093 [details] [diff] [review]
Improve the recursion detection in the CSS loader to detect mutual recursion scenarios.
Comment 11 Jonas Sicking (:sicking) PTO Until July 5th 2011-08-03 20:35:23 PDT
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
Comment 12 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-08-07 22:04:26 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/3b5da2d84538
Comment 13 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-08-08 05:47:54 PDT
http://hg.mozilla.org/mozilla-central/rev/3b5da2d84538

Note You need to log in before you can comment on or make changes to this bug.