Fix crash when ExtensionPopup::host_ is nullptr.

If a user uninstalls multiple extensions simultaneously, where the first
one has an extension popup open, the second notification will cause a
crash because the bubble will try to access |host_|, which is already
nullptr. To fix this, we unregister from further notifications when
destroying |host_|. Also adds a test.

Bug: 1099456
Change-Id: Ia89e5d1e25e801d83bae49c858ff8f95488cd311
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2292330
Commit-Queue: Bret Sepulveda <bsep@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#790302}

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

@@ -128,12 +128,17 @@ void ExtensionPopup::OnExtensionUnloaded(
     content::BrowserContext* browser_context,
     const extensions::Extension* extension,
     extensions::UnloadedExtensionReason reason) {
+  CHECK(host_);
   if (extension->id() == host_->extension_id()) {
     // To ensure |extension_view_| cannot receive any messages that cause it to
     // try to access the host during Widget closure, destroy it immediately.
     RemoveChildViewT(extension_view_);
 
     host_.reset();
+    // Stop observing the registry immediately to prevent any subsequent
+    // notifications, since Widget::Close is asynchronous.
+    extension_registry_observer_.RemoveAll();
+
     GetWidget()->Close();
   }
 }

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

@@ -201,13 +201,20 @@ class ExtensionsMenuViewBrowserTest : public ExtensionsToolbarBrowserTest {
           container->GetViewForId(extensions()[0]->id())->GetVisible());
     }
   }
+
   void TriggerSingleExtensionButton() {
+    ASSERT_EQ(1u, GetExtensionsMenuItemViews().size());
+    TriggerExtensionButton(0u);
+  }
+
+  void TriggerExtensionButton(size_t item_index) {
     auto menu_items = GetExtensionsMenuItemViews();
-    ASSERT_EQ(1u, menu_items.size());
+    ASSERT_LT(item_index, menu_items.size());
+
     ui::MouseEvent click_event(ui::ET_MOUSE_RELEASED, gfx::Point(),
                                gfx::Point(), base::TimeTicks(),
                                ui::EF_LEFT_MOUSE_BUTTON, 0);
-    menu_items[0]
+    menu_items[item_index]
         ->primary_action_button_for_testing()
         ->button_controller()
         ->OnMouseReleased(click_event);
@@ -477,6 +484,33 @@ IN_PROC_BROWSER_TEST_F(ExtensionsMenuViewBrowserTest,
   EXPECT_TRUE(GetVisibleToolbarActionViews().empty());
 }
 
+// Test for crbug.com/1099456.
+IN_PROC_BROWSER_TEST_F(ExtensionsMenuViewBrowserTest,
+                       RemoveMultipleExtensionsWhileShowingPopup) {
+  auto& id1 = LoadTestExtension("extensions/simple_with_popup")->id();
+  auto& id2 = LoadTestExtension("extensions/uitest/window_open")->id();
+  ShowUi("");
+  VerifyUi();
+  TriggerExtensionButton(0u);
+
+  ExtensionsContainer* const extensions_container =
+      BrowserView::GetBrowserViewForBrowser(browser())
+          ->toolbar()
+          ->extensions_container();
+  ASSERT_NE(nullptr, extensions_container->GetPoppedOutAction());
+
+  auto* extension_service =
+      extensions::ExtensionSystem::Get(browser()->profile())
+          ->extension_service();
+
+  extension_service->DisableExtension(
+      id1, extensions::disable_reason::DISABLE_USER_ACTION);
+  extension_service->DisableExtension(
+      id2, extensions::disable_reason::DISABLE_USER_ACTION);
+
+  EXPECT_EQ(nullptr, extensions_container->GetPoppedOutAction());
+}
+
 IN_PROC_BROWSER_TEST_F(ExtensionsMenuViewBrowserTest,
                        TriggeringExtensionClosesMenu) {
   LoadTestExtension("extensions/trigger_actions/browser_action");