Skip to content

Commit

Permalink
[Site Isolation] Context menu is offset in iframes
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=269437
rdar://122994262

Reviewed by Alex Christensen.

The context menu is offset when clicked in an iframe because an iframe process is trying to convert to
root view coordinates. To convert to root view coordinates from an iframe process we need to convert
from each parent root frame.

* Source/WebKit/Shared/ContextMenuContextData.h:
(WebKit::ContextMenuContextData::setMenuLocation):
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::setTextIndicatorFromFrame):
(WebKit::WebPageProxy::showContextMenuFromFrame):
* Source/WebKit/UIProcess/WebPageProxy.h:
* Source/WebKit/UIProcess/WebPageProxy.messages.in:
* Source/WebKit/WebProcess/WebPage/WebContextMenu.cpp:
(WebKit::WebContextMenu::show):
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::remoteViewToRootView):
(WebKit::WebPage::remoteViewRectToRootView):
(WebKit::WebPage::remoteViewPointToRootView):
(WebKit::WebPage::remoteViewCoordinatesToRootView): Deleted.
* Source/WebKit/WebProcess/WebPage/WebPage.h:
* Source/WebKit/WebProcess/WebPage/WebPage.messages.in:

Canonical link: https://commits.webkit.org/274761@main
  • Loading branch information
charliewolfe committed Feb 15, 2024
1 parent 4d6afe2 commit fbda7ba
Show file tree
Hide file tree
Showing 8 changed files with 47 additions and 9 deletions.
1 change: 1 addition & 0 deletions Source/WebKit/Shared/ContextMenuContextData.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ class ContextMenuContextData {

Type type() const { return m_type; }
const WebCore::IntPoint& menuLocation() const { return m_menuLocation; }
void setMenuLocation(WebCore::IntPoint menuLocation) { m_menuLocation = menuLocation; }
const Vector<WebKit::WebContextMenuItemData>& menuItems() const { return m_menuItems; }

const std::optional<WebHitTestResultData>& webHitTestResultData() const { return m_webHitTestResultData; }
Expand Down
24 changes: 23 additions & 1 deletion Source/WebKit/UIProcess/WebPageProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8402,7 +8402,7 @@ void WebPageProxy::setTextIndicatorFromFrame(FrameIdentifier frameID, TextIndica

auto parentFrameID = rootFrameParent->frameID();
auto textBoundingRect = indicatorData.textBoundingRectInRootViewCoordinates;
sendToProcessContainingFrame(parentFrameID, Messages::WebPage::RemoteViewCoordinatesToRootView(frameID, textBoundingRect), [protectedThis = Ref { *this }, indicatorData = WTFMove(indicatorData), rootFrameParent = WTFMove(rootFrameParent), lifetime](FloatRect rect) mutable {
sendToProcessContainingFrame(parentFrameID, Messages::WebPage::RemoteViewRectToRootView(frameID, textBoundingRect), [protectedThis = Ref { *this }, indicatorData = WTFMove(indicatorData), rootFrameParent = WTFMove(rootFrameParent), lifetime](FloatRect rect) mutable {
indicatorData.textBoundingRectInRootViewCoordinates = rect;
protectedThis->setTextIndicatorFromFrame(rootFrameParent->rootFrame()->frameID(), WTFMove(indicatorData), lifetime);
});
Expand Down Expand Up @@ -8556,6 +8556,28 @@ void WebPageProxy::hidePopupMenu()

#if ENABLE(CONTEXT_MENUS)

void WebPageProxy::showContextMenuFromFrame(FrameIdentifier frameID, ContextMenuContextData&& contextMenuContextData, UserData&& userData)
{
RefPtr frame = WebFrameProxy::webFrame(frameID);
if (!frame)
return;

RefPtr rootFrameParent = frame->rootFrame()->parentFrame();
if (!rootFrameParent) {
showContextMenu(WTFMove(contextMenuContextData), userData);
return;
}

ASSERT(m_preferences->siteIsolationEnabled());

auto parentFrameID = rootFrameParent->frameID();
auto menuLocation = contextMenuContextData.menuLocation();
sendToProcessContainingFrame(parentFrameID, Messages::WebPage::RemoteViewPointToRootView(frameID, menuLocation), [protectedThis = Ref { *this }, contextMenuContextData = WTFMove(contextMenuContextData), userData = WTFMove(userData), rootFrameParent = WTFMove(rootFrameParent)](FloatPoint point) mutable {
contextMenuContextData.setMenuLocation(IntPoint(point));
protectedThis->showContextMenuFromFrame(rootFrameParent->rootFrame()->frameID(), WTFMove(contextMenuContextData), WTFMove(userData));
});
}

void WebPageProxy::showContextMenu(ContextMenuContextData&& contextMenuContextData, const UserData& userData)
{
// Showing a context menu runs a nested runloop, which can handle messages that cause |this| to get closed.
Expand Down
1 change: 1 addition & 0 deletions Source/WebKit/UIProcess/WebPageProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -2653,6 +2653,7 @@ class WebPageProxy final : public API::ObjectImpl<API::Object::Type::Page>, publ
void hidePopupMenu();

#if ENABLE(CONTEXT_MENUS)
void showContextMenuFromFrame(WebCore::FrameIdentifier, ContextMenuContextData&&, UserData&&);
void showContextMenu(ContextMenuContextData&&, const UserData&);
#endif

Expand Down
1 change: 1 addition & 0 deletions Source/WebKit/UIProcess/WebPageProxy.messages.in
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ messages -> WebPageProxy {

#if ENABLE(CONTEXT_MENUS)
ShowContextMenu(WebKit::ContextMenuContextData contextMenuContextData, WebKit::UserData userData)
ShowContextMenuFromFrame(WebCore::FrameIdentifier frameID, WebKit::ContextMenuContextData contextMenuContextData, WebKit::UserData userData)
#endif

DidFinishServiceWorkerPageRegistration(bool success)
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/WebProcess/WebPage/WebContextMenu.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ void WebContextMenu::show()
// Mark the WebPage has having a shown context menu then notify the UIProcess.
m_page->startWaitingForContextMenuToShow();
m_page->flushPendingEditorStateUpdate();
m_page->send(Messages::WebPageProxy::ShowContextMenu(contextMenuContextData, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get())));
m_page->send(Messages::WebPageProxy::ShowContextMenuFromFrame(frame->frameID(), contextMenuContextData, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get())));
}

void WebContextMenu::itemSelected(const WebContextMenuItemData& item)
Expand Down
20 changes: 15 additions & 5 deletions Source/WebKit/WebProcess/WebPage/WebPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9176,23 +9176,33 @@ void WebPage::requestTextExtraction(std::optional<FloatRect>&& collectionRectInR
completion(TextExtraction::extractItem(WTFMove(collectionRectInRootView), Ref { *corePage() }));
}

void WebPage::remoteViewCoordinatesToRootView(FrameIdentifier frameID, FloatRect rect, CompletionHandler<void(FloatRect)>&& completionHandler)
template<typename T> void WebPage::remoteViewToRootView(WebCore::FrameIdentifier frameID, T geometry, CompletionHandler<void(T)>&& completionHandler)
{
RefPtr webFrame = WebProcess::singleton().webFrame(frameID);
if (!webFrame)
return completionHandler(rect);
return completionHandler(geometry);

RefPtr coreRemoteFrame = webFrame->coreRemoteFrame();
if (!coreRemoteFrame) {
ASSERT_NOT_REACHED();
return completionHandler(rect);
return completionHandler(geometry);
}

RefPtr view = coreRemoteFrame->view();
if (!view)
return completionHandler(rect);
return completionHandler(geometry);

completionHandler(view->contentsToRootView(rect));
completionHandler(view->contentsToRootView(geometry));
}

void WebPage::remoteViewRectToRootView(FrameIdentifier frameID, FloatRect rect, CompletionHandler<void(FloatRect)>&& completionHandler)
{
remoteViewToRootView(frameID, rect, WTFMove(completionHandler));
}

void WebPage::remoteViewPointToRootView(FrameIdentifier frameID, FloatPoint point, CompletionHandler<void(FloatPoint)>&& completionHandler)
{
remoteViewToRootView(frameID, point, WTFMove(completionHandler));
}

} // namespace WebKit
Expand Down
4 changes: 3 additions & 1 deletion Source/WebKit/WebProcess/WebPage/WebPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -2211,7 +2211,9 @@ class WebPage : public API::ObjectImpl<API::Object::Type::BundlePage>, public IP
static void setHasLaunchedWebContentProcess();
#endif

void remoteViewCoordinatesToRootView(WebCore::FrameIdentifier, WebCore::FloatRect, CompletionHandler<void(WebCore::FloatRect)>&&);
template<typename T> void remoteViewToRootView(WebCore::FrameIdentifier, T, CompletionHandler<void(T)>&&);
void remoteViewRectToRootView(WebCore::FrameIdentifier, WebCore::FloatRect, CompletionHandler<void(WebCore::FloatRect)>&&);
void remoteViewPointToRootView(WebCore::FrameIdentifier, WebCore::FloatPoint, CompletionHandler<void(WebCore::FloatPoint)>&&);

WebCore::PageIdentifier m_identifier;

Expand Down
3 changes: 2 additions & 1 deletion Source/WebKit/WebProcess/WebPage/WebPage.messages.in
Original file line number Diff line number Diff line change
Expand Up @@ -778,5 +778,6 @@ GenerateSyntheticEditingCommand(enum:uint8_t WebKit::SyntheticEditingCommandType
TextReplacementSessionDidReceiveEditAction(WTF::UUID uuid, enum:uint8_t WebKit::WebTextReplacementData::EditAction action);
#endif

RemoteViewCoordinatesToRootView(WebCore::FrameIdentifier frameID, WebCore::FloatRect rect) -> (WebCore::FloatRect rect)
RemoteViewRectToRootView(WebCore::FrameIdentifier frameID, WebCore::FloatRect rect) -> (WebCore::FloatRect rect)
RemoteViewPointToRootView(WebCore::FrameIdentifier frameID, WebCore::FloatPoint point) -> (WebCore::FloatPoint point)
}

0 comments on commit fbda7ba

Please sign in to comment.