Skip to content

Commit 14162f8

Browse files
authored
browser(webkit): let web page close when it has open context menu (#2802)
Review URL: aslushnikov/WebKit@42f86e9 Currently, if web page has an open context menu, then it won't close. This prevents browser from quitting. In stock safari, this behavior can also be observed in a way that context menu will stay opened even if related page got closed. While investigating this behavior on Mac, a crucial observation was that `[NSMenu popUpContextMenu]` is spawning a nested event loop, keeping reference to `WebContextMenuProxyMac` instance, which in turn keeps references on associated `NSView` with `WKWebView`. To exit the loop, we need to explicitly cancel context menu. For this, this patch adds a method `hide` on `WebContextMenuProxy` that uses port-specific code to cancel context menu. Windows part of this patch is somewhat speculative: I didn't check it, but given the same symptomps, I applied the same solution. Fixes #2700
1 parent c188118 commit 14162f8

File tree

2 files changed

+93
-17
lines changed

2 files changed

+93
-17
lines changed

browser_patches/webkit/BUILD_NUMBER

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
1296
1+
1297

browser_patches/webkit/patches/bootstrap.diff

Lines changed: 92 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11258,6 +11258,18 @@ index ff90d3de4349c9a3385c20c059729b8e22ebe2e5..d5c4f2cd715551ddef6f5af93ada65cb
1125811258
#include <WebCore/MockWebAuthenticationConfiguration.h>
1125911259

1126011260
namespace WebKit {
11261+
diff --git a/Source/WebKit/UIProcess/WebContextMenuProxy.h b/Source/WebKit/UIProcess/WebContextMenuProxy.h
11262+
index 3f05f4395c08656ee60c1a467c6fe809115a0210..c29320a1b9bbde5cc372d4728ad0614beceed988 100644
11263+
--- a/Source/WebKit/UIProcess/WebContextMenuProxy.h
11264+
+++ b/Source/WebKit/UIProcess/WebContextMenuProxy.h
11265+
@@ -40,6 +40,7 @@ public:
11266+
virtual ~WebContextMenuProxy();
11267+
11268+
virtual void show() = 0;
11269+
+ virtual void hide() {}
11270+
11271+
virtual void showContextMenuWithItems(Vector<Ref<WebContextMenuItem>>&&) = 0;
11272+
1126111273
diff --git a/Source/WebKit/UIProcess/WebGeolocationManagerProxy.cpp b/Source/WebKit/UIProcess/WebGeolocationManagerProxy.cpp
1126211274
index 04f3227cd55c992a42cd96a3f25d697aed7965a2..f0d36935f47bab03ea2ec50b705092068ecd3efa 100644
1126311275
--- a/Source/WebKit/UIProcess/WebGeolocationManagerProxy.cpp
@@ -11883,7 +11895,7 @@ index 0000000000000000000000000000000000000000..20311d530090b0229010957a96fc60f4
1188311895
+
1188411896
+} // namespace WebKit
1188511897
diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
11886-
index 4037ceef8668619ae52f1aa1a4b2783818d9c091..df126d71a13b80595fccfa586b23b78c6a6feeeb 100644
11898+
index 4037ceef8668619ae52f1aa1a4b2783818d9c091..6f1b3494f7603e79d82164a07de831ca4767522f 100644
1188711899
--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
1188811900
+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
1188911901
@@ -953,6 +953,7 @@ void WebPageProxy::finishAttachingToWebProcess(ProcessLaunchReason reason)
@@ -12061,7 +12073,18 @@ index 4037ceef8668619ae52f1aa1a4b2783818d9c091..df126d71a13b80595fccfa586b23b78c
1206112073
}
1206212074

1206312075
void WebPageProxy::exitFullscreenImmediately()
12064-
@@ -5678,6 +5756,8 @@ void WebPageProxy::runJavaScriptAlert(FrameIdentifier frameID, FrameInfoData&& f
12076+
@@ -5659,6 +5737,10 @@ void WebPageProxy::closePage()
12077+
if (isClosed())
12078+
return;
12079+
12080+
+#if ENABLE(CONTEXT_MENUS)
12081+
+ if (m_activeContextMenu)
12082+
+ m_activeContextMenu->hide();
12083+
+#endif
12084+
RELEASE_LOG_IF_ALLOWED(Process, "closePage:");
12085+
pageClient().clearAllEditCommands();
12086+
m_uiClient->close(this);
12087+
@@ -5678,6 +5760,8 @@ void WebPageProxy::runJavaScriptAlert(FrameIdentifier frameID, FrameInfoData&& f
1206512088
if (auto* automationSession = process().processPool().automationSession())
1206612089
automationSession->willShowJavaScriptDialog(*this);
1206712090
}
@@ -12070,7 +12093,7 @@ index 4037ceef8668619ae52f1aa1a4b2783818d9c091..df126d71a13b80595fccfa586b23b78c
1207012093
m_uiClient->runJavaScriptAlert(*this, message, frame, WTFMove(frameInfo), WTFMove(reply));
1207112094
}
1207212095

12073-
@@ -5695,6 +5775,8 @@ void WebPageProxy::runJavaScriptConfirm(FrameIdentifier frameID, FrameInfoData&&
12096+
@@ -5695,6 +5779,8 @@ void WebPageProxy::runJavaScriptConfirm(FrameIdentifier frameID, FrameInfoData&&
1207412097
if (auto* automationSession = process().processPool().automationSession())
1207512098
automationSession->willShowJavaScriptDialog(*this);
1207612099
}
@@ -12079,7 +12102,7 @@ index 4037ceef8668619ae52f1aa1a4b2783818d9c091..df126d71a13b80595fccfa586b23b78c
1207912102

1208012103
m_uiClient->runJavaScriptConfirm(*this, message, frame, WTFMove(frameInfo), WTFMove(reply));
1208112104
}
12082-
@@ -5713,6 +5795,8 @@ void WebPageProxy::runJavaScriptPrompt(FrameIdentifier frameID, FrameInfoData&&
12105+
@@ -5713,6 +5799,8 @@ void WebPageProxy::runJavaScriptPrompt(FrameIdentifier frameID, FrameInfoData&&
1208312106
if (auto* automationSession = process().processPool().automationSession())
1208412107
automationSession->willShowJavaScriptDialog(*this);
1208512108
}
@@ -12088,7 +12111,7 @@ index 4037ceef8668619ae52f1aa1a4b2783818d9c091..df126d71a13b80595fccfa586b23b78c
1208812111

1208912112
m_uiClient->runJavaScriptPrompt(*this, message, defaultValue, frame, WTFMove(frameInfo), WTFMove(reply));
1209012113
}
12091-
@@ -5868,6 +5952,8 @@ void WebPageProxy::runBeforeUnloadConfirmPanel(FrameIdentifier frameID, FrameInf
12114+
@@ -5868,6 +5956,8 @@ void WebPageProxy::runBeforeUnloadConfirmPanel(FrameIdentifier frameID, FrameInf
1209212115
return;
1209312116
}
1209412117
}
@@ -12097,39 +12120,39 @@ index 4037ceef8668619ae52f1aa1a4b2783818d9c091..df126d71a13b80595fccfa586b23b78c
1209712120

1209812121
// Since runBeforeUnloadConfirmPanel() can spin a nested run loop we need to turn off the responsiveness timer and the tryClose timer.
1209912122
m_process->stopResponsivenessTimer();
12100-
@@ -6925,6 +7011,7 @@ void WebPageProxy::didReceiveEvent(uint32_t opaqueType, bool handled)
12123+
@@ -6925,6 +7015,7 @@ void WebPageProxy::didReceiveEvent(uint32_t opaqueType, bool handled)
1210112124
if (auto* automationSession = process().processPool().automationSession())
1210212125
automationSession->mouseEventsFlushedForPage(*this);
1210312126
didFinishProcessingAllPendingMouseEvents();
1210412127
+ m_inspectorController->didProcessAllPendingMouseEvents();
1210512128
}
1210612129

1210712130
break;
12108-
@@ -6951,7 +7038,6 @@ void WebPageProxy::didReceiveEvent(uint32_t opaqueType, bool handled)
12131+
@@ -6951,7 +7042,6 @@ void WebPageProxy::didReceiveEvent(uint32_t opaqueType, bool handled)
1210912132
case WebEvent::RawKeyDown:
1211012133
case WebEvent::Char: {
1211112134
LOG(KeyHandling, "WebPageProxy::didReceiveEvent: %s (queue empty %d)", webKeyboardEventTypeString(type), m_keyEventQueue.isEmpty());
1211212135
-
1211312136
MESSAGE_CHECK(m_process, !m_keyEventQueue.isEmpty());
1211412137
NativeWebKeyboardEvent event = m_keyEventQueue.takeFirst();
1211512138

12116-
@@ -6971,7 +7057,6 @@ void WebPageProxy::didReceiveEvent(uint32_t opaqueType, bool handled)
12139+
@@ -6971,7 +7061,6 @@ void WebPageProxy::didReceiveEvent(uint32_t opaqueType, bool handled)
1211712140
// The call to doneWithKeyEvent may close this WebPage.
1211812141
// Protect against this being destroyed.
1211912142
Ref<WebPageProxy> protect(*this);
1212012143
-
1212112144
pageClient().doneWithKeyEvent(event, handled);
1212212145
if (!handled)
1212312146
m_uiClient->didNotHandleKeyEvent(this, event);
12124-
@@ -6980,6 +7065,7 @@ void WebPageProxy::didReceiveEvent(uint32_t opaqueType, bool handled)
12147+
@@ -6980,6 +7069,7 @@ void WebPageProxy::didReceiveEvent(uint32_t opaqueType, bool handled)
1212512148
if (!canProcessMoreKeyEvents) {
1212612149
if (auto* automationSession = process().processPool().automationSession())
1212712150
automationSession->keyboardEventsFlushedForPage(*this);
1212812151
+ m_inspectorController->didProcessAllPendingKeyboardEvents();
1212912152
}
1213012153
break;
1213112154
}
12132-
@@ -7425,8 +7511,10 @@ static bool shouldReloadAfterProcessTermination(ProcessTerminationReason reason)
12155+
@@ -7425,8 +7515,10 @@ static bool shouldReloadAfterProcessTermination(ProcessTerminationReason reason)
1213312156
void WebPageProxy::dispatchProcessDidTerminate(ProcessTerminationReason reason)
1213412157
{
1213512158
RELEASE_LOG_ERROR_IF_ALLOWED(Loading, "dispatchProcessDidTerminate: reason = %d", reason);
@@ -12141,15 +12164,15 @@ index 4037ceef8668619ae52f1aa1a4b2783818d9c091..df126d71a13b80595fccfa586b23b78c
1214112164
if (m_loaderClient)
1214212165
handledByClient = reason != ProcessTerminationReason::RequestedByClient && m_loaderClient->processDidCrash(*this);
1214312166
else
12144-
@@ -7693,6 +7781,7 @@ void WebPageProxy::resetStateAfterProcessExited(ProcessTerminationReason termina
12167+
@@ -7693,6 +7785,7 @@ void WebPageProxy::resetStateAfterProcessExited(ProcessTerminationReason termina
1214512168

1214612169
WebPageCreationParameters WebPageProxy::creationParameters(WebProcessProxy& process, DrawingAreaProxy& drawingArea, RefPtr<API::WebsitePolicies>&& websitePolicies)
1214712170
{
1214812171
+
1214912172
WebPageCreationParameters parameters;
1215012173

1215112174
parameters.processDisplayName = configuration().processDisplayName();
12152-
@@ -7845,6 +7934,8 @@ WebPageCreationParameters WebPageProxy::creationParameters(WebProcessProxy& proc
12175+
@@ -7845,6 +7938,8 @@ WebPageCreationParameters WebPageProxy::creationParameters(WebProcessProxy& proc
1215312176
parameters.limitsNavigationsToAppBoundDomains = m_limitsNavigationsToAppBoundDomains;
1215412177
parameters.shouldRelaxThirdPartyCookieBlocking = m_configuration->shouldRelaxThirdPartyCookieBlocking();
1215512178

@@ -12158,7 +12181,7 @@ index 4037ceef8668619ae52f1aa1a4b2783818d9c091..df126d71a13b80595fccfa586b23b78c
1215812181
#if PLATFORM(GTK)
1215912182
parameters.themeName = pageClient().themeName();
1216012183
#endif
12161-
@@ -7916,6 +8007,14 @@ void WebPageProxy::gamepadActivity(const Vector<GamepadData>& gamepadDatas, bool
12184+
@@ -7916,6 +8011,14 @@ void WebPageProxy::gamepadActivity(const Vector<GamepadData>& gamepadDatas, bool
1216212185

1216312186
void WebPageProxy::didReceiveAuthenticationChallengeProxy(Ref<AuthenticationChallengeProxy>&& authenticationChallenge, NegotiatedLegacyTLS negotiatedLegacyTLS)
1216412187
{
@@ -12173,7 +12196,7 @@ index 4037ceef8668619ae52f1aa1a4b2783818d9c091..df126d71a13b80595fccfa586b23b78c
1217312196
if (negotiatedLegacyTLS == NegotiatedLegacyTLS::Yes) {
1217412197
m_navigationClient->shouldAllowLegacyTLS(*this, authenticationChallenge.get(), [this, protectedThis = makeRef(*this), authenticationChallenge = authenticationChallenge.copyRef()] (bool shouldAllowLegacyTLS) {
1217512198
if (shouldAllowLegacyTLS)
12176-
@@ -8001,7 +8100,8 @@ void WebPageProxy::requestGeolocationPermissionForFrame(GeolocationIdentifier ge
12199+
@@ -8001,7 +8104,8 @@ void WebPageProxy::requestGeolocationPermissionForFrame(GeolocationIdentifier ge
1217712200
MESSAGE_CHECK(m_process, frame);
1217812201

1217912202
// FIXME: Geolocation should probably be using toString() as its string representation instead of databaseIdentifier().
@@ -12183,7 +12206,7 @@ index 4037ceef8668619ae52f1aa1a4b2783818d9c091..df126d71a13b80595fccfa586b23b78c
1218312206
auto request = m_geolocationPermissionRequestManager.createRequest(geolocationID);
1218412207
Function<void(bool)> completionHandler = [request = WTFMove(request)](bool allowed) {
1218512208
if (allowed)
12186-
@@ -8010,6 +8110,14 @@ void WebPageProxy::requestGeolocationPermissionForFrame(GeolocationIdentifier ge
12209+
@@ -8010,6 +8114,14 @@ void WebPageProxy::requestGeolocationPermissionForFrame(GeolocationIdentifier ge
1218712210
request->deny();
1218812211
};
1218912212

@@ -13305,6 +13328,35 @@ index 7b7e66c79edaf84a8a14756524e16c26e57896c6..2d6224b2bad15eaf808101dec24026b3
1330513328
return m_impl->windowIsFrontWindowUnderMouse(event.nativeEvent());
1330613329
}
1330713330

13331+
diff --git a/Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.h b/Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.h
13332+
index 900ef0f178a926daacd53e645e5730aabf1eaf71..eb8321928a18a14817d2f4b583e7ee5518826e25 100644
13333+
--- a/Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.h
13334+
+++ b/Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.h
13335+
@@ -66,6 +66,7 @@ public:
13336+
private:
13337+
WebContextMenuProxyMac(NSView*, WebPageProxy&, ContextMenuContextData&&, const UserData&);
13338+
void show() override;
13339+
+ void hide() override;
13340+
13341+
RefPtr<WebContextMenuListenerProxy> m_contextMenuListener;
13342+
void getContextMenuItem(const WebContextMenuItemData&, CompletionHandler<void(NSMenuItem *)>&&);
13343+
diff --git a/Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.mm b/Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.mm
13344+
index 3daaa25d4ffebbbb1682efc00dc50262cd3886ec..680153dd2802f33e05f70b8fa91c9272ca963840 100644
13345+
--- a/Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.mm
13346+
+++ b/Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.mm
13347+
@@ -335,6 +335,12 @@ void WebContextMenuProxyMac::getShareMenuItem(CompletionHandler<void(NSMenuItem
13348+
}
13349+
#endif
13350+
13351+
+void WebContextMenuProxyMac::hide()
13352+
+{
13353+
+ if (m_menu)
13354+
+ [m_menu cancelTracking];
13355+
+}
13356+
+
13357+
void WebContextMenuProxyMac::show()
13358+
{
13359+
Ref<WebPageProxy> protect(m_page);
1330813360
diff --git a/Source/WebKit/UIProcess/mac/WebPageInspectorEmulationAgentMac.mm b/Source/WebKit/UIProcess/mac/WebPageInspectorEmulationAgentMac.mm
1330913361
new file mode 100644
1331013362
index 0000000000000000000000000000000000000000..6113f4cd60a5d72b8ead61176cb43200803478ed
@@ -13690,7 +13742,7 @@ index 0000000000000000000000000000000000000000..135a60361fa8fbf907382625e7c8dd4e
1369013742
+
1369113743
+} // namespace WebKit
1369213744
diff --git a/Source/WebKit/UIProcess/win/WebContextMenuProxyWin.cpp b/Source/WebKit/UIProcess/win/WebContextMenuProxyWin.cpp
13693-
index aa171f48bc40a96a86d951451b578f609c573fce..8cdc2ff1e3a2ce3d05aacc46a83521c02442dd44 100644
13745+
index aa171f48bc40a96a86d951451b578f609c573fce..e948342089abdf57c80be77e25416743b63179a2 100644
1369413746
--- a/Source/WebKit/UIProcess/win/WebContextMenuProxyWin.cpp
1369513747
+++ b/Source/WebKit/UIProcess/win/WebContextMenuProxyWin.cpp
1369613748
@@ -105,6 +105,8 @@ void WebContextMenuProxyWin::showContextMenuWithItems(Vector<Ref<WebContextMenuI
@@ -13702,6 +13754,30 @@ index aa171f48bc40a96a86d951451b578f609c573fce..8cdc2ff1e3a2ce3d05aacc46a83521c0
1370213754
::ClientToScreen(wnd, &pt);
1370313755
::TrackPopupMenuEx(m_menu, flags, pt.x, pt.y, m_page.viewWidget(), nullptr);
1370413756
}
13757+
@@ -122,5 +124,11 @@ WebContextMenuProxyWin::~WebContextMenuProxyWin()
13758+
::DestroyMenu(m_menu);
13759+
}
13760+
13761+
+void WebContextMenuProxyWin::hide()
13762+
+{
13763+
+ if (m_menu)
13764+
+ ::DestroyMenu(m_menu);
13765+
+}
13766+
+
13767+
} // namespace WebKit
13768+
#endif // ENABLE(CONTEXT_MENUS)
13769+
diff --git a/Source/WebKit/UIProcess/win/WebContextMenuProxyWin.h b/Source/WebKit/UIProcess/win/WebContextMenuProxyWin.h
13770+
index 8eda673849da2d1a38c37bb384ddef5e76095a9a..f6197e5a5da7a5527101e8447091e79f5c7a53fe 100644
13771+
--- a/Source/WebKit/UIProcess/win/WebContextMenuProxyWin.h
13772+
+++ b/Source/WebKit/UIProcess/win/WebContextMenuProxyWin.h
13773+
@@ -48,6 +48,7 @@ private:
13774+
WebContextMenuProxyWin(WebPageProxy&, ContextMenuContextData&&, const UserData&);
13775+
void showContextMenuWithItems(Vector<Ref<WebContextMenuItem>>&&) override;
13776+
void show() override;
13777+
+ void hide() override;
13778+
13779+
WebPageProxy& m_page;
13780+
RefPtr<WebContextMenuListenerProxy> m_contextMenuListener;
1370513781
diff --git a/Source/WebKit/UIProcess/win/WebPageInspectorEmulationAgentWin.cpp b/Source/WebKit/UIProcess/win/WebPageInspectorEmulationAgentWin.cpp
1370613782
new file mode 100644
1370713783
index 0000000000000000000000000000000000000000..62b841fe1d0de2296e1c61e328cff564f5aa1c0f

0 commit comments

Comments
 (0)