[webkit-changes] [147262] tags/Safari-537.35.4/Source/WebKit2

Fri, 29 Mar 2013 16:40:01 -0700

Title: [147262] tags/Safari-537.35.4/Source/WebKit2

Diff

Modified: tags/Safari-537.35.4/Source/WebKit2/ChangeLog (147261 => 147262)


--- tags/Safari-537.35.4/Source/WebKit2/ChangeLog	2013-03-29 23:32:46 UTC (rev 147261)
+++ tags/Safari-537.35.4/Source/WebKit2/ChangeLog	2013-03-29 23:41:54 UTC (rev 147262)
@@ -1,5 +1,32 @@
 2013-03-29  Lucas Forschler  <[email protected]>
 
+        Merge r147257
+
+    2013-03-29  Brady Eidson  <[email protected]>
+
+            Crash when "willSendRequest" causes the ResourceLoader to be cancelled.
+            <rdar://problem/13531679> and https://bugs.webkit.org/show_bug.cgi?id=113616
+
+            Reviewed by Alexey Proskuryakov.
+
+            These callbacks to the WebCore ResourceLoader can cause the WebResourceLoader to be destroyed.
+            A RefPtr<> protector fixes that.
+
+            Additionally we can invalidate the WebResourceLoader to avoid unnecessary callbacks to the NetworkProcess.
+
+            * WebProcess/Network/WebResourceLoadScheduler.cpp:
+            (WebKit::WebResourceLoadScheduler::remove): Call detachFromCoreLoader() on a removed WebResourceLoader.
+
+            * WebProcess/Network/WebResourceLoader.cpp:
+            (WebKit::WebResourceLoader::detachFromCoreLoader): Clear out the ResourceLoader pointer.
+            (WebKit::WebResourceLoader::willSendRequest): Protect this, and don't message back to the NetworkProcess if its not needed.
+            (WebKit::WebResourceLoader::canAuthenticateAgainstProtectionSpace): Ditto
+            (WebKit::WebResourceLoader::didReceiveResource): Paranoid hardening - Protect this before delivering the data to the WebCore
+              ResourceLoader, and null check it before delivering the "didFinishLoader" call.
+            * WebProcess/Network/WebResourceLoader.h:
+
+2013-03-29  Lucas Forschler  <[email protected]>
+
         Merge r147179
 
     2013-03-28  Brady Eidson  <[email protected]>

Modified: tags/Safari-537.35.4/Source/WebKit2/WebProcess/Network/WebResourceLoadScheduler.cpp (147261 => 147262)


--- tags/Safari-537.35.4/Source/WebKit2/WebProcess/Network/WebResourceLoadScheduler.cpp	2013-03-29 23:32:46 UTC (rev 147261)
+++ tags/Safari-537.35.4/Source/WebKit2/WebProcess/Network/WebResourceLoadScheduler.cpp	2013-03-29 23:41:54 UTC (rev 147262)
@@ -155,8 +155,12 @@
     // If a resource load was actually started within the NetworkProcess then the NetworkProcess handles clearing out the identifier.
     WebProcess::shared().networkConnection()->connection()->send(Messages::NetworkConnectionToWebProcess::RemoveLoadIdentifier(identifier), 0);
     
-    ASSERT(m_webResourceLoaders.contains(identifier));
-    m_webResourceLoaders.remove(identifier);
+    RefPtr<WebResourceLoader> loader = m_webResourceLoaders.take(identifier);
+    ASSERT(loader);
+
+    // It's possible that this WebResourceLoader might be just about to message back to the NetworkProcess (e.g. ContinueWillSendRequest)
+    // but there's no point in doing so anymore.
+    loader->detachFromCoreLoader();
 }
 
 void WebResourceLoadScheduler::crossOriginRedirectReceived(ResourceLoader*, const KURL&)

Modified: tags/Safari-537.35.4/Source/WebKit2/WebProcess/Network/WebResourceLoader.cpp (147261 => 147262)


--- tags/Safari-537.35.4/Source/WebKit2/WebProcess/Network/WebResourceLoader.cpp	2013-03-29 23:32:46 UTC (rev 147261)
+++ tags/Safari-537.35.4/Source/WebKit2/WebProcess/Network/WebResourceLoader.cpp	2013-03-29 23:41:54 UTC (rev 147262)
@@ -73,12 +73,23 @@
     m_coreLoader->cancel();
 }
 
+void WebResourceLoader::detachFromCoreLoader()
+{
+    m_coreLoader = 0;
+}
+
 void WebResourceLoader::willSendRequest(const ResourceRequest& proposedRequest, const ResourceResponse& redirectResponse)
 {
     LOG(Network, "(WebProcess) WebResourceLoader::willSendRequest to '%s'", proposedRequest.url().string().utf8().data());
+
+    RefPtr<WebResourceLoader> protector(this);
     
     ResourceRequest newRequest = proposedRequest;
     m_coreLoader->willSendRequest(newRequest, redirectResponse);
+    
+    if (!m_coreLoader)
+        return;
+    
     send(Messages::NetworkResourceLoader::ContinueWillSendRequest(newRequest));
 }
 
@@ -120,16 +131,27 @@
         return;
     }
 
+    RefPtr<WebResourceLoader> protector(this);
+
     // Only send data to the didReceiveData callback if it exists.
     if (buffer->size())
         m_coreLoader->didReceiveBuffer(buffer.get(), buffer->size(), DataPayloadWholeResource);
 
+    if (!m_coreLoader)
+        return;
+
     m_coreLoader->didFinishLoading(finishTime);
 }
 
 void WebResourceLoader::canAuthenticateAgainstProtectionSpace(const ProtectionSpace& protectionSpace)
 {
+    RefPtr<WebResourceLoader> protector(this);
+
     bool result = m_coreLoader->canAuthenticateAgainstProtectionSpace(protectionSpace);
+
+    if (!m_coreLoader)
+        return;
+
     send(Messages::NetworkResourceLoader::ContinueCanAuthenticateAgainstProtectionSpace(result));
 }

Modified: tags/Safari-537.35.4/Source/WebKit2/WebProcess/Network/WebResourceLoader.h (147261 => 147262)


--- tags/Safari-537.35.4/Source/WebKit2/WebProcess/Network/WebResourceLoader.h	2013-03-29 23:32:46 UTC (rev 147261)
+++ tags/Safari-537.35.4/Source/WebKit2/WebProcess/Network/WebResourceLoader.h	2013-03-29 23:41:54 UTC (rev 147262)
@@ -67,6 +67,8 @@
 
     WebCore::ResourceLoader* resourceLoader() const { return m_coreLoader.get(); }
 
+    void detachFromCoreLoader();
+
 private:
     WebResourceLoader(PassRefPtr<WebCore::ResourceLoader>);
 

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to