diff options
author | Peter Kasting <pkasting@google.com> | 2015-07-13 14:33:43 -0700 |
---|---|---|
committer | Peter Kasting <pkasting@google.com> | 2015-07-13 21:36:41 +0000 |
commit | a37c3113d25e7a1976531a35122a33f7509def82 (patch) | |
tree | 633db5bba37366e566ac428af8a06d8e20825ca0 | |
parent | 27becec88aa4f9d089b9b45dbce3c7ef7f51aae5 (diff) | |
download | chromium_src-a37c3113d25e7a1976531a35122a33f7509def82.zip chromium_src-a37c3113d25e7a1976531a35122a33f7509def82.tar.gz chromium_src-a37c3113d25e7a1976531a35122a33f7509def82.tar.bz2 |
Merge to M44 (branch 2403): Null-check the RWHV before using it.
TBR=sky
Turns out a non-null WebContents can have a null RWHV if it's crashed/killed.
BUG=504325
TEST=Use Chrome task manager to kill the process for the current tab, select cut from the Chrome menu, browser should not crash
Review URL: https://codereview.chromium.org/1216773003
Cr-Commit-Position: refs/heads/master@{#336933}
(cherry picked from commit c696a07432d2dd041a5053a061480cbb81dda1db)
Only dispatch cut/copy/paste accelerators to views which register for them.
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/1232833005.
Cr-Commit-Position: refs/branch-heads/2403@{#504}
Cr-Branched-From: f54b8097a9c45ed4ad308133d49f05325d6c5070-refs/heads/master@{#330231}
-rw-r--r-- | chrome/browser/ui/views/frame/browser_view.cc | 17 |
1 files changed, 9 insertions, 8 deletions
diff --git a/chrome/browser/ui/views/frame/browser_view.cc b/chrome/browser/ui/views/frame/browser_view.cc index a20297a..683a33709 100644 --- a/chrome/browser/ui/views/frame/browser_view.cc +++ b/chrome/browser/ui/views/frame/browser_view.cc @@ -2543,14 +2543,15 @@ ExclusiveAccessContext* BrowserView::GetExclusiveAccessContext() { bool BrowserView::DoCutCopyPasteForWebContents( WebContents* contents, void (WebContents::*method)()) { - // Using RWH interface rather than a fake key event for a WebContent is - // important since a fake event might be consumed by the web content. - if (contents->GetRenderWidgetHostView()->HasFocus()) { - (contents->*method)(); - return true; - } - - return false; + // It's possible for a non-null WebContents to have a null RWHV if it's + // crashed or otherwise been killed. + content::RenderWidgetHostView* rwhv = contents->GetRenderWidgetHostView(); + if (!rwhv || !rwhv->HasFocus()) + return false; + // Calling |method| rather than using a fake key event is important since a + // fake event might be consumed by the web content. + (contents->*method)(); + return true; } void BrowserView::ActivateAppModalDialog() const { |