Changeset 86478 in webkit

Timestamp:

May 13, 2011 9:20:04 PM (13 years ago)

Author:

ap@apple.com

Message:

2011-05-13 Alexey Proskuryakov <​ap@apple.com>

Reviewed by Darin Adler.

HTML5 Application Cache Update Failures on Page Refresh
​https://bugs.webkit.org/show_bug.cgi?id=60598
<rdar://problem/9418639>

• http/tests/appcache/interrupted-update-expected.txt: Added.
• http/tests/appcache/interrupted-update.html: Added.
• http/tests/appcache/resources/interrupted-update.manifest: Added.

• http/tests/appcache/main-resource-redirect.html:
• http/tests/appcache/non-html.xhtml: Now that we dispatch an error even when aborting a load due to navigation, error event handler was getting called after notifyDone().

2011-05-13 Alexey Proskuryakov <​ap@apple.com>

Reviewed by Darin Adler.

HTML5 Application Cache Update Failures on Page Refresh
​https://bugs.webkit.org/show_bug.cgi?id=60598
<rdar://problem/9418639>

Test: http/tests/appcache/interrupted-update.html

The reason was two-fold:
1) Cache group state wasn't fully reset after a failed cache update (or an initial load).
2) The cache group is not deleted until after after the navigation (from autorelease pool),
so the failed and sorry old version was picked up by cache selection algorithm.

The latter is more of a surprising behavior than a problem in itself. Keeping an appcache in
memory for reload is generally a good thing for performance.

• loader/appcache/ApplicationCacheGroup.cpp: (WebCore::ApplicationCacheGroup::stopLoading): Added a FIXME about half-baked state resetting code that sent me looking for a wrong fix originally. (WebCore::ApplicationCacheGroup::stopLoadingInFrame): Stopped load is an update failure like any other, so we should call cacheUpdateFailed(), which knows how to reset state.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/http/tests/appcache/interrupted-update-expected.txt (added)
• LayoutTests/http/tests/appcache/interrupted-update.html (added)
• LayoutTests/http/tests/appcache/main-resource-redirect.html (modified) (1 diff)
• LayoutTests/http/tests/appcache/non-html.xhtml (modified) (2 diffs)
• LayoutTests/http/tests/appcache/resources/interrupted-update.manifest (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2011-05-13  Alexey Proskuryakov  <ap@apple.com>
+
+        Reviewed by Darin Adler.
+
+        HTML5 Application Cache Update Failures on Page Refresh
+        https://bugs.webkit.org/show_bug.cgi?id=60598
+        <rdar://problem/9418639>
+
+        * http/tests/appcache/interrupted-update-expected.txt: Added.
+        * http/tests/appcache/interrupted-update.html: Added.
+        * http/tests/appcache/resources/interrupted-update.manifest: Added.
+
+        * http/tests/appcache/main-resource-redirect.html:
+        * http/tests/appcache/non-html.xhtml:
+        Now that we dispatch an error even when aborting a load due to navigation, error event
+        handler was getting called after notifyDone().
+
 2011-05-13  Anders Carlsson  <andersca@apple.com>
 

trunk/LayoutTests/http/tests/appcache/main-resource-redirect.html

@@ -28 +28 @@
 function done()
 {
-    if (window.layoutTestController)
+    if (window.layoutTestController) {
+        applicationCache.onerror = null;
         layoutTestController.notifyDone();
+    }
     setNetworkEnabled(true);
 }

trunk/LayoutTests/http/tests/appcache/non-html.xhtml

@@ -83 +83 @@
         log("FAIL: Frame.onload was called, but the image doesn't seem to be loaded.");
     
-    if (window.layoutTestController)
+    if (window.layoutTestController) {
+        applicationCache.onerror = null;
         layoutTestController.notifyDone();
+    }
 }
 
@@ -97 +99 @@
 applicationCache.addEventListener('noupdate', cached, false);
 
-applicationCache.addEventListener('error', error, false);
+applicationCache.onerror = error;
 
 </script>

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-05-13  Alexey Proskuryakov  <ap@apple.com>
+
+        Reviewed by Darin Adler.
+
+        HTML5 Application Cache Update Failures on Page Refresh
+        https://bugs.webkit.org/show_bug.cgi?id=60598
+        <rdar://problem/9418639>
+
+        Test: http/tests/appcache/interrupted-update.html
+
+        The reason was two-fold:
+        1) Cache group state wasn't fully reset after a failed cache update (or an initial load).
+        2) The cache group is not deleted until after after the navigation (from autorelease pool),
+        so the failed and sorry old version was picked up by cache selection algorithm.
+
+        The latter is more of a surprising behavior than a problem in itself. Keeping an appcache in
+        memory for reload is generally a good thing for performance.
+
+        * loader/appcache/ApplicationCacheGroup.cpp:
+        (WebCore::ApplicationCacheGroup::stopLoading): Added a FIXME about half-baked state resetting
+        code that sent me looking for a wrong fix originally.
+        (WebCore::ApplicationCacheGroup::stopLoadingInFrame): Stopped load is an update failure like
+        any other, so we should call cacheUpdateFailed(), which knows how to reset state.
+
 2011-05-13  Enrica Casucci  <enrica@apple.com>
 

trunk/Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp

@@ -346 +346 @@
         m_currentHandle = 0;
     }    
-    
+
+    // FIXME: Resetting just a tiny part of the state in this function is confusing. Callers have to take care of a lot more.
     m_cacheBeingUpdated = 0;
     m_pendingEntries.clear();
@@ -398 +399 @@
         return;
 
-    stopLoading();
+    cacheUpdateFailed();
 }