WebPageProxy::WillSubmitForm should use sendWithAsyncReply

https://bugs.webkit.org/show_bug.cgi?id=259754 rdar://113299797 Reviewed by Chris Dumez. No change in behavior, just more clear code flows and less manual accounting. * Source/WebKit/Scripts/webkit/messages.py: (serialized_identifiers): (headers_for_type): * Source/WebKit/Scripts/webkit/tests/MessageArgumentDescriptions.cpp: (IPC::serializedIdentifiers): * Source/WebKit/Shared/IdentifierTypes.h: * Source/WebKit/UIProcess/API/APIFormClient.h: (API::FormClient::willSubmitForm): * Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm: (-[WKWebView _setInputDelegate:]): * Source/WebKit/UIProcess/API/glib/WebKitFormClient.cpp: * Source/WebKit/UIProcess/WebFormClient.cpp: (WebKit::WebFormClient::willSubmitForm): * Source/WebKit/UIProcess/WebFormClient.h: * Source/WebKit/UIProcess/WebPageProxy.cpp: (WebKit::WebPageProxy::willSubmitForm): * Source/WebKit/UIProcess/WebPageProxy.h: * Source/WebKit/UIProcess/WebPageProxy.messages.in: * Source/WebKit/WebProcess/WebCoreSupport/WebLocalFrameLoaderClient.cpp: (WebKit::WebLocalFrameLoaderClient::dispatchWillSubmitForm): * Source/WebKit/WebProcess/WebPage/WebFrame.cpp: (WebKit::WebFrame::~WebFrame): (WebKit::WebFrame::invalidatePolicyListeners): (WebKit::WebFrame::setUpWillSubmitFormListener): Deleted. (WebKit::WebFrame::continueWillSubmitForm): Deleted. * Source/WebKit/WebProcess/WebPage/WebFrame.h: * Source/WebKit/WebProcess/WebPage/WebPage.cpp: (WebKit::WebPage::continueWillSubmitForm): Deleted. * Source/WebKit/WebProcess/WebPage/WebPage.h: * Source/WebKit/WebProcess/WebPage/WebPage.messages.in: Canonical link: https://commits.webkit.org/266540@main
WebKit · Aug 3, 2023 · c8e6a14 · c8e6a14
1 parent c85962a
commit c8e6a14
Show file tree

Hide file tree

Showing 17 changed files with 14 additions and 66 deletions.
diff --git a/Source/WebKit/Scripts/webkit/messages.py b/Source/WebKit/Scripts/webkit/messages.py
@@ -303,7 +303,6 @@ def serialized_identifiers():
         'WebKit::ContentWorldIdentifier',
         'WebKit::DataTaskIdentifier',
         'WebKit::DownloadID',
-        'WebKit::FormSubmitListenerIdentifier',
         'WebKit::GeolocationIdentifier',
         'WebKit::GraphicsContextGLIdentifier',
         'WebKit::IPCConnectionTesterIdentifier',
@@ -871,7 +870,6 @@ def headers_for_type(type):
         'WebKit::DocumentEditingContextRequest': ['"DocumentEditingContext.h"'],
         'WebKit::FindDecorationStyle': ['"WebFindOptions.h"'],
         'WebKit::FindOptions': ['"WebFindOptions.h"'],
-        'WebKit::FormSubmitListenerIdentifier': ['"IdentifierTypes.h"'],
         'WebKit::GestureRecognizerState': ['"GestureTypes.h"'],
         'WebKit::GestureType': ['"GestureTypes.h"'],
         'WebKit::LastNavigationWasAppInitiated': ['"AppPrivacyReport.h"'],

diff --git a/Source/WebKit/Scripts/webkit/tests/MessageArgumentDescriptions.cpp b/Source/WebKit/Scripts/webkit/tests/MessageArgumentDescriptions.cpp
@@ -449,7 +449,6 @@ Vector<ASCIILiteral> serializedIdentifiers()
     static_assert(sizeof(uint64_t) == sizeof(WebKit::ContentWorldIdentifier));
     static_assert(sizeof(uint64_t) == sizeof(WebKit::DataTaskIdentifier));
     static_assert(sizeof(uint64_t) == sizeof(WebKit::DownloadID));
-    static_assert(sizeof(uint64_t) == sizeof(WebKit::FormSubmitListenerIdentifier));
     static_assert(sizeof(uint64_t) == sizeof(WebKit::GeolocationIdentifier));
     static_assert(sizeof(uint64_t) == sizeof(WebKit::GraphicsContextGLIdentifier));
     static_assert(sizeof(uint64_t) == sizeof(WebKit::IPCConnectionTesterIdentifier));
@@ -548,7 +547,6 @@ Vector<ASCIILiteral> serializedIdentifiers()
         "WebKit::ContentWorldIdentifier"_s,
         "WebKit::DataTaskIdentifier"_s,
         "WebKit::DownloadID"_s,
-        "WebKit::FormSubmitListenerIdentifier"_s,
         "WebKit::GeolocationIdentifier"_s,
         "WebKit::GraphicsContextGLIdentifier"_s,
         "WebKit::IPCConnectionTesterIdentifier"_s,

diff --git a/Source/WebKit/Shared/IdentifierTypes.h b/Source/WebKit/Shared/IdentifierTypes.h
@@ -33,9 +33,6 @@ namespace WebKit {
 struct AuthenticationChallengeIdentifierType;
 using AuthenticationChallengeIdentifier = ObjectIdentifier<AuthenticationChallengeIdentifierType>;
 
-struct FormSubmitListenerIdentifierType;
-using FormSubmitListenerIdentifier = ObjectIdentifier<FormSubmitListenerIdentifierType>;
-
 struct PageGroupIdentifierType;
 using PageGroupIdentifier = ObjectIdentifier<PageGroupIdentifierType>;
 

diff --git a/Source/WebKit/UIProcess/API/APIFormClient.h b/Source/WebKit/UIProcess/API/APIFormClient.h
@@ -25,8 +25,7 @@
 
 #pragma once
 
-#include <wtf/Forward.h>
-#include <wtf/Function.h>
+#include <wtf/CompletionHandler.h>
 
 namespace WebKit {
 class WebFrameProxy;
@@ -41,7 +40,7 @@ class FormClient {
 public:
     virtual ~FormClient() { }
 
-    virtual void willSubmitForm(WebKit::WebPageProxy&, WebKit::WebFrameProxy&, WebKit::WebFrameProxy&, const Vector<std::pair<WTF::String, WTF::String>>&, API::Object*, WTF::Function<void(void)>&& completionHandler)
+    virtual void willSubmitForm(WebKit::WebPageProxy&, WebKit::WebFrameProxy&, WebKit::WebFrameProxy&, const Vector<std::pair<WTF::String, WTF::String>>&, API::Object*, CompletionHandler<void()>&& completionHandler)
     {
         completionHandler();
     }

diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
@@ -3690,11 +3690,11 @@ explicit FormClient(WKWebView *webView)
 
         virtual ~FormClient() { }
 
-        void willSubmitForm(WebKit::WebPageProxy&, WebKit::WebFrameProxy&, WebKit::WebFrameProxy& sourceFrame, const Vector<std::pair<WTF::String, WTF::String>>& textFieldValues, API::Object* userData, WTF::Function<void(void)>&& completionHandler) override
+        void willSubmitForm(WebKit::WebPageProxy&, WebKit::WebFrameProxy&, WebKit::WebFrameProxy& sourceFrame, const Vector<std::pair<WTF::String, WTF::String>>& textFieldValues, API::Object* userData, CompletionHandler<void()>&& completionHandler) override
         {
             auto webView = m_webView.get();
             if (!webView)
-                return;
+                return completionHandler();
 
             auto inputDelegate = webView->_inputDelegate.get();
 
@@ -3710,7 +3710,7 @@ void willSubmitForm(WebKit::WebPageProxy&, WebKit::WebFrameProxy&, WebKit::WebFr
             auto userObject = userData ? userData->toNSObject() : RetainPtr<NSObject<NSSecureCoding>>();
 
             auto checker = WebKit::CompletionHandlerCallChecker::create(inputDelegate.get(), @selector(_webView:willSubmitFormValues:userObject:submissionHandler:));
-            [inputDelegate _webView:webView.get() willSubmitFormValues:valueMap.get() userObject:userObject.get() submissionHandler:makeBlockPtr([completionHandler = WTFMove(completionHandler), checker = WTFMove(checker)] {
+            [inputDelegate _webView:webView.get() willSubmitFormValues:valueMap.get() userObject:userObject.get() submissionHandler:makeBlockPtr([completionHandler = WTFMove(completionHandler), checker = WTFMove(checker)] () mutable {
                 if (checker->completionHandlerHasBeenCalled())
                     return;
                 checker->didCallCompletionHandler();

diff --git a/Source/WebKit/UIProcess/API/glib/WebKitFormClient.cpp b/Source/WebKit/UIProcess/API/glib/WebKitFormClient.cpp
@@ -37,7 +37,7 @@ class FormClient final : public API::FormClient {
     }
 
 private:
-    void willSubmitForm(WebPageProxy&, WebFrameProxy&, WebFrameProxy&, const Vector<std::pair<String, String>>& values, API::Object*, WTF::Function<void(void)>&& completionHandler) override
+    void willSubmitForm(WebPageProxy&, WebFrameProxy&, WebFrameProxy&, const Vector<std::pair<String, String>>& values, API::Object*, CompletionHandler<void()>&& completionHandler) override
     {
         GRefPtr<WebKitFormSubmissionRequest> request = adoptGRef(webkitFormSubmissionRequestCreate(values, WebFormSubmissionListenerProxy::create(WTFMove(completionHandler))));
         webkitWebViewSubmitFormRequest(m_webView, request.get());

diff --git a/Source/WebKit/UIProcess/WebFormClient.cpp b/Source/WebKit/UIProcess/WebFormClient.cpp
@@ -40,7 +40,7 @@ WebFormClient::WebFormClient(const WKPageFormClientBase* wkClient)
     initialize(wkClient);
 }
 
-void WebFormClient::willSubmitForm(WebPageProxy& page, WebFrameProxy& frame, WebFrameProxy& sourceFrame, const Vector<std::pair<String, String>>& textFieldValues, API::Object* userData, WTF::Function<void(void)>&& completionHandler)
+void WebFormClient::willSubmitForm(WebPageProxy& page, WebFrameProxy& frame, WebFrameProxy& sourceFrame, const Vector<std::pair<String, String>>& textFieldValues, API::Object* userData, CompletionHandler<void()>&& completionHandler)
 {
     if (!m_client.willSubmitForm) {
         completionHandler();

diff --git a/Source/WebKit/UIProcess/WebFormClient.h b/Source/WebKit/UIProcess/WebFormClient.h
@@ -42,7 +42,7 @@ class WebFormClient : public API::FormClient, API::Client<WKPageFormClientBase>
 public:
     explicit WebFormClient(const WKPageFormClientBase*);
 
-    void willSubmitForm(WebPageProxy&, WebFrameProxy&, WebFrameProxy&, const Vector<std::pair<String, String>>& textFieldValues, API::Object* userData, WTF::Function<void(void)>&&) override;
+    void willSubmitForm(WebPageProxy&, WebFrameProxy&, WebFrameProxy&, const Vector<std::pair<String, String>>& textFieldValues, API::Object* userData, CompletionHandler<void(void)>&&) override;
 };
 
 } // namespace WebKit
diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
@@ -6655,20 +6655,18 @@ void WebPageProxy::triggerBrowsingContextGroupSwitchForNavigation(uint64_t navig
 
 // FormClient
 
-void WebPageProxy::willSubmitForm(FrameIdentifier frameID, FrameIdentifier sourceFrameID, const Vector<std::pair<String, String>>& textFieldValues, FormSubmitListenerIdentifier listenerID, const UserData& userData)
+void WebPageProxy::willSubmitForm(FrameIdentifier frameID, FrameIdentifier sourceFrameID, const Vector<std::pair<String, String>>& textFieldValues, const UserData& userData, CompletionHandler<void()>&& completionHandler)
 {
     RefPtr frame = WebFrameProxy::webFrame(frameID);
     MESSAGE_CHECK(m_process, frame);
 
-    auto* sourceFrame = WebFrameProxy::webFrame(sourceFrameID);
+    RefPtr sourceFrame = WebFrameProxy::webFrame(sourceFrameID);
     MESSAGE_CHECK(m_process, sourceFrame);
 
     for (auto& pair : textFieldValues)
         MESSAGE_CHECK(m_process, API::Dictionary::MapType::isValidKey(pair.first));
 
-    m_formClient->willSubmitForm(*this, *frame, *sourceFrame, textFieldValues, m_process->transformHandlesToObjects(userData.object()).get(), [protectedThis = Ref { *this }, frameID = frame->frameID(), listenerID]() {
-        protectedThis->send(Messages::WebPage::ContinueWillSubmitForm(frameID, listenerID));
-    });
+    m_formClient->willSubmitForm(*this, *frame, *sourceFrame, textFieldValues, m_process->transformHandlesToObjects(userData.object()).get(), WTFMove(completionHandler));
 }
 
 #if ENABLE(CONTENT_EXTENSIONS)

diff --git a/Source/WebKit/UIProcess/WebPageProxy.h b/Source/WebKit/UIProcess/WebPageProxy.h
@@ -413,7 +413,6 @@ struct DynamicViewportSizeUpdate;
 struct EditingRange;
 struct EditorState;
 struct FocusedElementInformation;
-struct FormSubmitListenerIdentifierType;
 struct FrameInfoData;
 struct FrameTreeCreationParameters;
 struct FrameTreeNodeData;
@@ -484,7 +483,6 @@ enum class WindowKind : uint8_t;
 template<typename> class MonotonicObjectIdentifier;
 
 using ActivityStateChangeID = uint64_t;
-using FormSubmitListenerIdentifier = ObjectIdentifier<FormSubmitListenerIdentifierType>;
 using GeolocationIdentifier = ObjectIdentifier<GeolocationIdentifierType>;
 using LayerHostingContextID = uint32_t;
 using NetworkResourceLoadIdentifier = ObjectIdentifier<NetworkResourceLoadIdentifierType>;
@@ -2360,7 +2358,7 @@ class WebPageProxy final : public API::ObjectImpl<API::Object::Type::Page>, publ
 
     WebContentMode effectiveContentModeAfterAdjustingPolicies(API::WebsitePolicies&, const WebCore::ResourceRequest&);
 
-    void willSubmitForm(WebCore::FrameIdentifier, WebCore::FrameIdentifier sourceFrameID, const Vector<std::pair<String, String>>& textFieldValues, FormSubmitListenerIdentifier, const UserData&);
+    void willSubmitForm(WebCore::FrameIdentifier, WebCore::FrameIdentifier sourceFrameID, const Vector<std::pair<String, String>>& textFieldValues, const UserData&, CompletionHandler<void()>&&);
 
 #if ENABLE(CONTENT_EXTENSIONS)
     void contentRuleListNotification(URL&&, WebCore::ContentRuleListResults&&);

diff --git a/Source/WebKit/UIProcess/WebPageProxy.messages.in b/Source/WebKit/UIProcess/WebPageProxy.messages.in
@@ -159,7 +159,7 @@ messages -> WebPageProxy {
     DidFinishLoadingDataForCustomContentProvider(String suggestedFilename, IPC::DataReference data)
 
     # Forms messages
-    WillSubmitForm(WebCore::FrameIdentifier frameID, WebCore::FrameIdentifier sourceFrameID, Vector<std::pair<String, String>> textFieldValues, WebKit::FormSubmitListenerIdentifier listenerID, WebKit::UserData userData)
+    WillSubmitForm(WebCore::FrameIdentifier frameID, WebCore::FrameIdentifier sourceFrameID, Vector<std::pair<String, String>> textFieldValues, WebKit::UserData userData) -> ()
 
 #if PLATFORM(IOS_FAMILY)
     InterpretKeyEvent(struct WebKit::EditorState state, bool isCharEvent) -> (bool handled) Synchronous

diff --git a/Source/WebKit/WebProcess/WebCoreSupport/WebLocalFrameLoaderClient.cpp b/Source/WebKit/WebProcess/WebCoreSupport/WebLocalFrameLoaderClient.cpp
@@ -1043,9 +1043,7 @@ void WebLocalFrameLoaderClient::dispatchWillSubmitForm(FormState& formState, Com
     RefPtr<API::Object> userData;
     webPage->injectedBundleFormClient().willSubmitForm(webPage, &form, m_frame.ptr(), sourceFrame, values, userData);
 
-    auto listenerID = m_frame->setUpWillSubmitFormListener(WTFMove(completionHandler));
-
-    webPage->send(Messages::WebPageProxy::WillSubmitForm(m_frame->frameID(), sourceFrame->frameID(), values, listenerID, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get())));
+    webPage->sendWithAsyncReply(Messages::WebPageProxy::WillSubmitForm(m_frame->frameID(), sourceFrame->frameID(), values, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get())), WTFMove(completionHandler));
 }
 
 void WebLocalFrameLoaderClient::revertToProvisionalState(DocumentLoader*)

diff --git a/Source/WebKit/WebProcess/WebPage/WebFrame.cpp b/Source/WebKit/WebProcess/WebPage/WebFrame.cpp
@@ -177,10 +177,6 @@ WebFrame::~WebFrame()
 {
     ASSERT(!m_coreFrame);
 
-    auto willSubmitFormCompletionHandlers = std::exchange(m_willSubmitFormCompletionHandlers, { });
-    for (auto& completionHandler : willSubmitFormCompletionHandlers.values())
-        completionHandler();
-
     ASSERT_WITH_MESSAGE(!WebProcess::singleton().webFrame(m_frameID), "invalidate should have removed this WebFrame before destruction");
 
 #ifndef NDEBUG
@@ -309,20 +305,6 @@ uint64_t WebFrame::setUpPolicyListener(WebCore::PolicyCheckIdentifier identifier
     return policyListenerID;
 }
 
-FormSubmitListenerIdentifier WebFrame::setUpWillSubmitFormListener(CompletionHandler<void()>&& completionHandler)
-{
-    auto identifier = FormSubmitListenerIdentifier::generate();
-    m_willSubmitFormCompletionHandlers.set(identifier, WTFMove(completionHandler));
-    return identifier;
-}
-
-void WebFrame::continueWillSubmitForm(FormSubmitListenerIdentifier listenerID)
-{
-    Ref<WebFrame> protectedThis(*this);
-    if (auto completionHandler = m_willSubmitFormCompletionHandlers.take(listenerID))
-        completionHandler();
-}
-
 void WebFrame::didCommitLoadInAnotherProcess(std::optional<WebCore::LayerHostingContextIdentifier> layerHostingContextIdentifier)
 {
     RefPtr coreFrame = m_coreFrame.get();
@@ -468,10 +450,6 @@ void WebFrame::invalidatePolicyListeners()
     auto pendingPolicyChecks = std::exchange(m_pendingPolicyChecks, { });
     for (auto& policyCheck : pendingPolicyChecks.values())
         policyCheck.policyFunction(PolicyAction::Ignore, policyCheck.corePolicyIdentifier);
-
-    auto willSubmitFormCompletionHandlers = WTFMove(m_willSubmitFormCompletionHandlers);
-    for (auto& completionHandler : willSubmitFormCompletionHandlers.values())
-        completionHandler();
 }
 
 void WebFrame::didReceivePolicyDecision(uint64_t listenerID, PolicyCheckIdentifier identifier, PolicyDecision&& policyDecision)

diff --git a/Source/WebKit/WebProcess/WebPage/WebFrame.h b/Source/WebKit/WebProcess/WebPage/WebFrame.h
@@ -108,9 +108,6 @@ class WebFrame : public API::ObjectImpl<API::Object::Type::BundleFrame>, public
     void invalidatePolicyListeners();
     void didReceivePolicyDecision(uint64_t listenerID, WebCore::PolicyCheckIdentifier, PolicyDecision&&);
 
-    FormSubmitListenerIdentifier setUpWillSubmitFormListener(CompletionHandler<void()>&&);
-    void continueWillSubmitForm(FormSubmitListenerIdentifier);
-
     void didCommitLoadInAnotherProcess(std::optional<WebCore::LayerHostingContextIdentifier>);
     void didFinishLoadInAnotherProcess();
     void removeFromTree();
@@ -244,7 +241,6 @@ class WebFrame : public API::ObjectImpl<API::Object::Type::BundleFrame>, public
     };
     HashMap<uint64_t, PolicyCheck> m_pendingPolicyChecks;
 
-    HashMap<FormSubmitListenerIdentifier, CompletionHandler<void()>> m_willSubmitFormCompletionHandlers;
     std::optional<DownloadID> m_policyDownloadID;
 
     WeakPtr<LoadListener> m_loadListener;

diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.cpp b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
@@ -1083,16 +1083,6 @@ void WebPage::getFrameTree(CompletionHandler<void(FrameTreeNodeData&&)>&& comple
     completionHandler(m_mainFrame->frameTreeData());
 }
 
-void WebPage::continueWillSubmitForm(WebCore::FrameIdentifier frameID, WebKit::FormSubmitListenerIdentifier formListenerID)
-{
-    auto* frame = WebProcess::singleton().webFrame(frameID);
-    if (!frame) {
-        ASSERT_NOT_REACHED();
-        return;
-    }
-    frame->continueWillSubmitForm(formListenerID);
-}
-
 void WebPage::didCommitLoadInAnotherProcess(WebCore::FrameIdentifier frameID, std::optional<WebCore::LayerHostingContextIdentifier> layerHostingContextIdentifier)
 {
     auto* frame = WebProcess::singleton().webFrame(frameID);

diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.h b/Source/WebKit/WebProcess/WebPage/WebPage.h
@@ -611,7 +611,6 @@ class WebPage : public API::ObjectImpl<API::Object::Type::BundlePage>, public IP
 
     void getFrameInfo(WebCore::FrameIdentifier, CompletionHandler<void(FrameInfoData&&)>&&);
     void getFrameTree(CompletionHandler<void(FrameTreeNodeData&&)>&&);
-    void continueWillSubmitForm(WebCore::FrameIdentifier, WebKit::FormSubmitListenerIdentifier);
     void didCommitLoadInAnotherProcess(WebCore::FrameIdentifier, std::optional<WebCore::LayerHostingContextIdentifier>);
     void didFinishLoadInAnotherProcess(WebCore::FrameIdentifier);
     void frameWasRemovedInAnotherProcess(WebCore::FrameIdentifier);

diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.messages.in b/Source/WebKit/WebProcess/WebPage/WebPage.messages.in
@@ -197,7 +197,6 @@ GenerateSyntheticEditingCommand(enum:uint8_t WebKit::SyntheticEditingCommandType
 
     GetFrameInfo(WebCore::FrameIdentifier frameID) -> (struct WebKit::FrameInfoData data)
     GetFrameTree() -> (struct WebKit::FrameTreeNodeData data)
-    ContinueWillSubmitForm(WebCore::FrameIdentifier frameID, WebKit::FormSubmitListenerIdentifier listenerID)
     DidCommitLoadInAnotherProcess(WebCore::FrameIdentifier frameID, std::optional<WebCore::LayerHostingContextIdentifier> layerHostingContextIdentifier)
     DidFinishLoadInAnotherProcess(WebCore::FrameIdentifier frameID)
     FrameWasRemovedInAnotherProcess(WebCore::FrameIdentifier frameID)