- Revision
- 261921
- Author
- [email protected]
- Date
- 2020-05-20 08:43:19 -0700 (Wed, 20 May 2020)
Log Message
[Mac] UI processes spin when creating the "Share" context menu item
https://bugs.webkit.org/show_bug.cgi?id=212137
<rdar://problem/54498394>
Reviewed by Wenson Hsieh.
Source/WebCore:
Ran update-webkit-localizable-strings.
* en.lproj/Localizable.strings:
Source/WebCore/PAL:
* pal/spi/mac/NSSharingServicePickerSPI.h: Declared -getMenuWithCompletion:.
Source/WebKit:
On Mac, WebKit has been using the +[NSMenuItem standardShareMenuItemForItems:] SPI to create
the "Share" context submenu and menu item. This call performs synchronous IPC and can result
in UI process spins.
Where available, switch to using the asynchronous
-[NSSharingServicePicker getMenuWithCompletion:] SPI instead. This method only creates the
submenu, not the menu item, so when using this new method we must create the "Share" menu
item ourselves.
* UIProcess/mac/WebContextMenuProxyMac.h:
* UIProcess/mac/WebContextMenuProxyMac.mm:
(WebKit::getStandardShareMenuItem):
(WebKit::WebContextMenuProxyMac::getShareMenuItem):
(WebKit::WebContextMenuProxyMac::getContextMenuFromItems):
(WebKit::WebContextMenuProxyMac::getContextMenuItem):
(WebKit::WebContextMenuProxyMac::showContextMenuWithItems):
(WebKit::WebContextMenuProxyMac::createShareMenuItem): Deleted.
(WebKit::WebContextMenuProxyMac::createContextMenuFromItems): Deleted.
(WebKit::WebContextMenuProxyMac::createContextMenuItem): Deleted.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (261920 => 261921)
--- trunk/Source/WebCore/ChangeLog 2020-05-20 15:38:50 UTC (rev 261920)
+++ trunk/Source/WebCore/ChangeLog 2020-05-20 15:43:19 UTC (rev 261921)
@@ -1,3 +1,15 @@
+2020-05-20 Andy Estes <[email protected]>
+
+ [Mac] UI processes spin when creating the "Share" context menu item
+ https://bugs.webkit.org/show_bug.cgi?id=212137
+ <rdar://problem/54498394>
+
+ Reviewed by Wenson Hsieh.
+
+ Ran update-webkit-localizable-strings.
+
+ * en.lproj/Localizable.strings:
+
2020-05-20 Zalan Bujtas <[email protected]>
[LFC][TFC] Internal table boxes should take collapsed border into account
Modified: trunk/Source/WebCore/PAL/ChangeLog (261920 => 261921)
--- trunk/Source/WebCore/PAL/ChangeLog 2020-05-20 15:38:50 UTC (rev 261920)
+++ trunk/Source/WebCore/PAL/ChangeLog 2020-05-20 15:43:19 UTC (rev 261921)
@@ -1,3 +1,13 @@
+2020-05-20 Andy Estes <[email protected]>
+
+ [Mac] UI processes spin when creating the "Share" context menu item
+ https://bugs.webkit.org/show_bug.cgi?id=212137
+ <rdar://problem/54498394>
+
+ Reviewed by Wenson Hsieh.
+
+ * pal/spi/mac/NSSharingServicePickerSPI.h: Declared -getMenuWithCompletion:.
+
2020-05-18 Andy Estes <[email protected]>
http/tests/ssl/applepay/ApplePayInstallmentConfiguration.https.html fails in public SDK builds
Modified: trunk/Source/WebCore/PAL/pal/spi/mac/NSSharingServicePickerSPI.h (261920 => 261921)
--- trunk/Source/WebCore/PAL/pal/spi/mac/NSSharingServicePickerSPI.h 2020-05-20 15:38:50 UTC (rev 261920)
+++ trunk/Source/WebCore/PAL/pal/spi/mac/NSSharingServicePickerSPI.h 2020-05-20 15:43:19 UTC (rev 261921)
@@ -41,6 +41,7 @@
@interface NSSharingServicePicker (Private)
@property NSSharingServicePickerStyle style;
- (NSMenu *)menu;
+- (void)getMenuWithCompletion:(void(^)(NSMenu *))completion;
- (void)hide;
@end
Modified: trunk/Source/WebCore/en.lproj/Localizable.strings (261920 => 261921)
--- trunk/Source/WebCore/en.lproj/Localizable.strings 2020-05-20 15:38:50 UTC (rev 261920)
+++ trunk/Source/WebCore/en.lproj/Localizable.strings 2020-05-20 15:43:19 UTC (rev 261921)
@@ -1,9 +1,3 @@
-/* Allow the specified bundle to use Touch ID to sign in to the specified website on this device */
-"“%@” would like to use Touch ID for “%@”." = "“%@” would like to use Touch ID for “%@”.";
-
-/* "Allow the specified bundle to sign in to the specified website */
-"“%@” would like to sign in to “%@”." = "“%@” would like to sign in to “%@”.";
-
/* accessibility help text for media controller time value >= 1 day */
"%1$d days %2$d hours %3$d minutes %4$d seconds" = "%1$d days %2$d hours %3$d minutes %4$d seconds";
@@ -229,7 +223,7 @@
/* Continue */
"Continue" = "Continue";
-/* Continue with Touch ID */
+/* Continue with Touch ID. */
"Continue with Touch ID." = "Continue with Touch ID.";
/* Media Controls context menu item */
@@ -646,6 +640,9 @@
/* default label for Reset buttons in forms on web pages */
"Reset" = "Reset";
+/* Reset button in date input context menu */
+"Reset Button Date/Time Context Menu" = "Reset";
+
/* Right to Left context menu item */
"Right to Left" = "Right to Left";
@@ -697,6 +694,9 @@
/* Label for the set up with Apple Pay button. */
"Set up with Apple Pay" = "Set up with Apple Pay";
+/* Title for Share context menu item. */
+"Share" = "Share";
+
/* Title for Share action button */
"Share…" = "Share…";
@@ -1324,9 +1324,6 @@
/* HTTP result code string */
"requested range not satisfiable" = "requested range not satisfiable";
-/* Reset button in date input popover */
-"Reset Button Date/Time Context Menu" = "Reset";
-
/* HTTP result code string */
"reset content" = "reset content";
@@ -1468,6 +1465,12 @@
/* Message for requesting access to the device motion and orientation */
"“%@” Would Like to Access Motion and Orientation" = "“%@” Would Like to Access Motion and Orientation";
+/* Allow the specified bundle to sign in to the specified website */
+"“%@” would like to sign in to “%@”." = "“%@” would like to sign in to “%@”.";
+
+/* Allow the specified bundle to use Touch ID to sign in to the specified website on this device */
+"“%@” would like to use Touch ID for “%@”." = "“%@” would like to use Touch ID for “%@”.";
+
/* Option in segmented control for choosing list type in text editing */
"•" = "•";
Modified: trunk/Source/WebKit/ChangeLog (261920 => 261921)
--- trunk/Source/WebKit/ChangeLog 2020-05-20 15:38:50 UTC (rev 261920)
+++ trunk/Source/WebKit/ChangeLog 2020-05-20 15:43:19 UTC (rev 261921)
@@ -1,3 +1,31 @@
+2020-05-20 Andy Estes <[email protected]>
+
+ [Mac] UI processes spin when creating the "Share" context menu item
+ https://bugs.webkit.org/show_bug.cgi?id=212137
+ <rdar://problem/54498394>
+
+ Reviewed by Wenson Hsieh.
+
+ On Mac, WebKit has been using the +[NSMenuItem standardShareMenuItemForItems:] SPI to create
+ the "Share" context submenu and menu item. This call performs synchronous IPC and can result
+ in UI process spins.
+
+ Where available, switch to using the asynchronous
+ -[NSSharingServicePicker getMenuWithCompletion:] SPI instead. This method only creates the
+ submenu, not the menu item, so when using this new method we must create the "Share" menu
+ item ourselves.
+
+ * UIProcess/mac/WebContextMenuProxyMac.h:
+ * UIProcess/mac/WebContextMenuProxyMac.mm:
+ (WebKit::getStandardShareMenuItem):
+ (WebKit::WebContextMenuProxyMac::getShareMenuItem):
+ (WebKit::WebContextMenuProxyMac::getContextMenuFromItems):
+ (WebKit::WebContextMenuProxyMac::getContextMenuItem):
+ (WebKit::WebContextMenuProxyMac::showContextMenuWithItems):
+ (WebKit::WebContextMenuProxyMac::createShareMenuItem): Deleted.
+ (WebKit::WebContextMenuProxyMac::createContextMenuFromItems): Deleted.
+ (WebKit::WebContextMenuProxyMac::createContextMenuItem): Deleted.
+
2020-05-20 Carlos Garcia Campos <[email protected]>
Unreviewed. Fix GTK4 build with GTK 3.98.4
Modified: trunk/Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.h (261920 => 261921)
--- trunk/Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.h 2020-05-20 15:38:50 UTC (rev 261920)
+++ trunk/Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.h 2020-05-20 15:43:19 UTC (rev 261921)
@@ -68,12 +68,12 @@
void show() override;
RefPtr<WebContextMenuListenerProxy> m_contextMenuListener;
- RetainPtr<NSMenuItem> createContextMenuItem(const WebContextMenuItemData&);
- RetainPtr<NSMenu> createContextMenuFromItems(const Vector<WebContextMenuItemData>&);
+ void getContextMenuItem(const WebContextMenuItemData&, CompletionHandler<void(NSMenuItem *)>&&);
+ void getContextMenuFromItems(const Vector<WebContextMenuItemData>&, CompletionHandler<void(NSMenu *)>&&);
void showContextMenu();
#if ENABLE(SERVICE_CONTROLS)
- RetainPtr<NSMenuItem> createShareMenuItem();
+ void getShareMenuItem(CompletionHandler<void(NSMenuItem *)>&&);
void showServicesMenu();
void setupServicesMenu();
#endif
Modified: trunk/Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.mm (261920 => 261921)
--- trunk/Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.mm 2020-05-20 15:38:50 UTC (rev 261920)
+++ trunk/Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.mm 2020-05-20 15:43:19 UTC (rev 261921)
@@ -44,6 +44,7 @@
#import "WebProcessProxy.h"
#import <WebCore/GraphicsContext.h>
#import <WebCore/IntRect.h>
+#import <WebCore/LocalizedStrings.h>
#import <pal/spi/mac/NSMenuSPI.h>
#import <pal/spi/mac/NSSharingServicePickerSPI.h>
#import <pal/spi/mac/NSSharingServiceSPI.h>
@@ -260,8 +261,27 @@
m_menu = nullptr;
}
-RetainPtr<NSMenuItem> WebContextMenuProxyMac::createShareMenuItem()
+static void getStandardShareMenuItem(NSArray *items, void (^completionHandler)(NSMenuItem *))
{
+#if HAVE(NSSHARINGSERVICEPICKER_ASYNC_MENUS)
+ // FIXME (<rdar://problem/54551500>): Replace this with the async variant of +[NSMenuItem standardShareMenuItemForItems:] when it's available.
+ auto sharingServicePicker = adoptNS([[NSSharingServicePicker alloc] initWithItems:items]);
+ [sharingServicePicker setStyle:NSSharingServicePickerStyleMenu];
+ [sharingServicePicker getMenuWithCompletion:^(NSMenu *shareMenu) {
+ ASSERT(isMainThread());
+ shareMenu.delegate = (id <NSMenuDelegate>)sharingServicePicker.get();
+ auto shareMenuItem = adoptNS([[NSMenuItem alloc] initWithTitle:WEB_UI_STRING("Share", "Title for Share context menu item.") action:nil keyEquivalent:@""]);
+ [shareMenuItem setRepresentedObject:sharingServicePicker.get()];
+ [shareMenuItem setSubmenu:shareMenu];
+ completionHandler(shareMenuItem.get());
+ }];
+#else
+ completionHandler([NSMenuItem standardShareMenuItemForItems:items]);
+#endif
+}
+
+void WebContextMenuProxyMac::getShareMenuItem(CompletionHandler<void(NSMenuItem *)>&& completionHandler)
+{
const WebHitTestResultData& hitTestData = m_context.webHitTestResultData();
auto items = adoptNS([[NSMutableArray alloc] init]);
@@ -286,26 +306,32 @@
if (!m_context.selectedText().isEmpty())
[items addObject:(NSString *)m_context.selectedText()];
- if (![items count])
- return nil;
+ if (![items count]) {
+ completionHandler(nil);
+ return;
+ }
- RetainPtr<NSMenuItem> item = [NSMenuItem standardShareMenuItemForItems:items.get()];
- if (!item)
- return nil;
+ getStandardShareMenuItem(items.get(), makeBlockPtr([completionHandler = WTFMove(completionHandler), protectedThis = makeRef(*this), this](NSMenuItem *item) mutable {
+ if (!item) {
+ completionHandler(nil);
+ return;
+ }
- NSSharingServicePicker *sharingServicePicker = [item representedObject];
- sharingServicePicker.delegate = [WKSharingServicePickerDelegate sharedSharingServicePickerDelegate];
+ NSSharingServicePicker *sharingServicePicker = item.representedObject;
+ WKSharingServicePickerDelegate *sharingServicePickerDelegate = WKSharingServicePickerDelegate.sharedSharingServicePickerDelegate;
+ sharingServicePicker.delegate = sharingServicePickerDelegate;
- [[WKSharingServicePickerDelegate sharedSharingServicePickerDelegate] setFiltersEditingServices:NO];
- [[WKSharingServicePickerDelegate sharedSharingServicePickerDelegate] setHandlesEditingReplacement:NO];
- [[WKSharingServicePickerDelegate sharedSharingServicePickerDelegate] setMenuProxy:this];
+ sharingServicePickerDelegate.filtersEditingServices = NO;
+ sharingServicePickerDelegate.handlesEditingReplacement = NO;
+ sharingServicePickerDelegate.menuProxy = this;
- // Setting the picker lets the delegate retain it to keep it alive, but this picker is kept alive by the menu item.
- [[WKSharingServicePickerDelegate sharedSharingServicePickerDelegate] setPicker:nil];
+ // Setting the picker lets the delegate retain it to keep it alive, but this picker is kept alive by the menu item.
+ sharingServicePickerDelegate.picker = nil;
- [item setIdentifier:_WKMenuItemIdentifierShareMenu];
+ item.identifier = _WKMenuItemIdentifierShareMenu;
- return item;
+ completionHandler(item);
+ }).get());
}
#endif
@@ -323,17 +349,34 @@
showContextMenu();
}
-RetainPtr<NSMenu> WebContextMenuProxyMac::createContextMenuFromItems(const Vector<WebContextMenuItemData>& items)
+void WebContextMenuProxyMac::getContextMenuFromItems(const Vector<WebContextMenuItemData>& items, CompletionHandler<void(NSMenu *)>&& completionHandler)
{
auto menu = adoptNS([[NSMenu alloc] initWithTitle:@""]);
[menu setAutoenablesItems:NO];
- for (auto& item : items) {
- if (auto menuItem = createContextMenuItem(item))
- [menu addItem:menuItem.get()];
+ if (items.isEmpty()) {
+ completionHandler(menu.get());
+ return;
}
- return menu;
+ auto sparseMenuItems = retainPtr([NSPointerArray strongObjectsPointerArray]);
+ auto insertMenuItem = makeBlockPtr([completionHandler = WTFMove(completionHandler), itemsRemaining = items.size(), menu = WTFMove(menu), sparseMenuItems](NSMenuItem *item, NSUInteger index) mutable {
+ ASSERT(index < [sparseMenuItems count]);
+ ASSERT(![sparseMenuItems pointerAtIndex:index]);
+ [sparseMenuItems replacePointerAtIndex:index withPointer:item];
+ if (--itemsRemaining)
+ return;
+
+ [menu setItemArray:[sparseMenuItems allObjects]];
+ completionHandler(menu.get());
+ });
+
+ for (size_t i = 0; i < items.size(); ++i) {
+ [sparseMenuItems addPointer:nullptr];
+ getContextMenuItem(items[i], [insertMenuItem, i](NSMenuItem *menuItem) {
+ insertMenuItem(menuItem, i);
+ });
+ }
}
static NSString *menuItemIdentifier(const WebCore::ContextMenuAction action)
@@ -416,11 +459,13 @@
}
}
-RetainPtr<NSMenuItem> WebContextMenuProxyMac::createContextMenuItem(const WebContextMenuItemData& item)
+void WebContextMenuProxyMac::getContextMenuItem(const WebContextMenuItemData& item, CompletionHandler<void(NSMenuItem *)>&& completionHandler)
{
#if ENABLE(SERVICE_CONTROLS)
- if (item.action() == ContextMenuItemTagShareMenu)
- return createShareMenuItem();
+ if (item.action() == ContextMenuItemTagShareMenu) {
+ getShareMenuItem(WTFMove(completionHandler));
+ return;
+ }
#endif
switch (item.type()) {
@@ -439,19 +484,23 @@
[menuItem setRepresentedObject:wrapper.get()];
}
- return menuItem;
+ completionHandler(menuItem.get());
+ return;
}
case WebCore::SeparatorType:
- return [NSMenuItem separatorItem];
+ completionHandler(NSMenuItem.separatorItem);
+ return;
case WebCore::SubmenuType: {
- auto menuItem = adoptNS([[NSMenuItem alloc] initWithTitle:item.title() action:nullptr keyEquivalent:@""]);
- [menuItem setEnabled:item.enabled()];
- [menuItem setSubmenu:createContextMenuFromItems(item.submenu()).get()];
- [menuItem setIdentifier:menuItemIdentifier(item.action())];
-
- return menuItem;
+ getContextMenuFromItems(item.submenu(), [action = "" completionHandler = WTFMove(completionHandler), enabled = item.enabled(), title = item.title()](NSMenu *menu) mutable {
+ auto menuItem = adoptNS([[NSMenuItem alloc] initWithTitle:title action:nullptr keyEquivalent:@""]);
+ [menuItem setEnabled:enabled];
+ [menuItem setSubmenu:menu];
+ [menuItem setIdentifier:menuItemIdentifier(action)];
+ completionHandler(menuItem.get());
+ });
+ return;
}
}
}
@@ -468,19 +517,20 @@
data.reserveInitialCapacity(items.size());
for (auto& item : items)
data.uncheckedAppend(item->data());
-
- auto menu = createContextMenuFromItems(data);
- [[WKMenuTarget sharedMenuTarget] setMenuProxy:this];
- m_page.contextMenuClient().menuFromProposedMenu(m_page, menu.get(), m_context.webHitTestResultData(), m_userData.object(), [this, protectedThis = makeRef(*this)] (RetainPtr<NSMenu>&& menu) {
- m_menu = WTFMove(menu);
- NSPoint menuLocation = [m_webView convertPoint:m_context.menuLocation() toView:nil];
- NSEvent *event = [NSEvent mouseEventWithType:NSEventTypeRightMouseUp location:menuLocation modifierFlags:0 timestamp:0 windowNumber:m_webView.window.windowNumber context:nil eventNumber:0 clickCount:0 pressure:0];
- [NSMenu popUpContextMenu:m_menu.get() withEvent:event forView:m_webView];
-
- if (m_contextMenuListener) {
- m_contextMenuListener->invalidate();
- m_contextMenuListener = nullptr;
- }
+
+ getContextMenuFromItems(data, [protectedThis = makeRef(*this), this](NSMenu *menu) mutable {
+ [[WKMenuTarget sharedMenuTarget] setMenuProxy:this];
+ m_page.contextMenuClient().menuFromProposedMenu(m_page, menu, m_context.webHitTestResultData(), m_userData.object(), [this, protectedThis = WTFMove(protectedThis)] (RetainPtr<NSMenu>&& menu) {
+ m_menu = WTFMove(menu);
+ NSPoint menuLocation = [m_webView convertPoint:m_context.menuLocation() toView:nil];
+ NSEvent *event = [NSEvent mouseEventWithType:NSEventTypeRightMouseUp location:menuLocation modifierFlags:0 timestamp:0 windowNumber:m_webView.window.windowNumber context:nil eventNumber:0 clickCount:0 pressure:0];
+ [NSMenu popUpContextMenu:m_menu.get() withEvent:event forView:m_webView];
+
+ if (m_contextMenuListener) {
+ m_contextMenuListener->invalidate();
+ m_contextMenuListener = nullptr;
+ }
+ });
});
}
