Views: Modify Textfield::ExecuteCommand to use Textfield::IsCommandIdEnabled instead of a virtual ca

ui/views/controls/textfield/textfield.cc

Index: ui/views/controls/textfield/textfield.cc
diff --git a/ui/views/controls/textfield/textfield.cc b/ui/views/controls/textfield/textfield.cc
index cfa18d1299c2098d7e46ee4fe3ef00ad9063e7f4..40b15f83e154670490e7f2fe0c40853438643cc0 100644
--- a/ui/views/controls/textfield/textfield.cc
+++ b/ui/views/controls/textfield/textfield.cc
@@ -1302,8 +1302,6 @@ bool Textfield::GetAcceleratorForCommandId(int command_id,
 }
 
 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()) {
@@ -1319,9 +1317,13 @@ void Textfield::ExecuteCommand(int command_id, int event_flags) {
     }
   }
 
-  if (!IsCommandIdEnabled(command_id))
+  // We only execute the commands enabled in Textfield::IsCommandIdEnabled
+  // below. Hence don't do a virtual IsCommandIdEnabled call.
+  if (!Textfield::IsCommandIdEnabled(command_id))
     return;
 
+  DestroyTouchSelection();

[tapted, 2016/06/09 05:10:09]

The comment above is good, but now the remaining goal is to convince the code
reviewer that moving `DestroyTouchSelection()` to come after the check is the
right thing to do.

The description for http://crrev.com/267813 says "Whenever a mouse or keyboard
event happens anywhere in any display, touch text seleciton should be
deactivated.". That suggests to me that it might belong above the check
regardless of whether the command is enabled, but also that we might want to
duplicate the call to DestroyTouchSelection() inside ExecuteEditCommand (also
before checking whether the command_id is enabled) since the codepaths for menus
and edit commands will have less overlap.

+
   bool text_changed = false;
   bool cursor_changed = false;
   bool rtl = GetTextDirection() == base::i18n::RIGHT_TO_LEFT;