Begin implementing RemoteDOMWindow::close

https://bugs.webkit.org/show_bug.cgi?id=263572
rdar://111064432

Reviewed by Pascoe.

This begins to work as expected.  Radars are filed for future work.

* Source/WebCore/page/DOMWindow.idl:
* Source/WebCore/page/LocalDOMWindow.idl:
* Source/WebCore/page/RemoteDOMWindow.cpp:
(WebCore::RemoteDOMWindow::close):
* Source/WebCore/page/RemoteDOMWindow.idl:
* Source/WebCore/page/RemoteFrameClient.h:
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::close):
(WebKit::WebPageProxy::removeOpenedRemotePageProxy):
* Source/WebKit/UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::closeRemoteFrame):
* Source/WebKit/UIProcess/WebProcessProxy.h:
* Source/WebKit/UIProcess/WebProcessProxy.messages.in:
* Source/WebKit/WebProcess/WebCoreSupport/WebRemoteFrameClient.cpp:
(WebKit::WebRemoteFrameClient::close):
* Source/WebKit/WebProcess/WebCoreSupport/WebRemoteFrameClient.h:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/SiteIsolation.mm:
(TestWebKitAPI::openerAndOpenedViews):
(TestWebKitAPI::TEST):
* Tools/TestWebKitAPI/cocoa/TestUIDelegate.h:
* Tools/TestWebKitAPI/cocoa/TestUIDelegate.mm:
(-[TestUIDelegate webViewDidClose:]):
(-[TestUIDelegate waitForDidClose]):

Canonical link: https://commits.webkit.org/269734@main

Source/WebCore/page/DOMWindow.idl

@@ -27,4 +27,5 @@ interface mixin DOMWindow {
     [CallWith=CurrentGlobalObject&IncumbentWindow, DoNotCheckSecurity] undefined postMessage(any message, USVString targetOrigin, optional sequence<object> transfer = []);
     [CallWith=CurrentGlobalObject&IncumbentWindow, DoNotCheckSecurity] undefined postMessage(any message, optional WindowPostMessageOptions options);
     [DoNotCheckSecurity, PutForwards=href, LegacyUnforgeable] readonly attribute Location location;
+    [DoNotCheckSecurity, CallWith=IncumbentDocument] undefined close();
 };

Source/WebCore/page/LocalDOMWindow.idl

@@ -60,7 +60,6 @@
     [Replaceable] readonly attribute BarProp statusbar;
     [Replaceable] readonly attribute BarProp toolbar;
     attribute DOMString status;
-    [DoNotCheckSecurity, CallWith=IncumbentDocument] undefined close();
     [DoNotCheckSecurity] readonly attribute boolean closed;
     undefined stop();
     [DoNotCheckSecurity, CallWith=IncumbentWindow] undefined focus();

Source/WebCore/page/RemoteDOMWindow.cpp

@@ -60,7 +60,10 @@ WindowProxy* RemoteDOMWindow::self() const
 
 void RemoteDOMWindow::close(Document&)
 {
-    // FIXME: Implemented this. <rdar://116203970>
+    // FIXME: <rdar://117381050> Add security checks here equivalent to LocalDOMWindow::close (both with and without Document& parameter).
+    // Or refactor to share code.
+    if (m_frame && m_frame->isMainFrame())
+        m_frame->client().close();
 }
 
 bool RemoteDOMWindow::closed() const

Source/WebCore/page/RemoteDOMWindow.idl

@@ -43,7 +43,6 @@
 ] interface RemoteDOMWindow {
     [LegacyUnforgeable, ImplementedAs=self] readonly attribute WindowProxy window;
     [Replaceable] readonly attribute WindowProxy self;
-    [CallWith=IncumbentDocument] undefined close();
     readonly attribute boolean closed;
     [CallWith=IncumbentWindow] undefined focus();
     undefined blur();

Source/WebCore/page/RemoteFrameClient.h

@@ -46,6 +46,7 @@ class RemoteFrameClient {
     virtual void changeLocation(FrameLoadRequest&&) = 0;
     virtual String renderTreeAsText(size_t baseIndent, OptionSet<RenderAsTextFlag>) = 0;
     virtual void broadcastFrameRemovalToOtherProcesses() = 0;
+    virtual void close() = 0;
     virtual ~RemoteFrameClient() { }
 };
 

Source/WebKit/UIProcess/WebPageProxy.cpp

@@ -1478,6 +1478,9 @@ void WebPageProxy::close()
 
     stopAllURLSchemeTasks();
     updatePlayingMediaDidChange(MediaProducer::IsNotPlaying);
+
+    if (RefPtr openerPage = m_openerFrame ? m_openerFrame->page() : nullptr)
+        openerPage->removeOpenedRemotePageProxy(identifier());
 }
 
 bool WebPageProxy::tryClose()
@@ -12895,11 +12898,9 @@ RefPtr<RemotePageProxy> WebPageProxy::takeRemotePageProxyInOpenerProcessIfDomain
     return std::exchange(internals().remotePageProxyInOpenerProcess, nullptr);
 }
 
-// FIXME: Add a remove call if the opened page is closed or destroyed. <rdar://111064432>
 void WebPageProxy::removeOpenedRemotePageProxy(WebPageProxyIdentifier pageID)
 {
-    bool contained = internals().openedRemotePageProxies.remove(pageID);
-    ASSERT_UNUSED(contained, contained);
+    internals().openedRemotePageProxies.remove(pageID);
 }
 
 void WebPageProxy::addOpenedRemotePageProxy(WebPageProxyIdentifier pageID, Ref<RemotePageProxy>&& page)

Source/WebKit/UIProcess/WebProcessProxy.cpp

@@ -1410,6 +1410,21 @@ void WebProcessProxy::postMessageToRemote(WebCore::FrameIdentifier identifier, s
     destinationFrame->process().send(Messages::WebProcess::RemotePostMessage(identifier, target, message), 0);
 }
 
+void WebProcessProxy::closeRemoteFrame(WebCore::FrameIdentifier frameID)
+{
+    // FIXME: <rdar://117383252> This, postMessageToRemote, renderTreeAsText, etc. should be messages to the WebPageProxy instead of the process.
+    // They are more the page doing things than the process.
+    RefPtr destinationFrame = WebFrameProxy::webFrame(frameID);
+    if (!destinationFrame)
+        return;
+    if (!destinationFrame->isMainFrame())
+        return;
+    RefPtr page = destinationFrame->page();
+    if (!page)
+        return;
+    page->closePage();
+}
+
 void WebProcessProxy::renderTreeAsText(WebCore::FrameIdentifier frameIdentifier, size_t baseIndent, OptionSet<WebCore::RenderAsTextFlag> behavior, CompletionHandler<void(String&&)>&& completionHandler)
 {
     auto* frame = WebFrameProxy::webFrame(frameIdentifier);

Source/WebKit/UIProcess/WebProcessProxy.h

@@ -539,6 +539,7 @@ class WebProcessProxy : public AuxiliaryProcessProxy, private ProcessThrottlerCl
     void didDestroyFrame(WebCore::FrameIdentifier, WebPageProxyIdentifier);
     void didDestroyUserGestureToken(uint64_t);
     void postMessageToRemote(WebCore::FrameIdentifier, std::optional<WebCore::SecurityOriginData>, const WebCore::MessageWithMessagePorts&);
+    void closeRemoteFrame(WebCore::FrameIdentifier);
     void renderTreeAsText(WebCore::FrameIdentifier, size_t baseIndent, OptionSet<WebCore::RenderAsTextFlag>, CompletionHandler<void(String&&)>&&);
 
     bool canBeAddedToWebProcessCache() const;

Source/WebKit/UIProcess/WebProcessProxy.messages.in

@@ -89,6 +89,7 @@ messages -> WebProcessProxy LegacyReceiver {
     SetClientBadge(WebKit::WebPageProxyIdentifier pageIdentifier, WebCore::SecurityOriginData origin, std::optional<uint64_t> badge)
 
     PostMessageToRemote(WebCore::FrameIdentifier identifier, std::optional<WebCore::SecurityOriginData> target, struct WebCore::MessageWithMessagePorts message)
+    CloseRemoteFrame(WebCore::FrameIdentifier identifier)
 
     RenderTreeAsText(WebCore::FrameIdentifier identifier, size_t baseIndent, OptionSet<WebCore::RenderAsTextFlag> behavior) -> (String renderTreeDump) Synchronous
 }

Source/WebKit/WebProcess/WebCoreSupport/WebRemoteFrameClient.cpp

@@ -27,6 +27,7 @@
 #include "WebRemoteFrameClient.h"
 
 #include "MessageSenderInlines.h"
+#include "WebPage.h"
 #include "WebProcess.h"
 #include "WebProcessProxyMessages.h"
 #include <WebCore/FrameLoadRequest.h>
@@ -97,4 +98,10 @@ void WebRemoteFrameClient::broadcastFrameRemovalToOtherProcesses()
     WebFrameLoaderClient::broadcastFrameRemovalToOtherProcesses();
 }
 
+void WebRemoteFrameClient::close()
+{
+    // FIXME: <rdar://117381050> Consider if this needs the same logic as WebChromeClient::closeWindow, or refactor to share code.
+    WebProcess::singleton().send(Messages::WebProcessProxy::CloseRemoteFrame(m_frame->frameID()), 0);
+}
+
 }

Source/WebKit/WebProcess/WebCoreSupport/WebRemoteFrameClient.h

@@ -49,6 +49,7 @@ class WebRemoteFrameClient final : public WebCore::RemoteFrameClient, public Web
     void changeLocation(WebCore::FrameLoadRequest&&) final;
     String renderTreeAsText(size_t baseIndent, OptionSet<WebCore::RenderAsTextFlag>) final;
     void broadcastFrameRemovalToOtherProcesses() final;
+    void close() final;
 
     ScopeExit<Function<void()>> m_frameInvalidator;
 };

Tools/TestWebKitAPI/Tests/WebKitCocoa/SiteIsolation.mm

@@ -333,55 +333,92 @@ HTTPServer server({
     EXPECT_WK_STREQ(alert.get(), "opened page received pong");
 }
 
-TEST(SiteIsolation, NavigationAfterWindowOpen)
-{
-    HTTPServer server({
-        { "/example_opener"_s, { "<script>w = window.open('https://webkit.org/webkit')</script>"_s } },
-        { "/webkit"_s, { "hi"_s } },
-        { "/example_opened_after_navigation"_s, { "hi"_s } }
-    }, HTTPServer::Protocol::HttpsProxy);
+struct WebViewAndDelegates {
+    RetainPtr<WKWebView> webView;
+    RetainPtr<TestNavigationDelegate> navigationDelegate;
+    RetainPtr<TestUIDelegate> uiDelegate;
+};
 
-    auto navigationDelegate = adoptNS([TestNavigationDelegate new]);
-    [navigationDelegate allowAnyTLSCertificate];
+static std::pair<WebViewAndDelegates, WebViewAndDelegates> openerAndOpenedViews(const HTTPServer& server)
+{
+    __block WebViewAndDelegates opener;
+    __block WebViewAndDelegates opened;
+    opener.navigationDelegate = adoptNS([TestNavigationDelegate new]);
+    [opener.navigationDelegate allowAnyTLSCertificate];
     auto configuration = server.httpsProxyConfiguration();
     enableSiteIsolation(configuration);
     enableWindowOpenPSON(configuration);
-    auto openerWebView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration]);
-    openerWebView.get().navigationDelegate = navigationDelegate.get();
-    auto uiDelegate = adoptNS([TestUIDelegate new]);
-    __block RetainPtr<WKWebView> openedWebView;
-    __block RetainPtr<TestNavigationDelegate> openedNavigationDelegate;
-    uiDelegate.get().createWebViewWithConfiguration = ^(WKWebViewConfiguration *configuration, WKNavigationAction *action, WKWindowFeatures *windowFeatures) {
+    opener.webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration]);
+    opener.webView.get().navigationDelegate = opener.navigationDelegate.get();
+    opener.uiDelegate = adoptNS([TestUIDelegate new]);
+    opener.uiDelegate.get().createWebViewWithConfiguration = ^(WKWebViewConfiguration *configuration, WKNavigationAction *action, WKWindowFeatures *windowFeatures) {
         enableSiteIsolation(configuration);
         enableWindowOpenPSON(configuration);
-        openedWebView = adoptNS([[WKWebView alloc] initWithFrame:CGRectZero configuration:configuration]);
-        openedNavigationDelegate = adoptNS([TestNavigationDelegate new]);
-        [openedNavigationDelegate allowAnyTLSCertificate];
-        openedWebView.get().navigationDelegate = openedNavigationDelegate.get();
-        return openedWebView.get();
+        opened.webView = adoptNS([[WKWebView alloc] initWithFrame:CGRectZero configuration:configuration]);
+        opened.navigationDelegate = adoptNS([TestNavigationDelegate new]);
+        [opened.navigationDelegate allowAnyTLSCertificate];
+        opened.uiDelegate = adoptNS([TestUIDelegate new]);
+        opened.webView.get().navigationDelegate = opened.navigationDelegate.get();
+        opened.webView.get().UIDelegate = opened.uiDelegate.get();
+        return opened.webView.get();
     };
-    [openerWebView setUIDelegate:uiDelegate.get()];
-    openerWebView.get().configuration.preferences.javaScriptCanOpenWindowsAutomatically = YES;
-
-    [openerWebView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"https://example.com/example_opener"]]];
-    while (!openedWebView)
+    [opener.webView setUIDelegate:opener.uiDelegate.get()];
+    opener.webView.get().configuration.preferences.javaScriptCanOpenWindowsAutomatically = YES;
+    [opener.webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"https://example.com/example"]]];
+    while (!opened.webView)
         Util::spinRunLoop();
-    [openedNavigationDelegate waitForDidFinishNavigation];
+    [opened.navigationDelegate waitForDidFinishNavigation];
+    return { WTFMove(opener), WTFMove(opened) };
+}
+
+TEST(SiteIsolation, NavigationAfterWindowOpen)
+{
+    HTTPServer server({
+        { "/example"_s, { "<script>w = window.open('https://webkit.org/webkit')</script>"_s } },
+        { "/webkit"_s, { "hi"_s } },
+        { "/example_opened_after_navigation"_s, { "hi"_s } }
+    }, HTTPServer::Protocol::HttpsProxy);
+
+    auto [opener, opened] = openerAndOpenedViews(server);
+    checkFrameTreesInProcesses(opener.webView.get(), { { "https://example.com"_s }, { RemoteFrame } });
+    checkFrameTreesInProcesses(opened.webView.get(), { { RemoteFrame }, { "https://webkit.org"_s } });
+    pid_t webKitPid = findFramePID(frameTrees(opener.webView.get()).get(), FrameType::Remote);
 
-    checkFrameTreesInProcesses(openerWebView.get(), { { "https://example.com"_s }, { RemoteFrame } });
-    checkFrameTreesInProcesses(openedWebView.get(), { { RemoteFrame }, { "https://webkit.org"_s } });
-    pid_t webKitPid = findFramePID(frameTrees(openerWebView.get()).get(), FrameType::Remote);
+    [opened.webView evaluateJavaScript:@"window.location = 'https://example.com/example_opened_after_navigation'" completionHandler:nil];
+    [opened.navigationDelegate waitForDidFinishNavigation];
 
-    [openedWebView evaluateJavaScript:@"window.location = 'https://example.com/example_opened_after_navigation'" completionHandler:nil];
-    [openedNavigationDelegate waitForDidFinishNavigation];
+    checkFrameTreesInProcesses(opener.webView.get(), { { "https://example.com"_s } });
+    checkFrameTreesInProcesses(opened.webView.get(), { { "https://example.com"_s } });
 
-    checkFrameTreesInProcesses(openerWebView.get(), { { "https://example.com"_s } });
-    checkFrameTreesInProcesses(openedWebView.get(), { { "https://example.com"_s } });
+    while (processStillRunning(webKitPid))
+        Util::spinRunLoop();
+}
 
+TEST(SiteIsolation, CloseAfterWindowOpen)
+{
+    HTTPServer server({
+        { "/example"_s, { "<script>w = window.open('https://webkit.org/webkit')</script>"_s } },
+        { "/webkit"_s, { "hi"_s } }
+    }, HTTPServer::Protocol::HttpsProxy);
+
+    auto [opener, opened] = openerAndOpenedViews(server);
+    pid_t webKitPid = findFramePID(frameTrees(opener.webView.get()).get(), FrameType::Remote);
+    [opener.webView evaluateJavaScript:@"w.close()" completionHandler:nil];
+    [opened.uiDelegate waitForDidClose];
+    opened.webView = nullptr;
     while (processStillRunning(webKitPid))
         Util::spinRunLoop();
+    checkFrameTreesInProcesses(opener.webView.get(), { { "https://example.com"_s } });
 }
 
+// FIXME: <rdar://117383420> Add a test that deallocates the opened WKWebView without being asked to by JS.
+// Check state using native *and* JS APIs. Make sure processes are torn down as expected.
+// Same with the opener WKWebView. We would probably need to set remotePageProxyInOpenerProcess
+// to null manually to make the process terminate.
+//
+// Also test when the opener frame (if it's an iframe) is removed from the tree and garbage collected.
+// That should probably do some teardown that should be visible from the API.
+
 TEST(SiteIsolation, PostMessageWithMessagePorts)
 {
     auto exampleHTML = "<script>"

Tools/TestWebKitAPI/cocoa/TestUIDelegate.h

@@ -37,10 +37,12 @@
 #endif
 @property (nonatomic, copy) void (^saveDataToFile)(WKWebView *, NSData *, NSString *, NSString *, NSURL *);
 @property (nonatomic, copy) void (^focusWebView)(WKWebView *);
+@property (nonatomic, copy) void (^webViewDidClose)(WKWebView *);
 
 - (NSString *)waitForAlert;
 - (NSString *)waitForConfirm;
 - (NSString *)waitForPromptWithReply:(NSString *)reply;
+- (void)waitForDidClose;
 
 @end
 

Tools/TestWebKitAPI/cocoa/TestUIDelegate.mm

@@ -89,6 +89,12 @@ - (void)_focusWebView:(WKWebView *)webView
 }
 #endif // PLATFORM(MAC)
 
+- (void)webViewDidClose:(WKWebView *)webView
+{
+    if (_webViewDidClose)
+        _webViewDidClose(webView);
+}
+
 - (void)_webView:(WKWebView *)webView saveDataToFile:(NSData *)data suggestedFilename:(NSString *)suggestedFilename mimeType:(NSString *)mimeType originatingURL:(NSURL *)url
 {
     if (_saveDataToFile)
@@ -150,6 +156,16 @@ - (NSString *)waitForPromptWithReply:(NSString *)reply
     return result.autorelease();
 }
 
+- (void)waitForDidClose
+{
+    EXPECT_FALSE(self.webViewDidClose);
+    __block bool closed { false };
+    self.webViewDidClose = ^(WKWebView *) {
+        closed = true;
+    };
+    TestWebKitAPI::Util::run(&closed);
+}
+
 - (void)_webView:(WKWebView *)webView didAttachLocalInspector:(_WKInspector *)inspector
 {
     _showedInspector = YES;