Skip to content

Commit

Permalink
[Site Isolation] iframe processes do not handle mouse events after op…
Browse files Browse the repository at this point in the history
…ening a context menu

https://bugs.webkit.org/show_bug.cgi?id=269446
rdar://122997776

Reviewed by Alex Christensen.

When the web process is waiting for the client to open a context menu, all mouse events are ignored. With
site isolation, iframe processes weren't being notified when a context menu had opened. So, the processes
would not handle mouse events after one context menu was opened. Instead of updating this value in each
web process, we should just not send the event from the UI process when waiting for a context menu to
open.

* LayoutTests/http/tests/site-isolation/mouse-events/context-menu-event-twice-expected.txt: Added.
* LayoutTests/http/tests/site-isolation/mouse-events/context-menu-event-twice.html: Added.
* Source/WebKit/UIProcess/WebContextMenuProxy.cpp:
(WebKit::WebContextMenuProxy::useContextMenuItems):
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::processNextQueuedMouseEvent):
(WebKit::WebPageProxy::showContextMenu):
* Source/WebKit/UIProcess/WebPageProxy.h:
* Source/WebKit/WebProcess/WebPage/WebContextMenu.cpp:
(WebKit::WebContextMenu::show):
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::mouseEvent):
(WebKit::WebPage::didShowContextMenu): Deleted.
* Source/WebKit/WebProcess/WebPage/WebPage.h:
(WebKit::WebPage::startWaitingForContextMenuToShow): Deleted.
* Source/WebKit/WebProcess/WebPage/WebPage.messages.in:

Canonical link: https://commits.webkit.org/274771@main
  • Loading branch information
charliewolfe committed Feb 15, 2024
1 parent f8d35c1 commit 42d2cca
Show file tree
Hide file tree
Showing 9 changed files with 62 additions and 29 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
Verifies that the iframe receives a context menu event twice.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


PASS iframe received context menu event.
PASS iframe received context menu event.
PASS successfullyParsed is true

TEST COMPLETE

Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<!-- webkit-test-runner [ SiteIsolationEnabled=true ] -->
<script src="/js-test-resources/js-test.js"></script>
<script>
description("Verifies that the iframe receives a context menu event twice.");
jsTestIsAsync = true;

var messageCount = 0;
addEventListener("message", (event) => {
if (event.data != "contextmenu")
return;

testPassed("iframe received context menu event.");
messageCount++;
if (messageCount == 1) {
eventSender.mouseDown(2);
eventSender.mouseUp(2);
} else if (messageCount == 2)
finishJSTest();
});

function onLoad() {
let frame = document.getElementById("frame");
let x = frame.offsetParent.offsetLeft + frame.offsetLeft + frame.offsetWidth / 2;
let y = frame.offsetParent.offsetTop + frame.offsetTop + frame.offsetHeight / 2;
eventSender.mouseMoveTo(x, y);
eventSender.mouseDown(2);
eventSender.mouseUp(2);
}
</script>
<iframe onload="onLoad()" id="frame" src="http://localhost:8000/site-isolation/mouse-events/resources/context-menu-event-listener.html"></iframe>
4 changes: 1 addition & 3 deletions Source/WebKit/UIProcess/WebContextMenuProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,7 @@ void WebContextMenuProxy::useContextMenuItems(Vector<Ref<WebContextMenuItem>>&&
// Protect |this| from being deallocated if WebPageProxy code is re-entered from the menu runloop or delegates.
Ref protectedThis { *this };
showContextMenuWithItems(WTFMove(items));

// No matter the result of showContextMenuWithItems, always notify the WebProcess that the menu is hidden so it starts handling mouse events again.
page->send(Messages::WebPage::DidShowContextMenu());
page->clearWaitingForContextMenuToShow();
}

} // namespace WebKit
Expand Down
15 changes: 12 additions & 3 deletions Source/WebKit/UIProcess/WebPageProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3469,6 +3469,13 @@ void WebPageProxy::processNextQueuedMouseEvent()

const NativeWebMouseEvent& event = internals().mouseEventQueue.first();

#if ENABLE(CONTEXT_MENUS)
if (m_waitingForContextMenuToShow) {
mouseEventHandlingCompleted(event.type(), false);
return;
}
#endif

if (protectedPageClient()->windowIsFrontWindowUnderMouse(event))
setToolTip(String());

Expand Down Expand Up @@ -8586,12 +8593,14 @@ void WebPageProxy::showContextMenu(ContextMenuContextData&& contextMenuContextDa
// If the page is controlled by automation, entering a nested run loop while the menu is open
// can hang the page / WebDriver test. Pretend to show and immediately dismiss the context menu.
if (RefPtr automationSession = process().processPool().automationSession()) {
if (m_controlledByAutomation && automationSession->isSimulatingUserInteraction()) {
send(Messages::WebPage::DidShowContextMenu());
if (m_controlledByAutomation && automationSession->isSimulatingUserInteraction())
return;
}
}

#if ENABLE(CONTEXT_MENUS)
m_waitingForContextMenuToShow = true;
#endif

// Discard any enqueued mouse events that have been delivered to the UIProcess whilst the WebProcess is still processing the
// MouseDown event that triggered this ShowContextMenu message. This can happen if we take too long to enter the nested runloop.
discardQueuedMouseEvents();
Expand Down
8 changes: 8 additions & 0 deletions Source/WebKit/UIProcess/WebPageProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -2386,6 +2386,10 @@ class WebPageProxy final : public API::ObjectImpl<API::Object::Type::Page>, publ

void addConsoleMessage(WebCore::FrameIdentifier, JSC::MessageSource, JSC::MessageLevel, const String&, std::optional<WebCore::ResourceLoaderIdentifier> = std::nullopt);

#if ENABLE(CONTEXT_MENUS)
void clearWaitingForContextMenuToShow() { m_waitingForContextMenuToShow = false; }
#endif

private:
WebPageProxy(PageClient&, WebProcessProxy&, Ref<API::PageConfiguration>&&);
void platformInitialize();
Expand Down Expand Up @@ -3496,6 +3500,10 @@ class WebPageProxy final : public API::ObjectImpl<API::Object::Type::Page>, publ

RefPtr<WebPageProxy> m_pageToCloneSessionStorageFrom;
Ref<BrowsingContextGroup> m_browsingContextGroup;

#if ENABLE(CONTEXT_MENUS)
bool m_waitingForContextMenuToShow { false };
#endif
};

} // namespace WebKit
2 changes: 0 additions & 2 deletions Source/WebKit/WebProcess/WebPage/WebContextMenu.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,6 @@ void WebContextMenu::show()

ContextMenuContextData contextMenuContextData(menuLocation, menuItems, controller.context());

// Mark the WebPage has having a shown context menu then notify the UIProcess.
m_page->startWaitingForContextMenuToShow();
m_page->flushPendingEditorStateUpdate();
m_page->send(Messages::WebPageProxy::ShowContextMenuFromFrame(frame->frameID(), contextMenuContextData, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get())));
}
Expand Down
11 changes: 0 additions & 11 deletions Source/WebKit/WebProcess/WebPage/WebPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3375,11 +3375,6 @@ class CurrentEvent {

#if ENABLE(CONTEXT_MENUS)

void WebPage::didShowContextMenu()
{
m_waitingForContextMenuToShow = false;
}

void WebPage::didDismissContextMenu()
{
corePage()->contextMenuController().didDismissContextMenu();
Expand Down Expand Up @@ -3412,12 +3407,6 @@ void WebPage::mouseEvent(FrameIdentifier frameID, const WebMouseEvent& mouseEven
m_userActivity.impulse();

bool shouldHandleEvent = true;

#if ENABLE(CONTEXT_MENUS)
// Don't try to handle any pending mouse events if a context menu is showing.
if (m_waitingForContextMenuToShow)
shouldHandleEvent = false;
#endif
#if ENABLE(DRAG_SUPPORT)
if (m_isStartingDrag)
shouldHandleEvent = false;
Expand Down
9 changes: 0 additions & 9 deletions Source/WebKit/WebProcess/WebPage/WebPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -1202,10 +1202,6 @@ class WebPage : public API::ObjectImpl<API::Object::Type::BundlePage>, public IP
void handleAlternativeTextUIResult(const String&);
#endif

#if ENABLE(CONTEXT_MENUS)
void startWaitingForContextMenuToShow() { m_waitingForContextMenuToShow = true; }
#endif

void handleWheelEvent(WebCore::FrameIdentifier, const WebWheelEvent&, const OptionSet<WebCore::WheelEventProcessingSteps>&, std::optional<bool> willStartSwipe, CompletionHandler<void(std::optional<WebCore::ScrollingNodeID>, std::optional<WebCore::WheelScrollGestureState>, bool handled, std::optional<WebCore::RemoteUserInputEventData>)>&&);
WebCore::HandleUserInputEventResult wheelEvent(const WebCore::FrameIdentifier&, const WebWheelEvent&, OptionSet<WebCore::WheelEventProcessingSteps>);

Expand Down Expand Up @@ -1861,7 +1857,6 @@ class WebPage : public API::ObjectImpl<API::Object::Type::BundlePage>, public IP
void touchWithIdentifierWasRemoved(WebCore::PointerID);

#if ENABLE(CONTEXT_MENUS)
void didShowContextMenu();
void didDismissContextMenu();
#endif
#if ENABLE(CONTEXT_MENU_EVENT)
Expand Down Expand Up @@ -2486,10 +2481,6 @@ class WebPage : public API::ObjectImpl<API::Object::Type::BundlePage>, public IP
bool m_isWindowResizingEnabled { false };
#endif

#if ENABLE(CONTEXT_MENUS)
bool m_waitingForContextMenuToShow { false };
#endif

RefPtr<WebCore::Element> m_focusedElement;
RefPtr<WebCore::Element> m_recentlyBlurredElement;
bool m_hasPendingInputContextUpdateAfterBlurringAndRefocusingElement { false };
Expand Down
1 change: 0 additions & 1 deletion Source/WebKit/WebProcess/WebPage/WebPage.messages.in
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,6 @@ GenerateSyntheticEditingCommand(enum:uint8_t WebKit::SyntheticEditingCommandType

#if ENABLE(CONTEXT_MENUS)
# Context menu.
DidShowContextMenu()
DidDismissContextMenu()
DidSelectItemFromActiveContextMenu(WebKit::WebContextMenuItemData menuItem)
ContextMenuForKeyEvent()
Expand Down

0 comments on commit 42d2cca

Please sign in to comment.