ASSERTION FAILURE: Completion handlers not invalidated when WebPage::…

…~WebPage() invoked navigating to docs.google.com and signing in

https://bugs.webkit.org/show_bug.cgi?id=214098
<rdar://problem/64848288>

Reviewed by Geoffrey Garen.

Source/WebKit:

Make sure the WebPage destructor calls its "markLayersAsVolatile" CompletionHandlers before destroying
them.

Change is covered by new API test.

* UIProcess/API/Cocoa/WKWebViewPrivateForTesting.h:
* UIProcess/API/Cocoa/WKWebViewTesting.mm:
(-[WKWebView _processWillSuspendForTesting:]):
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::~WebPage):

Tools:

Add API test coverage.

* TestWebKitAPI/Tests/WebKitCocoa/ProcessSuspension.mm:
(TEST):


Canonical link: https://commits.webkit.org/226915@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@264138 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/ChangeLog

@@ -1,3 +1,22 @@
+2020-07-08  Chris Dumez  <cdumez@apple.com>
+
+        ASSERTION FAILURE: Completion handlers not invalidated when WebPage::~WebPage() invoked navigating to docs.google.com and signing in
+        https://bugs.webkit.org/show_bug.cgi?id=214098
+        <rdar://problem/64848288>
+
+        Reviewed by Geoffrey Garen.
+
+        Make sure the WebPage destructor calls its "markLayersAsVolatile" CompletionHandlers before destroying
+        them.
+
+        Change is covered by new API test.
+
+        * UIProcess/API/Cocoa/WKWebViewPrivateForTesting.h:
+        * UIProcess/API/Cocoa/WKWebViewTesting.mm:
+        (-[WKWebView _processWillSuspendForTesting:]):
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::~WebPage):
+
 2020-07-08  Per Arne Vollan  <pvollan@apple.com>
 
         [Cocoa] Update Launch Services database in the WebContent process from the Network process

Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivateForTesting.h

@@ -60,6 +60,7 @@ typedef enum {
 
 @property (nonatomic, setter=_setScrollingUpdatesDisabledForTesting:) BOOL _scrollingUpdatesDisabledForTesting;
 
+- (void)_processWillSuspendForTesting:(void (^)(void))completionHandler;
 - (void)_processWillSuspendImminentlyForTesting;
 - (void)_processDidResumeForTesting;
 @property (nonatomic, readonly) BOOL _hasServiceWorkerBackgroundActivityForTesting;

Source/WebKit/UIProcess/API/Cocoa/WKWebViewTesting.mm

@@ -170,6 +170,17 @@ - (BOOL)_hasInspectorFrontend
     return _page && _page->hasInspectorFrontend();
 }
 
+- (void)_processWillSuspendForTesting:(void (^)(void))completionHandler
+{
+    if (!_page) {
+        completionHandler();
+        return;
+    }
+    _page->process().sendPrepareToSuspend(WebKit::IsSuspensionImminent::No, [completionHandler = makeBlockPtr(completionHandler)] {
+        completionHandler();
+    });
+}
+
 - (void)_processWillSuspendImminentlyForTesting
 {
     if (_page)

Source/WebKit/WebProcess/WebPage/WebPage.cpp

@@ -895,6 +895,9 @@ WebPage::~WebPage()
     if (m_videoFullscreenManager)
         m_videoFullscreenManager->invalidate();
 #endif
+
+    for (auto& completionHandler : std::exchange(m_markLayersAsVolatileCompletionHandlers, { }))
+        completionHandler(false);
 }
 
 IPC::Connection* WebPage::messageSenderConnection() const

Tools/ChangeLog

@@ -1,3 +1,16 @@
+2020-07-08  Chris Dumez  <cdumez@apple.com>
+
+        ASSERTION FAILURE: Completion handlers not invalidated when WebPage::~WebPage() invoked navigating to docs.google.com and signing in
+        https://bugs.webkit.org/show_bug.cgi?id=214098
+        <rdar://problem/64848288>
+
+        Reviewed by Geoffrey Garen.
+
+        Add API test coverage.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSuspension.mm:
+        (TEST):
+
 2020-07-08  Sam Weinig  <weinig@apple.com>
 
         Part 3 of SimpleColor and SRGBA<uint8_t> are essentially the same - let's converge them

Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSuspension.mm

@@ -28,6 +28,7 @@
 #import "PlatformUtilities.h"
 #import "TestWKWebView.h"
 #import <WebKit/WKWebViewConfiguration.h>
+#import <WebKit/WKWebViewConfigurationPrivate.h>
 #import <WebKit/WKWebViewPrivate.h>
 #import <WebKit/WKWebViewPrivateForTesting.h>
 
@@ -51,3 +52,34 @@
     }];
     TestWebKitAPI::Util::run(&done);
 }
+
+TEST(ProcessSuspension, DestroyWebPageDuringWebProcessSuspension)
+{
+    auto configuration1 = adoptNS([[WKWebViewConfiguration alloc] init]);
+    auto webView1 = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 100, 100) configuration:configuration1.get() addToWindow:YES]);
+    [webView1 synchronouslyLoadTestPageNamed:@"large-red-square-image"];
+
+    auto pid1 = [webView1 _webProcessIdentifier];
+    EXPECT_NE(pid1, 0);
+
+    auto configuration2 = adoptNS([[WKWebViewConfiguration alloc] init]);
+    configuration2.get().processPool = configuration1.get().processPool;
+    configuration2.get()._relatedWebView = webView1.get();
+    auto webView2 = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(100, 0, 100, 100) configuration:configuration2.get() addToWindow:YES]);
+    [webView2 synchronouslyLoadTestPageNamed:@"large-red-square-image"];
+
+    auto pid2 = [webView2 _webProcessIdentifier];
+    EXPECT_EQ(pid1, pid2);
+
+    auto webView3 = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(200, 0, 100, 100) configuration:configuration2.get() addToWindow:YES]);
+    [webView3 synchronouslyLoadTestPageNamed:@"large-red-square-image"];
+
+    [webView3 _processWillSuspendForTesting:^{ }];
+    [webView1 _close];
+    TestWebKitAPI::Util::sleep(0.1);
+    [webView2 _close];
+
+    EXPECT_EQ(pid1, [webView3 _webProcessIdentifier]);
+    TestWebKitAPI::Util::sleep(1);
+    EXPECT_EQ(pid1, [webView3 _webProcessIdentifier]);
+}