Eliminate faux-RTTI code from BrowserView.

Eliminate faux-RTTI code from BrowserView.

Use the existing View::AcceleratorPressed() method to invoke a Textfield's
clipboard functions from the browser menu.

BUG=319035
TEST=none

Committed: https://crrev.com/cd3f08bceeac7819208d8f0f1b7b3a3528f4883b
Cr-Commit-Position: refs/heads/master@{#325781}

[Peter Kasting, 2015-04-16 01:02:31]

pkasting@chromium.org changed reviewers:
+ sky@chromium.org

[Peter Kasting, 2015-04-16 01:02:32]

This implementation seems kind of like what we discussed?

[sky, 2015-04-16 15:47:25]

https://codereview.chromium.org/1091703002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/browser_view.cc (right):

https://codereview.chromium.org/1091703002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/browser_view.cc:1465: // If a WebContents is
focused, call its member method.
Seems like ideally this function would do what you have at 1489, 1490, and after
that GetWidget()->GetFocusMananger()->ProcessAccelerator(). The accelerator
would be routed to the focused view correctly and everything would work. For
everything to work you likely need to change Textfield and Webview to register
cut/copy/paste accelerators.

[Peter Kasting, 2015-04-17 21:43:17]

Thanks Rietveld for eating my first draft...

Patch set 3 implements what you suggested in the previous round of comments, and
seems to basically work.

I am concerned that this might break web content overriding of ctrl-X/C/V.  I
don't really understand the existing code in WebView that toggles whether we
send keystrokes to web content, and I don't have a test page that overrides
these to do something non-clipboardy to really test with, but it _looks_ like
WebView now always gets these as ui::Accelerators and handles them without them
ever showing up as keystrokes, so I'm concerned.

Other than that, I don't have a strong feeling as to which implementation is
better.  The previous hardcoding of WebContents and Textfield in BrowserView
kinda sucked.  OTOH, the current way all the CanHandleAccelerators() methods
have to check whether the object is focused also sucks.

I don't know whether the older patch can be improved to not check both
WebContentses by making use of WebView or FocusManager or something, so that at
least it hardcodes two targets for things instead of three.  I could also
possibly use the AcceleratorPressed() override mechanism in the older patch for
communicating with the content area things as well, instead of the icky member
function pointer stuff.

Let me know what combination of things here you think is safest and cleanest.

[sky, 2015-04-17 21:52:32]

I like this in that it means the accelerators work outside of a browserview
window. LGTM

[Peter Kasting, 2015-04-17 22:16:10]

On 2015/04/17 21:52:32, sky wrote:
> I like this in that it means the accelerators work outside of a browserview
> window. LGTM

I'm really worried about that webcontents not getting to see these keystrokes
issue.  I don't know a good way to solve it.

I'm strongly inclined to go with something like patch set 2 unless you know of a
good way to either address that or prove it's not a problem...

(I also don't actually know what "outside of a browserview window" means.  Like,
some other thing someone builds using views, that's not a browser?)

[Peter Kasting, 2015-04-18 00:55:30]

After brief discussion offline, I'm going to revert to patch set 2 and land
that.

[Peter Kasting, 2015-04-18 00:58:33]

The CQ bit was checked by pkasting@chromium.org

[Peter Kasting, 2015-04-18 00:58:34]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1091703002/#ps60001
(title: "Revert to patch set 2")

[commit-bot: I haz the power, 2015-04-18 00:59:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1091703002/60001

[commit-bot: I haz the power, 2015-04-18 13:37:28]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-04-18 13:39:17]

Patchset 4 (id:??) landed as
https://crrev.com/cd3f08bceeac7819208d8f0f1b7b3a3528f4883b
Cr-Commit-Position: refs/heads/master@{#325781}