REGRESSION (NetworkProcess): Trying to use appcache fallback crashes in ApplicationCa...
authorap@apple.com <ap@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 18 May 2014 05:29:36 +0000 (05:29 +0000)
committerap@apple.com <ap@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 18 May 2014 05:29:36 +0000 (05:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=133007
<rdar://problem/13702706>

appcache tests often fail on the Mac WebKit2 bot
https://bugs.webkit.org/show_bug.cgi?id=82061

Reviewed by Maciej Stachowiak.

Source/WebCore:
Covered by existing tests, which this patch enables.

* WebCore.exp.in: Export ResourceLoader::cancel(const ResourceError&). It used
to be virtual, but marking subclasses final has resulted in the compiler calling
it directly from WebKitLegacy framework. Seeing that no subclass overrides it,
I then made it non-virtual.
Also export ApplicationCacheHost functions that are now used from WebKit2.

* loader/ResourceLoader.h: Made the class abstract (as I didn't realize at first
that it's always either a SubresourceLoader or a NetscapePlugInStreamLoader).
Made ResourceHandleClient functions private, as they should never be called other
than via a ResourceHandleClient pointer.

* loader/NetscapePlugInStreamLoader.h:
* loader/SubresourceLoader.h:
Marked these final.

* loader/ResourceLoader.cpp: (WebCore::ResourceLoader::willSwitchToSubstituteResource):
Added a function to be called when switching to a substitute resource. We still
need a ResourceLoader at this point, as substitute resource will be delivered through
it, but we don't want it to continue its current load.

* loader/appcache/ApplicationCacheHost.cpp:
(WebCore::ApplicationCacheHost::scheduleLoadFallbackResourceFromApplicationCache):
Call the ResourceLoader function instead of using handle, which is null when
using out of process networking.

Source/WebKit2:
* WebProcess/Network/WebResourceLoader.cpp:
(WebKit::WebResourceLoader::willSendRequest):
(WebKit::WebResourceLoader::didReceiveResponseWithCertificateInfo):
(WebKit::WebResourceLoader::didFailResourceLoad):
Perform the same appcache checks that ResourceHandleClient implementation in ResourceLoader
does. We should eventually come up with a way to share the code. Perhaps add a class that
isolates ResourceLoader from networking details? But ResourceLoader was itself supposed
to be the class that isolates DocumentLoader from networking details. So, unsure.

LayoutTests:
* platform/mac-wk2/TestExpectations: Let's enable all the appcache tests (except
for a couple that fail), and see what happens on bots. I don't see any reason
why WebKit2 would be any more flaky than WebKit1 here.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@169006 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/platform/mac-wk2/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/WebCore.exp.in
Source/WebCore/loader/NetscapePlugInStreamLoader.h
Source/WebCore/loader/ResourceLoader.cpp
Source/WebCore/loader/ResourceLoader.h
Source/WebCore/loader/SubresourceLoader.h
Source/WebCore/loader/appcache/ApplicationCacheHost.cpp
Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/Network/WebResourceLoader.cpp

index 278d8c3..7fcd85b 100644 (file)
@@ -1,3 +1,18 @@
+2014-05-17  Alexey Proskuryakov  <ap@apple.com>
+
+        REGRESSION (NetworkProcess): Trying to use appcache fallback crashes in ApplicationCacheHost::scheduleLoadFallbackResourceFromApplicationCache
+        https://bugs.webkit.org/show_bug.cgi?id=133007
+        <rdar://problem/13702706>
+
+        appcache tests often fail on the Mac WebKit2 bot
+        https://bugs.webkit.org/show_bug.cgi?id=82061
+
+        Reviewed by Maciej Stachowiak.
+
+        * platform/mac-wk2/TestExpectations: Let's enable all the appcache tests (except
+        for a couple that fail), and see what happens on bots. I don't see any reason
+        why WebKit2 would be any more flaky than WebKit1 here.
+
 2014-05-17  Piotr Grad  <p.grad@samsung.com>
 
         Setting playback rate on video with media controller is not ignored.
index 7a746e3..2c86e4e 100644 (file)
@@ -177,10 +177,6 @@ fast/events/pageshow-pagehide.html
 # https://bugs.webkit.org/show_bug.cgi?id=81833
 fast/events/drag-link.html
 
-# appcache tests often fail on the Mac WebKit2 bot
-# https://bugs.webkit.org/show_bug.cgi?id=82061 rdar://problem/11108649
-http/tests/appcache
-
 # flaky storage tests
 # https://bugs.webkit.org/show_bug.cgi?id=82679
 storage/websql/statement-success-callback-isolated-world.html
@@ -355,6 +351,8 @@ webkit.org/b/131477 fast/repaint/obscured-background-no-repaint.html [ Pass Fail
 # Subpixel wrong cliprect on WK2
 webkit.org/b/132100 fast/forms/hidpi-textarea-on-subpixel-position.html [ ImageOnlyFailure ]
 
+webkit.org/b/93980 http/tests/appcache/load-from-appcache-defer-resume-crash.html [ Skip ]
+
 ### END OF (1) Classified failures with bug reports
 ########################################
 
@@ -376,6 +374,9 @@ editing/spelling/spellcheck-sequencenum.html
 editing/spelling/spelling-insert-html.html
 editing/spelling/spelling-marker-description.html
 
+# This test times out, likely because appcache still loads in WebProcess, not NetworkProcess
+http/tests/appcache/auth.html [ Skip ]
+
 ### END OF (2) Classified failures without bug reports (yet)
 ########################################
 
index 6456f6a..baf68c1 100644 (file)
@@ -1,3 +1,41 @@
+2014-05-17  Alexey Proskuryakov  <ap@apple.com>
+
+        REGRESSION (NetworkProcess): Trying to use appcache fallback crashes in ApplicationCacheHost::scheduleLoadFallbackResourceFromApplicationCache
+        https://bugs.webkit.org/show_bug.cgi?id=133007
+        <rdar://problem/13702706>
+
+        appcache tests often fail on the Mac WebKit2 bot
+        https://bugs.webkit.org/show_bug.cgi?id=82061
+
+        Reviewed by Maciej Stachowiak.
+
+        Covered by existing tests, which this patch enables.
+
+        * WebCore.exp.in: Export ResourceLoader::cancel(const ResourceError&). It used
+        to be virtual, but marking subclasses final has resulted in the compiler calling
+        it directly from WebKitLegacy framework. Seeing that no subclass overrides it,
+        I then made it non-virtual.
+        Also export ApplicationCacheHost functions that are now used from WebKit2.
+
+        * loader/ResourceLoader.h: Made the class abstract (as I didn't realize at first
+        that it's always either a SubresourceLoader or a NetscapePlugInStreamLoader).
+        Made ResourceHandleClient functions private, as they should never be called other
+        than via a ResourceHandleClient pointer.
+
+        * loader/NetscapePlugInStreamLoader.h:
+        * loader/SubresourceLoader.h:
+        Marked these final.
+
+        * loader/ResourceLoader.cpp: (WebCore::ResourceLoader::willSwitchToSubstituteResource):
+        Added a function to be called when switching to a substitute resource. We still
+        need a ResourceLoader at this point, as substitute resource will be delivered through
+        it, but we don't want it to continue its current load.
+
+        * loader/appcache/ApplicationCacheHost.cpp:
+        (WebCore::ApplicationCacheHost::scheduleLoadFallbackResourceFromApplicationCache):
+        Call the ResourceLoader function instead of using handle, which is null when
+        using out of process networking.
+
 2014-05-17  Andreas Kling  <akling@apple.com>
 
         We shouldn't make a ScrollingThread on iOS.
index 4af43c6..fbbfff1 100644 (file)
@@ -377,6 +377,7 @@ __ZN7WebCore14ResourceHandle45continueCanAuthenticateAgainstProtectionSpaceEb
 __ZN7WebCore14ResourceHandle6createEPNS_17NetworkingContextERKNS_15ResourceRequestEPNS_20ResourceHandleClientEbb
 __ZN7WebCore14ResourceLoader14cancelledErrorEv
 __ZN7WebCore14ResourceLoader32didCancelAuthenticationChallengeERKNS_23AuthenticationChallengeE
+__ZN7WebCore14ResourceLoader6cancelERKNS_13ResourceErrorE
 __ZN7WebCore14ResourceLoader6cancelEv
 __ZN7WebCore14SQLiteDatabase11tableExistsERKN3WTF6StringE
 __ZN7WebCore14SQLiteDatabase12lastErrorMsgEv
@@ -824,6 +825,9 @@ __ZN7WebCore19floatValueForLengthERKNS_6LengthEfPNS_10RenderViewE
 __ZN7WebCore19getFileCreationTimeERKN3WTF6StringERl
 __ZN7WebCore19toInt32EnforceRangeEPN3JSC9ExecStateENS0_7JSValueE
 __ZN7WebCore20ApplicationCacheHost17maybeLoadResourceEPNS_14ResourceLoaderERKNS_15ResourceRequestERKNS_3URLE
+__ZN7WebCore20ApplicationCacheHost25maybeLoadFallbackForErrorEPNS_14ResourceLoaderERKNS_13ResourceErrorE
+__ZN7WebCore20ApplicationCacheHost28maybeLoadFallbackForResponseEPNS_14ResourceLoaderERKNS_16ResourceResponseE
+__ZN7WebCore20ApplicationCacheHost28maybeLoadFallbackForRedirectEPNS_14ResourceLoaderERNS_15ResourceRequestERKNS_16ResourceResponseE
 __ZN7WebCore20CachedResourceLoader31garbageCollectDocumentResourcesEv
 __ZN7WebCore20DictationAlternativeC1Ejjy
 __ZN7WebCore20DictationAlternativeC1Ev
index fa03dc0..bf7cc18 100644 (file)
@@ -48,7 +48,7 @@ protected:
     virtual ~NetscapePlugInStreamLoaderClient() { }
 };
 
-class NetscapePlugInStreamLoader : public ResourceLoader {
+class NetscapePlugInStreamLoader final : public ResourceLoader {
 public:
     static PassRefPtr<NetscapePlugInStreamLoader> create(Frame*, NetscapePlugInStreamLoaderClient*, const ResourceRequest&);
     virtual ~NetscapePlugInStreamLoader();
index a90f91b..b9e174b 100644 (file)
@@ -207,6 +207,13 @@ void ResourceLoader::setDataBufferingPolicy(DataBufferingPolicy dataBufferingPol
         m_resourceData = 0;
 }
     
+void ResourceLoader::willSwitchToSubstituteResource()
+{
+    ASSERT(!m_documentLoader->isSubstituteLoadPending(this));
+    platformStrategies()->loaderStrategy()->resourceLoadScheduler()->remove(this);
+    if (m_handle)
+        m_handle->cancel();
+}
 
 void ResourceLoader::addDataOrBuffer(const char* data, unsigned length, SharedBuffer* buffer, DataPayloadType dataPayloadType)
 {
index 2d41fa5..f5de3ee 100644 (file)
@@ -55,7 +55,7 @@ class QuickLookHandle;
 
 class ResourceLoader : public RefCounted<ResourceLoader>, protected ResourceHandleClient {
 public:
-    virtual ~ResourceLoader();
+    virtual ~ResourceLoader() = 0;
 
     void cancel();
 
@@ -69,14 +69,13 @@ public:
     }
 
     virtual const ResourceRequest& iOSOriginalRequest() const { return request(); }
-    virtual RetainPtr<CFDictionaryRef> connectionProperties(ResourceHandle*) override;
 #endif
 
     FrameLoader* frameLoader() const;
     DocumentLoader* documentLoader() const { return m_documentLoader.get(); }
     const ResourceRequest& originalRequest() const { return m_originalRequest; }
     
-    virtual void cancel(const ResourceError&);
+    void cancel(const ResourceError&);
     ResourceError cancelledError();
     ResourceError blockedError();
     ResourceError cannotShowURLError();
@@ -114,46 +113,6 @@ public:
 #endif
     virtual void receivedCancellation(const AuthenticationChallenge&);
 
-    // ResourceHandleClient
-    virtual void willSendRequest(ResourceHandle*, ResourceRequest&, const ResourceResponse& redirectResponse) override;
-    virtual void didSendData(ResourceHandle*, unsigned long long bytesSent, unsigned long long totalBytesToBeSent) override;
-    virtual void didReceiveResponse(ResourceHandle*, const ResourceResponse&) override;
-    virtual void didReceiveData(ResourceHandle*, const char*, unsigned, int encodedDataLength) override;
-    virtual void didReceiveBuffer(ResourceHandle*, PassRefPtr<SharedBuffer>, int encodedDataLength) override;
-    virtual void didFinishLoading(ResourceHandle*, double finishTime) override;
-    virtual void didFail(ResourceHandle*, const ResourceError&) override;
-    virtual void wasBlocked(ResourceHandle*) override;
-    virtual void cannotShowURL(ResourceHandle*) override;
-    virtual bool shouldUseCredentialStorage(ResourceHandle*) override { return shouldUseCredentialStorage(); }
-    virtual void didReceiveAuthenticationChallenge(ResourceHandle*, const AuthenticationChallenge& challenge) override { didReceiveAuthenticationChallenge(challenge); } 
-    virtual void didCancelAuthenticationChallenge(ResourceHandle*, const AuthenticationChallenge& challenge) override { didCancelAuthenticationChallenge(challenge); } 
-    virtual void receivedCancellation(ResourceHandle*, const AuthenticationChallenge& challenge) override { receivedCancellation(challenge); }
-
-#if USE(NETWORK_CFDATA_ARRAY_CALLBACK)
-    virtual void didReceiveDataArray(ResourceHandle*, CFArrayRef dataArray) override;
-#endif
-
-#if USE(PROTECTION_SPACE_AUTH_CALLBACK)
-    virtual bool canAuthenticateAgainstProtectionSpace(ResourceHandle*, const ProtectionSpace& protectionSpace) override { return canAuthenticateAgainstProtectionSpace(protectionSpace); }
-#endif
-
-#if PLATFORM(COCOA) && USE(CFNETWORK)
-    virtual CFCachedURLResponseRef willCacheResponse(ResourceHandle*, CFCachedURLResponseRef) override;
-#endif
-
-#if PLATFORM(WIN) && USE(CFNETWORK)
-    // FIXME: Windows should use willCacheResponse - <https://bugs.webkit.org/show_bug.cgi?id=57257>.
-    virtual bool shouldCacheResponse(ResourceHandle*, CFCachedURLResponseRef) override;
-#endif
-
-#if PLATFORM(COCOA) && !USE(CFNETWORK)
-    virtual NSCachedURLResponse* willCacheResponse(ResourceHandle*, NSCachedURLResponse*) override;
-#endif
-
-#if USE(QUICK_LOOK)
-    virtual void didCreateQuickLookHandle(QuickLookHandle&) override;
-#endif
-
     const URL& url() const { return m_request.url(); }
     ResourceHandle* handle() const { return m_handle.get(); }
     bool shouldSendResourceLoadCallbacks() const { return m_options.sendLoadCallbacks == SendCallbacks; }
@@ -167,6 +126,8 @@ public:
 
     void setDataBufferingPolicy(DataBufferingPolicy);
 
+    void willSwitchToSubstituteResource();
+
 #if PLATFORM(MAC)
     void schedule(WTF::SchedulePair&);
     void unschedule(WTF::SchedulePair&);
@@ -200,6 +161,43 @@ private:
 
     void addDataOrBuffer(const char*, unsigned, SharedBuffer*, DataPayloadType);
 
+    // ResourceHandleClient
+    virtual void willSendRequest(ResourceHandle*, ResourceRequest&, const ResourceResponse& redirectResponse) override;
+    virtual void didSendData(ResourceHandle*, unsigned long long bytesSent, unsigned long long totalBytesToBeSent) override;
+    virtual void didReceiveResponse(ResourceHandle*, const ResourceResponse&) override;
+    virtual void didReceiveData(ResourceHandle*, const char*, unsigned, int encodedDataLength) override;
+    virtual void didReceiveBuffer(ResourceHandle*, PassRefPtr<SharedBuffer>, int encodedDataLength) override;
+    virtual void didFinishLoading(ResourceHandle*, double finishTime) override;
+    virtual void didFail(ResourceHandle*, const ResourceError&) override;
+    virtual void wasBlocked(ResourceHandle*) override;
+    virtual void cannotShowURL(ResourceHandle*) override;
+#if USE(NETWORK_CFDATA_ARRAY_CALLBACK)
+    virtual void didReceiveDataArray(ResourceHandle*, CFArrayRef dataArray) override;
+#endif
+    virtual bool shouldUseCredentialStorage(ResourceHandle*) override { return shouldUseCredentialStorage(); }
+    virtual void didReceiveAuthenticationChallenge(ResourceHandle*, const AuthenticationChallenge& challenge) override { didReceiveAuthenticationChallenge(challenge); } 
+    virtual void didCancelAuthenticationChallenge(ResourceHandle*, const AuthenticationChallenge& challenge) override { didCancelAuthenticationChallenge(challenge); } 
+#if USE(PROTECTION_SPACE_AUTH_CALLBACK)
+    virtual bool canAuthenticateAgainstProtectionSpace(ResourceHandle*, const ProtectionSpace& protectionSpace) override { return canAuthenticateAgainstProtectionSpace(protectionSpace); }
+#endif
+    virtual void receivedCancellation(ResourceHandle*, const AuthenticationChallenge& challenge) override { receivedCancellation(challenge); }
+#if PLATFORM(COCOA) && !USE(CFNETWORK)
+    virtual NSCachedURLResponse* willCacheResponse(ResourceHandle*, NSCachedURLResponse*) override;
+#endif
+#if PLATFORM(COCOA) && USE(CFNETWORK)
+    virtual CFCachedURLResponseRef willCacheResponse(ResourceHandle*, CFCachedURLResponseRef) override;
+#endif
+#if PLATFORM(IOS)
+    virtual RetainPtr<CFDictionaryRef> connectionProperties(ResourceHandle*) override;
+#endif
+#if PLATFORM(WIN) && USE(CFNETWORK)
+    // FIXME: Windows should use willCacheResponse - <https://bugs.webkit.org/show_bug.cgi?id=57257>.
+    virtual bool shouldCacheResponse(ResourceHandle*, CFCachedURLResponseRef) override;
+#endif
+#if USE(QUICK_LOOK)
+    virtual void didCreateQuickLookHandle(QuickLookHandle&) override;
+#endif
+
     ResourceRequest m_request;
     ResourceRequest m_originalRequest; // Before redirects.
     RefPtr<ResourceBuffer> m_resourceData;
index 93cc41c..5f57382 100644 (file)
@@ -42,7 +42,7 @@ class Document;
 class PageActivityAssertionToken;
 class ResourceRequest;
 
-class SubresourceLoader : public ResourceLoader {
+class SubresourceLoader final : public ResourceLoader {
 public:
     static PassRefPtr<SubresourceLoader> create(Frame*, CachedResource*, const ResourceRequest&, const ResourceLoaderOptions&);
 
index c8f0814..9910679 100644 (file)
@@ -399,10 +399,10 @@ bool ApplicationCacheHost::scheduleLoadFallbackResourceFromApplicationCache(Reso
     if (!getApplicationCacheFallbackResource(loader->request(), resource, cache))
         return false;
 
+    loader->willSwitchToSubstituteResource();
+
     m_documentLoader->m_pendingSubstituteResources.set(loader, resource);
     m_documentLoader->deliverSubstituteResourcesAfterDelay();
-    
-    loader->handle()->cancel();
 
     return true;
 }
index 7d92b40..fc09d3d 100644 (file)
@@ -1,3 +1,23 @@
+2014-05-17  Alexey Proskuryakov  <ap@apple.com>
+
+        REGRESSION (NetworkProcess): Trying to use appcache fallback crashes in ApplicationCacheHost::scheduleLoadFallbackResourceFromApplicationCache
+        https://bugs.webkit.org/show_bug.cgi?id=133007
+        <rdar://problem/13702706>
+
+        appcache tests often fail on the Mac WebKit2 bot
+        https://bugs.webkit.org/show_bug.cgi?id=82061
+
+        Reviewed by Maciej Stachowiak.
+
+        * WebProcess/Network/WebResourceLoader.cpp:
+        (WebKit::WebResourceLoader::willSendRequest):
+        (WebKit::WebResourceLoader::didReceiveResponseWithCertificateInfo):
+        (WebKit::WebResourceLoader::didFailResourceLoad):
+        Perform the same appcache checks that ResourceHandleClient implementation in ResourceLoader
+        does. We should eventually come up with a way to share the code. Perhaps add a class that
+        isolates ResourceLoader from networking details? But ResourceLoader was itself supposed
+        to be the class that isolates DocumentLoader from networking details. So, unsure.
+
 2014-05-17  Jae Hyun Park  <jaepark@webkit.org>
 
         [EFL] Remove m_contentPosition from PageViewportControllerClientEfl
index 5534344..719061c 100644 (file)
@@ -35,6 +35,7 @@
 #include "WebCoreArgumentCoders.h"
 #include "WebErrors.h"
 #include "WebProcess.h"
+#include <WebCore/ApplicationCacheHost.h>
 #include <WebCore/CertificateInfo.h>
 #include <WebCore/DocumentLoader.h>
 #include <WebCore/ResourceBuffer.h>
@@ -84,8 +85,10 @@ void WebResourceLoader::willSendRequest(const ResourceRequest& proposedRequest,
     LOG(Network, "(WebProcess) WebResourceLoader::willSendRequest to '%s'", proposedRequest.url().string().utf8().data());
 
     Ref<WebResourceLoader> protect(*this);
-    
+
     ResourceRequest newRequest = proposedRequest;
+    if (m_coreLoader->documentLoader()->applicationCacheHost()->maybeLoadFallbackForRedirect(m_coreLoader.get(), newRequest, redirectResponse))
+        return;
     m_coreLoader->willSendRequest(newRequest, redirectResponse);
     
     if (!m_coreLoader)
@@ -120,6 +123,8 @@ void WebResourceLoader::didReceiveResponseWithCertificateInfo(const ResourceResp
     responseCopy.setSoupMessageCertificate(certificateInfo.certificate());
     responseCopy.setSoupMessageTLSErrors(certificateInfo.tlsErrors());
 #endif
+    if (m_coreLoader->documentLoader()->applicationCacheHost()->maybeLoadFallbackForResponse(m_coreLoader.get(), responseCopy))
+        return;
     m_coreLoader->didReceiveResponse(responseCopy);
 
     // If m_coreLoader becomes null as a result of the didReceiveResponse callback, we can't use the send function(). 
@@ -163,6 +168,8 @@ void WebResourceLoader::didFailResourceLoad(const ResourceError& error)
     if (m_quickLookHandle)
         m_quickLookHandle->didFail();
 #endif
+    if (m_coreLoader->documentLoader()->applicationCacheHost()->maybeLoadFallbackForError(m_coreLoader.get(), error))
+        return;
     m_coreLoader->didFail(error);
 }