summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorPeter Kasting <pkasting@google.com>2015-07-13 14:33:43 -0700
committerPeter Kasting <pkasting@google.com>2015-07-13 21:36:41 +0000
commita37c3113d25e7a1976531a35122a33f7509def82 (patch)
tree633db5bba37366e566ac428af8a06d8e20825ca0
parent27becec88aa4f9d089b9b45dbce3c7ef7f51aae5 (diff)
downloadchromium_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.cc17
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 {