Cancel navigation policy checks like we do content policy checks.

https://bugs.webkit.org/show_bug.cgi?id=150582
rdar://problem/22077579

Reviewed by Brent Fulgham.

This was verified manually and I'll write a layout test for it soon.

* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::DocumentLoader):
(WebCore::DocumentLoader::~DocumentLoader):
(WebCore::DocumentLoader::willSendRequest):
(WebCore::DocumentLoader::continueAfterNavigationPolicy):
(WebCore::DocumentLoader::cancelPolicyCheckIfNeeded):
* loader/DocumentLoader.h:
Add a bool to keep track of whether we are waiting for navigation policy checks, like we do with content policy checks.
Without this check, sometimes callbacks are made to DocumentLoaders that do not exist any more because they do not get
cancelled by cancelPolicyCheckIfNeeded when detaching from the frame.

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

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 872ca6a..93f2104 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,24 @@
+2015-10-27  Alex Christensen  <achristensen@webkit.org>
+
+        Cancel navigation policy checks like we do content policy checks.
+        https://bugs.webkit.org/show_bug.cgi?id=150582
+        rdar://problem/22077579
+
+        Reviewed by Brent Fulgham.
+
+        This was verified manually and I'll write a layout test for it soon.
+
+        * loader/DocumentLoader.cpp:
+        (WebCore::DocumentLoader::DocumentLoader):
+        (WebCore::DocumentLoader::~DocumentLoader):
+        (WebCore::DocumentLoader::willSendRequest):
+        (WebCore::DocumentLoader::continueAfterNavigationPolicy):
+        (WebCore::DocumentLoader::cancelPolicyCheckIfNeeded):
+        * loader/DocumentLoader.h:
+        Add a bool to keep track of whether we are waiting for navigation policy checks, like we do with content policy checks.
+        Without this check, sometimes callbacks are made to DocumentLoaders that do not exist any more because they do not get
+        cancelled by cancelPolicyCheckIfNeeded when detaching from the frame.
+
 2015-10-27  Brady Eidson  <beidson@apple.com>
 
         Modern IDB: Support IDBObjectStore.put/get support.

diff --git a/Source/WebCore/loader/DocumentLoader.cpp b/Source/WebCore/loader/DocumentLoader.cpp
index 42bb0e1..a97f695 100644 (file)
--- a/Source/WebCore/loader/DocumentLoader.cpp
+++ b/Source/WebCore/loader/DocumentLoader.cpp
@@ -142,7 +142,6 @@ DocumentLoader::DocumentLoader(const ResourceRequest& req, const SubstituteData&
     , m_timeOfLastDataReceived(0.0)
     , m_identifierForLoadWithoutResourceLoader(0)
     , m_dataLoadTimer(*this, &DocumentLoader::handleSubstituteDataLoadNow)
-    , m_waitingForContentPolicy(false)
     , m_subresourceLoadersArePageCacheAcceptable(false)
     , m_applicationCacheHost(std::make_unique<ApplicationCacheHost>(*this))
 #if ENABLE(CONTENT_FILTERING)
@@ -167,6 +166,7 @@ DocumentLoader::~DocumentLoader()
 {
     ASSERT(!m_frame || frameLoader()->activeDocumentLoader() != this || !isLoading());
     ASSERT_WITH_MESSAGE(!m_waitingForContentPolicy, "The content policy callback should never outlive its DocumentLoader.");
+    ASSERT_WITH_MESSAGE(!m_waitingForNavigationPolicy, "The navigation policy callback should never outlive its DocumentLoader.");
     if (m_iconLoadDecisionCallback)
         m_iconLoadDecisionCallback->invalidate();
     if (m_iconDataCallback)
@@ -570,6 +570,8 @@ void DocumentLoader::willSendRequest(ResourceRequest& newRequest, const Resource
     if (redirectResponse.isNull())
         return;
 
+    ASSERT(!m_waitingForNavigationPolicy);
+    m_waitingForNavigationPolicy = true;
     frameLoader()->policyChecker().checkNavigationPolicy(newRequest, [this](const ResourceRequest& request, PassRefPtr<FormState>, bool shouldContinue) {
         continueAfterNavigationPolicy(request, shouldContinue);
     });
@@ -577,6 +579,8 @@ void DocumentLoader::willSendRequest(ResourceRequest& newRequest, const Resource
 
 void DocumentLoader::continueAfterNavigationPolicy(const ResourceRequest&, bool shouldContinue)
 {
+    ASSERT(m_waitingForNavigationPolicy);
+    m_waitingForNavigationPolicy = false;
     if (!shouldContinue)
         stopLoadingForPolicyChange();
     else if (m_substituteData.isValid()) {
@@ -1485,9 +1489,10 @@ void DocumentLoader::cancelPolicyCheckIfNeeded()
 {
     RELEASE_ASSERT(frameLoader());
 
-    if (m_waitingForContentPolicy) {
+    if (m_waitingForContentPolicy || m_waitingForNavigationPolicy) {
         frameLoader()->policyChecker().cancelCheck();
         m_waitingForContentPolicy = false;
+        m_waitingForNavigationPolicy = false;
     }
 }
 

diff --git a/Source/WebCore/loader/DocumentLoader.h b/Source/WebCore/loader/DocumentLoader.h
index c7c1bf3..d472255 100644 (file)
--- a/Source/WebCore/loader/DocumentLoader.h
+++ b/Source/WebCore/loader/DocumentLoader.h
@@ -320,7 +320,6 @@ namespace WebCore {
         bool isPostOrRedirectAfterPost(const ResourceRequest&, const ResourceResponse&);
 
         void continueAfterNavigationPolicy(const ResourceRequest&, bool shouldContinue);
-
         void continueAfterContentPolicy(PolicyAction);
 
         void stopLoadingForPolicyChange();
@@ -432,7 +431,8 @@ namespace WebCore {
         unsigned long m_identifierForLoadWithoutResourceLoader;
 
         DocumentLoaderTimer m_dataLoadTimer;
-        bool m_waitingForContentPolicy;
+        bool m_waitingForContentPolicy { false };
+        bool m_waitingForNavigationPolicy { false };
 
         RefPtr<IconLoadDecisionCallback> m_iconLoadDecisionCallback;
         RefPtr<IconDataCallback> m_iconDataCallback;