[MacViews] Implemented text context menu

chrome/browser/ui/cocoa/renderer_context_menu/render_view_context_menu_mac.mm

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/cocoa/renderer_context_menu/render_view_context_menu_mac.h"
 
 #include <utility>
 
 #include "base/compiler_specific.h"
 #include "base/mac/mac_util.h"
 #import "base/mac/scoped_objc_class_swizzler.h"
 #import "base/mac/scoped_sending_event.h"
 #include "base/macros.h"
 #include "base/message_loop/message_loop.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/tracked_objects.h"
 #include "chrome/app/chrome_command_ids.h"
 #import "chrome/browser/mac/nsprocessinfo_additions.h"
 #include "chrome/grit/generated_resources.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/render_widget_host.h"
 #include "content/public/browser/render_widget_host_view.h"
 #import "ui/base/cocoa/menu_controller.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/strings/grit/ui_strings.h"
 
 using content::WebContents;
 
 namespace {
 
 IMP g_original_populatemenu_implementation = nullptr;
 
 // |g_filtered_entries_array| is only set during testing (see
 // +[ChromeSwizzleServicesMenuUpdater storeFilteredEntriesForTestingInArray:]).
 // Otherwise it remains nil.
@@ skipping 141 matching lines @@
 };
 
 // Obj-C bridge class that is the target of all items in the context menu.
 // Relies on the tag being set to the command id.
 
 RenderViewContextMenuMac::RenderViewContextMenuMac(
     content::RenderFrameHost* render_frame_host,
     const content::ContextMenuParams& params,
     NSView* parent_view)
     : RenderViewContextMenu(render_frame_host, params),
-      speech_submenu_model_(this),
-      bidi_submenu_model_(this),
-      parent_view_(parent_view) {
+      parent_view_(parent_view),
+      text_services_context_menu_(this) {
   std::unique_ptr<ToolkitDelegate> delegate(new ToolkitDelegateMac(this));
   set_toolkit_delegate(std::move(delegate));
 }
 
 RenderViewContextMenuMac::~RenderViewContextMenuMac() {
 }
 
 void RenderViewContextMenuMac::Show() {
   menu_controller_.reset(
       [[MenuController alloc] initWithModel:&menu_model_
@@ skipping 39 matching lines @@
     stopwatch.Start();
     // Show the menu.
     [NSMenu popUpContextMenu:[menu_controller_ menu]
                    withEvent:clickEvent
                      forView:parent_view_];
     stopwatch.Stop();
   }
 }
 
 void RenderViewContextMenuMac::ExecuteCommand(int command_id, int event_flags) {
-  switch (command_id) {
-    case IDC_CONTENT_CONTEXT_LOOK_UP:
-      LookUpInDictionary();
-      break;
-
-    case IDC_CONTENT_CONTEXT_SPEECH_START_SPEAKING:
-      StartSpeaking();
-      break;
-
-    case IDC_CONTENT_CONTEXT_SPEECH_STOP_SPEAKING:
-      StopSpeaking();
-      break;
-
-    case IDC_WRITING_DIRECTION_DEFAULT:
-      // WebKit's current behavior is for this menu item to always be disabled.
-      NOTREACHED();
-      break;
-
-    case IDC_WRITING_DIRECTION_RTL:
-    case IDC_WRITING_DIRECTION_LTR: {
-      content::RenderViewHost* view_host = GetRenderViewHost();
-      blink::WebTextDirection dir = blink::WebTextDirectionLeftToRight;
-      if (command_id == IDC_WRITING_DIRECTION_RTL)
-        dir = blink::WebTextDirectionRightToLeft;
-      view_host->GetWidget()->UpdateTextDirection(dir);
-      view_host->GetWidget()->NotifyTextDirection();
-      RenderViewContextMenu::RecordUsedItem(command_id);
-      break;
-    }
-
-    default:
-      RenderViewContextMenu::ExecuteCommand(command_id, event_flags);
-      break;
-  }
+  if (text_services_context_menu_.IsTextServicesCommandId(command_id))

[tapted, 2016/12/21 11:20:25]

I think this isn't needed now (text services commands should go straight to the
submenu?)

[spqchan, 2016/12/21 22:00:13]

Done.

+    text_services_context_menu_.ExecuteCommand(command_id, event_flags);
+  else if (command_id == IDC_CONTENT_CONTEXT_LOOK_UP)
+    LookUpInDictionary();
+  else
+    RenderViewContextMenu::ExecuteCommand(command_id, event_flags);
 }
 
 bool RenderViewContextMenuMac::IsCommandIdChecked(int command_id) const {
-  switch (command_id) {
-    case IDC_WRITING_DIRECTION_DEFAULT:
-      return params_.writing_direction_default &
-          blink::WebContextMenuData::CheckableMenuItemChecked;
-    case IDC_WRITING_DIRECTION_RTL:
-      return params_.writing_direction_right_to_left &
-          blink::WebContextMenuData::CheckableMenuItemChecked;
-    case IDC_WRITING_DIRECTION_LTR:
-      return params_.writing_direction_left_to_right &
-          blink::WebContextMenuData::CheckableMenuItemChecked;
-
-    default:
-      return RenderViewContextMenu::IsCommandIdChecked(command_id);
-  }
+  if (text_services_context_menu_.IsTextServicesCommandId(command_id))

[tapted, 2016/12/21 11:20:26]

same here, and below

[spqchan, 2016/12/21 22:00:13]

Done.

+    return text_services_context_menu_.IsCommandIdChecked(command_id);
+  else if (command_id == IDC_CONTENT_CONTEXT_LOOK_UP)
+    return false;
+  else

[tapted, 2016/12/21 11:20:26]

nit: "no else after return" -
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...
. same below

[spqchan, 2016/12/21 22:00:13]

Done.

+    return RenderViewContextMenu::IsCommandIdChecked(command_id);
 }
 
 bool RenderViewContextMenuMac::IsCommandIdEnabled(int command_id) const {
-  switch (command_id) {
-    case IDC_CONTENT_CONTEXT_LOOK_UP:
-      // This is OK because the menu is not shown when it isn't
-      // appropriate.
-      return true;
-
-    case IDC_CONTENT_CONTEXT_SPEECH_START_SPEAKING:
-      // This is OK because the menu is not shown when it isn't
-      // appropriate.
-      return true;
-
-    case IDC_CONTENT_CONTEXT_SPEECH_STOP_SPEAKING: {
-      content::RenderWidgetHostView* view =
-          GetRenderViewHost()->GetWidget()->GetView();
-      return view && view->IsSpeaking();
-    }
-
-    case IDC_WRITING_DIRECTION_DEFAULT:  // Provided to match OS defaults.
-      return params_.writing_direction_default &
-          blink::WebContextMenuData::CheckableMenuItemEnabled;
-    case IDC_WRITING_DIRECTION_RTL:
-      return params_.writing_direction_right_to_left &
-          blink::WebContextMenuData::CheckableMenuItemEnabled;
-    case IDC_WRITING_DIRECTION_LTR:
-      return params_.writing_direction_left_to_right &
-          blink::WebContextMenuData::CheckableMenuItemEnabled;
-
-    default:
-      return RenderViewContextMenu::IsCommandIdEnabled(command_id);
-  }
-}
-
-void RenderViewContextMenuMac::AppendPlatformEditableItems() {
-  // OS X provides a contextual menu to set writing direction for BiDi
-  // languages.
-  // This functionality is exposed as a keyboard shortcut on Windows & Linux.
-  AppendBidiSubMenu();
+  if (text_services_context_menu_.IsTextServicesCommandId(command_id))
+    return text_services_context_menu_.IsCommandIdEnabled(command_id);
+  else if (command_id == IDC_CONTENT_CONTEXT_LOOK_UP)
+    return true;
+  else
+    return RenderViewContextMenu::IsCommandIdEnabled(command_id);
 }
 
 void RenderViewContextMenuMac::InitToolkitMenu() {
-  if (params_.selection_text.empty())
-    return;
-
   if (params_.link_url.is_empty()) {
     // In case the user has selected a word that triggers spelling suggestions,
     // show the dictionary lookup under the group that contains the command to
     // “Add to Dictionary.”
-    int index = menu_model_.GetIndexOfCommandId(
-        IDC_SPELLCHECK_ADD_TO_DICTIONARY);
+    int index =
+        menu_model_.GetIndexOfCommandId(IDC_SPELLCHECK_ADD_TO_DICTIONARY);
     if (index < 0) {
       index = 0;
     } else {
-      while (menu_model_.GetTypeAt(index) != ui::MenuModel::TYPE_SEPARATOR) {
+      while (menu_model_.GetTypeAt(index) != ui::MenuModel::TYPE_SEPARATOR)
         index++;
-      }
-      index += 1; // Place it below the separator.
+
+      index += 1;  // Place it below the separator.
     }
 
     base::string16 printable_selection_text = PrintableSelectionText();
     EscapeAmpersands(&printable_selection_text);
     menu_model_.InsertItemAt(
-        index++,
-        IDC_CONTENT_CONTEXT_LOOK_UP,
+        index++, IDC_CONTENT_CONTEXT_LOOK_UP,
         l10n_util::GetStringFUTF16(IDS_CONTENT_CONTEXT_LOOK_UP,
                                    printable_selection_text));
-    menu_model_.InsertSeparatorAt(index++, ui::NORMAL_SEPARATOR);
+    menu_model_.InsertSeparatorAt(index + 1, ui::NORMAL_SEPARATOR);

[tapted, 2016/12/21 11:20:26]

I don't think this will be the same due to postfix increment - just |index|?

[spqchan, 2016/12/21 22:00:12]

Done.

   }
 
-  content::RenderWidgetHostView* view =
-      GetRenderViewHost()->GetWidget()->GetView();
-  if (view && view->SupportsSpeech()) {
-    menu_model_.AddSeparator(ui::NORMAL_SEPARATOR);
-    speech_submenu_model_.AddItemWithStringId(
-        IDC_CONTENT_CONTEXT_SPEECH_START_SPEAKING,
-        IDS_SPEECH_START_SPEAKING_MAC);
-    speech_submenu_model_.AddItemWithStringId(
-        IDC_CONTENT_CONTEXT_SPEECH_STOP_SPEAKING,
-        IDS_SPEECH_STOP_SPEAKING_MAC);
-    menu_model_.AddSubMenu(
-        IDC_CONTENT_CONTEXT_SPEECH_MENU,
-        l10n_util::GetStringUTF16(IDS_SPEECH_MAC),
-        &speech_submenu_model_);
-  }
+  text_services_context_menu_.AppendToContextMenu(&menu_model_);
 }
 
 void RenderViewContextMenuMac::CancelToolkitMenu() {
   [menu_controller_ cancel];
 }
 
 void RenderViewContextMenuMac::UpdateToolkitMenuItem(
     int command_id,
     bool enabled,
     bool hidden,
     const base::string16& title) {
   NSMenuItem* item = GetMenuItemByID(&menu_model_, [menu_controller_ menu],
                                      command_id);
   if (!item)
     return;
 
   // Update the returned NSMenuItem directly so we can update it immediately.
   [item setEnabled:enabled];
   [item setTitle:base::SysUTF16ToNSString(title)];
   [item setHidden:hidden];
   [[item menu] itemChanged:item];
 }
 
-void RenderViewContextMenuMac::AppendBidiSubMenu() {
-  bidi_submenu_model_.AddCheckItem(IDC_WRITING_DIRECTION_DEFAULT,
-      l10n_util::GetStringUTF16(IDS_CONTENT_CONTEXT_WRITING_DIRECTION_DEFAULT));
-  bidi_submenu_model_.AddCheckItem(IDC_WRITING_DIRECTION_LTR,
-      l10n_util::GetStringUTF16(IDS_CONTENT_CONTEXT_WRITING_DIRECTION_LTR));
-  bidi_submenu_model_.AddCheckItem(IDC_WRITING_DIRECTION_RTL,
-      l10n_util::GetStringUTF16(IDS_CONTENT_CONTEXT_WRITING_DIRECTION_RTL));
-
-  menu_model_.AddSubMenu(
-      IDC_WRITING_DIRECTION_MENU,
-      l10n_util::GetStringUTF16(IDS_CONTENT_CONTEXT_WRITING_DIRECTION_MENU),
-      &bidi_submenu_model_);
-}
-
 void RenderViewContextMenuMac::LookUpInDictionary() {
   content::RenderWidgetHostView* view =
       GetRenderViewHost()->GetWidget()->GetView();
   if (view)
     view->ShowDefinitionForSelection();
 }
 
-void RenderViewContextMenuMac::StartSpeaking() {
+base::string16 RenderViewContextMenuMac::GetSelectedText() const {
+  return params_.selection_text;
+}
+
+void RenderViewContextMenuMac::OnSpeakRequested() {
   content::RenderWidgetHostView* view =
       GetRenderViewHost()->GetWidget()->GetView();
   if (view)
     view->SpeakSelection();
 }
 
-void RenderViewContextMenuMac::StopSpeaking() {
-  content::RenderWidgetHostView* view =
-      GetRenderViewHost()->GetWidget()->GetView();
-  if (view)
-    view->StopSpeaking();
+bool RenderViewContextMenuMac::IsTextDirectionEnabled(

[tapted, 2016/12/21 11:20:25]

nit: method order should match the header

[spqchan, 2016/12/21 22:00:13]

Done.

+    base::i18n::TextDirection direction) const {
+  switch (direction) {
+    case base::i18n::TextDirection::UNKNOWN_DIRECTION:
+      return params_.writing_direction_default &

[tapted, 2016/12/21 11:20:26]

nit: I think this needs a private helper method:

int RenderViewContextMenuMac::BiDiParamsForDirection(base::i18n::TextDirection
direction) {
  switch (direction):
    case base::i18n::TextDirection::UNKNOWN_DIRECTION:
      return params_.writing_direction_default;
  etc.

Then just do

return BiDiParamsForDirection(direction) &
    blink::WebContextMenuData::CheckableMenuItemEnabled;

here and

return BiDiParamsForDirection(direction) &
    blink::WebContextMenuData::CheckableMenuItemChecked;

below

[spqchan, 2016/12/21 22:00:13]

Done.

+             blink::WebContextMenuData::CheckableMenuItemEnabled;
+    case base::i18n::TextDirection::RIGHT_TO_LEFT:
+      return params_.writing_direction_right_to_left &
+             blink::WebContextMenuData::CheckableMenuItemEnabled;
+    case base::i18n::TextDirection::LEFT_TO_RIGHT:
+      return params_.writing_direction_left_to_right &
+             blink::WebContextMenuData::CheckableMenuItemEnabled;
+    case base::i18n::TextDirection::TEXT_DIRECTION_NUM_DIRECTIONS:
+      NOTREACHED();
+      return false;
+  }
 }
+
+bool RenderViewContextMenuMac::IsTextDirectionChecked(
+    base::i18n::TextDirection direction) const {
+  switch (direction) {
+    case base::i18n::TextDirection::UNKNOWN_DIRECTION:
+      return params_.writing_direction_default &
+             blink::WebContextMenuData::CheckableMenuItemChecked;
+    case base::i18n::TextDirection::RIGHT_TO_LEFT:
+      return params_.writing_direction_right_to_left &
+             blink::WebContextMenuData::CheckableMenuItemChecked;
+    case base::i18n::TextDirection::LEFT_TO_RIGHT:
+      return params_.writing_direction_left_to_right &
+             blink::WebContextMenuData::CheckableMenuItemChecked;
+    case base::i18n::TextDirection::TEXT_DIRECTION_NUM_DIRECTIONS:
+      NOTREACHED();
+      return false;
+  }
+}
+
+void RenderViewContextMenuMac::UpdateTextDirection(
+    base::i18n::TextDirection direction) {
+  DCHECK_NE(direction, base::i18n::TextDirection::UNKNOWN_DIRECTION);
+
+  content::RenderViewHost* view_host = GetRenderViewHost();
+  blink::WebTextDirection dir = blink::WebTextDirectionLeftToRight;
+  int command_id = IDC_WRITING_DIRECTION_LTR;
+  if (direction == base::i18n::TextDirection::RIGHT_TO_LEFT) {
+    dir = blink::WebTextDirectionRightToLeft;
+    command_id = IDC_WRITING_DIRECTION_RTL;
+  }
+
+  view_host->GetWidget()->UpdateTextDirection(dir);
+  view_host->GetWidget()->NotifyTextDirection();
+
+  RenderViewContextMenu::RecordUsedItem(command_id);
+}