Changeset 30587 in webkit

Timestamp:

Feb 25, 2008 8:25:25 PM (16 years ago)

Author:

Beth Dakin

Message:

WebCore:

Reviewed by Geoff.

Fixed for <rdar://problem/5760530> REGRESSION: Assertion failure in
Document::removePendingSheet() from r30438

r30438 added a call to CachedResource::error() from inside
Loader::didReceiveData() if a CSS file 4xxs. There was an
assumption in the loader, though, that either error() would be
called, or didFinishLoading() would be called, so some work is
duplicated in each. Now that we are calling an error() on files
that will also make it to didFinishLoading() (since they succeeded
in the network layer), we need to make sure we do not duplicate the
work. CachedCSSStyleSheet::error() calls checkNotify, which ends up
decrementing the document's pending style sheet counter.
checkNotify() was still getting called, though, through the normal
didFinishLoading code path, and the counter was being decremented
twice. Bad!

• loader/loader.cpp: (WebCore::Loader::didFinishLoading): (WebCore::Loader::didReceiveData):

LayoutTests:

Reviewed by Geoff.

Test for <rdar://problem/5760530> REGRESSION: Assertion failure in
Document::removePendingSheet() from r30438

• http/tests/misc/missing-style-sheet-expected.txt: Added.
• http/tests/misc/missing-style-sheet.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/http/tests/misc/missing-style-sheet-expected.txt (added)
• LayoutTests/http/tests/misc/missing-style-sheet.html (added)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/WebCore.xcodeproj/project.pbxproj (modified) (2 diffs)
• WebCore/loader/loader.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2008-02-25  Beth Dakin  <bdakin@apple.com>
+
+        Reviewed by Geoff.
+
+        Test for <rdar://problem/5760530> REGRESSION: Assertion failure in 
+        Document::removePendingSheet() from r30438
+
+        * http/tests/misc/missing-style-sheet-expected.txt: Added.
+        * http/tests/misc/missing-style-sheet.html: Added.
+
 2008-02-25  Brady Eidson  <beidson@apple.com>
 

trunk/WebCore/ChangeLog

@@ +1 @@
+2008-02-25  Beth Dakin  <bdakin@apple.com>
+
+        Reviewed by Geoff.
+
+        Fixed for <rdar://problem/5760530> REGRESSION: Assertion failure in 
+        Document::removePendingSheet() from r30438
+
+        r30438 added a call to CachedResource::error() from inside 
+        Loader::didReceiveData() if a CSS file 4xxs. There was an 
+        assumption in the loader, though, that either error() would be 
+        called, or didFinishLoading() would be called, so some work is 
+        duplicated in each. Now that we are calling an error() on files 
+        that will also make it to didFinishLoading() (since they succeeded 
+        in the network layer), we need to make sure we do not duplicate the 
+        work. CachedCSSStyleSheet::error() calls checkNotify, which ends up 
+        decrementing the document's pending style sheet counter. 
+        checkNotify() was still getting called, though, through the normal  
+        didFinishLoading code path, and the counter was being decremented 
+        twice. Bad!
+
+        * loader/loader.cpp:
+        (WebCore::Loader::didFinishLoading):
+        (WebCore::Loader::didReceiveData):
+
 2008-02-25  Mark Rowe  <mrowe@apple.com>
 

trunk/WebCore/WebCore.xcodeproj/project.pbxproj

@@ -14308 +14308 @@
                         isa = PBXProject;
                         buildConfigurationList = 149C284308902B11008A9EFC /* Build configuration list for PBXProject "WebCore" */;
+                        compatibilityVersion = "Xcode 2.4";
                         hasScannedForEncodings = 1;
                         knownRegions = (
@@ -14321 +14322 @@
                         productRefGroup = 034768DFFF38A50411DB9C8B /* Products */;
                         projectDirPath = "";
+                        projectRoot = "";
                         targets = (
                                 93F198A508245E59001E9ABC /* WebCore */,

trunk/WebCore/loader/loader.cpp

@@ -110 +110 @@
     CachedResource* object = req->cachedResource();
 
-    docLoader->setLoadInProgress(true);
-    object->data(loader->resourceData(), true);
-    docLoader->setLoadInProgress(false);
-    object->finish();
+    // If we got a 4xx response, we're pretending to have received a network
+    // error, so we can't send the successful data() and finish() callbacks.
+    if (!object->errorOccurred()) {
+        docLoader->setLoadInProgress(true);
+        object->data(loader->resourceData(), true);
+        docLoader->setLoadInProgress(false);
+        object->finish();
+    }
 
     delete req;
@@ -199 +203 @@
     
     if (object->response().httpStatusCode() / 100 == 4) {
-        // Make sure the 4xx error codes result in an error.
+        // Treat a 4xx response like a network error.
         object->error();
         return;