Diff
Modified: branches/safari-608.1.5.1-branch/Source/WebKit/ChangeLog (241345 => 241346)
--- branches/safari-608.1.5.1-branch/Source/WebKit/ChangeLog 2019-02-13 06:36:02 UTC (rev 241345)
+++ branches/safari-608.1.5.1-branch/Source/WebKit/ChangeLog 2019-02-13 06:36:06 UTC (rev 241346)
@@ -1,5 +1,60 @@
2019-02-12 Babak Shafiei <[email protected]>
+ Cherry-pick r241306. rdar://problem/47789393
+
+ WebPage::close needs to remove all message receivers associated with that WebPage, not WebPage::~WebPage
+ https://bugs.webkit.org/show_bug.cgi?id=194522
+ <rdar://problem/47789393>
+
+ Reviewed by Chris Dumez.
+
+ Source/WebKit:
+
+ The InjectedBundle SPI can retain the WebPage or wrapping objects (WKWebProcessPlugInBrowserContextController/WKBundlePageRef).
+ This can make it so WebPage::close is called before WebPage::~WebPage, and if the SuspendedPageProxy is reused for a subsequent
+ navigation to the same domain, the WebProcess is reused with a different WebPage instance with the same PageID, which causes problems
+ when another WebPage registers message handlers and then the previous WebPage is destroyed, which removes both message handlers.
+
+ * WebProcess/WebPage/WebPage.cpp:
+ (WebKit::WebPage::~WebPage):
+ (WebKit::WebPage::close):
+ (WebKit::WebPage::mainFrameDidLayout):
+ * WebProcess/WebPage/WebPage.h:
+ * WebProcess/WebProcess.h:
+ (WebKit::WebProcess::eventDispatcher):
+
+ Tools:
+
+ * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+ * TestWebKitAPI/Tests/WebKitCocoa/BundleRetainPagePlugIn.mm: Added.
+ (-[BundleRetainPagePlugIn webProcessPlugIn:didCreateBrowserContextController:]):
+ * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@241306 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2019-02-12 Alex Christensen <[email protected]>
+
+ WebPage::close needs to remove all message receivers associated with that WebPage, not WebPage::~WebPage
+ https://bugs.webkit.org/show_bug.cgi?id=194522
+ <rdar://problem/47789393>
+
+ Reviewed by Chris Dumez.
+
+ The InjectedBundle SPI can retain the WebPage or wrapping objects (WKWebProcessPlugInBrowserContextController/WKBundlePageRef).
+ This can make it so WebPage::close is called before WebPage::~WebPage, and if the SuspendedPageProxy is reused for a subsequent
+ navigation to the same domain, the WebProcess is reused with a different WebPage instance with the same PageID, which causes problems
+ when another WebPage registers message handlers and then the previous WebPage is destroyed, which removes both message handlers.
+
+ * WebProcess/WebPage/WebPage.cpp:
+ (WebKit::WebPage::~WebPage):
+ (WebKit::WebPage::close):
+ (WebKit::WebPage::mainFrameDidLayout):
+ * WebProcess/WebPage/WebPage.h:
+ * WebProcess/WebProcess.h:
+ (WebKit::WebProcess::eventDispatcher):
+
+2019-02-12 Babak Shafiei <[email protected]>
+
Cherry-pick r241112. rdar://problem/47764549
AX: com.apple.WebKit.WebContent at WebKit: -[WKAccessibilityWebPageObjectBase axObjectCache]
Modified: branches/safari-608.1.5.1-branch/Source/WebKit/WebProcess/WebPage/WebPage.cpp (241345 => 241346)
--- branches/safari-608.1.5.1-branch/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2019-02-13 06:36:02 UTC (rev 241345)
+++ branches/safari-608.1.5.1-branch/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2019-02-13 06:36:06 UTC (rev 241346)
@@ -364,7 +364,7 @@
#endif
, m_layerHostingMode(parameters.layerHostingMode)
#if PLATFORM(COCOA)
- , m_viewGestureGeometryCollector(makeUniqueRef<ViewGestureGeometryCollector>(*this))
+ , m_viewGestureGeometryCollector(std::make_unique<ViewGestureGeometryCollector>(*this))
#elif HAVE(ACCESSIBILITY) && PLATFORM(GTK)
, m_accessibilityObject(nullptr)
#endif
@@ -747,12 +747,6 @@
{
ASSERT(!m_page);
- auto& webProcess = WebProcess::singleton();
-#if ENABLE(ASYNC_SCROLLING)
- if (m_useAsyncScrolling)
- webProcess.eventDispatcher().removeScrollingTreeForPage(this);
-#endif
-
platformDetach();
m_sandboxExtensionTracker.invalidate();
@@ -767,16 +761,6 @@
m_footerBanner->detachFromPage();
#endif // !PLATFORM(IOS_FAMILY)
- webProcess.removeMessageReceiver(Messages::WebPage::messageReceiverName(), m_pageID);
-
- // FIXME: This should be done in the object destructors, and the objects themselves should be message receivers.
- webProcess.removeMessageReceiver(Messages::WebInspector::messageReceiverName(), m_pageID);
- webProcess.removeMessageReceiver(Messages::WebInspectorUI::messageReceiverName(), m_pageID);
- webProcess.removeMessageReceiver(Messages::RemoteWebInspectorUI::messageReceiverName(), m_pageID);
-#if ENABLE(FULLSCREEN_API)
- webProcess.removeMessageReceiver(Messages::WebFullScreenManager::messageReceiverName(), m_pageID);
-#endif
-
#ifndef NDEBUG
webPageCounter.decrement();
#endif
@@ -1315,6 +1299,23 @@
bool isRunningModal = m_isRunningModal;
m_isRunningModal = false;
+ auto& webProcess = WebProcess::singleton();
+#if ENABLE(ASYNC_SCROLLING)
+ if (m_useAsyncScrolling)
+ webProcess.eventDispatcher().removeScrollingTreeForPage(this);
+#endif
+ webProcess.removeMessageReceiver(Messages::WebPage::messageReceiverName(), m_pageID);
+ // FIXME: This should be done in the object destructors, and the objects themselves should be message receivers.
+ webProcess.removeMessageReceiver(Messages::WebInspector::messageReceiverName(), m_pageID);
+ webProcess.removeMessageReceiver(Messages::WebInspectorUI::messageReceiverName(), m_pageID);
+ webProcess.removeMessageReceiver(Messages::RemoteWebInspectorUI::messageReceiverName(), m_pageID);
+#if ENABLE(FULLSCREEN_API)
+ webProcess.removeMessageReceiver(Messages::WebFullScreenManager::messageReceiverName(), m_pageID);
+#endif
+#if PLATFORM(COCOA) || PLATFORM(GTK)
+ m_viewGestureGeometryCollector = nullptr;
+#endif
+
// The WebPage can be destroyed by this call.
WebProcess::singleton().removeWebPage(m_pageID);
@@ -4219,7 +4220,8 @@
}
#if PLATFORM(COCOA)
- m_viewGestureGeometryCollector->mainFrameDidLayout();
+ if (m_viewGestureGeometryCollector)
+ m_viewGestureGeometryCollector->mainFrameDidLayout();
#endif
#if PLATFORM(IOS_FAMILY)
if (FrameView* frameView = mainFrameView()) {
Modified: branches/safari-608.1.5.1-branch/Source/WebKit/WebProcess/WebPage/WebPage.h (241345 => 241346)
--- branches/safari-608.1.5.1-branch/Source/WebKit/WebProcess/WebPage/WebPage.h 2019-02-13 06:36:02 UTC (rev 241345)
+++ branches/safari-608.1.5.1-branch/Source/WebKit/WebProcess/WebPage/WebPage.h 2019-02-13 06:36:06 UTC (rev 241346)
@@ -1577,7 +1577,7 @@
RetainPtr<WKAccessibilityWebPageObject> m_mockAccessibilityElement;
- UniqueRef<ViewGestureGeometryCollector> m_viewGestureGeometryCollector;
+ std::unique_ptr<ViewGestureGeometryCollector> m_viewGestureGeometryCollector;
RetainPtr<NSDictionary> m_dataDetectionContext;
#endif
Modified: branches/safari-608.1.5.1-branch/Source/WebKit/WebProcess/WebProcess.h (241345 => 241346)
--- branches/safari-608.1.5.1-branch/Source/WebKit/WebProcess/WebProcess.h 2019-02-13 06:36:02 UTC (rev 241345)
+++ branches/safari-608.1.5.1-branch/Source/WebKit/WebProcess/WebProcess.h 2019-02-13 06:36:06 UTC (rev 241346)
@@ -165,7 +165,7 @@
PluginProcessConnectionManager& pluginProcessConnectionManager();
#endif
- EventDispatcher& eventDispatcher() { return *m_eventDispatcher; }
+ EventDispatcher& eventDispatcher() { return m_eventDispatcher.get(); }
NetworkProcessConnection& ensureNetworkProcessConnection();
void networkProcessConnectionClosed(NetworkProcessConnection*);
@@ -402,7 +402,7 @@
HashMap<uint64_t, RefPtr<WebPageGroupProxy>> m_pageGroupMap;
RefPtr<InjectedBundle> m_injectedBundle;
- RefPtr<EventDispatcher> m_eventDispatcher;
+ Ref<EventDispatcher> m_eventDispatcher;
#if PLATFORM(IOS_FAMILY)
RefPtr<ViewUpdateDispatcher> m_viewUpdateDispatcher;
#endif
Modified: branches/safari-608.1.5.1-branch/Tools/ChangeLog (241345 => 241346)
--- branches/safari-608.1.5.1-branch/Tools/ChangeLog 2019-02-13 06:36:02 UTC (rev 241345)
+++ branches/safari-608.1.5.1-branch/Tools/ChangeLog 2019-02-13 06:36:06 UTC (rev 241346)
@@ -1,3 +1,50 @@
+2019-02-12 Babak Shafiei <[email protected]>
+
+ Cherry-pick r241306. rdar://problem/47789393
+
+ WebPage::close needs to remove all message receivers associated with that WebPage, not WebPage::~WebPage
+ https://bugs.webkit.org/show_bug.cgi?id=194522
+ <rdar://problem/47789393>
+
+ Reviewed by Chris Dumez.
+
+ Source/WebKit:
+
+ The InjectedBundle SPI can retain the WebPage or wrapping objects (WKWebProcessPlugInBrowserContextController/WKBundlePageRef).
+ This can make it so WebPage::close is called before WebPage::~WebPage, and if the SuspendedPageProxy is reused for a subsequent
+ navigation to the same domain, the WebProcess is reused with a different WebPage instance with the same PageID, which causes problems
+ when another WebPage registers message handlers and then the previous WebPage is destroyed, which removes both message handlers.
+
+ * WebProcess/WebPage/WebPage.cpp:
+ (WebKit::WebPage::~WebPage):
+ (WebKit::WebPage::close):
+ (WebKit::WebPage::mainFrameDidLayout):
+ * WebProcess/WebPage/WebPage.h:
+ * WebProcess/WebProcess.h:
+ (WebKit::WebProcess::eventDispatcher):
+
+ Tools:
+
+ * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+ * TestWebKitAPI/Tests/WebKitCocoa/BundleRetainPagePlugIn.mm: Added.
+ (-[BundleRetainPagePlugIn webProcessPlugIn:didCreateBrowserContextController:]):
+ * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@241306 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2019-02-12 Alex Christensen <[email protected]>
+
+ WebPage::close needs to remove all message receivers associated with that WebPage, not WebPage::~WebPage
+ https://bugs.webkit.org/show_bug.cgi?id=194522
+ <rdar://problem/47789393>
+
+ Reviewed by Chris Dumez.
+
+ * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+ * TestWebKitAPI/Tests/WebKitCocoa/BundleRetainPagePlugIn.mm: Added.
+ (-[BundleRetainPagePlugIn webProcessPlugIn:didCreateBrowserContextController:]):
+ * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
2019-02-04 Youenn Fablet <[email protected]>
Capture state should be managed consistently when doing process swapping
Modified: branches/safari-608.1.5.1-branch/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (241345 => 241346)
--- branches/safari-608.1.5.1-branch/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2019-02-13 06:36:02 UTC (rev 241345)
+++ branches/safari-608.1.5.1-branch/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2019-02-13 06:36:06 UTC (rev 241346)
@@ -303,6 +303,7 @@
5C4A84951F7EEFFC00ACFC54 /* Configuration.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5C4A84941F7EEFD400ACFC54 /* Configuration.mm */; };
5C69BDD51F82A7EF000F4F4B /* _javascript_DuringNavigation.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5C69BDD41F82A7EB000F4F4B /* _javascript_DuringNavigation.mm */; };
5C7148952123A40A00FDE3C5 /* WKWebsiteDatastore.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5C7148942123A40700FDE3C5 /* WKWebsiteDatastore.mm */; };
+ 5C75716122124C5200B9E5AC /* BundleRetainPagePlugIn.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5C75715F221249BD00B9E5AC /* BundleRetainPagePlugIn.mm */; };
5C7964101EB0278D0075D74C /* EventModifiers.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 5C79640F1EB0269B0075D74C /* EventModifiers.cpp */; };
5C7C74CB1FB529BA002F9ABE /* WebViewScheduleInRunLoop.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5C7C74CA1FB528D4002F9ABE /* WebViewScheduleInRunLoop.mm */; };
5C838F7F1DB04F900082858F /* LoadInvalidURLRequest.mm in Sources */ = {isa = PBXBuildFile; fileRef = 57901FAE1CAF137100ED64F9 /* LoadInvalidURLRequest.mm */; };
@@ -1678,6 +1679,7 @@
5C5E633D1D0B67940085A025 /* UniqueRef.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = UniqueRef.cpp; sourceTree = "<group>"; };
5C69BDD41F82A7EB000F4F4B /* _javascript_DuringNavigation.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = _javascript_DuringNavigation.mm; sourceTree = "<group>"; };
5C7148942123A40700FDE3C5 /* WKWebsiteDatastore.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = WKWebsiteDatastore.mm; sourceTree = "<group>"; };
+ 5C75715F221249BD00B9E5AC /* BundleRetainPagePlugIn.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = BundleRetainPagePlugIn.mm; sourceTree = "<group>"; };
5C79640F1EB0269B0075D74C /* EventModifiers.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = EventModifiers.cpp; sourceTree = "<group>"; };
5C7C74CA1FB528D4002F9ABE /* WebViewScheduleInRunLoop.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = WebViewScheduleInRunLoop.mm; sourceTree = "<group>"; };
5C8BC798218CF3E900813886 /* NetworkProcess.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = NetworkProcess.mm; sourceTree = "<group>"; };
@@ -2454,6 +2456,7 @@
37A709AD1E3EA8B000CA5969 /* BundleRangeHandle.mm */,
37A709AA1E3EA79000CA5969 /* BundleRangeHandlePlugIn.mm */,
37A709AC1E3EA7E800CA5969 /* BundleRangeHandleProtocol.h */,
+ 5C75715F221249BD00B9E5AC /* BundleRetainPagePlugIn.mm */,
1C2B817E1C891E4200A5529F /* CancelFontSubresource.mm */,
1C2B81811C891EFA00A5529F /* CancelFontSubresourcePlugIn.mm */,
5CB18BA71F5645B200EE23C4 /* ClickAutoFillButton.mm */,
@@ -4369,6 +4372,7 @@
374B7A611DF371CF00ACCB6C /* BundleEditingDelegatePlugIn.mm in Sources */,
A13EBBB01B87436F00097110 /* BundleParametersPlugIn.mm in Sources */,
37A709AF1E3EA97E00CA5969 /* BundleRangeHandlePlugIn.mm in Sources */,
+ 5C75716122124C5200B9E5AC /* BundleRetainPagePlugIn.mm in Sources */,
1C2B81831C891F0900A5529F /* CancelFontSubresourcePlugIn.mm in Sources */,
5CB18BA81F5645E300EE23C4 /* ClickAutoFillButton.mm in Sources */,
A14FC58B1B89927100D107EB /* ContentFilteringPlugIn.mm in Sources */,
Added: branches/safari-608.1.5.1-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/BundleRetainPagePlugIn.mm (0 => 241346)
--- branches/safari-608.1.5.1-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/BundleRetainPagePlugIn.mm (rev 0)
+++ branches/safari-608.1.5.1-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/BundleRetainPagePlugIn.mm 2019-02-13 06:36:06 UTC (rev 241346)
@@ -0,0 +1,45 @@
+/*
+ * Copyright (C) 2019 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#import "config.h"
+
+#if WK_API_ENABLED
+
+#import <WebKit/WKWebProcessPlugIn.h>
+#import <wtf/RetainPtr.h>
+
+@interface BundleRetainPagePlugIn : NSObject <WKWebProcessPlugIn>
+@end
+
+@implementation BundleRetainPagePlugIn
+
+- (void)webProcessPlugIn:(WKWebProcessPlugInController *)plugInController didCreateBrowserContextController:(WKWebProcessPlugInBrowserContextController *)browserContextController
+{
+ dispatch_after(dispatch_time(DISPATCH_TIME_NOW, 0.5 * NSEC_PER_SEC), dispatch_get_main_queue(), [retainedPage = retainPtr(browserContextController)] { });
+}
+
+@end
+
+#endif // WK_API_ENABLED
Modified: branches/safari-608.1.5.1-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (241345 => 241346)
--- branches/safari-608.1.5.1-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2019-02-13 06:36:02 UTC (rev 241345)
+++ branches/safari-608.1.5.1-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2019-02-13 06:36:06 UTC (rev 241346)
@@ -2404,11 +2404,18 @@
</body>
)PSONRESOURCE";
-TEST(ProcessSwap, ReuseSuspendedProcessForRegularNavigation)
+enum class RetainPageInBundle { No, Yes };
+
+void testReuseSuspendedProcessForRegularNavigation(RetainPageInBundle retainPageInBundle)
{
auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
[processPoolConfiguration setProcessSwapsOnNavigation:YES];
+ if (retainPageInBundle == RetainPageInBundle::Yes)
+ [processPoolConfiguration setInjectedBundleURL:[[NSBundle mainBundle] URLForResource:@"TestWebKitAPI" withExtension:@"wkbundle"]];
+
auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
+ if (retainPageInBundle == RetainPageInBundle::Yes)
+ [processPool _setObject:@"BundleRetainPagePlugIn" forBundleParameter:TestWebKitAPI::Util::TestPlugInClassNameParameter];
auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
[webViewConfiguration setProcessPool:processPool.get()];
@@ -2446,6 +2453,16 @@
EXPECT_EQ(webkitPID, [webView _webProcessIdentifier]);
}
+TEST(ProcessSwap, ReuseSuspendedProcessForRegularNavigationRetainBundlePage)
+{
+ testReuseSuspendedProcessForRegularNavigation(RetainPageInBundle::Yes);
+}
+
+TEST(ProcessSwap, ReuseSuspendedProcessForRegularNavigation)
+{
+ testReuseSuspendedProcessForRegularNavigation(RetainPageInBundle::No);
+}
+
static const char* mainFramesOnlyMainFrame = R"PSONRESOURCE(
<script>
function loaded() {
