Fix an ExtensionPopup crash on browser window close.

Check for NULL |activation_client| in ExtensionPopup::OnWidgetDestroying.

Add a BrowserActionInteractiveTest that crashes without this fix.
Add a BrowserActionTestUtil::InspectPopup helper on Views.

BUG=327776
TEST=Inspecting an extension popup bubble and closing the underlying browser window does not crash.
R=sky@chromium.org,finnur@chromium.org

Review URL: https://codereview.chromium.org/275783002

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

chrome/browser/extensions/api/extension_action/browser_action_interactive_test.cc

@@ -263,5 +263,30 @@ IN_PROC_BROWSER_TEST_F(BrowserActionInteractiveTest, TabSwitchClosesPopup) {
   EXPECT_FALSE(BrowserActionTestUtil(browser()).HasPopup());
 }
 
+#if defined(TOOLKIT_VIEWS)
+// Test closing the browser while inspecting an extension popup with dev tools.
+IN_PROC_BROWSER_TEST_F(BrowserActionInteractiveTest, CloseBrowserWithDevTools) {
+  if (!ShouldRunPopupTest())
+    return;
+
+  // Load a first extension that can open a popup.
+  ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII(
+      "browser_action/popup")));
+  const Extension* extension = GetSingleLoadedExtension();
+  ASSERT_TRUE(extension) << message_;
+
+  // Open an extension popup by clicking the browser action button.
+  content::WindowedNotificationObserver frame_observer(
+      content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME,
+      content::NotificationService::AllSources());
+  BrowserActionTestUtil(browser()).InspectPopup(0);
+  frame_observer.Wait();
+  EXPECT_TRUE(BrowserActionTestUtil(browser()).HasPopup());
+
+  // Close the browser window, this should not cause a crash.
+  chrome::CloseWindow(browser());
+}
+#endif  // TOOLKIT_VIEWS
+
 }  // namespace
 }  // namespace extensions

chrome/browser/extensions/browser_action_test_util.h

@@ -31,6 +31,9 @@ class BrowserActionTestUtil {
 #if defined(TOOLKIT_VIEWS)
   // Returns the ExtensionAction for the given index.
   ExtensionAction* GetExtensionAction(int index);
+
+  // Inspects the extension popup for the action at the given index.
+  void InspectPopup(int index);
 #endif
 
   // Returns whether the browser action at |index| has a non-null icon. Note

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

@@ -158,7 +158,10 @@ void ExtensionPopup::OnWidgetDestroying(views::Widget* widget) {
   aura::Window* bubble_window = GetWidget()->GetNativeWindow();
   aura::client::ActivationClient* activation_client =
       aura::client::GetActivationClient(bubble_window->GetRootWindow());
-  activation_client->RemoveObserver(this);
+  // If the popup was being inspected with devtools and the browser window was
+  // closed, then the root window and activation client are already destroyed.
+  if (activation_client)
+    activation_client->RemoveObserver(this);
 #endif
 }
 

chrome/browser/ui/views/toolbar/browser_action_test_util_views.cc

@@ -19,10 +19,8 @@
 namespace {
 
 BrowserActionsContainer* GetContainer(Browser* browser) {
-  BrowserActionsContainer* container =
-      browser->window()->GetBrowserWindowTesting()->GetToolbarView()->
-          browser_actions();
-  return container;
+  return browser->window()->GetBrowserWindowTesting()->GetToolbarView()->
+      browser_actions();
 }
 
 }  // namespace
@@ -41,6 +39,10 @@ ExtensionAction* BrowserActionTestUtil::GetExtensionAction(int index) {
                        button()->extension());
 }
 
+void BrowserActionTestUtil::InspectPopup(int index) {
+  GetContainer(browser_)->InspectPopup(GetExtensionAction(index));
+}
+
 bool BrowserActionTestUtil::HasIcon(int index) {
   return GetContainer(browser_)->GetBrowserActionViewAt(index)->button()->
       HasIcon();
@@ -78,8 +80,7 @@ gfx::Rect BrowserActionTestUtil::GetPopupBounds() {
 }
 
 bool BrowserActionTestUtil::HidePopup() {
-  BrowserActionsContainer* container = GetContainer(browser_);
-  container->HidePopup();
+  GetContainer(browser_)->HidePopup();
   return !HasPopup();
 }