Changeset 238356 in webkit


Ignore:
Timestamp:
Nov 17, 2018 4:25:20 PM (5 years ago)
Author:
Chris Dumez
Message:

ASSERTION FAILED: m_messageReceivers.contains(...) under ViewGestureController removeMessageReceiver
https://bugs.webkit.org/show_bug.cgi?id=191734
<rdar://problem/46151497>

Reviewed by Ryosuke Niwa.

Source/WebKit:

When a WebProcess crashes, we destroy the ViewGestureController and reconstruct it later
after we've relaunched a new WebProcess. The ViewGestureController controller takes care
of adding itself as an IPC message receiver to the WebProcessProxy, and the destructor
takes care of removing itself as an IPC message receiver.

However, when process-swapping on navigation, we do not destroy the ViewGestureController
because doing so would take down the swipe gesture snapshot on cross-site swipe navigation.
This led to hitting this assertion later on because the ViewGestureController is still
registered as an IPC message receiver with the old process after process swapping.

To address the issue, we now make sure the ViewGestureController unregisters itself from
the old process and registers itself with the new process on process-swap.

  • UIProcess/Cocoa/ViewGestureController.cpp:

(WebKit::ViewGestureController::ViewGestureController):
(WebKit::ViewGestureController::~ViewGestureController):
(WebKit::ViewGestureController::disconnectFromProcess):
(WebKit::ViewGestureController::connectToProcess):

  • UIProcess/Cocoa/ViewGestureController.h:
  • UIProcess/Cocoa/WebViewImpl.mm:

(WebKit::WebViewImpl::processWillSwap):
(WebKit::WebViewImpl::didRelaunchProcess):

Tools:

Add API test coverage.

  • TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
Location:
trunk
Files:
6 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebKit/ChangeLog

    r238353 r238356  
     12018-11-17  Chris Dumez  <cdumez@apple.com>
     2
     3        ASSERTION FAILED: m_messageReceivers.contains(...) under ViewGestureController removeMessageReceiver
     4        https://bugs.webkit.org/show_bug.cgi?id=191734
     5        <rdar://problem/46151497>
     6
     7        Reviewed by Ryosuke Niwa.
     8
     9        When a WebProcess crashes, we destroy the ViewGestureController and reconstruct it later
     10        after we've relaunched a new WebProcess. The ViewGestureController controller takes care
     11        of adding itself as an IPC message receiver to the WebProcessProxy, and the destructor
     12        takes care of removing itself as an IPC message receiver.
     13
     14        However, when process-swapping on navigation, we do not destroy the ViewGestureController
     15        because doing so would take down the swipe gesture snapshot on cross-site swipe navigation.
     16        This led to hitting this assertion later on because the ViewGestureController is still
     17        registered as an IPC message receiver with the old process after process swapping.
     18
     19        To address the issue, we now make sure the ViewGestureController unregisters itself from
     20        the old process and registers itself with the new process on process-swap.
     21
     22        * UIProcess/Cocoa/ViewGestureController.cpp:
     23        (WebKit::ViewGestureController::ViewGestureController):
     24        (WebKit::ViewGestureController::~ViewGestureController):
     25        (WebKit::ViewGestureController::disconnectFromProcess):
     26        (WebKit::ViewGestureController::connectToProcess):
     27        * UIProcess/Cocoa/ViewGestureController.h:
     28        * UIProcess/Cocoa/WebViewImpl.mm:
     29        (WebKit::WebViewImpl::processWillSwap):
     30        (WebKit::WebViewImpl::didRelaunchProcess):
     31
    1322018-11-17  Chris Dumez  <cdumez@apple.com>
    233
  • trunk/Source/WebKit/UIProcess/Cocoa/ViewGestureController.cpp

    r236966 r238356  
    6464#endif
    6565{
     66    connectToProcess();
     67
     68    viewGestureControllersForAllPages().add(webPageProxy.pageID(), this);
     69}
     70
     71ViewGestureController::~ViewGestureController()
     72{
     73    platformTeardown();
     74
     75    viewGestureControllersForAllPages().remove(m_webPageProxy.pageID());
     76
     77    disconnectFromProcess();
     78}
     79
     80void ViewGestureController::disconnectFromProcess()
     81{
     82    if (!m_isConnectedToProcess)
     83        return;
     84
     85    m_webPageProxy.process().removeMessageReceiver(Messages::ViewGestureController::messageReceiverName(), m_webPageProxy.pageID());
     86    m_isConnectedToProcess = false;
     87}
     88
     89void ViewGestureController::connectToProcess()
     90{
     91    if (m_isConnectedToProcess)
     92        return;
     93
    6694    m_webPageProxy.process().addMessageReceiver(Messages::ViewGestureController::messageReceiverName(), m_webPageProxy.pageID(), *this);
    67 
    68     viewGestureControllersForAllPages().add(webPageProxy.pageID(), this);
    69 }
    70 
    71 ViewGestureController::~ViewGestureController()
    72 {
    73     platformTeardown();
    74 
    75     viewGestureControllersForAllPages().remove(m_webPageProxy.pageID());
    76 
    77     m_webPageProxy.process().removeMessageReceiver(Messages::ViewGestureController::messageReceiverName(), m_webPageProxy.pageID());
     95    m_isConnectedToProcess = true;
    7896}
    7997
  • trunk/Source/WebKit/UIProcess/Cocoa/ViewGestureController.h

    r237266 r238356  
    6969    ~ViewGestureController();
    7070    void platformTeardown();
     71
     72    void disconnectFromProcess();
     73    void connectToProcess();
    7174   
    7275    enum class ViewGestureType {
     
    307310    uint64_t m_snapshotRemovalTargetRenderTreeSize { 0 };
    308311#endif
     312    bool m_isConnectedToProcess { false };
    309313
    310314    SnapshotRemovalTracker m_snapshotRemovalTracker;
  • trunk/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm

    r238212 r238356  
    14781478{
    14791479    handleProcessSwapOrExit();
     1480    if (m_gestureController)
     1481        m_gestureController->disconnectFromProcess();
    14801482}
    14811483
     
    14931495void WebViewImpl::didRelaunchProcess()
    14941496{
     1497    if (m_gestureController)
     1498        m_gestureController->connectToProcess();
     1499
    14951500    accessibilityRegisterUIProcessTokens();
    14961501}
  • trunk/Tools/ChangeLog

    r238349 r238356  
     12018-11-17  Chris Dumez  <cdumez@apple.com>
     2
     3        ASSERTION FAILED: m_messageReceivers.contains(...) under ViewGestureController removeMessageReceiver
     4        https://bugs.webkit.org/show_bug.cgi?id=191734
     5        <rdar://problem/46151497>
     6
     7        Reviewed by Ryosuke Niwa.
     8
     9        Add API test coverage.
     10
     11        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
     12
    1132018-11-17  Zalan Bujtas  <zalan@apple.com>
    214
  • trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm

    r238339 r238356  
    2828#import "PlatformUtilities.h"
    2929#import "Test.h"
     30#import "TestNavigationDelegate.h"
    3031#import <WebKit/WKNavigationDelegatePrivate.h>
    3132#import <WebKit/WKNavigationPrivate.h>
     
    27282729#if PLATFORM(MAC)
    27292730
     2731TEST(ProcessSwap, TerminateProcessAfterProcessSwap)
     2732{
     2733    auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
     2734    [processPoolConfiguration setProcessSwapsOnNavigation:YES];
     2735    auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
     2736
     2737    auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
     2738    [webViewConfiguration setProcessPool:processPool.get()];
     2739    auto handler = adoptNS([[PSONScheme alloc] init]);
     2740    [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
     2741
     2742    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
     2743    [webView setAllowsBackForwardNavigationGestures:YES];
     2744
     2745    auto navigationDelegate = adoptNS([[TestNavigationDelegate alloc] init]);
     2746    [webView setNavigationDelegate:navigationDelegate.get()];
     2747    __block bool webProcessTerminated = false;
     2748    [navigationDelegate setWebContentProcessDidTerminate:^(WKWebView *) {
     2749        webProcessTerminated = true;
     2750    }];
     2751    [navigationDelegate setDidFinishNavigation:^(WKWebView *, WKNavigation *) {
     2752        done = true;
     2753    }];
     2754
     2755    // Make sure there is a gesture controller.
     2756    [webView _setCustomSwipeViewsTopContentInset:2.];
     2757
     2758    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
     2759    [webView loadRequest:request];
     2760    TestWebKitAPI::Util::run(&done);
     2761    done = false;
     2762
     2763    auto webkitPID = [webView _webProcessIdentifier];
     2764
     2765    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main.html"]];
     2766
     2767    [webView loadRequest:request];
     2768    TestWebKitAPI::Util::run(&done);
     2769    done = false;
     2770
     2771    EXPECT_NE(webkitPID, [webView _webProcessIdentifier]);
     2772
     2773    webProcessTerminated = false;
     2774    kill([webView _webProcessIdentifier], 9);
     2775
     2776    TestWebKitAPI::Util::run(&webProcessTerminated);
     2777
     2778    TestWebKitAPI::Util::spinRunLoop(1);
     2779
     2780    [webView reload];
     2781    TestWebKitAPI::Util::run(&done);
     2782    done = false;
     2783}
     2784
    27302785TEST(ProcessSwap, GoBackToSuspendedPageWithMainFrameIDThatIsNotOne)
    27312786{
Note: See TracChangeset for help on using the changeset viewer.