Views::Textfield: Decouple handling of menu and text editing commands.

ui/views/controls/textfield/textfield.cc

 // Copyright (c) 2012 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 "ui/views/controls/textfield/textfield.h"
 
 #include <string>
 #include <utility>
 
 #include "base/trace_event/trace_event.h"
@@ skipping 230 matching lines @@
   }
   return kNoCommand;
 }
 #endif
 
 const gfx::FontList& GetDefaultFontList() {
   ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
   return rb.GetFontListWithDelta(ui::kLabelFontSizeDelta);
 }
 
+// Returns true if |command_id| is a menu action installed by this textfield.
+// Keep in sync with UpdateContextMenu.
+bool IsMenuCommand(int command_id) {
+  switch (command_id) {
+    case IDS_APP_UNDO:
+    case IDS_APP_CUT:
+    case IDS_APP_COPY:
+    case IDS_APP_PASTE:
+    case IDS_APP_DELETE:
+    case IDS_APP_SELECT_ALL:
+      return true;
+  }
+  return false;
+}
+
 }  // namespace
 
 // static
 const char Textfield::kViewClassName[] = "Textfield";
 const int Textfield::kTextPadding = 3;
 
 // static
 size_t Textfield::GetCaretBlinkMs() {
   static const size_t default_value = 500;
 #if defined(OS_WIN)
@@ skipping 302 matching lines @@
 }
 
 void Textfield::ClearEditHistory() {
   model_->ClearEditHistory();
 }
 
 void Textfield::SetAccessibleName(const base::string16& name) {
   accessible_name_ = name;
 }
 
-void Textfield::ExecuteCommand(int command_id) {
-  ExecuteCommand(command_id, ui::EF_NONE);
-}
-
 bool Textfield::HasTextBeingDragged() {
   return initiating_drag_;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // Textfield, View overrides:
 
 gfx::Insets Textfield::GetInsets() const {
   gfx::Insets insets = View::GetInsets();
   insets += gfx::Insets(kTextPadding, kTextPadding, kTextPadding, kTextPadding);
@@ skipping 123 matching lines @@
     return handled;
 
 #if defined(OS_LINUX) && !defined(OS_CHROMEOS)
   ui::TextEditKeyBindingsDelegateAuraLinux* delegate =
       ui::GetTextEditKeyBindingsDelegate();
   std::vector<ui::TextEditCommandAuraLinux> commands;
   if (!handled && delegate && delegate->MatchEvent(event, &commands)) {
     const bool rtl = GetTextDirection() == base::i18n::RIGHT_TO_LEFT;
     for (size_t i = 0; i < commands.size(); ++i) {
       const int command = GetViewsCommand(commands[i], rtl);
-      if (IsCommandIdEnabled(command)) {
-        ExecuteCommand(command);
+      if (IsEditCommandEnabled(command)) {
+        ExecuteEditCommand(command);
         handled = true;
       }
     }
     return handled;
   }
 #endif
 
   if (edit_command == kNoCommand)
     edit_command = GetCommandForKeyEvent(event);
 
-  if (!handled && IsCommandIdEnabled(edit_command)) {
-    ExecuteCommand(edit_command);
+  if (!handled && IsEditCommandEnabled(edit_command)) {
+    ExecuteEditCommand(edit_command);
     handled = true;
   }
   return handled;
 }
 
 void Textfield::OnGestureEvent(ui::GestureEvent* event) {
   switch (event->type()) {
     case ui::ET_GESTURE_TAP_DOWN:
       RequestFocus();
       ShowImeIfNeeded();
@@ skipping 83 matching lines @@
       break;
     default:
       return;
   }
 }
 
 // This function is called by BrowserView to execute clipboard commands.
 bool Textfield::AcceleratorPressed(const ui::Accelerator& accelerator) {
   ui::KeyEvent event(accelerator.type(), accelerator.key_code(),
                      accelerator.modifiers());
-  ExecuteCommand(GetCommandForKeyEvent(event));
+  ExecuteEditCommand(GetCommandForKeyEvent(event));
   return true;
 }
 
 bool Textfield::CanHandleAccelerators() const {
   return GetRenderText()->focused() && View::CanHandleAccelerators();
 }
 
 void Textfield::AboutToRequestFocusFromTabTraversal(bool reverse) {
   SelectAll(false);
 }
 
 bool Textfield::SkipDefaultKeyEventProcessing(const ui::KeyEvent& event) {
 #if defined(OS_LINUX) && !defined(OS_CHROMEOS)
   // Skip any accelerator handling that conflicts with custom keybindings.
   ui::TextEditKeyBindingsDelegateAuraLinux* delegate =
       ui::GetTextEditKeyBindingsDelegate();
   std::vector<ui::TextEditCommandAuraLinux> commands;
   if (delegate && delegate->MatchEvent(event, &commands)) {
     const bool rtl = GetTextDirection() == base::i18n::RIGHT_TO_LEFT;
     for (size_t i = 0; i < commands.size(); ++i)
-      if (IsCommandIdEnabled(GetViewsCommand(commands[i], rtl)))
+      if (IsEditCommandEnabled(GetViewsCommand(commands[i], rtl)))
         return true;
   }
 #endif
 
   // Skip backspace accelerator handling; editable textfields handle this key.
   // Also skip processing Windows [Alt]+<num-pad digit> Unicode alt-codes.
   const bool is_backspace = event.key_code() == ui::VKEY_BACK;
   return (is_backspace && !read_only()) || event.IsUnicodeKeyCode();
 }
 
@@ skipping 352 matching lines @@
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // Textfield, ui::SimpleMenuModel::Delegate overrides:
 
 bool Textfield::IsCommandIdChecked(int command_id) const {
   return true;
 }
 
 bool Textfield::IsCommandIdEnabled(int command_id) const {
-  base::string16 result;
-  bool editable = !read_only();
-  bool readable = text_input_type_ != ui::TEXT_INPUT_TYPE_PASSWORD;
-  switch (command_id) {
-    case IDS_APP_UNDO:
-      return editable && model_->CanUndo();
-    case IDS_APP_REDO:
-      return editable && model_->CanRedo();
-    case IDS_APP_CUT:
-      return editable && readable && model_->HasSelection();
-    case IDS_APP_COPY:
-      return readable && model_->HasSelection();
-    case IDS_APP_PASTE:
-      ui::Clipboard::GetForCurrentThread()->ReadText(
-          ui::CLIPBOARD_TYPE_COPY_PASTE, &result);
-      return editable && !result.empty();
-    case IDS_APP_DELETE:
-      return editable && model_->HasSelection();
-    case IDS_APP_SELECT_ALL:
-      return !text().empty();
-    case IDS_DELETE_FORWARD:
-    case IDS_DELETE_BACKWARD:
-    case IDS_DELETE_TO_BEGINNING_OF_LINE:
-    case IDS_DELETE_TO_END_OF_LINE:
-    case IDS_DELETE_WORD_BACKWARD:
-    case IDS_DELETE_WORD_FORWARD:
-      return editable;
-    case IDS_MOVE_LEFT:
-    case IDS_MOVE_LEFT_AND_MODIFY_SELECTION:
-    case IDS_MOVE_RIGHT:
-    case IDS_MOVE_RIGHT_AND_MODIFY_SELECTION:
-    case IDS_MOVE_WORD_LEFT:
-    case IDS_MOVE_WORD_LEFT_AND_MODIFY_SELECTION:
-    case IDS_MOVE_WORD_RIGHT:
-    case IDS_MOVE_WORD_RIGHT_AND_MODIFY_SELECTION:
-    case IDS_MOVE_TO_BEGINNING_OF_LINE:
-    case IDS_MOVE_TO_BEGINNING_OF_LINE_AND_MODIFY_SELECTION:
-    case IDS_MOVE_TO_END_OF_LINE:
-    case IDS_MOVE_TO_END_OF_LINE_AND_MODIFY_SELECTION:
-      return true;
-    default:
-      return false;
-  }
+  return IsMenuCommand(command_id) &&
+         Textfield::IsEditCommandEnabled(command_id);
 }
 
 bool Textfield::GetAcceleratorForCommandId(int command_id,
                                            ui::Accelerator* accelerator) {
   switch (command_id) {
     case IDS_APP_UNDO:
       *accelerator = ui::Accelerator(ui::VKEY_Z, ui::EF_CONTROL_DOWN);
       return true;
 
     case IDS_APP_CUT:
@@ skipping 12 matching lines @@
       *accelerator = ui::Accelerator(ui::VKEY_A, ui::EF_CONTROL_DOWN);
       return true;
 
     default:
       return false;
   }
 }
 
 void Textfield::ExecuteCommand(int command_id, int event_flags) {
   DestroyTouchSelection();
-
-  // Some codepaths may bypass GetCommandForKeyEvent, so any selection-dependent
-  // modifications of the command should happen here.
-  if (HasSelection()) {
-    switch (command_id) {
-      case IDS_DELETE_WORD_BACKWARD:
-      case IDS_DELETE_TO_BEGINNING_OF_LINE:
-        command_id = IDS_DELETE_BACKWARD;
-        break;
-      case IDS_DELETE_WORD_FORWARD:
-      case IDS_DELETE_TO_END_OF_LINE:
-        command_id = IDS_DELETE_FORWARD;
-        break;
-    }
-  }
-
-  if (!IsCommandIdEnabled(command_id))
+  if (!Textfield::IsCommandIdEnabled(command_id))

[karandeepb, 2016/06/09 10:47:10]

This was a virtual call earlier. However doing Textfield::IsCommandIdEnabled
looks more correct to me. Thoughts? 

[msw, 2016/06/10 00:07:50]

Yeah, this is probably more correct; subclasses should override
IsCommandIdEnabled *and* ExecuteCommand to enable additional commands, etc.

[msw, 2016/06/10 00:07:50]

optional nit: invert this check and inline the ExecuteEditCommand call.

[karandeepb, 2016/06/10 05:52:36]

Done.

     return;
-
-  bool text_changed = false;
-  bool cursor_changed = false;
-  bool rtl = GetTextDirection() == base::i18n::RIGHT_TO_LEFT;
-  gfx::VisualCursorDirection begin = rtl ? gfx::CURSOR_RIGHT : gfx::CURSOR_LEFT;
-  gfx::VisualCursorDirection end = rtl ? gfx::CURSOR_LEFT : gfx::CURSOR_RIGHT;
-  gfx::SelectionModel selection_model = GetSelectionModel();
-
-  OnBeforeUserAction();
-  switch (command_id) {
-    case IDS_APP_UNDO:
-      text_changed = cursor_changed = model_->Undo();
-      break;
-    case IDS_APP_REDO:
-      text_changed = cursor_changed = model_->Redo();
-      break;
-    case IDS_APP_CUT:
-      text_changed = cursor_changed = Cut();
-      break;
-    case IDS_APP_COPY:
-      Copy();
-      break;
-    case IDS_APP_PASTE:
-      text_changed = cursor_changed = Paste();
-      break;
-    case IDS_APP_DELETE:
-      text_changed = cursor_changed = model_->Delete();
-      break;
-    case IDS_APP_SELECT_ALL:
-      SelectAll(false);
-      break;
-    case IDS_DELETE_BACKWARD:
-      text_changed = cursor_changed = model_->Backspace();
-      break;
-    case IDS_DELETE_FORWARD:
-      text_changed = cursor_changed = model_->Delete();
-      break;
-    case IDS_DELETE_TO_END_OF_LINE:
-      model_->MoveCursor(gfx::LINE_BREAK, end, true);
-      text_changed = cursor_changed = model_->Delete();
-      break;
-    case IDS_DELETE_TO_BEGINNING_OF_LINE:
-      model_->MoveCursor(gfx::LINE_BREAK, begin, true);
-      text_changed = cursor_changed = model_->Backspace();
-      break;
-    case IDS_DELETE_WORD_BACKWARD:
-      model_->MoveCursor(gfx::WORD_BREAK, begin, true);
-      text_changed = cursor_changed = model_->Backspace();
-      break;
-    case IDS_DELETE_WORD_FORWARD:
-      model_->MoveCursor(gfx::WORD_BREAK, end, true);
-      text_changed = cursor_changed = model_->Delete();
-      break;
-    case IDS_MOVE_LEFT:
-      model_->MoveCursor(gfx::CHARACTER_BREAK, gfx::CURSOR_LEFT, false);
-      break;
-    case IDS_MOVE_LEFT_AND_MODIFY_SELECTION:
-      model_->MoveCursor(gfx::CHARACTER_BREAK, gfx::CURSOR_LEFT, true);
-      break;
-    case IDS_MOVE_RIGHT:
-      model_->MoveCursor(gfx::CHARACTER_BREAK, gfx::CURSOR_RIGHT, false);
-      break;
-    case IDS_MOVE_RIGHT_AND_MODIFY_SELECTION:
-      model_->MoveCursor(gfx::CHARACTER_BREAK, gfx::CURSOR_RIGHT, true);
-      break;
-    case IDS_MOVE_WORD_LEFT:
-      model_->MoveCursor(gfx::WORD_BREAK, gfx::CURSOR_LEFT, false);
-      break;
-    case IDS_MOVE_WORD_LEFT_AND_MODIFY_SELECTION:
-      model_->MoveCursor(gfx::WORD_BREAK, gfx::CURSOR_LEFT, true);
-      break;
-    case IDS_MOVE_WORD_RIGHT:
-      model_->MoveCursor(gfx::WORD_BREAK, gfx::CURSOR_RIGHT, false);
-      break;
-    case IDS_MOVE_WORD_RIGHT_AND_MODIFY_SELECTION:
-      model_->MoveCursor(gfx::WORD_BREAK, gfx::CURSOR_RIGHT, true);
-      break;
-    case IDS_MOVE_TO_BEGINNING_OF_LINE:
-      model_->MoveCursor(gfx::LINE_BREAK, begin, false);
-      break;
-    case IDS_MOVE_TO_BEGINNING_OF_LINE_AND_MODIFY_SELECTION:
-      model_->MoveCursor(gfx::LINE_BREAK, begin, true);
-      break;
-    case IDS_MOVE_TO_END_OF_LINE:
-      model_->MoveCursor(gfx::LINE_BREAK, end, false);
-      break;
-    case IDS_MOVE_TO_END_OF_LINE_AND_MODIFY_SELECTION:
-      model_->MoveCursor(gfx::LINE_BREAK, end, true);
-      break;
-    default:
-      NOTREACHED();
-      break;
-  }
-
-  cursor_changed |= GetSelectionModel() != selection_model;
-  if (cursor_changed)
-    UpdateSelectionClipboard();
-  UpdateAfterChange(text_changed, cursor_changed);
-  OnAfterUserAction();
+  Textfield::ExecuteEditCommand(command_id);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // Textfield, ui::TextInputClient overrides:
 
 void Textfield::SetCompositionText(const ui::CompositionText& composition) {
   if (GetTextInputType() == ui::TEXT_INPUT_TYPE_NONE)
     return;
 
   OnBeforeUserAction();
@@ skipping 202 matching lines @@
   range.set_start(range.start() - before);
   range.set_end(range.end() + after);
   gfx::Range text_range;
   if (GetTextRange(&text_range) && text_range.Contains(range))
     DeleteRange(range);
 }
 
 void Textfield::EnsureCaretInRect(const gfx::Rect& rect) {}
 
 bool Textfield::IsEditCommandEnabled(int command_id) const {
-  return IsCommandIdEnabled(command_id);
+  base::string16 result;
+  bool editable = !read_only();
+  bool readable = text_input_type_ != ui::TEXT_INPUT_TYPE_PASSWORD;
+  switch (command_id) {
+    case IDS_APP_UNDO:
+      return editable && model_->CanUndo();
+    case IDS_APP_REDO:
+      return editable && model_->CanRedo();
+    case IDS_APP_CUT:
+      return editable && readable && model_->HasSelection();
+    case IDS_APP_COPY:
+      return readable && model_->HasSelection();
+    case IDS_APP_PASTE:
+      ui::Clipboard::GetForCurrentThread()->ReadText(
+          ui::CLIPBOARD_TYPE_COPY_PASTE, &result);
+      return editable && !result.empty();
+    case IDS_APP_DELETE:
+      return editable && model_->HasSelection();
+    case IDS_APP_SELECT_ALL:
+      return !text().empty();
+    case IDS_DELETE_FORWARD:
+    case IDS_DELETE_BACKWARD:
+    case IDS_DELETE_TO_BEGINNING_OF_LINE:
+    case IDS_DELETE_TO_END_OF_LINE:
+    case IDS_DELETE_WORD_BACKWARD:
+    case IDS_DELETE_WORD_FORWARD:
+      return editable;
+    case IDS_MOVE_LEFT:
+    case IDS_MOVE_LEFT_AND_MODIFY_SELECTION:
+    case IDS_MOVE_RIGHT:
+    case IDS_MOVE_RIGHT_AND_MODIFY_SELECTION:
+    case IDS_MOVE_WORD_LEFT:
+    case IDS_MOVE_WORD_LEFT_AND_MODIFY_SELECTION:
+    case IDS_MOVE_WORD_RIGHT:
+    case IDS_MOVE_WORD_RIGHT_AND_MODIFY_SELECTION:
+    case IDS_MOVE_TO_BEGINNING_OF_LINE:
+    case IDS_MOVE_TO_BEGINNING_OF_LINE_AND_MODIFY_SELECTION:
+    case IDS_MOVE_TO_END_OF_LINE:
+    case IDS_MOVE_TO_END_OF_LINE_AND_MODIFY_SELECTION:
+      return true;
+    default:
+      return false;
+  }
 }
 
 void Textfield::SetEditCommandForNextKeyEvent(int command_id) {
   DCHECK_EQ(kNoCommand, scheduled_edit_command_);
   scheduled_edit_command_ = command_id;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // Textfield, protected:
 
@@ skipping 14 matching lines @@
   return model_->render_text();
 }
 
 base::string16 Textfield::GetSelectionClipboardText() const {
   base::string16 selection_clipboard_text;
   ui::Clipboard::GetForCurrentThread()->ReadText(
       ui::CLIPBOARD_TYPE_SELECTION, &selection_clipboard_text);
   return selection_clipboard_text;
 }
 
+void Textfield::ExecuteEditCommand(int command_id) {
+  DestroyTouchSelection();
+
+  // Some codepaths may bypass GetCommandForKeyEvent, so any selection-dependent
+  // modifications of the command should happen here.
+  if (HasSelection()) {
+    switch (command_id) {
+      case IDS_DELETE_WORD_BACKWARD:
+      case IDS_DELETE_TO_BEGINNING_OF_LINE:
+        command_id = IDS_DELETE_BACKWARD;
+        break;
+      case IDS_DELETE_WORD_FORWARD:
+      case IDS_DELETE_TO_END_OF_LINE:
+        command_id = IDS_DELETE_FORWARD;
+        break;
+    }
+  }
+
+  // We only execute the commands enabled in Textfield::IsEditCommandEnabled
+  // below. Hence don't do a virtual IsEditCommandEnabled call.
+  if (!Textfield::IsEditCommandEnabled(command_id))
+    return;
+
+  bool text_changed = false;
+  bool cursor_changed = false;
+  bool rtl = GetTextDirection() == base::i18n::RIGHT_TO_LEFT;
+  gfx::VisualCursorDirection begin = rtl ? gfx::CURSOR_RIGHT : gfx::CURSOR_LEFT;
+  gfx::VisualCursorDirection end = rtl ? gfx::CURSOR_LEFT : gfx::CURSOR_RIGHT;
+  gfx::SelectionModel selection_model = GetSelectionModel();
+
+  OnBeforeUserAction();
+  switch (command_id) {
+    case IDS_APP_UNDO:
+      text_changed = cursor_changed = model_->Undo();
+      break;
+    case IDS_APP_REDO:
+      text_changed = cursor_changed = model_->Redo();
+      break;
+    case IDS_APP_CUT:
+      text_changed = cursor_changed = Cut();
+      break;
+    case IDS_APP_COPY:
+      Copy();
+      break;
+    case IDS_APP_PASTE:
+      text_changed = cursor_changed = Paste();
+      break;
+    case IDS_APP_DELETE:
+      text_changed = cursor_changed = model_->Delete();
+      break;
+    case IDS_APP_SELECT_ALL:
+      SelectAll(false);
+      break;
+    case IDS_DELETE_BACKWARD:
+      text_changed = cursor_changed = model_->Backspace();
+      break;
+    case IDS_DELETE_FORWARD:
+      text_changed = cursor_changed = model_->Delete();
+      break;
+    case IDS_DELETE_TO_END_OF_LINE:
+      model_->MoveCursor(gfx::LINE_BREAK, end, true);
+      text_changed = cursor_changed = model_->Delete();
+      break;
+    case IDS_DELETE_TO_BEGINNING_OF_LINE:
+      model_->MoveCursor(gfx::LINE_BREAK, begin, true);
+      text_changed = cursor_changed = model_->Backspace();
+      break;
+    case IDS_DELETE_WORD_BACKWARD:
+      model_->MoveCursor(gfx::WORD_BREAK, begin, true);
+      text_changed = cursor_changed = model_->Backspace();
+      break;
+    case IDS_DELETE_WORD_FORWARD:
+      model_->MoveCursor(gfx::WORD_BREAK, end, true);
+      text_changed = cursor_changed = model_->Delete();
+      break;
+    case IDS_MOVE_LEFT:
+      model_->MoveCursor(gfx::CHARACTER_BREAK, gfx::CURSOR_LEFT, false);
+      break;
+    case IDS_MOVE_LEFT_AND_MODIFY_SELECTION:
+      model_->MoveCursor(gfx::CHARACTER_BREAK, gfx::CURSOR_LEFT, true);
+      break;
+    case IDS_MOVE_RIGHT:
+      model_->MoveCursor(gfx::CHARACTER_BREAK, gfx::CURSOR_RIGHT, false);
+      break;
+    case IDS_MOVE_RIGHT_AND_MODIFY_SELECTION:
+      model_->MoveCursor(gfx::CHARACTER_BREAK, gfx::CURSOR_RIGHT, true);
+      break;
+    case IDS_MOVE_WORD_LEFT:
+      model_->MoveCursor(gfx::WORD_BREAK, gfx::CURSOR_LEFT, false);
+      break;
+    case IDS_MOVE_WORD_LEFT_AND_MODIFY_SELECTION:
+      model_->MoveCursor(gfx::WORD_BREAK, gfx::CURSOR_LEFT, true);
+      break;
+    case IDS_MOVE_WORD_RIGHT:
+      model_->MoveCursor(gfx::WORD_BREAK, gfx::CURSOR_RIGHT, false);
+      break;
+    case IDS_MOVE_WORD_RIGHT_AND_MODIFY_SELECTION:
+      model_->MoveCursor(gfx::WORD_BREAK, gfx::CURSOR_RIGHT, true);
+      break;
+    case IDS_MOVE_TO_BEGINNING_OF_LINE:
+      model_->MoveCursor(gfx::LINE_BREAK, begin, false);
+      break;
+    case IDS_MOVE_TO_BEGINNING_OF_LINE_AND_MODIFY_SELECTION:
+      model_->MoveCursor(gfx::LINE_BREAK, begin, true);
+      break;
+    case IDS_MOVE_TO_END_OF_LINE:
+      model_->MoveCursor(gfx::LINE_BREAK, end, false);
+      break;
+    case IDS_MOVE_TO_END_OF_LINE_AND_MODIFY_SELECTION:
+      model_->MoveCursor(gfx::LINE_BREAK, end, true);
+      break;
+    default:
+      NOTREACHED();
+      break;
+  }
+
+  cursor_changed |= GetSelectionModel() != selection_model;
+  if (cursor_changed)
+    UpdateSelectionClipboard();
+  UpdateAfterChange(text_changed, cursor_changed);
+  OnAfterUserAction();
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // Textfield, private:
 
 void Textfield::AccessibilitySetValue(const base::string16& new_value) {
   if (!read_only()) {
     SetText(new_value);
     ClearSelection();
   }
 }
 
@@ skipping 156 matching lines @@
     context_menu_contents_.reset(new ui::SimpleMenuModel(this));
     context_menu_contents_->AddItemWithStringId(IDS_APP_UNDO, IDS_APP_UNDO);
     context_menu_contents_->AddSeparator(ui::NORMAL_SEPARATOR);
     context_menu_contents_->AddItemWithStringId(IDS_APP_CUT, IDS_APP_CUT);
     context_menu_contents_->AddItemWithStringId(IDS_APP_COPY, IDS_APP_COPY);
     context_menu_contents_->AddItemWithStringId(IDS_APP_PASTE, IDS_APP_PASTE);
     context_menu_contents_->AddItemWithStringId(IDS_APP_DELETE, IDS_APP_DELETE);
     context_menu_contents_->AddSeparator(ui::NORMAL_SEPARATOR);
     context_menu_contents_->AddItemWithStringId(IDS_APP_SELECT_ALL,
                                                 IDS_APP_SELECT_ALL);
+
+    // If the controller adds menu commands, also override ExecuteCommand() and
+    // IsCommandIdEnabled() as appropriate, for the commands added.

[karandeepb, 2016/06/09 10:47:10]

It seems weird that the controller can update the context menu, but we don't
have a way to forward IsCommandIdEnabled and ExecuteCommand to the controller.
Hence if a controller wants to add commands to the context menu, it will also
have to inherit from Textfield (like Omnibox does). Does it make more sense to
remove UpdateContextMenu from TextfieldController and give subclasses a way to
update the context menu instead.

[msw, 2016/06/10 00:07:50]

That's a good point. Making Textfield::UpdateContextMenu virtual, and letting
OmniboxViewViews override that would be nice, but we'd need to move the
|context_menu_runner_.reset...| statement elsewhere. It's not critical, but
that'd be a nice fix after you're done with this set of CLs.

[karandeepb, 2016/06/10 05:52:36]

WIll do this in a follow-up after this set of CLs land.

     if (controller_)
       controller_->UpdateContextMenu(context_menu_contents_.get());
   }
   context_menu_runner_.reset(
       new MenuRunner(context_menu_contents_.get(),
                      MenuRunner::HAS_MNEMONICS | MenuRunner::CONTEXT_MENU));
 }
 
 void Textfield::TrackMouseClicks(const ui::MouseEvent& event) {
   if (event.IsOnlyLeftMouseButton()) {
@@ skipping 63 matching lines @@
     RequestFocus();
   model_->MoveCursorTo(mouse);
   if (!selection_clipboard_text.empty()) {
     model_->InsertText(selection_clipboard_text);
     UpdateAfterChange(true, true);
   }
   OnAfterUserAction();
 }
 
 }  // namespace views