Skip to content

Commit

Permalink
PDF context menu should be shown asynchronously
Browse files Browse the repository at this point in the history 
https://bugs.webkit.org/show_bug.cgi?id=268973
rdar://122525514

Reviewed by Simon Fraser.

WebPageProxy::ShowPDFContextMenu is a sync IPC call that blocks the WP.
Among other things, this manifests itself in repaint requests queued
right before the IPC call to be fulfilled only _after_ a context menu
action has been taken.

There is no reason for this synchronicity, so this patch async-ifies
WebPageProxy::ShowPDFContextMenu, which we call with sendWithAsyncReply
to learn about the context menu item selected by the user.

We also make some drive-by fixes such as avoiding blindly casting
integral values from the UIP into ContextMenuItemTag and moving some
function definitions around so we can annotate function groups with
`#pragma mark`.

* Source/WebKit/UIProcess/WebPageProxy.messages.in:
* Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:
(WebKit::PDFPlugin::handleContextMenuEvent):
* Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/UnifiedPDFPlugin.h:
* Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/UnifiedPDFPlugin.mm:
(WebKit::UnifiedPDFPlugin::handleMouseEvent):
(WebKit::UnifiedPDFPlugin::handleMouseEnterEvent):
(WebKit::UnifiedPDFPlugin::handleMouseLeaveEvent):
(WebKit::UnifiedPDFPlugin::handleContextMenuEvent):
(WebKit::UnifiedPDFPlugin::contextMenuItemTagFromDisplayMode const):
(WebKit::UnifiedPDFPlugin::displayModeFromContextMenuItemTag const):
(WebKit::UnifiedPDFPlugin::performContextMenuAction):

Canonical link: https://commits.webkit.org/274320@main
  • Loading branch information
@aprotyas
aprotyas committed Feb 8, 2024
1 parent 03a60be commit 3babd34
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 40 deletions.
2 changes: 1 addition & 1 deletion Source/WebKit/UIProcess/WebPageProxy.messages.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ messages -> WebPageProxy {
#endif

#if ENABLE(PDF_PLUGIN) && PLATFORM(MAC)
ShowPDFContextMenu(struct WebKit::PDFContextMenu contextMenu, WebKit::PDFPluginIdentifier identifier) -> (std::optional<int32_t> selectedItem) Synchronous
ShowPDFContextMenu(struct WebKit::PDFContextMenu contextMenu, WebKit::PDFPluginIdentifier identifier) -> (std::optional<int32_t> selectedItem)
#endif

#if ENABLE(PDF_HUD)
Expand Down
13 changes: 7 additions & 6 deletions Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1092,7 +1092,7 @@ static bool getEventTypeFromWebEvent(const WebEvent& event, NSEventType& eventTy
IntPoint point = frameView->contentsToScreen(IntRect(frameView->windowToContents(event.position()), IntSize())).location();

NSUserInterfaceLayoutDirection uiLayoutDirection = webPage->userInterfaceLayoutDirection() == UserInterfaceLayoutDirection::LTR ? NSUserInterfaceLayoutDirectionLeftToRight : NSUserInterfaceLayoutDirectionRightToLeft;
NSMenu *nsMenu = [m_pdfLayerController menuForEvent:nsEventForWebMouseEvent(event) withUserInterfaceLayoutDirection:uiLayoutDirection];
RetainPtr nsMenu = [m_pdfLayerController menuForEvent:nsEventForWebMouseEvent(event) withUserInterfaceLayoutDirection:uiLayoutDirection];

if (!nsMenu)
return false;
Expand All @@ -1115,11 +1115,12 @@ static bool getEventTypeFromWebEvent(const WebEvent& event, NSEventType& eventTy
}
PDFContextMenu contextMenu { point, WTFMove(items), WTFMove(openInPreviewTag) };

auto sendResult = webPage->sendSync(Messages::WebPageProxy::ShowPDFContextMenu(contextMenu, m_identifier));
auto [selectedIndex] = sendResult.takeReplyOr(-1);

if (selectedIndex && *selectedIndex >= 0 && *selectedIndex < itemCount)
[nsMenu performActionForItemAtIndex:*selectedIndex];
webPage->sendWithAsyncReply(Messages::WebPageProxy::ShowPDFContextMenu { contextMenu, m_identifier }, [itemCount, nsMenu = WTFMove(nsMenu), weakThis = WeakPtr { *this }](std::optional<int32_t>&& selectedIndex) {
if (RefPtr protectedThis = weakThis.get()) {
if (selectedIndex && selectedIndex.value() >= 0 && selectedIndex.value() < itemCount)
[nsMenu performActionForItemAtIndex:*selectedIndex];
}
});

return true;
}
Expand Down
7 changes: 5 additions & 2 deletions Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/UnifiedPDFPlugin.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,9 @@ class UnifiedPDFPlugin final : public PDFPluginBase, public WebCore::GraphicsLay
bool forwardEditingCommandToEditor(const String& commandName, const String& argument) const;
void selectAll();

enum class ContextMenuItemTag : uint8_t {
// Context Menu
enum class ContextMenuItemTag : int8_t {
Invalid = -1,
OpenWithPreview,
SinglePage,
SinglePageContinuous,
Expand All @@ -169,15 +171,16 @@ class UnifiedPDFPlugin final : public PDFPluginBase, public WebCore::GraphicsLay
ZoomIn,
ZoomOut,
ActualSize,
Unknown,
};

#if PLATFORM(MAC)
PDFContextMenu createContextMenu(const WebCore::IntPoint& contextMenuPoint) const;
ContextMenuItemTag toContextMenuItemTag(int tagValue) const;
void performContextMenuAction(ContextMenuItemTag);

ContextMenuItemTag contextMenuItemTagFromDisplayMode(const PDFDocumentLayout::DisplayMode&) const;
PDFDocumentLayout::DisplayMode displayModeFromContextMenuItemTag(const ContextMenuItemTag&) const;
static constexpr int invalidContextMenuItemTag { -1 };
#endif

// Selections
Expand Down
87 changes: 56 additions & 31 deletions Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/UnifiedPDFPlugin.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1181,6 +1181,39 @@ static AffineTransform documentSpaceToPageSpaceTransform(const IntDegrees& pageR
return false;
}

bool UnifiedPDFPlugin::handleContextMenuEvent(const WebMouseEvent& event)
{
#if PLATFORM(MAC)
if (!m_frame || !m_frame->coreLocalFrame())
return false;
RefPtr webPage = m_frame->page();
if (!webPage)
return false;
RefPtr frameView = m_frame->coreLocalFrame()->view();
if (!frameView)
return false;

auto contextMenu = createContextMenu(frameView->contentsToScreen(IntRect(frameView->windowToContents(event.position()), IntSize())).location());

webPage->sendWithAsyncReply(Messages::WebPageProxy::ShowPDFContextMenu { contextMenu, m_identifier }, [this, weakThis = WeakPtr { *this }](std::optional<int32_t>&& selectedItemTag) {
RefPtr protectedThis = weakThis.get();
if (!protectedThis)
return;
if (selectedItemTag)
performContextMenuAction(toContextMenuItemTag(selectedItemTag.value()));
});

return true;
#else
return false;
#endif // PLATFORM(MAC)
}

bool UnifiedPDFPlugin::handleKeyboardEvent(const WebKeyboardEvent&)
{
return false;
}

void UnifiedPDFPlugin::didClickLinkAnnotation(const PDFAnnotation *annotation)
{
// FIXME: Handle links with in-document destinations.
Expand All @@ -1189,6 +1222,8 @@ static AffineTransform documentSpaceToPageSpaceTransform(const IntDegrees& pageR
navigateToURL(url);
}

#pragma mark Context Menu

#if PLATFORM(MAC)
UnifiedPDFPlugin::ContextMenuItemTag UnifiedPDFPlugin::contextMenuItemTagFromDisplayMode(const PDFDocumentLayout::DisplayMode& displayMode) const
{
Expand All @@ -1213,12 +1248,30 @@ static AffineTransform documentSpaceToPageSpaceTransform(const IntDegrees& pageR
}
}

auto UnifiedPDFPlugin::toContextMenuItemTag(int tagValue) const -> ContextMenuItemTag
{
static constexpr std::array regularContextMenuItemTags {
ContextMenuItemTag::OpenWithPreview,
ContextMenuItemTag::SinglePage,
ContextMenuItemTag::SinglePageContinuous,
ContextMenuItemTag::TwoPages,
ContextMenuItemTag::TwoPagesContinuous,
ContextMenuItemTag::ZoomIn,
ContextMenuItemTag::ZoomOut,
ContextMenuItemTag::ActualSize,
};
const auto isKnownContextMenuItemTag = WTF::anyOf(regularContextMenuItemTags, [tagValue](ContextMenuItemTag tag) {
return tagValue == enumToUnderlyingType(tag);
});
return isKnownContextMenuItemTag ? static_cast<ContextMenuItemTag>(tagValue) : ContextMenuItemTag::Unknown;
}

PDFContextMenu UnifiedPDFPlugin::createContextMenu(const IntPoint& contextMenuPoint) const
{
Vector<PDFContextMenuItem> menuItems;

auto addSeparator = [&] {
menuItems.append({ String(), 0, invalidContextMenuItemTag, ContextMenuItemEnablement::Disabled, ContextMenuItemHasAction::No, ContextMenuItemIsSeparator::Yes });
menuItems.append({ String(), 0, enumToUnderlyingType(ContextMenuItemTag::Invalid), ContextMenuItemEnablement::Disabled, ContextMenuItemHasAction::No, ContextMenuItemIsSeparator::Yes });
};

menuItems.append({ WebCore::contextMenuItemPDFOpenWithPreview(), 0,
Expand Down Expand Up @@ -1268,40 +1321,12 @@ static AffineTransform documentSpaceToPageSpaceTransform(const IntDegrees& pageR
case ContextMenuItemTag::ActualSize:
m_view->setPageScaleFactor(scaleForActualSize(), std::nullopt);
break;
default:
RELEASE_ASSERT_NOT_REACHED();
}
}
#endif // PLATFORM(MAC)

bool UnifiedPDFPlugin::handleContextMenuEvent(const WebMouseEvent& event)
{
#if PLATFORM(MAC)
if (!m_frame || !m_frame->coreLocalFrame())
return false;
RefPtr webPage = m_frame->page();
if (!webPage)
return false;
RefPtr frameView = m_frame->coreLocalFrame()->view();
if (!frameView)
return false;

auto contextMenu = createContextMenu(frameView->contentsToScreen(IntRect(frameView->windowToContents(event.position()), IntSize())).location());

auto sendResult = webPage->sendSync(Messages::WebPageProxy::ShowPDFContextMenu(contextMenu, m_identifier));
auto [selectedItemTag] = sendResult.takeReplyOr(-1);

if (selectedItemTag >= 0)
performContextMenuAction(static_cast<ContextMenuItemTag>(selectedItemTag.value()));
return true;
#else
return false;
#endif // PLATFORM(MAC)
}

bool UnifiedPDFPlugin::handleKeyboardEvent(const WebKeyboardEvent&)
{
return false;
}

#pragma mark Editing Commands

bool UnifiedPDFPlugin::handleEditingCommand(const String& commandName, const String& argument)
Expand Down

0 comments on commit 3babd34

Please sign in to comment.