From 34c6495a4a25d34ab900c8e698f4ccc2cf9b39fd Mon Sep 17 00:00:00 2001 From: "brettw@chromium.org" Date: Fri, 16 Jan 2009 17:41:33 +0000 Subject: Fix a crash when handling close tab accelerators. The class would still be accessed after the accelerator was handled, and close tab accelerators will cause the view to be deleted out from under us. BUG=6321 Review URL: http://codereview.chromium.org/18151 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@8182 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/tab_contents/web_contents_view_win.cc | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/chrome/browser/tab_contents/web_contents_view_win.cc b/chrome/browser/tab_contents/web_contents_view_win.cc index 29af1f6..54eb162 100644 --- a/chrome/browser/tab_contents/web_contents_view_win.cc +++ b/chrome/browser/tab_contents/web_contents_view_win.cc @@ -277,9 +277,19 @@ void WebContentsViewWin::HandleKeyboardEvent(const WebKeyboardEvent& event) { WebInputEvent::CTRL_KEY, (event.modifiers & WebInputEvent::ALT_KEY) == WebInputEvent::ALT_KEY); + + // This is tricky: we want to set ignore_next_char_event_ if + // ProcessAccelerator returns true. But ProcessAccelerator might delete + // |this| if the accelerator is a "close tab" one. So we speculatively + // set the flag and fix it if no event was handled. + ignore_next_char_event_ = true; if (focus_manager->ProcessAccelerator(accelerator, false)) { - ignore_next_char_event_ = true; + // DANGER: |this| could be deleted now! return; + } else { + // ProcessAccelerator didn't handle the accelerator, so we know both + // that |this| is still valid, and that we didn't want to set the flag. + ignore_next_char_event_ = false; } } } -- cgit v1.1