WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
12973
REGRESSION: Reproducible assert while loading this test file if css is already in the cache
https://bugs.webkit.org/show_bug.cgi?id=12973
Summary
REGRESSION: Reproducible assert while loading this test file if css is alread...
Grace Kloba
Reported
2007-03-05 14:33:07 PST
Load LayoutTests/fast/dom/css-insert-import-rule.html first to get the resource in the cache. Change address to be LayoutTests/fast/dom/css-insert-import-rule.txt. As there is no existing file, you should see an error page. Now change address back to LayoutTests/fast/dom/css-insert-import-rule.html, you should see assertion like this, WebCore/dom/Document.cpp:1878: failed assertion `m_pendingStylesheets > 0'
Attachments
Sample fix
(384 bytes, patch)
2007-03-09 15:01 PST
,
mitz
no flags
Details
Formatted Diff
Diff
Keep loadCompleted() in sync with the pending stylesheet count
(11.51 KB, patch)
2007-03-10 02:43 PST
,
mitz
darin
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
David Kilzer (:ddkilzer)
Comment 1
2007-03-05 21:00:55 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
Mark Rowe (bdash)
Comment 2
2007-03-07 06:47:33 PST
<
rdar://problem/5045718
>
mitz
Comment 3
2007-03-09 13:45:09 PST
(In reply to
comment #1
)
> Console output on debug build (CrashReporter does not launch!):
Evil lowercase assert()s!
mitz
Comment 4
2007-03-09 15:01:59 PST
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...
mitz
Comment 5
2007-03-10 02:43:54 PST
Created
attachment 13573
[details]
Keep loadCompleted() in sync with the pending stylesheet count
Darin Adler
Comment 6
2007-03-10 07:16:07 PST
Comment on
attachment 13573
[details]
Keep loadCompleted() in sync with the pending stylesheet count r=me
David Kilzer (:ddkilzer)
Comment 7
2007-03-10 12:09:39 PST
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(); > }
mitz
Comment 8
2007-03-10 12:37:04 PST
(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.
David Kilzer (:ddkilzer)
Comment 9
2007-03-10 14:57:05 PST
Committed revision 20098.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug