Closed Bug 486044 Opened 15 years ago Closed 15 years ago

[FIX]"ASSERTION: Bad loading table" on canadianimmigrant.ca

Categories

(Core :: CSS Parsing and Computation, defect)

x86
macOS
defect
Not set
normal

Tracking

()

VERIFIED FIXED

People

(Reporter: jruderman, Assigned: bzbarsky)

References

()

Details

(Keywords: assertion, verified1.9.1)

Attachments

(2 files)

Loading http://www.canadianimmigrant.ca/ triggers 2x

###!!! ASSERTION: Bad loading table: 'mLoadingDatas.Get(&key, &loadingData) && loadingData == aLoadData', file /Users/jruderman/central/layout/style/nsCSSLoader.cpp, line 1632

The URL from bug 486039 also triggers this assertion before hitting an unrelated crash.
Attached file Testcase
So this sucks.  I thought we'd eliminated all instances of this when we stopped blocking the parser on CSS loads and when we switched to having DoSheetComplete ... but clearly not.

Still trying to figure out what's going on here; with the DoSheetComplete setup this really shouldn't happen.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Jesse, did the build in which you hit this have the fix for bug 457809 in it?  I just rebuild parser/htmlparser after backing that out, and can't reproduce this anymore...
Yes.
Attached patch FixSplinter Review
OK, this is totally my fault; when I started passing principals through this code I forgot to pass it to CreateSheet in this case.  So we were not finding the existing load when going to preload (the site in question has two <link>s separated by a script pointing to the same stylesheet).  But we put the right principal in the load data, so the lookup in SheetComplete found the right thing and removed it from the hashtable.  We thus had two (at least) loads racing to remove the same hashtable entry; whichever one lost the race asserted.

This fix just passes the right principal to CreateSheet, and everything is happy, since we properly coalesce the loads.

I guess the tests for bug 457809 were not hitting this in terms of the pageload time because they were using the HTTP cache or something; we really need tests for that bug that enforce non-cacheable stylesheets.
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #370126 - Flags: superreview?(peterv)
Attachment #370126 - Flags: review?(peterv)
Blocks: 457809
Keywords: testcase-wanted
Summary: "ASSERTION: Bad loading table" on canadianimmigrant.ca → [FIX]"ASSERTION: Bad loading table" on canadianimmigrant.ca
> I guess the tests for bug 457809 were not hitting this in terms of the pageload
> time because they were using the HTTP cache or something; we really need tests
> for that bug that enforce non-cacheable stylesheets.

See bug #457809 comment #19. There are no tests available for this at the moment.
I meant the manual tests you presumably ran to test your patch, not the automated tests you can't write yet.
Blocks: 457810
No longer blocks: 457809
Attachment #370126 - Flags: superreview?(peterv)
Attachment #370126 - Flags: superreview+
Attachment #370126 - Flags: review?(peterv)
Attachment #370126 - Flags: review+
Pushed http://hg.mozilla.org/mozilla-central/rev/1d85e364668e
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Comment on attachment 370126 [details] [diff] [review]
Fix

We should take this on branch; needed for stylesheet prefetch.
Attachment #370126 - Flags: approval1.9.1?
Attachment #370126 - Flags: approval1.9.1? → approval1.9.1+
verified FIXED on builds: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090414 Minefield/3.6a1pre ID:20090414030735

and

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b4pre) Gecko/20090414 Shiretoko/3.5b4pre ID:20090414035212
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: