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

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
 

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);

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.

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));
+}

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,18 +56,15 @@ 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.
   void DismissPopup();
 
 #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);
 };
 

chrome/browser/renderer_host/render_widget_host_view.h

@@ -51,6 +51,11 @@ class RenderWidgetHostView {
   // going to be a regular RenderWidgetHost or a RenderViewHost (a subclass).
   static RenderWidgetHostView* CreateViewForWidget(RenderWidgetHost* widget);
 
+  // Retrieves the RenderWidgetHostView corresponding to the specified
+  // |native_view|, or NULL if there is no such instance.
+  static RenderWidgetHostView* GetRenderWidgetHostViewFromNativeView(
+      gfx::NativeView native_view);
+
   // Perform all the initialization steps necessary for this object to represent
   // a popup (such as a <select> dropdown), then shows the popup at |pos|.
   virtual void InitAsPopup(RenderWidgetHostView* parent_host_view,
@@ -204,6 +209,10 @@ class RenderWidgetHostView {
   }
   const SkBitmap& background() const { return background_; }
 
+  // Returns true if the native view, |native_view|, is contained within in the
+  // widget associated with this RenderWidgetHostView.
+  virtual bool ContainsNativeView(gfx::NativeView native_view) const = 0;
+
  protected:
   // Interface class only, do not construct.
   RenderWidgetHostView() : activatable_(true) {}

chrome/browser/renderer_host/render_widget_host_view_gtk.cc

@@ -43,6 +43,8 @@ static const int kMaxWindowHeight = 4000;
 // TODO(brettw) make this a command line option.
 static const bool kUseGPURendering = false;
 
+static const char* kRenderWidgetHostViewKey = "__RENDER_WIDGET_HOST_VIEW__";
+
 using WebKit::WebInputEventFactory;
 
 // This class is a simple convenience wrapper for Gtk functions. It has only
@@ -102,6 +104,9 @@ class RenderWidgetHostViewGtkWidget {
     g_signal_connect_after(widget, "scroll-event",
                            G_CALLBACK(MouseScrollEvent), host_view);
 
+    g_object_set_data(G_OBJECT(widget), kRenderWidgetHostViewKey,
+                      static_cast<RenderWidgetHostView*>(host_view));
+
     return widget;
   }
 
@@ -744,6 +749,14 @@ void RenderWidgetHostViewGtk::DestroyPluginContainer(
   plugin_container_manager_.DestroyPluginContainer(id);
 }
 
+bool RenderWidgetHostViewGtk::ContainsNativeView(
+    gfx::NativeView native_view) const {
+  // TODO(port)
+  NOTREACHED() <<
+    "RenderWidgetHostViewGtk::ContainsNativeView not implemented.";
+  return false;
+}
+
 void RenderWidgetHostViewGtk::ForwardKeyboardEvent(
     const NativeWebKeyboardEvent& event) {
   if (!host_)
@@ -755,3 +768,12 @@ void RenderWidgetHostViewGtk::ForwardKeyboardEvent(
   }
   host_->ForwardKeyboardEvent(event);
 }
+
+// static
+RenderWidgetHostView*
+    RenderWidgetHostView::GetRenderWidgetHostViewFromNativeView(
+        gfx::NativeView widget) {
+  gpointer user_data = g_object_get_data(G_OBJECT(widget),
+                                         kRenderWidgetHostViewKey);
+  return reinterpret_cast<RenderWidgetHostView*>(user_data);
+}

chrome/browser/renderer_host/render_widget_host_view_gtk.h

@@ -71,6 +71,7 @@ class RenderWidgetHostViewGtk : public RenderWidgetHostView {
   virtual void SetBackground(const SkBitmap& background);
   virtual void CreatePluginContainer(gfx::PluginWindowHandle id);
   virtual void DestroyPluginContainer(gfx::PluginWindowHandle id);
+  virtual bool ContainsNativeView(gfx::NativeView native_view) const;
 
   gfx::NativeView native_view() const { return view_.get(); }
 

chrome/browser/renderer_host/render_widget_host_view_mac.h

@@ -115,6 +115,7 @@ class RenderWidgetHostViewMac : public RenderWidgetHostView {
   virtual void SetWindowVisibility(bool visible);
   virtual void WindowFrameChanged();
   virtual void SetBackground(const SkBitmap& background);
+  virtual bool ContainsNativeView(gfx::NativeView native_view) const;
 
   // Methods associated with GPU plugin instances
   virtual gfx::PluginWindowHandle AllocateFakePluginWindowHandle();

chrome/browser/renderer_host/render_widget_host_view_mac.mm

@@ -93,6 +93,16 @@ -(void)drawInCGLContext:(CGLContextObj)glContext
   return new RenderWidgetHostViewMac(widget);
 }
 
+// static
+RenderWidgetHostView* RenderWidgetHostView::
+    GetRenderWidgetHostViewFromNativeView(gfx::NativeView native_view) {
+  // TODO(port)
+  NOTREACHED() <<
+      "RenderWidgetHostView::GetRenderWidgetHostViewFromNativeView not"
+      "implemented";
+  return NULL;
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // RenderWidgetHostViewMac, public:
 
@@ -645,6 +655,14 @@ -(void)drawInCGLContext:(CGLContextObj)glContext
         render_widget_host_->routing_id(), background));
 }
 
+bool RenderWidgetHostViewMac::ContainsNativeView(
+    gfx::NativeView native_view) const {
+  // TODO(port)
+  NOTREACHED() <<
+    "RenderWidgetHostViewMac::ContainsNativeView not implemented.";
+  return false;
+}
+
 // EditCommandMatcher ---------------------------------------------------------
 
 // This class is used to capture the shortcuts that a given key event maps to.