Commit e26cb1a1 authored by msw@chromium.org's avatar msw@chromium.org

Don't close ExtensionPopups on child window focus (Win, non-Aura).

Fixes crash on JS dialog popups from the extension bubbles.

Check for child HWND in WidgetFocusChangeListener::OnNativeFocusChange.
Code from BubbleWidget in browser_bubble_win.cc (crrev.com/112278)

See Issue 106958 for Aura, the dialog&bubble close early without crashing.

BUG=106723,106958
TEST=The js alert extension attached to the issue doesn't crash, popup works on non-aura.

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@113936 0039d316-1c4b-4281-b951-d872f2087c98
parent cce130fe
...@@ -39,7 +39,12 @@ ExtensionPopup::ExtensionPopup( ...@@ -39,7 +39,12 @@ ExtensionPopup::ExtensionPopup(
SetLayoutManager(new views::FillLayout()); SetLayoutManager(new views::FillLayout());
AddChildView(host->view()); AddChildView(host->view());
host->view()->SetContainer(this); host->view()->SetContainer(this);
#if defined(OS_WIN) && !defined(USE_AURA)
// Use OnNativeFocusChange to check for child window activation on deactivate.
set_close_on_deactivate(false);
#else
set_close_on_deactivate(!inspect_with_devtools); set_close_on_deactivate(!inspect_with_devtools);
#endif
// We wait to show the popup until the contained host finishes loading. // We wait to show the popup until the contained host finishes loading.
registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_HOST_DID_STOP_LOADING, registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_HOST_DID_STOP_LOADING,
...@@ -48,9 +53,12 @@ ExtensionPopup::ExtensionPopup( ...@@ -48,9 +53,12 @@ ExtensionPopup::ExtensionPopup(
// Listen for the containing view calling window.close(); // Listen for the containing view calling window.close();
registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_HOST_VIEW_SHOULD_CLOSE, registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_HOST_VIEW_SHOULD_CLOSE,
content::Source<Profile>(host->profile())); content::Source<Profile>(host->profile()));
views::WidgetFocusManager::GetInstance()->AddFocusChangeListener(this);
} }
ExtensionPopup::~ExtensionPopup() { ExtensionPopup::~ExtensionPopup() {
views::WidgetFocusManager::GetInstance()->RemoveFocusChangeListener(this);
} }
void ExtensionPopup::Observe(int type, void ExtensionPopup::Observe(int type,
...@@ -103,6 +111,27 @@ gfx::Size ExtensionPopup::GetPreferredSize() { ...@@ -103,6 +111,27 @@ gfx::Size ExtensionPopup::GetPreferredSize() {
return sz; return sz;
} }
void ExtensionPopup::OnNativeFocusChange(gfx::NativeView focused_before,
gfx::NativeView focused_now) {
// TODO(msw): Implement something equivalent for Aura. See crbug.com/106958
#if defined(OS_WIN) && !defined(USE_AURA)
// Don't close if a child of this window is activated (only needed on Win).
// ExtensionPopups can create Javascipt dialogs; see crbug.com/106723.
gfx::NativeView this_window = GetWidget()->GetNativeView();
if (!inspect_with_devtools_ && focused_before == this_window) {
DCHECK_NE(focused_now, this_window);
if (::GetWindow(focused_now, GW_OWNER) == this_window)
return;
gfx::NativeView focused_parent = focused_now;
while (focused_parent = ::GetParent(focused_parent)) {
if (this_window == focused_parent)
return;
}
GetWidget()->Close();
}
#endif
}
// static // static
ExtensionPopup* ExtensionPopup::ShowPopup( ExtensionPopup* ExtensionPopup::ShowPopup(
const GURL& url, const GURL& url,
......
...@@ -12,12 +12,14 @@ ...@@ -12,12 +12,14 @@
#include "content/public/browser/notification_observer.h" #include "content/public/browser/notification_observer.h"
#include "googleurl/src/gurl.h" #include "googleurl/src/gurl.h"
#include "ui/views/bubble/bubble_delegate.h" #include "ui/views/bubble/bubble_delegate.h"
#include "ui/views/focus/widget_focus_manager.h"
class Browser; class Browser;
class ExtensionPopup : public views::BubbleDelegateView, class ExtensionPopup : public views::BubbleDelegateView,
public ExtensionView::Container, public ExtensionView::Container,
public content::NotificationObserver { public content::NotificationObserver,
public views::WidgetFocusChangeListener {
public: public:
virtual ~ExtensionPopup(); virtual ~ExtensionPopup();
...@@ -50,9 +52,13 @@ class ExtensionPopup : public views::BubbleDelegateView, ...@@ -50,9 +52,13 @@ class ExtensionPopup : public views::BubbleDelegateView,
// ExtensionView::Container overrides. // ExtensionView::Container overrides.
virtual void OnExtensionPreferredSizeChanged(ExtensionView* view) OVERRIDE; virtual void OnExtensionPreferredSizeChanged(ExtensionView* view) OVERRIDE;
// view::View overrides. // views::View overrides.
virtual gfx::Size GetPreferredSize() OVERRIDE; virtual gfx::Size GetPreferredSize() OVERRIDE;
// views::WidgetFocusChangeListener overrides.
virtual void OnNativeFocusChange(gfx::NativeView focused_before,
gfx::NativeView focused_now) OVERRIDE;
// The min/max height of popups. // The min/max height of popups.
static const int kMinWidth; static const int kMinWidth;
static const int kMinHeight; static const int kMinHeight;
......
...@@ -27,7 +27,7 @@ void WidgetFocusManager::RemoveFocusChangeListener( ...@@ -27,7 +27,7 @@ void WidgetFocusManager::RemoveFocusChangeListener(
void WidgetFocusManager::OnWidgetFocusEvent(gfx::NativeView focused_before, void WidgetFocusManager::OnWidgetFocusEvent(gfx::NativeView focused_before,
gfx::NativeView focused_now) { gfx::NativeView focused_now) {
if (enabled_) { if (enabled_ && focused_before != focused_now) {
FOR_EACH_OBSERVER(WidgetFocusChangeListener, focus_change_listeners_, FOR_EACH_OBSERVER(WidgetFocusChangeListener, focus_change_listeners_,
OnNativeFocusChange(focused_before, focused_now)); OnNativeFocusChange(focused_before, focused_now));
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment