Skip to content

Commit

Permalink
Fix a crash in ChromeFrame which would occur if we clicked both mouse…
Browse files Browse the repository at this point in the history
… buttons on a link.

The right click would run the TrackPopupMenuEx API which runs a system modal loop and the left click
would cause the current document to be destroyed causing a crash on return while accessing member
variables on a destroyed object.

Fix is to grab a reference on the current active document while handling the context menu event to protect
against us getting destroyed in the context of the TrackPopupMenuEx call.

We also need to NULL check the automation_client_ member in ChromeFramePlugin as this could be detached
from the existing active document and attached to the new active document instance which comes up to
handle the link navigation.

Fixes bug http://code.google.com/p/chromium/issues/detail?id=37220

Bug=37220

Review URL: http://codereview.chromium.org/664009

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@40547 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
ananta@chromium.org committed Mar 3, 2010
1 parent 32da100 commit 045229a
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 12 deletions.
8 changes: 8 additions & 0 deletions chrome_frame/chrome_frame_activex_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,14 @@ END_MSG_MAP()
OnOpenURL(tab_handle, GURL(url), GURL(), disposition);
}

virtual void OnHandleContextMenu(int tab_handle, HANDLE menu_handle,
int align_flags,
const IPC::ContextMenuParams& params) {
scoped_refptr<Base> ref(this);
ChromeFramePlugin<T>::OnHandleContextMenu(tab_handle, menu_handle,
align_flags, params);
}

LRESULT OnCreate(UINT message, WPARAM wparam, LPARAM lparam,
BOOL& handled) { // NO_LINT
ModifyStyle(0, WS_CLIPCHILDREN | WS_CLIPSIBLINGS, 0);
Expand Down
29 changes: 17 additions & 12 deletions chrome_frame/chrome_frame_plugin.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class ChromeFramePlugin : public ChromeFrameDelegateImpl {
Uninitialize();
}

BEGIN_MSG_MAP(ChromeFrameActivex)
BEGIN_MSG_MAP(T)
MESSAGE_HANDLER(WM_SETFOCUS, OnSetFocus)
MESSAGE_HANDLER(WM_SIZE, OnSize)
MESSAGE_HANDLER(WM_PARENTNOTIFY, OnParentNotify)
Expand All @@ -43,7 +43,7 @@ END_MSG_MAP()
}

void Uninitialize() {
if (automation_client_.get()) {
if (IsValid()) {
automation_client_->Uninitialize();
automation_client_ = NULL;
}
Expand All @@ -52,6 +52,7 @@ END_MSG_MAP()
bool InitializeAutomation(const std::wstring& profile_name,
const std::wstring& extra_chrome_arguments,
bool incognito) {
DCHECK(IsValid());
// We don't want to do incognito when privileged, since we're
// running in browser chrome or some other privileged context.
bool incognito_mode = !is_privileged_ && incognito;
Expand All @@ -78,7 +79,7 @@ END_MSG_MAP()
virtual void OnAutomationServerReady() {
// Issue the extension automation request if we're privileged to
// allow this control to handle extension requests from Chrome.
if (is_privileged_)
if (is_privileged_ && IsValid())
automation_client_->SetEnableExtensionAutomation(functions_enabled_);
}

Expand All @@ -87,7 +88,8 @@ END_MSG_MAP()
}

virtual void OnHostMoved() {
automation_client_->OnChromeFrameHostMoved();
if (IsValid())
automation_client_->OnChromeFrameHostMoved();
}

protected:
Expand Down Expand Up @@ -123,7 +125,8 @@ END_MSG_MAP()
params.screen_y, GetWindow(), NULL);
// Menu is over now give focus back to chrome
GiveFocusToChrome();
if (selected != 0 && !self->HandleContextMenuCommand(selected, params)) {
if (IsValid() && selected != 0 &&
!self->HandleContextMenuCommand(selected, params)) {
automation_client_->SendContextMenuCommandToChromeFrame(selected);
}
}
Expand All @@ -133,7 +136,7 @@ END_MSG_MAP()

LRESULT OnSetFocus(UINT message, WPARAM wparam, LPARAM lparam,
BOOL& handled) { // NO_LINT
if (!ignore_setfocus_ && automation_client_ != NULL) {
if (!ignore_setfocus_ && IsValid()) {
GiveFocusToChrome();
}
return 0;
Expand All @@ -143,7 +146,7 @@ END_MSG_MAP()
BOOL& handled) { // NO_LINT
handled = FALSE;
// When we get resized, we need to resize the external tab window too.
if (automation_client_.get())
if (IsValid())
automation_client_->Resize(LOWORD(lparam), HIWORD(lparam),
SWP_NOACTIVATE | SWP_NOZORDER);
return 0;
Expand Down Expand Up @@ -197,11 +200,13 @@ END_MSG_MAP()
}

void GiveFocusToChrome() {
TabProxy* tab = automation_client_->tab();
HWND chrome_window = automation_client_->tab_window();
if (tab && ::IsWindow(chrome_window)) {
DLOG(INFO) << "Setting initial focus";
tab->SetInitialFocus(win_util::IsShiftPressed());
if (IsValid()) {
TabProxy* tab = automation_client_->tab();
HWND chrome_window = automation_client_->tab_window();
if (tab && ::IsWindow(chrome_window)) {
DLOG(INFO) << "Setting initial focus";
tab->SetInitialFocus(win_util::IsShiftPressed());
}
}
}

Expand Down

0 comments on commit 045229a

Please sign in to comment.