Closed Bug 645998 Opened 13 years ago Closed 13 years ago

Loading forever on www.cleansui.com

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla8

People

(Reporter: alice0775, Assigned: bzbarsky)

References

()

Details

(Keywords: regression)

Attachments

(2 files)

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.
Component: Document Navigation → HTML: Parser
QA Contact: docshell → parser
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.
Component: HTML: Parser → DOM
OS: Windows 7 → All
QA Contact: parser → general
Hardware: x86 → All
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.
Component: DOM → Style System (CSS)
QA Contact: general → style-system
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.
Blocks: 482919
Keywords: regression
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.
Assignee: nobody → bzbarsky
Priority: -- → P1
I can not reproduce anymore.

Maybe the issue was fixed by the site.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → WORKSFORME
I can most certainly reproduce using the steps in comment 7!
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Whiteboard: [need review]
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
Attachment #550093 - Flags: review?(jonas) → review+
Whiteboard: [need review] → [need landing]
http://hg.mozilla.org/integration/mozilla-inbound/rev/3b5da2d84538
Flags: in-testsuite+
Whiteboard: [need landing]
Target Milestone: --- → mozilla8
http://hg.mozilla.org/mozilla-central/rev/3b5da2d84538
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
See Also: → 1844683
You need to log in before you can comment on or make changes to this bug.