Changeset 39474 in webkit

Timestamp:

Dec 25, 2008 1:12:07 PM (15 years ago)

Author:

ap@webkit.org

Message:

Reviewed by Darin Adler.

​https://bugs.webkit.org/show_bug.cgi?id=22993
Application cache shouldn't be inherited by subframes

<rdar://problem/6284708> AppCache crashes in ApplicationCacheResource::addType()
This crash happened because main resource for subframe was looked up in top frame's
appcache. If not for the spec change, it could have been fixed by preventing the load
altogether.

Test: http/tests/appcache/foreign-iframe-main.html
I intend to write additional tests for the behavior change.

• loader/DocumentLoader.cpp: (WebCore::DocumentLoader::shouldLoadResourceFromApplicationCache):
• loader/DocumentLoader.h: (WebCore::DocumentLoader::applicationCache):
• loader/MainResourceLoader.cpp: (WebCore::MainResourceLoader::load):
• loader/appcache/DOMApplicationCache.cpp: (WebCore::DOMApplicationCache::associatedCache): Removed DocumentLoader::topLevelApplicationCache(), and changed callers accordingly.

• loader/appcache/ApplicationCacheGroup.cpp: (WebCore::ApplicationCacheGroup::selectCache): (WebCore::ApplicationCacheGroup::selectCacheWithoutManifestURL): Removed checks for the frame being top-level one, now that subframes are cached independently.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/http/tests/appcache/foreign-iframe-main-expected.txt (added)
• LayoutTests/http/tests/appcache/foreign-iframe-main.html (added)
• LayoutTests/http/tests/appcache/resources/foreign-iframe-subresource.js (added)
• LayoutTests/http/tests/appcache/resources/foreign-iframe.html (added)
• LayoutTests/http/tests/appcache/resources/foreign-iframe.manifest (added)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/loader/DocumentLoader.cpp (modified) (2 diffs)
• WebCore/loader/DocumentLoader.h (modified) (1 diff)
• WebCore/loader/MainResourceLoader.cpp (modified) (1 diff)
• WebCore/loader/appcache/ApplicationCacheGroup.cpp (modified) (2 diffs)
• WebCore/loader/appcache/DOMApplicationCache.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2008-12-25  Alexey Proskuryakov  <ap@webkit.org>
+
+        Reviewed by Darin Adler.
+
+        https://bugs.webkit.org/show_bug.cgi?id=22993
+        Application cache shouldn't be inherited by subframes
+
+        <rdar://problem/6284708> AppCache crashes in ApplicationCacheResource::addType()
+
+        * http/tests/appcache/foreign-iframe-main-expected.txt: Added.
+        * http/tests/appcache/foreign-iframe-main.html: Added.
+        * http/tests/appcache/resources/foreign-iframe-subresource.js: Added.
+        * http/tests/appcache/resources/foreign-iframe.html: Added.
+        * http/tests/appcache/resources/foreign-iframe.manifest: Added.
+
 2008-12-24  Simon Fraser  <simon.fraser@apple.com>
 

trunk/WebCore/ChangeLog

@@ +1 @@
+2008-12-25  Alexey Proskuryakov  <ap@webkit.org>
+
+        Reviewed by Darin Adler.
+
+        https://bugs.webkit.org/show_bug.cgi?id=22993
+        Application cache shouldn't be inherited by subframes
+
+        <rdar://problem/6284708> AppCache crashes in ApplicationCacheResource::addType()
+        This crash happened because main resource for subframe was looked up in top frame's
+        appcache. If not for the spec change, it could have been fixed by preventing the load
+        altogether.
+
+        Test: http/tests/appcache/foreign-iframe-main.html
+        I intend to write additional tests for the behavior change.
+
+        * loader/DocumentLoader.cpp:
+        (WebCore::DocumentLoader::shouldLoadResourceFromApplicationCache):
+        * loader/DocumentLoader.h:
+        (WebCore::DocumentLoader::applicationCache):
+        * loader/MainResourceLoader.cpp:
+        (WebCore::MainResourceLoader::load):
+        * loader/appcache/DOMApplicationCache.cpp:
+        (WebCore::DOMApplicationCache::associatedCache):
+        Removed DocumentLoader::topLevelApplicationCache(), and changed callers accordingly.
+
+        * loader/appcache/ApplicationCacheGroup.cpp:
+        (WebCore::ApplicationCacheGroup::selectCache):
+        (WebCore::ApplicationCacheGroup::selectCacheWithoutManifestURL):
+        Removed checks for the frame being top-level one, now that subframes are cached independently.
+
 2008-12-25  Antti Koivisto  <antti@apple.com>
 

trunk/WebCore/loader/DocumentLoader.cpp

@@ -853 +853 @@
 }
 
-ApplicationCache* DocumentLoader::topLevelApplicationCache() const
-{
-    if (!m_frame)
-        return 0;
-    
-    if (m_applicationCache)
-        return m_applicationCache.get();
-    
-    if (Page* page = m_frame->page())
-        return page->mainFrame()->loader()->documentLoader()->applicationCache();
-    
-    return 0;
-}
-
 ApplicationCache* DocumentLoader::mainResourceApplicationCache() const
 {
@@ -878 +864 @@
 bool DocumentLoader::shouldLoadResourceFromApplicationCache(const ResourceRequest& request, ApplicationCacheResource*& resource)
 {
-    ApplicationCache* cache = topLevelApplicationCache();    
+    ApplicationCache* cache = applicationCache();
     if (!cache)
         return false;

trunk/WebCore/loader/DocumentLoader.h

@@ -206 +206 @@
         void setApplicationCache(PassRefPtr<ApplicationCache> applicationCache);
         ApplicationCache* applicationCache() const { return m_applicationCache.get(); }
-        ApplicationCache* topLevelApplicationCache() const;
 
         ApplicationCache* mainResourceApplicationCache() const;

trunk/WebCore/loader/MainResourceLoader.cpp

@@ -434 +434 @@
         ASSERT(!m_applicationCache);
 
-        if (Page* page = frameLoader()->frame()->page()) {
-            if (frameLoader()->frame() == page->mainFrame())
-                m_applicationCache = ApplicationCacheGroup::cacheForMainRequest(r, m_documentLoader.get());
-            else
-                m_applicationCache = frameLoader()->documentLoader()->topLevelApplicationCache();
-        }
+        m_applicationCache = ApplicationCacheGroup::cacheForMainRequest(r, m_documentLoader.get());
             
         if (m_applicationCache) {

trunk/WebCore/loader/appcache/ApplicationCacheGroup.cpp

@@ -113 +113 @@
     ApplicationCache* mainResourceCache = documentLoader->mainResourceApplicationCache();
     
-    // Check if the main resource is being loaded as part of navigation of the main frame
-    bool isMainFrame = frame->page()->mainFrame() == frame;
-    
-    if (!isMainFrame) {
-        if (mainResourceCache && manifestURL != mainResourceCache->group()->manifestURL()) {
-            ApplicationCacheResource* resource = mainResourceCache->resourceForURL(documentLoader->originalURL());
-            ASSERT(resource);
-            
-            resource->addType(ApplicationCacheResource::Foreign);
-        }
-
-        return;
-    }
-    
     if (mainResourceCache) {
         if (manifestURL == mainResourceCache->group()->m_manifestURL) {
@@ -201 +187 @@
 
     ApplicationCache* mainResourceCache = documentLoader->mainResourceApplicationCache();
-    bool isMainFrame = frame->page()->mainFrame() == frame;
-
-    if (isMainFrame && mainResourceCache) {
+
+    if (mainResourceCache) {
         mainResourceCache->group()->associateDocumentLoaderWithCache(documentLoader, mainResourceCache);
         mainResourceCache->group()->update(frame);

trunk/WebCore/loader/appcache/DOMApplicationCache.cpp

@@ -56 +56 @@
         return 0;
  
-    return m_frame->loader()->documentLoader()->topLevelApplicationCache();
+    return m_frame->loader()->documentLoader()->applicationCache();
 }