[MacViews] Implemented text context menu

ui/views/controls/views_text_services_context_menu_mac.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_VIEWS_CONTROLS_VIEWS_TEXT_SERVICES_CONTEXT_MENU_MAC_H_

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

Is this header required?

(can all of ViewsTextServicesContextMenuMac be defined in an anonymous
namespace?)

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

Done.

+#define UI_VIEWS_CONTROLS_VIEWS_TEXT_SERVICES_CONTEXT_MENU_MAC_H_
+
+#include "ui/base/cocoa/text_services_context_menu.h"
+#include "ui/views/controls/views_text_services_context_menu.h"
+
+namespace ui {
+class SimpleMenuModel;
+class TextInputClient;
+}
+
+namespace views {
+
+class View;
+class WordLookupClient;
+
+// This class serves as a bridge to TextServicesContextMenu to add and handle
+// text service items in the context menu. The items include Speech, Look Up
+// and BiDi.
+class ViewsTextServicesContextMenuMac
+    : public ViewsTextServicesContextMenu,
+      public ui::TextServicesContextMenu::Delegate {
+ public:
+  explicit ViewsTextServicesContextMenuMac(ui::TextInputClient* text_client,

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

nit: explicit not required

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

Done.

+                                           WordLookupClient* lookup_client,
+                                           ui::SimpleMenuModel* menu,
+                                           View* view);
+  ~ViewsTextServicesContextMenuMac() override;
+
+  // ViewsTextContextMenu:

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

ViewsTextServicesContextMenu:

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

Done.

+  bool ShouldUpdateMenu() const override;
+  bool IsTextServicesCommandId(int command_id) const override;
+  void ExecuteCommand(int command_id, int event_flags) override;
+  bool IsCommandIdChecked(int command_id) const override;
+  bool IsCommandIdEnabled(int command_id) const override;
+
+  // TextServicesContextMenu::Delegate

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

nit: colon at end

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

Done.

+  base::string16 GetSelectedText() const override;
+  void OnSpeakRequested() override;
+  bool IsLookUpAvailable() const override;
+  void LookUpInDictionary() override;
+  bool IsWritingDirectionEnabled(ui::WritingDirection direction) const override;
+  bool IsWritingDirectionChecked(ui::WritingDirection direction) const override;
+  void UpdateTextDirection(ui::WritingDirection direction) override;
+
+ private:
+  // Used to retrieve the selected text. Weak.
+  ui::TextInputClient* text_client_;
+
+  // Used to retrieve the decorated text for Look Up. Weak.
+  WordLookupClient* lookup_client_;

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

I think text_client_ and lookup_client_ will both be a views::TextField? I think
we can just use views::Textfield here. Then the ownership is clearer, since it's
the Textfield that will own |this|. i.e. we can say

views::TextField* client_;  // Weak. Owns |this|.

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

Done.

+
+  // Appends and handles the text service menu.
+  ui::TextServicesContextMenu menu_;
+
+  // The view associated with the menu. Weak.
+  View* view_;

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

ah, this is probably the TextField too?

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

Done.

+
+  // The selected text that appears in the context menu. This is used to
+  // determine if the context menu needs to updated.

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

insert "be" -> "needs to be updated"

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

Done.

+  base::string16 selection_text_;
+
+  DISALLOW_COPY_AND_ASSIGN(ViewsTextServicesContextMenuMac);
+};
+
+}  // namespace views
+
+#endif  // UI_VIEWS_CONTROLS_VIEWS_TEXT_SERVICES_CONTEXT_MENU_MAC_H_