Merge to M44 (branch 2403): Only dispatch cut/copy/paste accelerators to views

which register for them.

TBR=sky

This restores some of https://codereview.chromium.org/1091703002/#ps40001 ; we
once again use the standard accelerator processing code to dispatch accelerators
instead of calling AcceleratorPressed() directly.  To avoid the concerns I had
which led me to discard that patch originally, WebContents does not make use of
this route; the manual dispatch for its sake is still in place.

This prevents AccleratorPressed() from being called on focused views which
didn't register for the cut/copy/paste accelerators.  Views differ about whether
to handle a call in such a case by simply returning false, or by DCHECK failing
(or similar).  For those that do the latter, the old code would lead to a DCHECK
failure followed by unconditionally executing some other accelerator's handling
code, e.g. "escape was pressed, close the window".

This means that another potential fix here would be to change all instances of
AcceleratorPressed() to return false if the provided accelerator wasn't one they
were interested in.  Not only is this a much larger change, it seems arguably
incorrect -- classes ought not to need to be defensive here.

BUG=495472
TEST=Open print preview, select cut from wrench menu, print preview should not close.

Review URL: https://codereview.chromium.org/1214343006

Cr-Commit-Position: refs/heads/master@{#336925}
(cherry picked from commit 25614a51c26a6e5da0956c86772f4302f298b793)

Review URL: https://codereview.chromium.org/1234983002.

Cr-Commit-Position: refs/branch-heads/2403@{#503}
Cr-Branched-From: f54b8097a9c45ed4ad308133d49f05325d6c5070-refs/heads/master@{#330231}

3 files changed, 31 insertions, 10 deletions

diff --git a/chrome/browser/ui/views/frame/browser_view.cc b/chrome/browser/ui/views/frame/browser_view.cc
index 8936588..a20297a 100644
--- a/chrome/browser/ui/views/frame/browser_view.cc
+++ b/chrome/browser/ui/views/frame/browser_view.cc
@@ -1172,7 +1172,7 @@ void BrowserView::FocusAppMenu() {
 }
 
 void BrowserView::RotatePaneFocus(bool forwards) {
-  GetWidget()->GetFocusManager()->RotatePaneFocus(
+  GetFocusManager()->RotatePaneFocus(
       forwards ?
           views::FocusManager::kForward : views::FocusManager::kBackward,
       views::FocusManager::kWrap);
@@ -1465,6 +1465,12 @@ void BrowserView::HandleKeyboardEvent(const NativeWebKeyboardEvent& event) {
 // manager to do that.
 void BrowserView::CutCopyPaste(int command_id) {
   // If a WebContents is focused, call its member method.
+  //
+  // We could make WebContents register accelerators and then just use the
+  // plumbing for accelerators below to dispatch these, but it's not clear
+  // whether that would still allow keypresses of ctrl-X/C/V to be sent as
+  // key events (and not accelerators) to the WebContents so it can give the web
+  // page a chance to override them.
   WebContents* contents = browser_->tab_strip_model()->GetActiveWebContents();
   if (contents) {
     void (WebContents::*method)();
@@ -1483,15 +1489,18 @@ void BrowserView::CutCopyPaste(int command_id) {
       return;
   }
 
-  // Execute the command on the focused view, if possible, by calling
-  // AcceleratorPressed().  Textfields override this to translate and execute
-  // the provided accelerator.
-  views::View* focused = GetFocusManager()->GetFocusedView();
-  if (focused && focused->CanHandleAccelerators()) {
-    ui::Accelerator accelerator;
-    GetAccelerator(command_id, &accelerator);
-    focused->AcceleratorPressed(accelerator);
-  }
+  // Any Views which want to handle the clipboard commands in the Chrome menu
+  // should:
+  //   (a) Register ctrl-X/C/V as accelerators
+  //   (b) Implement CanHandleAccelerators() to not return true unless they're
+  //       focused, as the FocusManager will try all registered accelerator
+  //       handlers, not just the focused one.
+  // Currently, Textfield (which covers the omnibox and find bar, and likely any
+  // other native UI in the future that wants to deal with clipboard commands)
+  // does the above.
+  ui::Accelerator accelerator;
+  GetAccelerator(command_id, &accelerator);
+  GetFocusManager()->ProcessAccelerator(accelerator);
 }
 
 WindowOpenDisposition BrowserView::GetDispositionForPopupBounds(
diff --git a/ui/views/controls/textfield/textfield.cc b/ui/views/controls/textfield/textfield.cc
index 73cccd5..d0410cb 100644
--- a/ui/views/controls/textfield/textfield.cc
+++ b/ui/views/controls/textfield/textfield.cc
@@ -288,6 +288,13 @@ Textfield::Textfield()
     password_reveal_duration_ = ViewsDelegate::views_delegate->
         GetDefaultTextfieldObscuredRevealDuration();
   }
+
+  // These allow BrowserView to pass edit commands from the Chrome menu to us
+  // when we're focused by simply asking the FocusManager to
+  // ProcessAccelerator() with the relevant accelerators.
+  AddAccelerator(ui::Accelerator(ui::VKEY_X, ui::EF_CONTROL_DOWN));
+  AddAccelerator(ui::Accelerator(ui::VKEY_C, ui::EF_CONTROL_DOWN));
+  AddAccelerator(ui::Accelerator(ui::VKEY_V, ui::EF_CONTROL_DOWN));
 }
 
 Textfield::~Textfield() {}
@@ -817,6 +824,10 @@ bool Textfield::AcceleratorPressed(const ui::Accelerator& accelerator) {
   return true;
 }
 
+bool Textfield::CanHandleAccelerators() const {
+  return GetRenderText()->focused() && View::CanHandleAccelerators();
+}
+
 void Textfield::AboutToRequestFocusFromTabTraversal(bool reverse) {
   SelectAll(false);
 }
diff --git a/ui/views/controls/textfield/textfield.h b/ui/views/controls/textfield/textfield.h
index 6814946..9a6e68c 100644
--- a/ui/views/controls/textfield/textfield.h
+++ b/ui/views/controls/textfield/textfield.h
@@ -221,6 +221,7 @@ class VIEWS_EXPORT Textfield : public View,
   ui::TextInputClient* GetTextInputClient() override;
   void OnGestureEvent(ui::GestureEvent* event) override;
   bool AcceleratorPressed(const ui::Accelerator& accelerator) override;
+  bool CanHandleAccelerators() const override;
   void AboutToRequestFocusFromTabTraversal(bool reverse) override;
   bool SkipDefaultKeyEventProcessing(const ui::KeyEvent& event) override;
   bool GetDropFormats(