Avoid keeping FormState alive longer than necessary
authorbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 22 May 2018 22:01:34 +0000 (22:01 +0000)
committerbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 22 May 2018 22:01:34 +0000 (22:01 +0000)
https://bugs.webkit.org/show_bug.cgi?id=185877
<rdar://problem/39329219>

Reviewed by Ryosuke Niwa.

A number of crash fixes were done to prevent FormState objects from being
accessed after their relevant Frames had been destroyed. Unfortunately, this
could cause the FormState to persist after the owning Frame had been
destroyed, resulting in nullptr dereferences.

This patch does the following:

1. Changes to use WeakPtr's for FormState objects passed to completion handlers,
   rather than RefPtr, since those completion handlers might fire as part of
   the clean-up process during Frame destruction. This allows us to use the FormState
   if they are still valid, but gracefully handle cases where a form submission
   is cancelled in-flight.
2. Removes some extraneous WTFMove() calls being made on bare FormState pointers.
3. Changes the trap from Bug 183704 so that it only fires if the FormState object
   is being retained more than once.

* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::willSendRequest): Update for new CompletionHandler
signature.
* loader/FormState.cpp:
(WebCore::FormState::willDetachPage): Revise trap to check for retain counts
above one.
* loader/FormState.h:
(WebCore::FormState::weakPtrFactory const): Added.
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::loadFrameRequest): Revise to use WeakPtr for FormState
passed to the completion handler.
(WebCore::FrameLoader::loadURL): Update for new CompletionHandler signature.
(WebCore::FrameLoader::load): Ditto.
(WebCore::FrameLoader::loadWithDocumentLoader): Ditto.
(WebCore::FrameLoader::loadPostRequest): Ditto.
* loader/PolicyChecker.cpp:
(WebCore::PolicyChecker::checkNavigationPolicy): Revise to use WeakPtr for
FormState passed to the completion handler. Remove some extraneous WTFMove()
calls on bare pointers.
(WebCore::PolicyChecker::checkNewWindowPolicy): Ditto.
* loader/PolicyChecker.h:

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

Source/WebCore/ChangeLog
Source/WebCore/loader/DocumentLoader.cpp
Source/WebCore/loader/FormState.cpp
Source/WebCore/loader/FormState.h
Source/WebCore/loader/FrameLoader.cpp
Source/WebCore/loader/PolicyChecker.cpp
Source/WebCore/loader/PolicyChecker.h

index 4deb286..d54854e 100644 (file)
@@ -1,3 +1,49 @@
+2018-05-22  Brent Fulgham  <bfulgham@apple.com>
+
+        Avoid keeping FormState alive longer than necessary
+        https://bugs.webkit.org/show_bug.cgi?id=185877
+        <rdar://problem/39329219>
+
+        Reviewed by Ryosuke Niwa.
+
+        A number of crash fixes were done to prevent FormState objects from being
+        accessed after their relevant Frames had been destroyed. Unfortunately, this
+        could cause the FormState to persist after the owning Frame had been
+        destroyed, resulting in nullptr dereferences.
+
+        This patch does the following:
+
+        1. Changes to use WeakPtr's for FormState objects passed to completion handlers,
+           rather than RefPtr, since those completion handlers might fire as part of
+           the clean-up process during Frame destruction. This allows us to use the FormState
+           if they are still valid, but gracefully handle cases where a form submission
+           is cancelled in-flight.
+        2. Removes some extraneous WTFMove() calls being made on bare FormState pointers.
+        3. Changes the trap from Bug 183704 so that it only fires if the FormState object
+           is being retained more than once.
+
+        * loader/DocumentLoader.cpp:
+        (WebCore::DocumentLoader::willSendRequest): Update for new CompletionHandler
+        signature.
+        * loader/FormState.cpp:
+        (WebCore::FormState::willDetachPage): Revise trap to check for retain counts
+        above one.
+        * loader/FormState.h:
+        (WebCore::FormState::weakPtrFactory const): Added.
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::loadFrameRequest): Revise to use WeakPtr for FormState
+        passed to the completion handler.
+        (WebCore::FrameLoader::loadURL): Update for new CompletionHandler signature.
+        (WebCore::FrameLoader::load): Ditto.
+        (WebCore::FrameLoader::loadWithDocumentLoader): Ditto.
+        (WebCore::FrameLoader::loadPostRequest): Ditto.
+        * loader/PolicyChecker.cpp:
+        (WebCore::PolicyChecker::checkNavigationPolicy): Revise to use WeakPtr for
+        FormState passed to the completion handler. Remove some extraneous WTFMove()
+        calls on bare pointers.
+        (WebCore::PolicyChecker::checkNewWindowPolicy): Ditto.
+        * loader/PolicyChecker.h:
+
 2018-05-22  Sihui Liu  <sihui_liu@apple.com>
 
         Conversion between SecurityOriginData and DatabaseIdentifier is asymmetric when port is null
index 9e09bd3..7ed4f64 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2006-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2006-2018 Apple Inc. All rights reserved.
  * Copyright (C) 2011 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -640,7 +640,7 @@ void DocumentLoader::willSendRequest(ResourceRequest&& newRequest, const Resourc
     if (!didReceiveRedirectResponse && shouldContinue != ShouldContinue::ForSuspension)
         return completionHandler(WTFMove(newRequest));
 
-    auto navigationPolicyCompletionHandler = [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)] (ResourceRequest&& request, FormState*, ShouldContinue shouldContinue) mutable {
+    auto navigationPolicyCompletionHandler = [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)] (ResourceRequest&& request, WeakPtr<FormState>&&, ShouldContinue shouldContinue) mutable {
         m_waitingForNavigationPolicy = false;
         switch (shouldContinue) {
         case ShouldContinue::ForSuspension:
index 9fa8bdd..f8700b1 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2006-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2006-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -52,7 +52,7 @@ Ref<FormState> FormState::create(HTMLFormElement& form, StringPairVector&& textF
 void FormState::willDetachPage()
 {
     // Beartrap for <rdar://problem/37579354>
-    RELEASE_ASSERT_NOT_REACHED();
+    RELEASE_ASSERT(hasOneRef());
 }
 
 }
index 2634a97..7758432 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2006-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2006-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -29,6 +29,7 @@
 #pragma once
 
 #include "FrameDestructionObserver.h"
+#include <wtf/WeakPtr.h>
 #include <wtf/text/WTFString.h>
 
 namespace WebCore {
@@ -49,6 +50,8 @@ public:
     Document& sourceDocument() const { return m_sourceDocument; }
     FormSubmissionTrigger formSubmissionTrigger() const { return m_formSubmissionTrigger; }
 
+    auto& weakPtrFactory() const { return m_weakFactory; }
+
 private:
     FormState(HTMLFormElement&, StringPairVector&& textFieldValues, Document&, FormSubmissionTrigger);
     void willDetachPage() override;
@@ -57,6 +60,7 @@ private:
     StringPairVector m_textFieldValues;
     Ref<Document> m_sourceDocument;
     FormSubmissionTrigger m_formSubmissionTrigger;
+    WeakPtrFactory<FormState> m_weakFactory;
 };
 
 } // namespace WebCore
index 8c6e54b..6904394 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2006-2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2006-2018 Apple Inc. All rights reserved.
  * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  * Copyright (C) 2008 Alp Toker <alp@atoker.com>
@@ -1228,7 +1228,7 @@ void FrameLoader::loadFrameRequest(FrameLoadRequest&& request, Event* event, For
     else
         loadType = FrameLoadType::Standard;
 
-    auto completionHandler = [this, protectedFrame = makeRef(m_frame), formState = makeRefPtr(formState), frameName = request.frameName()] {
+    auto completionHandler = [this, protectedFrame = makeRef(m_frame), formState = makeWeakPtr(formState), frameName = request.frameName()] {
         // FIXME: It's possible this targetFrame will not be the same frame that was targeted by the actual
         // load if frame names have changed.
         Frame* sourceFrame = formState ? formState->sourceDocument().frame() : &m_frame;
@@ -1338,8 +1338,8 @@ void FrameLoader::loadURL(FrameLoadRequest&& frameLoadRequest, const String& ref
 
     if (!targetFrame && !frameName.isEmpty()) {
         action = action.copyWithShouldOpenExternalURLsPolicy(shouldOpenExternalURLsPolicyToApply(m_frame, frameLoadRequest));
-        policyChecker().checkNewWindowPolicy(WTFMove(action), WTFMove(request), formState, frameName, [this, allowNavigationToInvalidURL, openerPolicy, completionHandler = completionHandlerCaller.release()] (const ResourceRequest& request, FormState* formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue) {
-            continueLoadAfterNewWindowPolicy(request, formState, frameName, action, shouldContinue, allowNavigationToInvalidURL, openerPolicy);
+        policyChecker().checkNewWindowPolicy(WTFMove(action), WTFMove(request), formState, frameName, [this, allowNavigationToInvalidURL, openerPolicy, completionHandler = completionHandlerCaller.release()] (const ResourceRequest& request, WeakPtr<FormState>&& formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue) {
+            continueLoadAfterNewWindowPolicy(request, formState.get(), frameName, action, shouldContinue, allowNavigationToInvalidURL, openerPolicy);
             completionHandler();
         });
         return;
@@ -1358,7 +1358,7 @@ void FrameLoader::loadURL(FrameLoadRequest&& frameLoadRequest, const String& ref
         oldDocumentLoader->setLastCheckedRequest(ResourceRequest());
         policyChecker().stopCheck();
         policyChecker().setLoadType(newLoadType);
-        policyChecker().checkNavigationPolicy(WTFMove(request), false /* didReceiveRedirectResponse */, oldDocumentLoader.get(), formState, [this, protectedFrame = makeRef(m_frame)] (const ResourceRequest& request, FormState*, ShouldContinue shouldContinue) {
+        policyChecker().checkNavigationPolicy(WTFMove(request), false /* didReceiveRedirectResponse */, oldDocumentLoader.get(), formState, [this, protectedFrame = makeRef(m_frame)] (const ResourceRequest& request, WeakPtr<FormState>&&, ShouldContinue shouldContinue) {
             continueFragmentScrollAfterNavigationPolicy(request, shouldContinue == ShouldContinue::Yes);
         }, PolicyDecisionMode::Synchronous);
         return;
@@ -1414,8 +1414,8 @@ void FrameLoader::load(FrameLoadRequest&& request)
 
     if (request.shouldCheckNewWindowPolicy()) {
         NavigationAction action { request.requester(), request.resourceRequest(), InitiatedByMainFrame::Unknown, NavigationType::Other, request.shouldOpenExternalURLsPolicy() };
-        policyChecker().checkNewWindowPolicy(WTFMove(action), WTFMove(request.resourceRequest()), nullptr, request.frameName(), [this] (const ResourceRequest& request, FormState* formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue) {
-            continueLoadAfterNewWindowPolicy(request, formState, frameName, action, shouldContinue, AllowNavigationToInvalidURL::Yes, NewFrameOpenerPolicy::Suppress);
+        policyChecker().checkNewWindowPolicy(WTFMove(action), WTFMove(request.resourceRequest()), nullptr, request.frameName(), [this] (const ResourceRequest& request, WeakPtr<FormState>&& formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue) {
+            continueLoadAfterNewWindowPolicy(request, formState.get(), frameName, action, shouldContinue, AllowNavigationToInvalidURL::Yes, NewFrameOpenerPolicy::Suppress);
         });
 
         return;
@@ -1528,7 +1528,7 @@ void FrameLoader::loadWithDocumentLoader(DocumentLoader* loader, FrameLoadType t
         oldDocumentLoader->setTriggeringAction(action);
         oldDocumentLoader->setLastCheckedRequest(ResourceRequest());
         policyChecker().stopCheck();
-        policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), false /* didReceiveRedirectResponse */, oldDocumentLoader.get(), formState, [this, protectedFrame = makeRef(m_frame)] (const ResourceRequest& request, FormState*, ShouldContinue shouldContinue) {
+        policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), false /* didReceiveRedirectResponse */, oldDocumentLoader.get(), formState, [this, protectedFrame = makeRef(m_frame)] (const ResourceRequest& request, WeakPtr<FormState>&&, ShouldContinue shouldContinue) {
             continueFragmentScrollAfterNavigationPolicy(request, shouldContinue == ShouldContinue::Yes);
         }, PolicyDecisionMode::Synchronous);
         return;
@@ -1562,8 +1562,8 @@ void FrameLoader::loadWithDocumentLoader(DocumentLoader* loader, FrameLoadType t
         return;
     }
 
-    policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), false /* didReceiveRedirectResponse */, loader, formState, [this, protectedFrame = makeRef(m_frame), allowNavigationToInvalidURL, completionHandler = completionHandlerCaller.release()] (const ResourceRequest& request, FormState* formState, ShouldContinue shouldContinue) {
-        continueLoadAfterNavigationPolicy(request, formState, shouldContinue, allowNavigationToInvalidURL);
+    policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), false /* didReceiveRedirectResponse */, loader, formState, [this, protectedFrame = makeRef(m_frame), allowNavigationToInvalidURL, completionHandler = completionHandlerCaller.release()] (const ResourceRequest& request, WeakPtr<FormState>&& formState, ShouldContinue shouldContinue) {
+        continueLoadAfterNavigationPolicy(request, formState.get(), shouldContinue, allowNavigationToInvalidURL);
         completionHandler();
     });
 }
@@ -2856,13 +2856,13 @@ void FrameLoader::loadPostRequest(FrameLoadRequest&& request, const String& refe
 
     if (!frameName.isEmpty()) {
         // The search for a target frame is done earlier in the case of form submission.
-        if (Frame* targetFrame = formState ? 0 : findFrameForNavigation(frameName)) {
-            targetFrame->loader().loadWithNavigationAction(workingResourceRequest, action, lockHistory, loadType, WTFMove(formState), allowNavigationToInvalidURL, WTFMove(completionHandler));
+        if (auto* targetFrame = formState ? nullptr : findFrameForNavigation(frameName)) {
+            targetFrame->loader().loadWithNavigationAction(workingResourceRequest, action, lockHistory, loadType, formState, allowNavigationToInvalidURL, WTFMove(completionHandler));
             return;
         }
 
-        policyChecker().checkNewWindowPolicy(WTFMove(action), WTFMove(workingResourceRequest), WTFMove(formState), frameName, [this, allowNavigationToInvalidURL, openerPolicy, completionHandler = WTFMove(completionHandler)] (const ResourceRequest& request, FormState* formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue) {
-            continueLoadAfterNewWindowPolicy(request, formState, frameName, action, shouldContinue, allowNavigationToInvalidURL, openerPolicy);
+        policyChecker().checkNewWindowPolicy(WTFMove(action), WTFMove(workingResourceRequest), formState, frameName, [this, allowNavigationToInvalidURL, openerPolicy, completionHandler = WTFMove(completionHandler)] (const ResourceRequest& request, WeakPtr<FormState>&& formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue) {
+            continueLoadAfterNewWindowPolicy(request, formState.get(), frameName, action, shouldContinue, allowNavigationToInvalidURL, openerPolicy);
             completionHandler();
         });
         return;
@@ -2870,7 +2870,7 @@ void FrameLoader::loadPostRequest(FrameLoadRequest&& request, const String& refe
 
     // must grab this now, since this load may stop the previous load and clear this flag
     bool isRedirect = m_quickRedirectComing;
-    loadWithNavigationAction(workingResourceRequest, action, lockHistory, loadType, WTFMove(formState), allowNavigationToInvalidURL, [this, isRedirect, protectedFrame = makeRef(m_frame), completionHandler = WTFMove(completionHandler)] {
+    loadWithNavigationAction(workingResourceRequest, action, lockHistory, loadType, formState, allowNavigationToInvalidURL, [this, isRedirect, protectedFrame = makeRef(m_frame), completionHandler = WTFMove(completionHandler)] {
         if (isRedirect) {
             m_quickRedirectComing = false;
             if (m_provisionalDocumentLoader)
index b6690ea..1be70e1 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2006-2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2006-2018 Apple Inc. All rights reserved.
  * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  *
@@ -109,7 +109,7 @@ void PolicyChecker::checkNavigationPolicy(ResourceRequest&& request, bool didRec
     // Don't ask more than once for the same request or if we are loading an empty URL.
     // This avoids confusion on the part of the client.
     if (equalIgnoringHeaderFields(request, loader->lastCheckedRequest()) || (!request.isNull() && request.url().isEmpty())) {
-        function(ResourceRequest(request), nullptr, ShouldContinue::Yes);
+        function(ResourceRequest(request), { }, ShouldContinue::Yes);
         loader->setLastCheckedRequest(WTFMove(request));
         return;
     }
@@ -124,7 +124,7 @@ void PolicyChecker::checkNavigationPolicy(ResourceRequest&& request, bool didRec
 #endif
         if (isBackForwardLoadType(m_loadType))
             m_loadType = FrameLoadType::Reload;
-        function(WTFMove(request), nullptr, shouldContinue ? ShouldContinue::Yes : ShouldContinue::No);
+        function(WTFMove(request), { }, shouldContinue ? ShouldContinue::Yes : ShouldContinue::No);
         return;
     }
 
@@ -134,7 +134,7 @@ void PolicyChecker::checkNavigationPolicy(ResourceRequest&& request, bool didRec
             // reveal that the frame was blocked. This way, it looks like any other cross-origin page load.
             m_frame.ownerElement()->dispatchEvent(Event::create(eventNames().loadEvent, false, false));
         }
-        function(WTFMove(request), nullptr, ShouldContinue::No);
+        function(WTFMove(request), { }, ShouldContinue::No);
         return;
     }
 
@@ -147,7 +147,7 @@ void PolicyChecker::checkNavigationPolicy(ResourceRequest&& request, bool didRec
 #if USE(QUICK_LOOK)
     // Always allow QuickLook-generated URLs based on the protocol scheme.
     if (!request.isNull() && isQuickLookPreviewURL(request.url()))
-        return function(WTFMove(request), formState, ShouldContinue::Yes);
+        return function(WTFMove(request), makeWeakPtr(formState), ShouldContinue::Yes);
 #endif
 
 #if ENABLE(CONTENT_FILTERING)
@@ -168,7 +168,7 @@ void PolicyChecker::checkNavigationPolicy(ResourceRequest&& request, bool didRec
 
     m_delegateIsDecidingNavigationPolicy = true;
     String suggestedFilename = action.downloadAttribute().isEmpty() ? nullAtom() : action.downloadAttribute();
-    m_frame.loader().client().dispatchDecidePolicyForNavigationAction(action, request, didReceiveRedirectResponse, formState, policyDecisionMode, [this, function = WTFMove(function), request = ResourceRequest(request), formState = makeRefPtr(formState), suggestedFilename = WTFMove(suggestedFilename), blobURLLifetimeExtension = WTFMove(blobURLLifetimeExtension)](PolicyAction policyAction) mutable {
+    m_frame.loader().client().dispatchDecidePolicyForNavigationAction(action, request, didReceiveRedirectResponse, formState, policyDecisionMode, [this, function = WTFMove(function), request = ResourceRequest(request), formState = makeWeakPtr(formState), suggestedFilename = WTFMove(suggestedFilename), blobURLLifetimeExtension = WTFMove(blobURLLifetimeExtension)](PolicyAction policyAction) mutable {
         m_delegateIsDecidingNavigationPolicy = false;
 
         switch (policyAction) {
@@ -185,7 +185,7 @@ void PolicyChecker::checkNavigationPolicy(ResourceRequest&& request, bool didRec
                 handleUnimplementablePolicy(m_frame.loader().client().cannotShowURLError(request));
                 return function({ }, nullptr, ShouldContinue::No);
             }
-            return function(WTFMove(request), formState.get(), ShouldContinue::Yes);
+            return function(WTFMove(request), WTFMove(formState), ShouldContinue::Yes);
         }
         ASSERT_NOT_REACHED();
     });
@@ -201,7 +201,7 @@ void PolicyChecker::checkNewWindowPolicy(NavigationAction&& navigationAction, Re
 
     auto blobURLLifetimeExtension = extendBlobURLLifetimeIfNecessary(request);
 
-    m_frame.loader().client().dispatchDecidePolicyForNewWindowAction(navigationAction, request, formState, frameName, [frame = makeRef(m_frame), request, formState = makeRefPtr(formState), frameName, navigationAction, function = WTFMove(function), blobURLLifetimeExtension = WTFMove(blobURLLifetimeExtension)](PolicyAction policyAction) mutable {
+    m_frame.loader().client().dispatchDecidePolicyForNewWindowAction(navigationAction, request, formState, frameName, [frame = makeRef(m_frame), request, formState = makeWeakPtr(formState), frameName, navigationAction, function = WTFMove(function), blobURLLifetimeExtension = WTFMove(blobURLLifetimeExtension)](PolicyAction policyAction) mutable {
         switch (policyAction) {
         case PolicyAction::Download:
             frame->loader().client().startDownload(request);
@@ -213,7 +213,7 @@ void PolicyChecker::checkNewWindowPolicy(NavigationAction&& navigationAction, Re
             // It is invalid to get a "Suspend" policy for new windows, as the old document is not going away.
             RELEASE_ASSERT_NOT_REACHED();
         case PolicyAction::Use:
-            function(request, formState.get(), frameName, navigationAction, ShouldContinue::Yes);
+            function(request, WTFMove(formState), frameName, navigationAction, ShouldContinue::Yes);
             return;
         }
         ASSERT_NOT_REACHED();
index c329ec4..6f11211 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2006-2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2006-2018 Apple Inc. All rights reserved.
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  *
  * Redistribution and use in source and binary forms, with or without
@@ -31,6 +31,7 @@
 
 #include "FrameLoaderTypes.h"
 #include "ResourceRequest.h"
+#include <wtf/WeakPtr.h>
 #include <wtf/text/WTFString.h>
 
 #if ENABLE(CONTENT_FILTERING)
@@ -59,8 +60,8 @@ enum class ShouldContinue {
 
 enum class PolicyDecisionMode { Synchronous, Asynchronous };
 
-using NewWindowPolicyDecisionFunction = CompletionHandler<void(const ResourceRequest&, FormState*, const String& frameName, const NavigationAction&, ShouldContinue)>;
-using NavigationPolicyDecisionFunction = CompletionHandler<void(ResourceRequest&&, FormState*, ShouldContinue)>;
+using NewWindowPolicyDecisionFunction = CompletionHandler<void(const ResourceRequest&, WeakPtr<FormState>&&, const String& frameName, const NavigationAction&, ShouldContinue)>;
+using NavigationPolicyDecisionFunction = CompletionHandler<void(ResourceRequest&&, WeakPtr<FormState>&&, ShouldContinue)>;
 
 class PolicyChecker {
     WTF_MAKE_NONCOPYABLE(PolicyChecker);