Changeset 191636 in webkit

Timestamp:

Oct 27, 2015 1:41:34 PM (9 years ago)

Author:

achristensen@apple.com

Message:

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.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• loader/DocumentLoader.cpp (modified) (5 diffs)
• loader/DocumentLoader.h (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/loader/DocumentLoader.cpp

@@ -143 +143 @@
     , m_identifierForLoadWithoutResourceLoader(0)
     , m_dataLoadTimer(*this, &DocumentLoader::handleSubstituteDataLoadNow)
-    , m_waitingForContentPolicy(false)
     , m_subresourceLoadersArePageCacheAcceptable(false)
     , m_applicationCacheHost(std::make_unique<ApplicationCacheHost>(*this))
@@ -168 +167 @@
     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();
@@ -571 +571 @@
         return;
 
+    ASSERT(!m_waitingForNavigationPolicy);
+    m_waitingForNavigationPolicy = true;
     frameLoader()->policyChecker().checkNavigationPolicy(newRequest, [this](const ResourceRequest& request, PassRefPtr<FormState>, bool shouldContinue) {
         continueAfterNavigationPolicy(request, shouldContinue);
@@ -578 +580 @@
 void DocumentLoader::continueAfterNavigationPolicy(const ResourceRequest&, bool shouldContinue)
 {
+    ASSERT(m_waitingForNavigationPolicy);
+    m_waitingForNavigationPolicy = false;
     if (!shouldContinue)
         stopLoadingForPolicyChange();
@@ -1486 +1490 @@
     RELEASE_ASSERT(frameLoader());
 
-    if (m_waitingForContentPolicy) {
+    if (m_waitingForContentPolicy || m_waitingForNavigationPolicy) {
         frameLoader()->policyChecker().cancelCheck();
         m_waitingForContentPolicy = false;
+        m_waitingForNavigationPolicy = false;
     }
 }

trunk/Source/WebCore/loader/DocumentLoader.h

@@ -321 +321 @@
 
         void continueAfterNavigationPolicy(const ResourceRequest&, bool shouldContinue);
-
         void continueAfterContentPolicy(PolicyAction);
 
@@ -433 +432 @@
 
         DocumentLoaderTimer m_dataLoadTimer;
-        bool m_waitingForContentPolicy;
+        bool m_waitingForContentPolicy { false };
+        bool m_waitingForNavigationPolicy { false };
 
         RefPtr<IconLoadDecisionCallback> m_iconLoadDecisionCallback;