645998 - Loading forever on www.cleansui.com

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P1)

Description

[Alice0775 White]

Attachment: 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]

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]

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)]

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)]

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)]

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]

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 [:bzbarsky]]

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]

I can not reproduce anymore.

Maybe the issue was fixed by the site.

Comment 9

[Boris Zbarsky [:bzbarsky]]

I can most certainly reproduce using the steps in comment 7!

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: Improve the recursion detection in the CSS loader to detect mutual recursion scenarios.

Comment 11

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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 [:bzbarsky]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/3b5da2d84538

Comment 13

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

http://hg.mozilla.org/mozilla-central/rev/3b5da2d84538