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

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 1284 matching lines @@
     case IDS_APP_SELECT_ALL:
       *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))
+  // 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;
   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:
@@ skipping 594 matching lines @@
     RequestFocus();
   model_->MoveCursorTo(mouse);
   if (!selection_clipboard_text.empty()) {
     model_->InsertText(selection_clipboard_text);
     UpdateAfterChange(true, true);
   }
   OnAfterUserAction();
 }
 
 }  // namespace views