[MacViews] Implemented text context menu

ui/base/cocoa/text_services_context_menu.h

+// Copyright 2016 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.
+
+#ifndef UI_BASE_COCOA_TEXT_SERVICES_CONTEXT_MENU_H_
+#define UI_BASE_COCOA_TEXT_SERVICES_CONTEXT_MENU_H_
+
+#include <ApplicationServices/ApplicationServices.h>
+#include <CoreAudio/CoreAudio.h>

[tapted, 2016/12/13 05:11:23]

move to the .mm?

[spqchan, 2016/12/15 23:29:00]

Done. 

+
+#include "base/macros.h"
+#include "ui/base/models/simple_menu_model.h"
+
+namespace ui {
+
+// Enum class that represents the writing directions on the BiDi submenu.
+enum class WritingDirection { DEFAULT, LTR, RTL };

[tapted, 2016/12/13 05:11:23]

I think ui:: is too broad a scope for something new like this. But can we use
i18n::TextDirection from base/i18n/rtl.h instead? This would make
ViewsTextServicesContextMenuMac::UpdateTextDirection() simpler too

[spqchan, 2016/12/15 23:29:00]

Done.

+
+// This class is used to append and handle text service items to the context
+// menu. These items are comprised of the Speech, Look Up, and BiDi items.
+class UI_BASE_EXPORT TextServicesContextMenu

[tapted, 2016/12/13 05:11:23]

nit: #include ui/base/ui_base_export.h

[spqchan, 2016/12/15 23:29:00]

Done.

+    : public ui::SimpleMenuModel::Delegate {

[tapted, 2016/12/13 05:11:23]

nit: no `ui::`

[spqchan, 2016/12/15 23:29:00]

Done.

+ public:
+  class UI_BASE_EXPORT Delegate {
+   public:
+    // Returns the selected text

[tapted, 2016/12/13 05:11:23]

nit: full stop at end

[spqchan, 2016/12/15 23:29:00]

Done.

+    virtual base::string16 GetSelectedText() const = 0;

[tapted, 2016/12/13 05:11:23]

#include string16

[spqchan, 2016/12/15 23:29:00]

Done.

+
+    // Handles the "Start Speaking" command. The delegate is expected to
+    // provide text and call SpeakText().
+    virtual void OnSpeakRequested() = 0;
+
+    // Returns true if the "Look Up" item is available on the menu.
+    virtual bool IsLookUpAvailable() const = 0;
+
+    // Handles the "Look Up" command.
+    virtual void LookUpInDictionary() = 0;
+
+    // Returns true if |direction| should be enabled in the BiDi submenu.
+    virtual bool IsWritingDirectionEnabled(
+        WritingDirection direction) const = 0;
+
+    // Returns true if |direction| should be checked in the BiDi submenu.
+    virtual bool IsWritingDirectionChecked(
+        WritingDirection direction) const = 0;
+
+    // Updates the text direction to |direction|.
+    virtual void UpdateTextDirection(WritingDirection direction) = 0;
+  };
+
+  explicit TextServicesContextMenu(Delegate* delegate);
+
+  // Methods for speaking.
+  static void SpeakText(const base::string16& text);
+  static void StopSpeaking();
+  static bool IsSpeaking();
+
+  // Methods for appending items to the context menu.

[tapted, 2016/12/13 05:11:23]

The comment should say what the difference between the two methods is, since
it's not obvious.

[spqchan, 2016/12/15 23:29:00]

Removed AppendPlatformEditableItems, I don't think there's any point with
keeping it

+  void AppendToContextMenu(ui::SimpleMenuModel* model);

[tapted, 2016/12/13 05:11:23]

nit: no ui::, (4 others, below)

[spqchan, 2016/12/15 23:29:00]

Done.

+  void AppendPlatformEditableItems(ui::SimpleMenuModel* model);
+
+  // Returns true if the command_id is supported by the text services menu.
+  bool IsTextServicesCommandId(int command_id) const;
+
+  // SimpleMenuModel::Delegate implementation.
+  void ExecuteCommand(int command_id, int event_flags) override;
+  bool IsCommandIdChecked(int command_id) const override;
+  bool IsCommandIdEnabled(int command_id) const override;
+  bool GetAcceleratorForCommandId(int command_id,
+                                  ui::Accelerator* accelerator) const override;
+
+ private:
+  // Returns the WritingDirection associated associated with the given
+  // BiDi |command_id|.
+  WritingDirection GetWritingDirectionFromCommandId(int command_id) const;

[tapted, 2016/12/13 05:11:23]

move to anonymous namespace in .mm

[spqchan, 2016/12/15 23:29:00]

Done.

+
+  // Model for the "Speech" submenu.
+  ui::SimpleMenuModel speech_submenu_model_;
+
+  // Model for the BiDi input submenu.
+  ui::SimpleMenuModel bidi_submenu_model_;
+
+  Delegate* delegate_;  // weak

[tapted, 2016/12/13 05:11:23]

nit: "// Weak."

[spqchan, 2016/12/15 23:29:00]

Done.

+
+  // The speech channel used for speaking.
+  static SpeechChannel speechChannel;

[tapted, 2016/12/13 05:11:23]

Can this move to an anonymous namespace in the .mm? 

I think we can get rid of #include <ApplicationServices/ApplicationServices.h>
then -- just have it in the .mm.

[spqchan, 2016/12/15 23:29:00]

Done.

+
+  DISALLOW_COPY_AND_ASSIGN(TextServicesContextMenu);
+};
+}

[tapted, 2016/12/13 05:11:22]

nit: blank line before + }  // namespace ui

[spqchan, 2016/12/15 23:29:00]

Done.

+
+#endif  // UI_BASE_COCOA_TEXT_SERVICES_CONTEXT_MENU_H_