RemoteMacViews: Break validateUserInterfaceItem across mojo

This is a step toward supporting shortcut keys and menu interface items
in PWAs. This breaks the function -[BrowserWindowCommandHandler
validateUserInterfaceItem] into two parts
- The part to look up the browser command and to update the interface
  item still remains in BrowserWindowCommandHandler (which will be in
  the in the PWA process).
- The part to access the Browser*, which will be in the browser process,
  which is
  - routed through mojo to the BridgedNativeWidgetHostImpl
  - to the NativeWidgetMac
  - which is subclassed as BrowserFrameMac
  - which calls BrowserWindowCommandHandlerValidateUserInterfaceItem
    to implemented the parts that used to be in
    BrowserWindowCommandHandler

Also add required interface to look up a BridgedNativeWidgetImpl from
an NSWindow.

Bug: 895169
Change-Id: Ie81a1e5a44537b07183d4c29b89d2da1951137c0
Reviewed-on: https://chromium-review.googlesource.com/c/1357829
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613255}

chrome/browser/ui/cocoa/browser_window_command_handler.mm

@@ -6,22 +6,17 @@
 
 #include "base/logging.h"
 #import "base/mac/foundation_util.h"
+#include "base/strings/sys_string_conversions.h"
 #include "chrome/app/chrome_command_ids.h"
 #import "chrome/browser/app_controller_mac.h"
-#include "chrome/browser/media/router/media_router_feature.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/ui/bookmarks/bookmark_utils.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_window.h"
-#include "chrome/common/chrome_features.h"
-#include "chrome/common/pref_names.h"
-#include "chrome/grit/generated_resources.h"
-#include "components/bookmarks/common/bookmark_pref_names.h"
 #include "content/public/browser/web_contents.h"
 #import "ui/base/cocoa/cocoa_base_utils.h"
-#include "ui/base/l10n/l10n_util.h"
-#include "ui/base/l10n/l10n_util_mac.h"
+#include "ui/views_bridge_mac/bridged_native_widget_impl.h"
+#include "ui/views_bridge_mac/mojo/bridged_native_widget_host.mojom.h"
 
 namespace {
 
@@ -41,69 +36,6 @@ void SetToggleState(bool toggled, id item) {
   }
 }
 
-// Update a toggle state for an item if modified. The item may be an NSMenuItem
-// or NSButton. Called by -validateUserInterfaceItem:.
-void UpdateToggleStateWithTag(NSInteger tag, id item, NSWindow* window) {
-  if (!base::mac::ObjCCast<NSMenuItem>(item) &&
-      !base::mac::ObjCCast<NSButton>(item))
-    return;
-
-  Browser* browser = chrome::FindBrowserWithWindow(window);
-  DCHECK(browser);
-
-  // On Windows this logic happens in bookmark_bar_view.cc. This simply updates
-  // the menu item; it does not display the bookmark bar itself.
-  if (tag == IDC_SHOW_BOOKMARK_BAR) {
-    PrefService* prefs = browser->profile()->GetPrefs();
-    SetToggleState(prefs->GetBoolean(bookmarks::prefs::kShowBookmarkBar), item);
-    return;
-  }
-
-  if (tag == IDC_TOGGLE_FULLSCREEN_TOOLBAR) {
-    PrefService* prefs = browser->profile()->GetPrefs();
-    SetToggleState(prefs->GetBoolean(prefs::kShowFullscreenToolbar), item);
-    return;
-  }
-
-  if (tag == IDC_TOGGLE_JAVASCRIPT_APPLE_EVENTS) {
-    PrefService* prefs = browser->profile()->GetPrefs();
-    SetToggleState(prefs->GetBoolean(prefs::kAllowJavascriptAppleEvents), item);
-    return;
-  }
-
-  if (tag == IDC_WINDOW_MUTE_SITE) {
-    TabStripModel* model = browser->tab_strip_model();
-    bool will_mute =
-        base::FeatureList::IsEnabled(features::kSoundContentSetting)
-            ? model->WillContextMenuMuteSites(model->active_index())
-            : model->WillContextMenuMute(model->active_index());
-    // Menu items may be validated during browser startup, before the
-    // TabStripModel has been populated.
-    SetToggleState(!model->empty() && !will_mute, item);
-    return;
-  }
-
-  if (tag == IDC_WINDOW_PIN_TAB) {
-    TabStripModel* model = browser->tab_strip_model();
-    SetToggleState(
-        !model->empty() && !model->WillContextMenuPin(model->active_index()),
-        item);
-    return;
-  }
-}
-
-NSString* GetTitleForViewsFullscreenMenuItem(Browser* browser) {
-  return l10n_util::GetNSString(browser->window()->IsFullscreen()
-                                    ? IDS_EXIT_FULLSCREEN_MAC
-                                    : IDS_ENTER_FULLSCREEN_MAC);
-}
-
-// Get the text for the "Enter/Exit Fullscreen" menu item.
-// TODO(jackhou): Remove the dependency on BrowserWindowController(Private).
-NSString* GetTitleForFullscreenMenuItem(Browser* browser) {
-  return GetTitleForViewsFullscreenMenuItem(browser);
-}
-
 // Identify the actual Browser to which the command should be dispatched. It
 // might belong to a background window, yet another dispatcher gets it because
 // it is the foreground window's dispatcher and thus in the responder chain.
@@ -138,74 +70,28 @@ Browser* FindBrowserForSender(id sender, NSWindow* window) {
     return YES;
   }
 
-  Browser* browser = chrome::FindBrowserWithWindow(window);
-  DCHECK(browser);
-  NSInteger tag = [item tag];
-  if (!chrome::SupportsCommand(browser, tag))
+  auto* bridge = views::BridgedNativeWidgetImpl::GetFromNativeWindow(window);
+  DCHECK(bridge);
+
+  views_bridge_mac::mojom::ValidateUserInterfaceItemResultPtr result;
+  if (!bridge->host()->ValidateUserInterfaceItem([item tag], &result))
     return NO;
 
-  // Generate return value (enabled state).
-  BOOL enable = chrome::IsCommandEnabled(browser, tag);
-  switch (tag) {
-    case IDC_CLOSE_TAB:
-      // Disable "close tab" if the receiving window is not tabbed.
-      // We simply check whether the item has a keyboard shortcut set here;
-      // app_controller_mac.mm actually determines whether the item should
-      // be enabled.
-      if (NSMenuItem* menuItem = base::mac::ObjCCast<NSMenuItem>(item))
-        enable &= !![[menuItem keyEquivalent] length];
-      break;
-    case IDC_FULLSCREEN: {
-      if (NSMenuItem* menuItem = base::mac::ObjCCast<NSMenuItem>(item))
-        [menuItem setTitle:GetTitleForFullscreenMenuItem(browser)];
-      break;
-    }
-    case IDC_BOOKMARK_PAGE: {
-      // Extensions have the ability to hide the bookmark page menu item.
-      // This only affects the bookmark page menu item under the main menu.
-      // The bookmark page menu item under the app menu has its visibility
-      // controlled by AppMenuModel.
-      bool shouldHide =
-          chrome::ShouldRemoveBookmarkThisPageUI(browser->profile());
-      NSMenuItem* menuItem = base::mac::ObjCCast<NSMenuItem>(item);
-      [menuItem setHidden:shouldHide];
-      break;
-    }
-    case IDC_BOOKMARK_ALL_TABS: {
-      // Extensions have the ability to hide the bookmark all tabs menu
-      // item.  This only affects the bookmark page menu item under the main
-      // menu.  The bookmark page menu item under the app menu has its
-      // visibility controlled by AppMenuModel.
-      bool shouldHide =
-          chrome::ShouldRemoveBookmarkOpenPagesUI(browser->profile());
-      NSMenuItem* menuItem = base::mac::ObjCCast<NSMenuItem>(item);
-      [menuItem setHidden:shouldHide];
-      break;
-    }
-    case IDC_SHOW_AS_TAB: {
-      // Hide this menu option if the window is tabbed or is the devtools
-      // window.
-      NSMenuItem* menuItem = base::mac::ObjCCast<NSMenuItem>(item);
-      [menuItem setHidden:browser->is_type_tabbed() || browser->is_devtools()];
-      break;
-    }
-    case IDC_ROUTE_MEDIA: {
-      // Hide this menu option if Media Router is disabled.
-      NSMenuItem* menuItem = base::mac::ObjCCast<NSMenuItem>(item);
-      [menuItem
-          setHidden:!media_router::MediaRouterEnabled(browser->profile())];
-      break;
-    }
-    default:
-      break;
-  }
+  if (result->set_toggle_state)
+    SetToggleState(result->new_toggle_state, item);
 
-  // If the item is toggleable, find its toggle state and
-  // try to update it.  This is a little awkward, but the alternative is
-  // to check after a commandDispatch, which seems worse.
-  UpdateToggleStateWithTag(tag, item, window);
+  if (NSMenuItem* menuItem = base::mac::ObjCCast<NSMenuItem>(item)) {
+    if (result->disable_if_has_no_key_equivalent)
+      result->enable &= !![[menuItem keyEquivalent] length];
+
+    if (result->set_hidden_state)
+      [menuItem setHidden:result->new_hidden_state];
+
+    if (result->new_title)
+      [menuItem setTitle:base::SysUTF16ToNSString(*result->new_title)];
+  }
 
-  return enable;
+  return result->enable;
 }
 
 - (void)commandDispatch:(id)sender window:(NSWindow*)window {

chrome/browser/ui/views/frame/browser_frame_mac.h

@@ -53,6 +53,10 @@ class BrowserFrameMac : public views::NativeWidgetMac,
   ~BrowserFrameMac() override;
 
   // Overridden from views::NativeWidgetMac:
+  void ValidateUserInterfaceItem(
+      int32_t command,
+      views_bridge_mac::mojom::ValidateUserInterfaceItemResult* result)
+      override;
   void PopulateCreateWindowParams(
       const views::Widget::InitParams& widget_params,
       views_bridge_mac::mojom::CreateWindowParams* params) override;

chrome/browser/ui/views/frame/browser_frame_mac.mm

@@ -8,6 +8,8 @@
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/apps/app_shim/extension_app_shim_handler_mac.h"
 #include "chrome/browser/global_keyboard_shortcuts_mac.h"
+#include "chrome/browser/media/router/media_router_feature.h"
+#include "chrome/browser/ui/bookmarks/bookmark_utils.h"
 #include "chrome/browser/ui/browser_command_controller.h"
 #include "chrome/browser/ui/browser_commands.h"
 #import "chrome/browser/ui/cocoa/browser_window_command_handler.h"
@@ -16,10 +18,16 @@
 #include "chrome/browser/ui/views/frame/browser_frame.h"
 #include "chrome/browser/ui/views/frame/browser_non_client_frame_view.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/common/chrome_features.h"
+#include "chrome/common/pref_names.h"
+#include "chrome/grit/generated_resources.h"
+#include "components/bookmarks/common/bookmark_pref_names.h"
 #include "components/web_modal/web_contents_modal_dialog_host.h"
 #include "content/public/browser/native_web_keyboard_event.h"
 #import "ui/base/cocoa/window_size_constants.h"
+#include "ui/base/l10n/l10n_util.h"
 #include "ui/views_bridge_mac/mojo/bridged_native_widget.mojom.h"
+#include "ui/views_bridge_mac/mojo/bridged_native_widget_host.mojom.h"
 #import "ui/views_bridge_mac/native_widget_mac_nswindow.h"
 #import "ui/views_bridge_mac/window_touch_bar_delegate.h"
 
@@ -136,6 +144,113 @@ void BrowserFrameMac::OnWindowFullscreenStateChange() {
   browser_view_->FullscreenStateChanged();
 }
 
+void BrowserFrameMac::ValidateUserInterfaceItem(
+    int32_t tag,
+    views_bridge_mac::mojom::ValidateUserInterfaceItemResult* result) {
+  Browser* browser = browser_view_->browser();
+  if (!chrome::SupportsCommand(browser, tag)) {
+    result->enable = false;
+    return;
+  }
+
+  // Generate return value (enabled state).
+  result->enable = chrome::IsCommandEnabled(browser, tag);
+  switch (tag) {
+    case IDC_CLOSE_TAB:
+      // Disable "close tab" if the receiving window is not tabbed.
+      // We simply check whether the item has a keyboard shortcut set here;
+      // app_controller_mac.mm actually determines whether the item should
+      // be enabled.
+      result->disable_if_has_no_key_equivalent = true;
+      break;
+    case IDC_FULLSCREEN: {
+      result->new_title.emplace(l10n_util::GetStringUTF16(
+          browser->window()->IsFullscreen() ? IDS_EXIT_FULLSCREEN_MAC
+                                            : IDS_ENTER_FULLSCREEN_MAC));
+      break;
+    }
+    case IDC_BOOKMARK_PAGE: {
+      // Extensions have the ability to hide the bookmark page menu item.
+      // This only affects the bookmark page menu item under the main menu.
+      // The bookmark page menu item under the app menu has its visibility
+      // controlled by AppMenuModel.
+      result->new_hidden_state =
+          chrome::ShouldRemoveBookmarkThisPageUI(browser->profile());
+      break;
+    }
+    case IDC_BOOKMARK_ALL_TABS: {
+      // Extensions have the ability to hide the bookmark all tabs menu
+      // item.  This only affects the bookmark page menu item under the main
+      // menu.  The bookmark page menu item under the app menu has its
+      // visibility controlled by AppMenuModel.
+      result->new_hidden_state =
+          chrome::ShouldRemoveBookmarkOpenPagesUI(browser->profile());
+      break;
+    }
+    case IDC_SHOW_AS_TAB: {
+      // Hide this menu option if the window is tabbed or is the devtools
+      // window.
+      result->new_hidden_state =
+          browser->is_type_tabbed() || browser->is_devtools();
+      break;
+    }
+    case IDC_ROUTE_MEDIA: {
+      // Hide this menu option if Media Router is disabled.
+      result->new_hidden_state =
+          !media_router::MediaRouterEnabled(browser->profile());
+      break;
+    }
+    default:
+      break;
+  }
+
+  // If the item is toggleable, find its toggle state and
+  // try to update it.  This is a little awkward, but the alternative is
+  // to check after a commandDispatch, which seems worse.
+  // On Windows this logic happens in bookmark_bar_view.cc. This simply updates
+  // the menu item; it does not display the bookmark bar itself.
+  result->set_toggle_state = true;
+  switch (tag) {
+    default:
+      result->set_toggle_state = false;
+      break;
+    case IDC_SHOW_BOOKMARK_BAR: {
+      PrefService* prefs = browser->profile()->GetPrefs();
+      result->new_toggle_state =
+          prefs->GetBoolean(bookmarks::prefs::kShowBookmarkBar);
+      break;
+    }
+    case IDC_TOGGLE_FULLSCREEN_TOOLBAR: {
+      PrefService* prefs = browser->profile()->GetPrefs();
+      result->new_toggle_state =
+          prefs->GetBoolean(prefs::kShowFullscreenToolbar);
+      break;
+    }
+    case IDC_TOGGLE_JAVASCRIPT_APPLE_EVENTS: {
+      PrefService* prefs = browser->profile()->GetPrefs();
+      result->new_toggle_state =
+          prefs->GetBoolean(prefs::kAllowJavascriptAppleEvents);
+      break;
+    }
+    case IDC_WINDOW_MUTE_SITE: {
+      TabStripModel* model = browser->tab_strip_model();
+      bool will_mute =
+          base::FeatureList::IsEnabled(features::kSoundContentSetting)
+              ? model->WillContextMenuMuteSites(model->active_index())
+              : model->WillContextMenuMute(model->active_index());
+      // Menu items may be validated during browser startup, before the
+      // TabStripModel has been populated.
+      result->new_toggle_state = !model->empty() && !will_mute;
+      break;
+    }
+    case IDC_WINDOW_PIN_TAB:
+      TabStripModel* model = browser->tab_strip_model();
+      result->new_toggle_state =
+          !model->empty() && !model->WillContextMenuPin(model->active_index());
+      break;
+  }
+}
+
 void BrowserFrameMac::InitNativeWidget(
     const views::Widget::InitParams& params) {
   views::NativeWidgetMac::InitNativeWidget(params);

ui/views/cocoa/bridged_native_widget_host_impl.h

@@ -268,6 +268,10 @@ class VIEWS_EXPORT BridgedNativeWidgetHostImpl
   bool GetWindowFrameTitlebarHeight(bool* override_titlebar_height,
                                     float* titlebar_height) override;
   void OnFocusWindowToolbar() override;
+  bool ValidateUserInterfaceItem(
+      int32_t command,
+      views_bridge_mac::mojom::ValidateUserInterfaceItemResultPtr* out_result)
+      override;
 
   // views_bridge_mac::mojom::BridgedNativeWidgetHost, synchronous callbacks:
   void DispatchKeyEventRemote(std::unique_ptr<ui::Event> event,
@@ -299,6 +303,9 @@ class VIEWS_EXPORT BridgedNativeWidgetHostImpl
   void GetAccessibilityTokens(const std::vector<uint8_t>& window_token,
                               const std::vector<uint8_t>& view_token,
                               GetAccessibilityTokensCallback callback) override;
+  void ValidateUserInterfaceItem(
+      int32_t command,
+      ValidateUserInterfaceItemCallback callback) override;
 
   // DialogObserver:
   void OnDialogChanged() override;

ui/views/cocoa/bridged_native_widget_host_impl.mm

@@ -892,6 +892,14 @@ void BridgedNativeWidgetHostImpl::OnFocusWindowToolbar() {
   native_widget_mac_->OnFocusWindowToolbar();
 }
 
+bool BridgedNativeWidgetHostImpl::ValidateUserInterfaceItem(
+    int32_t command,
+    views_bridge_mac::mojom::ValidateUserInterfaceItemResultPtr* out_result) {
+  *out_result = views_bridge_mac::mojom::ValidateUserInterfaceItemResult::New();
+  native_widget_mac_->ValidateUserInterfaceItem(command, out_result->get());
+  return true;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // BridgedNativeWidgetHostImpl,
 // views_bridge_mac::mojom::BridgedNativeWidgetHost synchronous callbacks:
@@ -1020,6 +1028,14 @@ void BridgedNativeWidgetHostImpl::GetAccessibilityTokens(
       getpid(), ui::RemoteAccessibility::GetTokenForLocalElement(element_id));
 }
 
+void BridgedNativeWidgetHostImpl::ValidateUserInterfaceItem(
+    int32_t command,
+    ValidateUserInterfaceItemCallback callback) {
+  views_bridge_mac::mojom::ValidateUserInterfaceItemResultPtr result;
+  ValidateUserInterfaceItem(command, &result);
+  std::move(callback).Run(std::move(result));
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // BridgedNativeWidgetHostImpl, DialogObserver:
 

ui/views/widget/native_widget_mac.h

@@ -19,6 +19,7 @@ namespace views_bridge_mac {
 namespace mojom {
 class BridgedNativeWidget;
 class CreateWindowParams;
+class ValidateUserInterfaceItemResult;
 }  // namespace mojom
 }  // namespace views_bridge_mac
 
@@ -63,6 +64,12 @@ class VIEWS_EXPORT NativeWidgetMac : public internal::NativeWidgetPrivate {
   // Handle "Move focus to the window toolbar" shortcut.
   virtual void OnFocusWindowToolbar() {}
 
+  // Allows subclasses to override the behavior for
+  // -[NSUserInterfaceValidations validateUserInterfaceItem].
+  virtual void ValidateUserInterfaceItem(
+      int32_t command,
+      views_bridge_mac::mojom::ValidateUserInterfaceItemResult* result) {}
+
   // internal::NativeWidgetPrivate:
   void InitNativeWidget(const Widget::InitParams& params) override;
   void OnWidgetInitDone() override;

ui/views_bridge_mac/bridged_native_widget_impl.h

@@ -66,8 +66,9 @@ class VIEWS_EXPORT BridgedNativeWidgetImpl
   static gfx::Size GetWindowSizeForClientSize(NSWindow* window,
                                               const gfx::Size& size);
 
-  // Retrieve a BridgedNativeWidgetImpl* from its id.
+  // Retrieve a BridgedNativeWidgetImpl* from its id or window.
   static BridgedNativeWidgetImpl* GetFromId(uint64_t bridged_native_widget_id);
+  static BridgedNativeWidgetImpl* GetFromNativeWindow(gfx::NativeWindow window);
 
   // Create an NSWindow for the specified parameters.
   static base::scoped_nsobject<NativeWidgetMacNSWindow> CreateNSWindow(

ui/views_bridge_mac/bridged_native_widget_impl.mm

@@ -244,6 +244,17 @@ BridgedNativeWidgetImpl* BridgedNativeWidgetImpl::GetFromId(
   return found->second;
 }
 
+// static
+BridgedNativeWidgetImpl* BridgedNativeWidgetImpl::GetFromNativeWindow(
+    gfx::NativeWindow native_window) {
+  NSWindow* window = native_window.GetNativeNSWindow();
+  if (NativeWidgetMacNSWindow* widget_window =
+          base::mac::ObjCCast<NativeWidgetMacNSWindow>(window)) {
+    return GetFromId([widget_window bridgedNativeWidgetId]);
+  }
+  return nullptr;
+}
+
 // static
 base::scoped_nsobject<NativeWidgetMacNSWindow>
 BridgedNativeWidgetImpl::CreateNSWindow(

ui/views_bridge_mac/mojo/bridged_native_widget_host.mojom

@@ -11,6 +11,29 @@ import "ui/events/mojo/event.mojom";
 import "ui/gfx/geometry/mojo/geometry.mojom";
 import "ui/gfx/mojo/ca_layer_params.mojom";
 
+struct ValidateUserInterfaceItemResult {
+  // Whether or not the specified sender should be enabled.
+  bool enable;
+
+  // If true, then the item should be disabled if there exists no key equivalent
+  // for the item.
+  bool disable_if_has_no_key_equivalent;
+
+  // The new title to set for the item (unset if the title is not to be
+  // changed).
+  mojo_base.mojom.String16? new_title;
+
+  // Whether or not to change the hidden state for the item, and the new hidden
+  // state to set.
+  bool set_hidden_state;
+  bool new_hidden_state;
+
+  // Whether or not to change the toggled state for the item, and the new toggle
+  // state to set.
+  bool set_toggle_state;
+  bool new_toggle_state;
+};
+
 // The interface through which a NativeWidgetMac may interact with an NSWindow
 // in another process.
 interface BridgedNativeWidgetHost {
@@ -175,4 +198,11 @@ interface BridgedNativeWidgetHost {
   GetAccessibilityTokens(array<uint8> window_token,
                          array<uint8> view_token) =>
       (int64 host_pid, array<uint8> element_token);
+
+  // Return the result for -[NSUserInterfaceValidations
+  // validateUserInterfaceItem] for a given command, along with any state for
+  // that item that should be updated.
+  [Sync]
+  ValidateUserInterfaceItem(int32 command) =>
+      (ValidateUserInterfaceItemResult result);
 };