Reland: Disable install event.prompt() after user has already installed the site

Before this CL we would only disable prompt() if the user installs the site
via the menu or the web app banner. This updates the web app omnibox install
button to also disable prompt() from being called.
Without disabling prompt() the site can re-prompt the user to install
which results in the browser crashing.

Note: This CL is a "quick fix" to resolve the browser crash in a mergeable
CL. A set of follow up CLs will change AppBannerManager to listen to web app
install events and clear the prompt() handle that way instead.

This is a reland of:
https://chromium-review.googlesource.com/c/chromium/src/+/1640742
The previous CL's test hit an MSAN error where the test class was being cast
as a sibling class erroneously. This CL moves the tests to
AppBannerManagerDesktopBrowserTest so the test class can subclass the existing
AppBannerManagerDesktop and avoid a sibling cast.
See reland diff:
https://chromium-review.googlesource.com/c/chromium/src/+/1653789/1..8

(cherry picked from commit 1e3a18a3eb3ae1b49a215422e2778f7ae8779741)

Bug: 956810
Change-Id: I31a3c4a6d6ae78a6a629d47119a89a4118efb702
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1653789
Auto-Submit: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#668655}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1662953
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/branch-heads/3809@{#398}
Cr-Branched-From: d82dec1a818f378c464ba307ddd9c92133eac355-refs/heads/master@{#665002}

diff --git a/chrome/browser/banners/app_banner_manager.cc b/chrome/browser/banners/app_banner_manager.cc
index 0d2301a..9fa0c4b 100644
--- a/chrome/browser/banners/app_banner_manager.cc
+++ b/chrome/browser/banners/app_banner_manager.cc

@@ -210,6 +210,20 @@
   observer_list_.RemoveObserver(observer);
 }
 
+void AppBannerManager::MigrateObserverListForTesting(
+    content::WebContents* web_contents) {
+  AppBannerManager* existing_manager = FromWebContents(web_contents);
+  for (Observer& observer : existing_manager->observer_list_)
+    observer.ObserveAppBannerManager(this);
+  DCHECK(existing_manager->observer_list_.begin() ==
+         existing_manager->observer_list_.end())
+      << "Old observer list must be empty after transfer to test instance.";
+}
+
+bool AppBannerManager::IsPromptAvailableForTesting() const {
+  return binding_.is_bound();
+}
+
 AppBannerManager::AppBannerManager(content::WebContents* web_contents)
     : content::WebContentsObserver(web_contents),
       SiteEngagementObserver(SiteEngagementService::Get(
@@ -489,16 +503,6 @@
     observer.OnInstallableWebAppStatusUpdated();
 }
 
-void AppBannerManager::MigrateObserverListForTesting(
-    content::WebContents* web_contents) {
-  AppBannerManager* existing_manager = FromWebContents(web_contents);
-  for (Observer& observer : existing_manager->observer_list_)
-    observer.ObserveAppBannerManager(this);
-  DCHECK(existing_manager->observer_list_.begin() ==
-         existing_manager->observer_list_.end())
-      << "Old observer list must be empty after transfer to test instance.";
-}
-
 void AppBannerManager::Stop(InstallableStatusCode code) {
   ReportStatus(code);
 

diff --git a/chrome/browser/banners/app_banner_manager.h b/chrome/browser/banners/app_banner_manager.h
index 38304c03..67a9638 100644
--- a/chrome/browser/banners/app_banner_manager.h
+++ b/chrome/browser/banners/app_banner_manager.h

@@ -152,7 +152,8 @@
   // performs logging related to the app installation. Appinstalled event is
   // redundant for the beforeinstallprompt event's promise being resolved, but
   // is required by the install event spec.
-  void OnInstall(bool is_native, blink::WebDisplayMode display);
+  // This is virtual for testing.
+  virtual void OnInstall(bool is_native, blink::WebDisplayMode display);
 
   // Sends a message to the renderer that the user accepted the banner.
   void SendBannerAccepted();
@@ -163,6 +164,17 @@
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
 
+  virtual base::WeakPtr<AppBannerManager> GetWeakPtr() = 0;
+
+  // Used by test subclasses that replace the existing AppBannerManager
+  // instance. The observer list must be transferred over to avoid dangling
+  // pointers in the observers.
+  void MigrateObserverListForTesting(content::WebContents* web_contents);
+
+  // Returns whether the site can call "event.prompt()" to prompt the user to
+  // install the site.
+  bool IsPromptAvailableForTesting() const;
+
  protected:
   explicit AppBannerManager(content::WebContents* web_contents);
   ~AppBannerManager() override;
@@ -189,7 +201,6 @@
   // alerting websites that a banner is about to be created.
   virtual std::string GetBannerType();
 
-  virtual base::WeakPtr<AppBannerManager> GetWeakPtr() = 0;
   virtual void InvalidateWeakPtrs() = 0;
 
   // Returns true if |has_sufficient_engagement_| is true or
@@ -296,11 +307,6 @@
   State state() const { return state_; }
   bool IsRunning() const;
 
-  // Used by test subclasses that replace the existing AppBannerManager
-  // instance. The observer list must be transferred over to avoid dangling
-  // pointers in the observers.
-  void MigrateObserverListForTesting(content::WebContents* web_contents);
-
   // The URL for which the banner check is being conducted.
   GURL validated_url_;
 

diff --git a/chrome/browser/banners/app_banner_manager_desktop_browsertest.cc b/chrome/browser/banners/app_banner_manager_desktop_browsertest.cc
index 68c1804..9c4f757 100644
--- a/chrome/browser/banners/app_banner_manager_desktop_browsertest.cc
+++ b/chrome/browser/banners/app_banner_manager_desktop_browsertest.cc

@@ -12,15 +12,20 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/bind_test_util.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/banners/app_banner_manager_browsertest_base.h"
 #include "chrome/browser/banners/app_banner_manager_desktop.h"
 #include "chrome/browser/banners/app_banner_settings_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_command_controller.h"
 #include "chrome/browser/ui/browser_dialogs.h"
+#include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/page_action/page_action_icon_container.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/web_applications/web_app_dialog_utils.h"
 #include "chrome/browser/web_applications/components/web_app_constants.h"
+#include "chrome/common/chrome_features.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test_utils.h"
@@ -37,11 +42,14 @@
 
   static FakeAppBannerManagerDesktop* CreateForWebContents(
       content::WebContents* web_contents) {
-    web_contents->SetUserData(
-        UserDataKey(),
-        std::make_unique<FakeAppBannerManagerDesktop>(web_contents));
-    return static_cast<FakeAppBannerManagerDesktop*>(
-        web_contents->GetUserData(UserDataKey()));
+    auto banner_manager =
+        std::make_unique<FakeAppBannerManagerDesktop>(web_contents);
+    banner_manager->MigrateObserverListForTesting(web_contents);
+
+    FakeAppBannerManagerDesktop* result = banner_manager.get();
+    web_contents->SetUserData(FakeAppBannerManagerDesktop::UserDataKey(),
+                              std::move(banner_manager));
+    return result;
   }
 
   // Configures a callback to be invoked when the app banner flow finishes.
@@ -49,7 +57,19 @@
 
   State state() { return AppBannerManager::state(); }
 
+  void AwaitAppInstall() {
+    base::RunLoop loop;
+    on_install_ = loop.QuitClosure();
+    loop.Run();
+  }
+
  protected:
+  void OnInstall(bool is_native, blink::WebDisplayMode display) override {
+    AppBannerManager::OnInstall(is_native, display);
+    if (on_install_)
+      std::move(on_install_).Run();
+  }
+
   void OnFinished() {
     if (on_done_) {
       base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
@@ -74,6 +94,7 @@
 
  private:
   base::OnceClosure on_done_;
+  base::OnceClosure on_install_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeAppBannerManagerDesktop);
 };
@@ -224,3 +245,65 @@
     EXPECT_TRUE(callback_called);
   }
 }
+
+IN_PROC_BROWSER_TEST_F(AppBannerManagerDesktopBrowserTest,
+                       InstallPromptAfterUserMenuInstall) {
+  // TODO(https://crbug.com/915043): Fix this test for the unified install
+  // codepath. This fails under unified install because the menu install path
+  // relies on extensions::TabHelper to call AppBannerManager::OnInstall which
+  // is no longer the case under unified install. This will be fixed in a follow
+  // up patch when AppBannerManager calls OnInstall by itself via
+  // AppRegistrarObserver::OnWebAppInstalled().
+  if (base::FeatureList::IsEnabled(features::kDesktopPWAsUnifiedInstall))
+    return;
+
+  FakeAppBannerManagerDesktop* manager =
+      FakeAppBannerManagerDesktop::CreateForWebContents(
+          browser()->tab_strip_model()->GetActiveWebContents());
+
+  {
+    base::RunLoop run_loop;
+    manager->PrepareDone(run_loop.QuitClosure());
+
+    ui_test_utils::NavigateToURL(browser(),
+                                 GetBannerURLWithAction("stash_event"));
+    run_loop.Run();
+    EXPECT_EQ(State::PENDING_PROMPT, manager->state());
+  }
+
+  // Install the app via the menu instead of the banner.
+  chrome::SetAutoAcceptPWAInstallConfirmationForTesting(true);
+  browser()->command_controller()->ExecuteCommand(IDC_INSTALL_PWA);
+  manager->AwaitAppInstall();
+  chrome::SetAutoAcceptPWAInstallConfirmationForTesting(false);
+
+  EXPECT_FALSE(manager->IsPromptAvailableForTesting());
+}
+
+IN_PROC_BROWSER_TEST_F(AppBannerManagerDesktopBrowserTest,
+                       InstallPromptAfterUserOmniboxInstall) {
+  FakeAppBannerManagerDesktop* manager =
+      FakeAppBannerManagerDesktop::CreateForWebContents(
+          browser()->tab_strip_model()->GetActiveWebContents());
+
+  {
+    base::RunLoop run_loop;
+    manager->PrepareDone(run_loop.QuitClosure());
+
+    ui_test_utils::NavigateToURL(browser(),
+                                 GetBannerURLWithAction("stash_event"));
+    run_loop.Run();
+    EXPECT_EQ(State::PENDING_PROMPT, manager->state());
+  }
+
+  // Install the app via the menu instead of the banner.
+  chrome::SetAutoAcceptPWAInstallConfirmationForTesting(true);
+  browser()
+      ->window()
+      ->GetOmniboxPageActionIconContainer()
+      ->ExecutePageActionIconForTesting(PageActionIconType::kPwaInstall);
+  manager->AwaitAppInstall();
+  chrome::SetAutoAcceptPWAInstallConfirmationForTesting(false);
+
+  EXPECT_FALSE(manager->IsPromptAvailableForTesting());
+}

diff --git a/chrome/browser/ui/browser_commands.cc b/chrome/browser/ui/browser_commands.cc
index 85f4b4dc..1e73054 100644
--- a/chrome/browser/ui/browser_commands.cc
+++ b/chrome/browser/ui/browser_commands.cc

@@ -1323,7 +1323,8 @@
 void CreateBookmarkAppFromCurrentWebContents(Browser* browser,
                                              bool force_shortcut_app) {
   base::RecordAction(UserMetricsAction("CreateHostedApp"));
-  web_app::CreateWebAppFromCurrentWebContents(browser, force_shortcut_app);
+  web_app::CreateWebAppFromCurrentWebContents(browser, force_shortcut_app,
+                                              base::DoNothing());
 }
 
 bool CanCreateBookmarkApp(const Browser* browser) {

diff --git a/chrome/browser/ui/page_action/page_action_icon_container.h b/chrome/browser/ui/page_action/page_action_icon_container.h
index f09ce779..e2a76c4 100644
--- a/chrome/browser/ui/page_action/page_action_icon_container.h
+++ b/chrome/browser/ui/page_action/page_action_icon_container.h

@@ -26,6 +26,8 @@
   // Signals a page action icon to update its visual state if it is present in
   // the browser window.
   virtual void UpdatePageActionIcon(PageActionIconType type) = 0;
+
+  virtual void ExecutePageActionIconForTesting(PageActionIconType type) = 0;
 };
 
 #endif  // CHROME_BROWSER_UI_PAGE_ACTION_PAGE_ACTION_ICON_CONTAINER_H_

diff --git a/chrome/browser/ui/views/page_action/omnibox_page_action_icon_container_view.cc b/chrome/browser/ui/views/page_action/omnibox_page_action_icon_container_view.cc
index a1072f3f..cf458b30 100644
--- a/chrome/browser/ui/views/page_action/omnibox_page_action_icon_container_view.cc
+++ b/chrome/browser/ui/views/page_action/omnibox_page_action_icon_container_view.cc

@@ -134,6 +134,13 @@
     icon->Update();
 }
 
+void OmniboxPageActionIconContainerView::ExecutePageActionIconForTesting(
+    PageActionIconType type) {
+  PageActionIconView* icon = GetPageActionIconView(type);
+  if (icon)
+    icon->ExecuteForTesting();
+}
+
 bool OmniboxPageActionIconContainerView::
     ActivateFirstInactiveBubbleForAccessibility() {
   for (PageActionIconView* icon : page_action_icons_) {

diff --git a/chrome/browser/ui/views/page_action/omnibox_page_action_icon_container_view.h b/chrome/browser/ui/views/page_action/omnibox_page_action_icon_container_view.h
index c588322..01496529 100644
--- a/chrome/browser/ui/views/page_action/omnibox_page_action_icon_container_view.h
+++ b/chrome/browser/ui/views/page_action/omnibox_page_action_icon_container_view.h

@@ -69,6 +69,7 @@
 
   // PageActionIconContainer:
   void UpdatePageActionIcon(PageActionIconType type) override;
+  void ExecutePageActionIconForTesting(PageActionIconType type) override;
 
  private:
   // views::View:

diff --git a/chrome/browser/ui/views/page_action/pwa_install_view.cc b/chrome/browser/ui/views/page_action/pwa_install_view.cc
index bdaeae717..412fbcf1 100644
--- a/chrome/browser/ui/views/page_action/pwa_install_view.cc
+++ b/chrome/browser/ui/views/page_action/pwa_install_view.cc

@@ -10,6 +10,7 @@
 #include "chrome/browser/banners/app_banner_manager.h"
 #include "chrome/browser/installable/installable_metrics.h"
 #include "chrome/browser/ui/web_applications/web_app_dialog_utils.h"
+#include "chrome/browser/web_applications/components/web_app_constants.h"
 #include "chrome/browser/web_applications/components/web_app_tab_helper_base.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/omnibox/browser/vector_icons.h"
@@ -54,9 +55,20 @@
 
 void PwaInstallView::OnExecuting(PageActionIconView::ExecuteSource source) {
   base::RecordAction(base::UserMetricsAction("PWAInstallIcon"));
-  web_app::CreateWebAppFromManifest(GetWebContents(),
-                                    WebappInstallSource::OMNIBOX_INSTALL_ICON,
-                                    base::DoNothing());
+  content::WebContents* web_contents = GetWebContents();
+  // TODO(https://crbug.com/956810): Make AppBannerManager listen for
+  // installations instead of having to notify it from every installation UI
+  // surface.
+  auto* manager = banners::AppBannerManager::FromWebContents(web_contents);
+  web_app::CreateWebAppFromManifest(
+      web_contents, WebappInstallSource::OMNIBOX_INSTALL_ICON,
+      base::BindOnce(
+          [](base::WeakPtr<banners::AppBannerManager> manager,
+             const web_app::AppId& app_id, web_app::InstallResultCode code) {
+            if (manager && code == web_app::InstallResultCode::kSuccess)
+              manager->OnInstall(false, blink::kWebDisplayModeStandalone);
+          },
+          manager ? manager->GetWeakPtr() : nullptr));
 }
 
 views::BubbleDialogDelegateView* PwaInstallView::GetBubble() const {

diff --git a/chrome/browser/ui/views/toolbar/toolbar_page_action_icon_container_view.cc b/chrome/browser/ui/views/toolbar/toolbar_page_action_icon_container_view.cc
index da46d67..3fcf042 100644
--- a/chrome/browser/ui/views/toolbar/toolbar_page_action_icon_container_view.cc
+++ b/chrome/browser/ui/views/toolbar/toolbar_page_action_icon_container_view.cc

@@ -106,6 +106,13 @@
     icon->Update();
 }
 
+void ToolbarPageActionIconContainerView::ExecutePageActionIconForTesting(
+    PageActionIconType type) {
+  PageActionIconView* icon = GetIconView(type);
+  if (icon)
+    icon->ExecuteForTesting();
+}
+
 SkColor ToolbarPageActionIconContainerView::GetPageActionInkDropColor() const {
   return GetToolbarInkDropBaseColor(this);
 }

diff --git a/chrome/browser/ui/views/toolbar/toolbar_page_action_icon_container_view.h b/chrome/browser/ui/views/toolbar/toolbar_page_action_icon_container_view.h
index 0558126..d525bc7 100644
--- a/chrome/browser/ui/views/toolbar/toolbar_page_action_icon_container_view.h
+++ b/chrome/browser/ui/views/toolbar/toolbar_page_action_icon_container_view.h

@@ -39,6 +39,7 @@
 
   // PageActionIconContainer:
   void UpdatePageActionIcon(PageActionIconType icon_type) override;
+  void ExecutePageActionIconForTesting(PageActionIconType icon_type) override;
 
   // PageActionIconView::Delegate:
   SkColor GetPageActionInkDropColor() const override;

diff --git a/chrome/browser/ui/web_applications/web_app_dialog_utils.cc b/chrome/browser/ui/web_applications/web_app_dialog_utils.cc
index 16d61236..2d69f63 100644
--- a/chrome/browser/ui/web_applications/web_app_dialog_utils.cc
+++ b/chrome/browser/ui/web_applications/web_app_dialog_utils.cc

@@ -80,8 +80,10 @@
   return provider->install_manager().CanInstallWebApp(web_contents);
 }
 
-void CreateWebAppFromCurrentWebContents(Browser* browser,
-                                        bool force_shortcut_app) {
+void CreateWebAppFromCurrentWebContents(
+    Browser* browser,
+    bool force_shortcut_app,
+    WebAppInstalledCallback installed_callback) {
   DCHECK(CanCreateWebApp(browser));
 
   content::WebContents* web_contents =
@@ -89,8 +91,6 @@
   auto* provider = WebAppProvider::GetForWebContents(web_contents);
   DCHECK(provider);
 
-  WebAppInstalledCallback installed_callback = base::DoNothing();
-
   WebappInstallSource install_source =
       InstallableMetrics::GetInstallSource(web_contents, InstallTrigger::MENU);
 

diff --git a/chrome/browser/ui/web_applications/web_app_dialog_utils.h b/chrome/browser/ui/web_applications/web_app_dialog_utils.h
index 0c91f59..41159af 100644
--- a/chrome/browser/ui/web_applications/web_app_dialog_utils.h
+++ b/chrome/browser/ui/web_applications/web_app_dialog_utils.h

@@ -25,13 +25,15 @@
 // Returns true if a WebApp installation is allowed for the current page.
 bool CanCreateWebApp(const Browser* browser);
 
-// Initiates install of a WebApp for the current page.
-void CreateWebAppFromCurrentWebContents(Browser* browser,
-                                        bool force_shortcut_app);
-
 using WebAppInstalledCallback =
     base::OnceCallback<void(const AppId& app_id, InstallResultCode code)>;
 
+// Initiates install of a WebApp for the current page.
+void CreateWebAppFromCurrentWebContents(
+    Browser* browser,
+    bool force_shortcut_app,
+    WebAppInstalledCallback installed_callback);
+
 // Starts install of a WebApp for a given |web_contents|, initiated from
 // a promotional banner or omnibox install icon.
 // Returns false if WebApps are disabled for the profile behind |web_contents|.

diff --git a/chrome/test/base/test_browser_window.h b/chrome/test/base/test_browser_window.h
index f8f433a..46eb04e 100644
--- a/chrome/test/base/test_browser_window.h
+++ b/chrome/test/base/test_browser_window.h

@@ -228,6 +228,7 @@
 
     // PageActionIconContainer:
     void UpdatePageActionIcon(PageActionIconType type) override {}
+    void ExecutePageActionIconForTesting(PageActionIconType type) override {}
 
    private:
     DISALLOW_COPY_AND_ASSIGN(TestOmniboxPageActionIconContainer);