Close ExtensionPopup on tab switches; add test.

chrome/browser/ui/views/extensions/extension_popup.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/extensions/extension_popup.h"
 
 #include "base/bind.h"
 #include "base/message_loop/message_loop.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/devtools/devtools_window.h"
 #include "chrome/browser/extensions/extension_view_host.h"
 #include "chrome/browser/extensions/extension_view_host_factory.h"
 #include "chrome/browser/platform_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/host_desktop.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "content/public/browser/devtools_agent_host.h"
 #include "content/public/browser/devtools_manager.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_view.h"
 #include "ui/gfx/insets.h"
 #include "ui/views/layout/fill_layout.h"
@@ skipping 59 matching lines @@
 
   // Wait to show the popup until the contained host finishes loading.
   registrar_.Add(this, content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME,
                  content::Source<WebContents>(host->host_contents()));
 
   // Listen for the containing view calling window.close();
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_HOST_VIEW_SHOULD_CLOSE,
                  content::Source<BrowserContext>(host->browser_context()));
   content::DevToolsManager::GetInstance()->AddAgentStateCallback(
       devtools_callback_);
+
+  Browser* browser = host_->view()->browser();
+  if (browser && browser->tab_strip_model())

[sky, 2013/12/20 00:14:51]

Can browser really be NULL here? browser->tab_strip_model should never to NULL.

[msw, 2013/12/20 00:46:28]

Done, I removed both checks. I tracked down that the ExtensionViewViews |host|
should never be constructed with a NULL |browser_| for extension popups, which
are VIEW_TYPE_EXTENSION_POPUP, as per extensions::CreateViewHostForExtension:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ext...
That's the only instantiation site, and its |browser_| member is never modified.
+CC jamescook for any additional comments or concerns. 

+    browser->tab_strip_model()->AddObserver(this);
 }
 
 ExtensionPopup::~ExtensionPopup() {
   content::DevToolsManager::GetInstance()->RemoveAgentStateCallback(
       devtools_callback_);
+
+  Browser* browser = host_->view()->browser();
+  if (browser && browser->tab_strip_model())
+    browser->tab_strip_model()->RemoveObserver(this);
 }
 
 void ExtensionPopup::Observe(int type,
                              const content::NotificationSource& source,
                              const content::NotificationDetails& details) {
   switch (type) {
     case content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME:
       DCHECK(content::Source<WebContents>(host()->host_contents()) == source);
       // Show when the content finishes loading and its width is computed.
       ShowBubble();
@@ skipping 73 matching lines @@
   chrome::HostDesktopType host_desktop_type =
       chrome::GetHostDesktopTypeForNativeWindow(this_window);
   if (!inspect_with_devtools_ && anchor_window == gained_active &&
       host_desktop_type != chrome::HOST_DESKTOP_TYPE_ASH &&
       this_window->GetRootWindow() == anchor_window->GetRootWindow() &&
       gained_active->transient_parent() != this_window)
     GetWidget()->Close();
 }
 #endif
 
+void ExtensionPopup::ActiveTabChanged(content::WebContents* old_contents,
+                                      content::WebContents* new_contents,
+                                      int index,
+                                      int reason) {
+  GetWidget()->Close();
+}
+
 // static
 ExtensionPopup* ExtensionPopup::ShowPopup(const GURL& url,
                                           Browser* browser,
                                           views::View* anchor_view,
                                           views::BubbleBorder::Arrow arrow,
                                           ShowAction show_action) {
   extensions::ExtensionViewHost* host =
       extensions::ExtensionViewHostFactory::CreatePopupHost(url, browser);
   ExtensionPopup* popup = new ExtensionPopup(host, anchor_view, arrow,
       show_action);
@@ skipping 29 matching lines @@
 
   // Focus on the host contents when the bubble is first shown.
   host()->host_contents()->GetView()->Focus();
 
   if (inspect_with_devtools_) {
     DevToolsWindow::ToggleDevToolsWindow(host()->render_view_host(),
         true,
         DevToolsToggleAction::ShowConsole());
   }
 }