Summary: | REGRESSION: Reproducible assert while loading this test file if css is already in the cache | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Grace Kloba <klobag> | ||||||
Component: | Page Loading | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | ddkilzer, mitz, rwlbuis | ||||||
Priority: | P1 | Keywords: | InRadar, Regression | ||||||
Version: | 523.x (Safari 3) | ||||||||
Hardware: | Mac | ||||||||
OS: | OS X 10.4 | ||||||||
URL: | LayoutTests/fast/dom/css-insert-import-rule.html | ||||||||
Attachments: |
|
Description
Grace Kloba
2007-03-05 14:33:07 PST
Confirmed with local debug build of WebKit r19972 with Safari 2.0.4 (419.3) on Mac OS X 10.4.8 (8L127). This is a regression from shipping Safari 2.0.4 (419.3) on Mac OS X 10.4.8 (8L127). Console output on debug build (CrashReporter does not launch!): /path/to/WebKit/WebCore/dom/Document.cpp:1894: failed assertion `m_pendingStylesheets > 0' Abort trap (In reply to comment #1) > Console output on debug build (CrashReporter does not launch!): Evil lowercase assert()s! Created attachment 13568 [details]
Sample fix
The problem is that the pending stylesheet counter and the stylesheet's loadCompleted() flag get out of sync. This patch fixes the test case, but I noticed several other calls to stylesheetLoaded() that need to be tested and possibly fixed...
Created attachment 13573 [details]
Keep loadCompleted() in sync with the pending stylesheet count
Comment on attachment 13573 [details]
Keep loadCompleted() in sync with the pending stylesheet count
r=me
Comment on attachment 13573 [details] Keep loadCompleted() in sync with the pending stylesheet count Why is the m_sheet null check kept in one place, but not the other two? In both methods were the null check is not kept, m_sheet is used previously in the method, but how do we know its value won't change between uses? Not kept: >Index: WebCore/dom/ProcessingInstruction.cpp >- // Tell the doc about the sheet. >- if (!isLoading() && m_sheet) >- document()->stylesheetLoaded(); >+ m_sheet->checkLoaded(); > } Kept: >Index: WebCore/dom/StyleElement.cpp >- if (!isLoading() && m_sheet) >- document->stylesheetLoaded(); >+ if (m_sheet) >+ m_sheet->checkLoaded(); > } Not kept: >Index: WebCore/html/HTMLLinkElement.cpp >- // Tell the doc about the sheet. >- if (!isLoading() && m_sheet && !isDisabled() && !isAlternate()) >- document()->stylesheetLoaded(); >+ m_sheet->checkLoaded(); > } (In reply to comment #7) > Why is the m_sheet null check kept in one place, but not the other two? Because it's needed in that one place (in case the type attribute is wrong or the media doesn't match) and not in the other two. > In > both methods were the null check is not kept, m_sheet is used previously in the > method, but how do we know its value won't change between uses? In HTMLLinkElement it's obvious (setMedia() cannot get m_sheet changed). In ProcessingInstruction it's quite easy to verify that it cannot become 0. Committed revision 20098. |