Bug 194522

Summary: WebPage::close needs to remove all message receivers associated with that WebPage, not WebPage::~WebPage
Product: WebKit Reporter: Alex Christensen <achristensen>
Component: New BugsAssignee: Alex Christensen <achristensen>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, jon, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
cdumez: review+, cdumez: commit-queue-
patch for the safari-607-branch none

Description Alex Christensen 2019-02-11 17:00:43 PST
WebPage::close needs to remove all message receivers associated with that WebPage, not WebPage::~WebPage
Comment 1 Alex Christensen 2019-02-11 17:08:33 PST
Created attachment 361735 [details]
Patch
Comment 2 Alex Christensen 2019-02-11 17:08:35 PST
<rdar://problem/47789393>
Comment 3 Chris Dumez 2019-02-11 18:55:43 PST
Comment on attachment 361735 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=361735&action=review

r=me with nits.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/BundleRetainPagePlugIn.mm:40
> +    dispatch_after(dispatch_time(DISPATCH_TIME_NOW, 0.5*NSEC_PER_SEC), dispatch_get_main_queue(), [retainedPage = retainPtr(browserContextController)] { });

I think we usually have spaces around the *

> Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:2407
> +void testReuseSuspendedProcessForRegularNavigation(bool retainPageInBundle)

We usually use enum classes for such boolean parameters, see other tests in this file.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:2456
> +    testReuseSuspendedProcessForRegularNavigation(true);

Otherwise, call sites are not really readable.
Comment 4 Chris Dumez 2019-02-11 18:57:00 PST
Comment on attachment 361735 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=361735&action=review

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:1318
> +    m_viewGestureGeometryCollector = nullptr;

This needs to be protected like so:
#if PLATFORM(COCOA) || PLATFORM(GTK)
    m_viewGestureGeometryCollector = nullptr;
#endif
Comment 5 Alex Christensen 2019-02-12 11:29:34 PST
http://trac.webkit.org/r241306
Comment 6 Alex Christensen 2019-02-12 11:59:59 PST
Created attachment 361819 [details]
patch for the safari-607-branch
Comment 7 Chris Dumez 2019-02-12 19:45:31 PST
*** Bug 193701 has been marked as a duplicate of this bug. ***
Comment 8 Chris Dumez 2019-02-12 19:47:26 PST
Although this bug does not make it clear, this was causing WebProcess hangs with PSON enabled because the WebPage would stop responding to IPC.