Changeset 241306 in webkit
- Timestamp:
- Feb 12, 2019 11:29:21 AM (5 years ago)
- Location:
- trunk
- Files:
-
- 1 added
- 7 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/Source/WebKit/ChangeLog
r241304 r241306 1 2019-02-12 Alex Christensen <achristensen@webkit.org> 2 3 WebPage::close needs to remove all message receivers associated with that WebPage, not WebPage::~WebPage 4 https://bugs.webkit.org/show_bug.cgi?id=194522 5 <rdar://problem/47789393> 6 7 Reviewed by Chris Dumez. 8 9 The InjectedBundle SPI can retain the WebPage or wrapping objects (WKWebProcessPlugInBrowserContextController/WKBundlePageRef). 10 This can make it so WebPage::close is called before WebPage::~WebPage, and if the SuspendedPageProxy is reused for a subsequent 11 navigation to the same domain, the WebProcess is reused with a different WebPage instance with the same PageID, which causes problems 12 when another WebPage registers message handlers and then the previous WebPage is destroyed, which removes both message handlers. 13 14 * WebProcess/WebPage/WebPage.cpp: 15 (WebKit::WebPage::~WebPage): 16 (WebKit::WebPage::close): 17 (WebKit::WebPage::mainFrameDidLayout): 18 * WebProcess/WebPage/WebPage.h: 19 * WebProcess/WebProcess.h: 20 (WebKit::WebProcess::eventDispatcher): 21 1 22 2019-02-12 Michael Catanzaro <mcatanzaro@igalia.com> 2 23 -
trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp
r241281 r241306 365 365 , m_layerHostingMode(parameters.layerHostingMode) 366 366 #if PLATFORM(COCOA) || PLATFORM(GTK) 367 , m_viewGestureGeometryCollector( makeUniqueRef<ViewGestureGeometryCollector>(*this))367 , m_viewGestureGeometryCollector(std::make_unique<ViewGestureGeometryCollector>(*this)) 368 368 #elif HAVE(ACCESSIBILITY) && PLATFORM(GTK) 369 369 , m_accessibilityObject(nullptr) … … 751 751 ASSERT(!m_page); 752 752 753 auto& webProcess = WebProcess::singleton();754 #if ENABLE(ASYNC_SCROLLING)755 if (m_useAsyncScrolling)756 webProcess.eventDispatcher().removeScrollingTreeForPage(this);757 #endif758 759 753 platformDetach(); 760 754 … … 770 764 m_footerBanner->detachFromPage(); 771 765 #endif // !PLATFORM(IOS_FAMILY) 772 773 webProcess.removeMessageReceiver(Messages::WebPage::messageReceiverName(), m_pageID);774 775 // FIXME: This should be done in the object destructors, and the objects themselves should be message receivers.776 webProcess.removeMessageReceiver(Messages::WebInspector::messageReceiverName(), m_pageID);777 webProcess.removeMessageReceiver(Messages::WebInspectorUI::messageReceiverName(), m_pageID);778 webProcess.removeMessageReceiver(Messages::RemoteWebInspectorUI::messageReceiverName(), m_pageID);779 #if ENABLE(FULLSCREEN_API)780 webProcess.removeMessageReceiver(Messages::WebFullScreenManager::messageReceiverName(), m_pageID);781 #endif782 766 783 767 #ifndef NDEBUG … … 1319 1303 m_isRunningModal = false; 1320 1304 1305 auto& webProcess = WebProcess::singleton(); 1306 #if ENABLE(ASYNC_SCROLLING) 1307 if (m_useAsyncScrolling) 1308 webProcess.eventDispatcher().removeScrollingTreeForPage(this); 1309 #endif 1310 webProcess.removeMessageReceiver(Messages::WebPage::messageReceiverName(), m_pageID); 1311 // FIXME: This should be done in the object destructors, and the objects themselves should be message receivers. 1312 webProcess.removeMessageReceiver(Messages::WebInspector::messageReceiverName(), m_pageID); 1313 webProcess.removeMessageReceiver(Messages::WebInspectorUI::messageReceiverName(), m_pageID); 1314 webProcess.removeMessageReceiver(Messages::RemoteWebInspectorUI::messageReceiverName(), m_pageID); 1315 #if ENABLE(FULLSCREEN_API) 1316 webProcess.removeMessageReceiver(Messages::WebFullScreenManager::messageReceiverName(), m_pageID); 1317 #endif 1318 #if PLATFORM(COCOA) || PLATFORM(GTK) 1319 m_viewGestureGeometryCollector = nullptr; 1320 #endif 1321 1321 1322 // The WebPage can be destroyed by this call. 1322 1323 WebProcess::singleton().removeWebPage(m_pageID); … … 4217 4218 4218 4219 #if PLATFORM(COCOA) || PLATFORM(GTK) 4219 m_viewGestureGeometryCollector->mainFrameDidLayout(); 4220 if (m_viewGestureGeometryCollector) 4221 m_viewGestureGeometryCollector->mainFrameDidLayout(); 4220 4222 #endif 4221 4223 #if PLATFORM(IOS_FAMILY) -
trunk/Source/WebKit/WebProcess/WebPage/WebPage.h
r241282 r241306 1586 1586 1587 1587 #if PLATFORM(COCOA) || PLATFORM(GTK) 1588 UniqueRef<ViewGestureGeometryCollector> m_viewGestureGeometryCollector;1588 std::unique_ptr<ViewGestureGeometryCollector> m_viewGestureGeometryCollector; 1589 1589 #endif 1590 1590 -
trunk/Source/WebKit/WebProcess/WebProcess.h
r240683 r241306 166 166 #endif 167 167 168 EventDispatcher& eventDispatcher() { return *m_eventDispatcher; }168 EventDispatcher& eventDispatcher() { return m_eventDispatcher.get(); } 169 169 170 170 NetworkProcessConnection& ensureNetworkProcessConnection(); … … 403 403 RefPtr<InjectedBundle> m_injectedBundle; 404 404 405 Ref Ptr<EventDispatcher> m_eventDispatcher;405 Ref<EventDispatcher> m_eventDispatcher; 406 406 #if PLATFORM(IOS_FAMILY) 407 407 RefPtr<ViewUpdateDispatcher> m_viewUpdateDispatcher; -
trunk/Tools/ChangeLog
r241299 r241306 1 2019-02-12 Alex Christensen <achristensen@webkit.org> 2 3 WebPage::close needs to remove all message receivers associated with that WebPage, not WebPage::~WebPage 4 https://bugs.webkit.org/show_bug.cgi?id=194522 5 <rdar://problem/47789393> 6 7 Reviewed by Chris Dumez. 8 9 * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj: 10 * TestWebKitAPI/Tests/WebKitCocoa/BundleRetainPagePlugIn.mm: Added. 11 (-[BundleRetainPagePlugIn webProcessPlugIn:didCreateBrowserContextController:]): 12 * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm: 13 1 14 2019-02-12 Andy Estes <aestes@apple.com> 2 15 -
trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
r241134 r241306 308 308 5C69BDD51F82A7EF000F4F4B /* JavaScriptDuringNavigation.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5C69BDD41F82A7EB000F4F4B /* JavaScriptDuringNavigation.mm */; }; 309 309 5C7148952123A40A00FDE3C5 /* WKWebsiteDatastore.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5C7148942123A40700FDE3C5 /* WKWebsiteDatastore.mm */; }; 310 5C75716122124C5200B9E5AC /* BundleRetainPagePlugIn.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5C75715F221249BD00B9E5AC /* BundleRetainPagePlugIn.mm */; }; 310 311 5C7964101EB0278D0075D74C /* EventModifiers.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 5C79640F1EB0269B0075D74C /* EventModifiers.cpp */; }; 311 312 5C7C74CB1FB529BA002F9ABE /* WebViewScheduleInRunLoop.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5C7C74CA1FB528D4002F9ABE /* WebViewScheduleInRunLoop.mm */; }; … … 1691 1692 5C69BDD41F82A7EB000F4F4B /* JavaScriptDuringNavigation.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = JavaScriptDuringNavigation.mm; sourceTree = "<group>"; }; 1692 1693 5C7148942123A40700FDE3C5 /* WKWebsiteDatastore.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = WKWebsiteDatastore.mm; sourceTree = "<group>"; }; 1694 5C75715F221249BD00B9E5AC /* BundleRetainPagePlugIn.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = BundleRetainPagePlugIn.mm; sourceTree = "<group>"; }; 1693 1695 5C79640F1EB0269B0075D74C /* EventModifiers.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = EventModifiers.cpp; sourceTree = "<group>"; }; 1694 1696 5C7C74CA1FB528D4002F9ABE /* WebViewScheduleInRunLoop.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = WebViewScheduleInRunLoop.mm; sourceTree = "<group>"; }; … … 2468 2470 37A709AA1E3EA79000CA5969 /* BundleRangeHandlePlugIn.mm */, 2469 2471 37A709AC1E3EA7E800CA5969 /* BundleRangeHandleProtocol.h */, 2472 5C75715F221249BD00B9E5AC /* BundleRetainPagePlugIn.mm */, 2470 2473 1C2B817E1C891E4200A5529F /* CancelFontSubresource.mm */, 2471 2474 1C2B81811C891EFA00A5529F /* CancelFontSubresourcePlugIn.mm */, … … 4390 4393 A13EBBB01B87436F00097110 /* BundleParametersPlugIn.mm in Sources */, 4391 4394 37A709AF1E3EA97E00CA5969 /* BundleRangeHandlePlugIn.mm in Sources */, 4395 5C75716122124C5200B9E5AC /* BundleRetainPagePlugIn.mm in Sources */, 4392 4396 1C2B81831C891F0900A5529F /* CancelFontSubresourcePlugIn.mm in Sources */, 4393 4397 5CB18BA81F5645E300EE23C4 /* ClickAutoFillButton.mm in Sources */, -
trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
r240928 r241306 2405 2405 )PSONRESOURCE"; 2406 2406 2407 TEST(ProcessSwap, ReuseSuspendedProcessForRegularNavigation) 2407 enum class RetainPageInBundle { No, Yes }; 2408 2409 void testReuseSuspendedProcessForRegularNavigation(RetainPageInBundle retainPageInBundle) 2408 2410 { 2409 2411 auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]); 2410 2412 [processPoolConfiguration setProcessSwapsOnNavigation:YES]; 2411 auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]); 2413 if (retainPageInBundle == RetainPageInBundle::Yes) 2414 [processPoolConfiguration setInjectedBundleURL:[[NSBundle mainBundle] URLForResource:@"TestWebKitAPI" withExtension:@"wkbundle"]]; 2415 2416 auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]); 2417 if (retainPageInBundle == RetainPageInBundle::Yes) 2418 [processPool _setObject:@"BundleRetainPagePlugIn" forBundleParameter:TestWebKitAPI::Util::TestPlugInClassNameParameter]; 2412 2419 2413 2420 auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]); … … 2445 2452 2446 2453 EXPECT_EQ(webkitPID, [webView _webProcessIdentifier]); 2454 } 2455 2456 TEST(ProcessSwap, ReuseSuspendedProcessForRegularNavigationRetainBundlePage) 2457 { 2458 testReuseSuspendedProcessForRegularNavigation(RetainPageInBundle::Yes); 2459 } 2460 2461 TEST(ProcessSwap, ReuseSuspendedProcessForRegularNavigation) 2462 { 2463 testReuseSuspendedProcessForRegularNavigation(RetainPageInBundle::No); 2447 2464 } 2448 2465
Note: See TracChangeset
for help on using the changeset viewer.