CL implementing focus-dismissal of the chrome.experimental.popup set of extension APIs.

Specifically, these changes cause a displayed pop-up to be dismissed when the focus shifts away from both the pop-up view, and the extension-view that launched the pop-up.I had to do a lot of investigating and trial-and-error to converge to the solution present here.  I was hoping to be able to piggy-back on the existing FocusManager's various listener routines, but because the pop-up is hosted in a BubbleWidget, which is a separate top-level window with its own focus manager, I could not rely on a given focus manager to take care of the focus change notifications. To get around the above issue, I added a new type of focus listener that can listen on native-window focus change events.  I added invocations to this listener throughout the Widget classes in the system so that registered listeners will be notified on focus change.  

I found some of the focus change events problematic, as the system will arbitrarily reassign the focus to the main browser window when shifting activation between chrome windows.  (SeefocusManagerWin::ClearNativeFocus).  To prevent this focus bounce from confusing focus listeners, I added a means to suppress notification of focus change during these operations.

I added GTK and Mac stubs for the new widget functions that will assert when called.  GTK and Cocoa development is not my expertise, so I thought // TODO(port) would be wiser.I'm uncertain of the best means to unit-test these changes.  Direction in this regard would be appreciated.

BUG=None
TEST=None

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@38685 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 119 insertions, 23 deletions

diff --git a/chrome/browser/extensions/extension_dom_ui.cc b/chrome/browser/extensions/extension_dom_ui.cc
index e9f1996..3428869 100644
--- a/chrome/browser/extensions/extension_dom_ui.cc
+++ b/chrome/browser/extensions/extension_dom_ui.cc
@@ -9,6 +9,7 @@
 #include "chrome/browser/browser_list.h"
 #include "chrome/browser/extensions/extensions_service.h"
 #include "chrome/browser/profile.h"
+#include "chrome/browser/renderer_host/render_widget_host_view.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/common/bindings_policy.h"
 #include "chrome/common/pref_service.h"
@@ -90,6 +91,10 @@ gfx::NativeWindow ExtensionDOMUI::GetFrameNativeWindow() {
   return native_window;
 }
 
+gfx::NativeView ExtensionDOMUI::GetNativeViewOfHost() {
+  return tab_contents()->GetRenderWidgetHostView()->GetNativeView();
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // chrome:// URL overrides
 
diff --git a/chrome/browser/extensions/extension_dom_ui.h b/chrome/browser/extensions/extension_dom_ui.h
index bd7a0ac..c7d2b97 100644
--- a/chrome/browser/extensions/extension_dom_ui.h
+++ b/chrome/browser/extensions/extension_dom_ui.h
@@ -47,6 +47,7 @@ class ExtensionDOMUI
   // ExtensionPopupHost::Delegate
   virtual RenderViewHost* GetRenderViewHost();
   virtual Profile* GetProfile();
+  virtual gfx::NativeView GetNativeViewOfHost();
 
   // BrowserURLHandler
   static bool HandleChromeURLOverride(GURL* url, Profile* profile);
diff --git a/chrome/browser/extensions/extension_host.h b/chrome/browser/extensions/extension_host.h
index 4087338..2bd7906 100644
--- a/chrome/browser/extensions/extension_host.h
+++ b/chrome/browser/extensions/extension_host.h
@@ -195,6 +195,9 @@ class ExtensionHost : public ExtensionPopupHost::PopupDelegate,
 
   // ExtensionPopupHost::Delegate
   virtual RenderViewHost* GetRenderViewHost() { return render_view_host(); }
+  virtual gfx::NativeView GetNativeViewOfHost() {
+    return view()->native_view();
+  }
 
   // Returns true if we're hosting a background page.
   // This isn't valid until CreateRenderView is called.
diff --git a/chrome/browser/extensions/extension_popup_host.cc b/chrome/browser/extensions/extension_popup_host.cc
index 1a964da..ec679e3 100644
--- a/chrome/browser/extensions/extension_popup_host.cc
+++ b/chrome/browser/extensions/extension_popup_host.cc
@@ -4,18 +4,88 @@
 
 #include "chrome/browser/extensions/extension_popup_host.h"
 
+#include "base/message_loop.h"
 #if defined(TOOLKIT_VIEWS)
 #include "chrome/browser/extensions/extension_popup_api.h"
 #endif
 #include "chrome/browser/profile.h"
 #include "chrome/browser/browser.h"
 #include "chrome/browser/renderer_host/render_view_host.h"
+#include "chrome/browser/renderer_host/render_widget_host_view.h"
 #if defined(TOOLKIT_VIEWS)
 #include "chrome/browser/views/extensions/extension_popup.h"
 #endif
 #include "chrome/common/notification_details.h"
 #include "chrome/common/notification_source.h"
 #include "chrome/common/notification_type.h"
+#if defined(TOOLKIT_VIEWS)
+#include "views/focus/focus_manager.h"
+#include "views/widget/root_view.h"
+#endif
+
+#if defined(TOOLKIT_VIEWS)
+// A helper class that monitors native focus change events.  Because the views
+// FocusManager does not listen for view-change notification across
+// native-windows, we need to use native-listener utilities.
+class ExtensionPopupHost::PopupFocusListener
+    : public views::WidgetFocusChangeListener {
+ public:
+  // Constructs and registers a new PopupFocusListener for the given
+  // |popup_host|.
+  explicit PopupFocusListener(ExtensionPopupHost* popup_host)
+      : popup_host_(popup_host) {
+    views::FocusManager::GetWidgetFocusManager()->AddFocusChangeListener(this);
+  }
+
+  virtual ~PopupFocusListener() {
+    views::FocusManager::GetWidgetFocusManager()->
+        RemoveFocusChangeListener(this);
+  }
+
+  virtual void NativeFocusWillChange(gfx::NativeView focused_before,
+                                     gfx::NativeView focused_now) {
+    // If no view is to be focused, then Chrome was deactivated, so hide the
+    // popup.
+    if (!focused_now) {
+      popup_host_->DismissPopupAsync();
+      return;
+    }
+
+    gfx::NativeView host_view = popup_host_->delegate()->GetNativeViewOfHost();
+
+    // If the widget hosting the popup contains the newly focused view, then
+    // don't dismiss the pop-up.
+    views::Widget* popup_root_widget =
+        popup_host_->child_popup()->host()->view()->GetWidget();
+    if (popup_root_widget &&
+        popup_root_widget->ContainsNativeView(focused_now)) {
+      return;
+    }
+
+    // If the widget or RenderWidgetHostView hosting the extension that
+    // launched the pop-up is receiving focus, then don't dismiss the popup.
+    views::Widget* host_widget =
+        views::Widget::GetWidgetFromNativeView(host_view);
+    if (host_widget && host_widget->ContainsNativeView(focused_now)) {
+      return;
+    }
+
+    RenderWidgetHostView* render_host_view =
+        RenderWidgetHostView::GetRenderWidgetHostViewFromNativeView(
+            host_view);
+    if (render_host_view &&
+        render_host_view->ContainsNativeView(focused_now)) {
+      return;
+    }
+
+    popup_host_->DismissPopupAsync();
+  }
+
+ private:
+  ExtensionPopupHost* popup_host_;
+  DISALLOW_COPY_AND_ASSIGN(PopupFocusListener);
+};
+#endif  // if defined(TOOLKIT_VIEWS)
 
 
 ExtensionPopupHost::PopupDelegate::~PopupDelegate() {
@@ -47,9 +117,11 @@ Profile* ExtensionPopupHost::PopupDelegate::GetProfile() {
 ExtensionPopupHost::ExtensionPopupHost(PopupDelegate* delegate)
     :  // NO LINT
 #if defined(TOOLKIT_VIEWS)
+      listener_(NULL),
       child_popup_(NULL),
 #endif
-      delegate_(delegate) {
+      delegate_(delegate),
+      ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)) {
   DCHECK(delegate_);
 
   // Listen for view close requests, so that we can dismiss a hosted pop-up
@@ -65,22 +137,21 @@ ExtensionPopupHost::~ExtensionPopupHost() {
 }
 
 #if defined(TOOLKIT_VIEWS)
-void ExtensionPopupHost::BubbleBrowserWindowMoved(BrowserBubble* bubble) {
+void ExtensionPopupHost::set_child_popup(ExtensionPopup* popup) {
+  // An extension may only have one popup active at a given time.
   DismissPopup();
-}
+  if (popup)
+    listener_.reset(new PopupFocusListener(this));
 
-void ExtensionPopupHost::BubbleBrowserWindowClosing(BrowserBubble* bubble) {
-  DismissPopup();
+  child_popup_ = popup;
 }
 
-void ExtensionPopupHost::BubbleGotFocus(BrowserBubble* bubble) {
+void ExtensionPopupHost::BubbleBrowserWindowMoved(BrowserBubble* bubble) {
+  DismissPopupAsync();
 }
 
-void ExtensionPopupHost::BubbleLostFocus(BrowserBubble* bubble,
-                                         gfx::NativeView focused_view) {
-  // TODO(twiz):  Dismiss the pop-up upon loss of focus of the bubble, but not
-  // if the focus is transitioning to the host which owns the popup!
-  // DismissPopup();
+void ExtensionPopupHost::BubbleBrowserWindowClosing(BrowserBubble* bubble) {
+  DismissPopupAsync();
 }
 #endif  // defined(TOOLKIT_VIEWS)
 
@@ -103,6 +174,7 @@ void ExtensionPopupHost::Observe(NotificationType type,
 
 void ExtensionPopupHost::DismissPopup() {
 #if defined(TOOLKIT_VIEWS)
+  listener_.reset(NULL);
   if (child_popup_) {
     child_popup_->Hide();
     child_popup_->DetachFromBrowser();
@@ -117,3 +189,14 @@ void ExtensionPopupHost::DismissPopup() {
   }
 #endif  // defined(TOOLKIT_VIEWS)
 }
+
+void ExtensionPopupHost::DismissPopupAsync() {
+  // Dismiss the popup asynchronously, as we could be deep in a message loop
+  // processing activations, and the focus manager may get confused if the
+  // currently focused view is destroyed.
+  method_factory_.RevokeAll();
+  MessageLoop::current()->PostNonNestableTask(
+      FROM_HERE,
+      method_factory_.NewRunnableMethod(
+          &ExtensionPopupHost::DismissPopup));
+}
diff --git a/chrome/browser/extensions/extension_popup_host.h b/chrome/browser/extensions/extension_popup_host.h
index 102c4ad..c11568b 100644
--- a/chrome/browser/extensions/extension_popup_host.h
+++ b/chrome/browser/extensions/extension_popup_host.h
@@ -5,7 +5,9 @@
 #ifndef CHROME_BROWSER_EXTENSIONS_EXTENSION_POPUP_HOST_H_
 #define CHROME_BROWSER_EXTENSIONS_EXTENSION_POPUP_HOST_H_
 
+#include "app/gfx/native_widget_types.h"
 #include "base/scoped_ptr.h"
+#include "base/task.h"
 #include "build/build_config.h"
 #if defined(TOOLKIT_VIEWS)
 #include "chrome/browser/views/browser_bubble.h"
@@ -39,6 +41,7 @@ class ExtensionPopupHost :  // NOLINT
     virtual ~PopupDelegate();
     virtual Browser* GetBrowser() const = 0;
     virtual RenderViewHost* GetRenderViewHost() = 0;
+    virtual gfx::NativeView GetNativeViewOfHost() = 0;
     virtual Profile* GetProfile();
 
     // Constructs, or returns the existing ExtensionPopupHost instance.
@@ -53,6 +56,7 @@ class ExtensionPopupHost :  // NOLINT
   explicit ExtensionPopupHost(PopupDelegate* delegate);
   virtual ~ExtensionPopupHost();
 
+  PopupDelegate* delegate() { return delegate_; }
   void RevokeDelegate() { delegate_ = NULL; }
 
   // Dismiss the hosted pop-up, if one is present.
@@ -60,11 +64,7 @@ class ExtensionPopupHost :  // NOLINT
 
 #if defined(TOOLKIT_VIEWS)
   ExtensionPopup* child_popup() const { return child_popup_; }
-  void set_child_popup(ExtensionPopup* popup) {
-    // An extension may only have one popup active at a given time.
-    DismissPopup();
-    child_popup_ = popup;
-  }
+  void set_child_popup(ExtensionPopup* popup);
 
   // BrowserBubble::Delegate implementation.
   // Called when the Browser Window that this bubble is attached to moves.
@@ -73,13 +73,6 @@ class ExtensionPopupHost :  // NOLINT
   // Called with the Browser Window that this bubble is attached to is
   // about to close.
   virtual void BubbleBrowserWindowClosing(BrowserBubble* bubble);
-
-  // Called when the bubble became active / got focus.
-  virtual void BubbleGotFocus(BrowserBubble* bubble);
-
-  // Called when the bubble became inactive / lost focus.
-  virtual void BubbleLostFocus(BrowserBubble* bubble,
-                               gfx::NativeView focused_view);
 #endif  // defined(TOOLKIT_VIEWS)
 
   // NotificationObserver implementation.
@@ -88,7 +81,16 @@ class ExtensionPopupHost :  // NOLINT
                        const NotificationDetails& details);
 
  private:
+  // Posts a task to the current thread's message-loop that will dismiss the
+  // popup.
+  void DismissPopupAsync();
+
 #if defined(TOOLKIT_VIEWS)
+  // A native-view focus listener that monitors when the pop-up should be
+  // dismissed due to focus change events.
+  class PopupFocusListener;
+  scoped_ptr<PopupFocusListener> listener_;
+
   // A popup view that is anchored to and owned by this ExtensionHost.  However,
   // the popup contains its own separate ExtensionHost
   ExtensionPopup* child_popup_;
@@ -103,6 +105,8 @@ class ExtensionPopupHost :  // NOLINT
   // notifications once.
   bool listeners_registered_;
 
+  ScopedRunnableMethodFactory<ExtensionPopupHost> method_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(ExtensionPopupHost);
 };