Call AppBannerManager::OnInstall in BookmarkAppHelper's finishing callback.

On desktop platforms, calling AppBannerManager::OnInstall in
BookmarkAppHelper::FinishInstallation resulted in a bug where the userChoice
promise on beforeinstalleventprompt was not resolved. This is because
OnInstall() clears Mojo bindings, wiping out the connection between
AppBannerManager and the beforeinstallpromptevent in the renderer.

This CL moves the call to AppBannerManager::OnInstall from
BookmarkAppHelper::FinishInstallation to BookmarkAppHelper::callback_. For
installations from app banners on desktop, this is
AppBannerManagerDesktop::DidFinishCreatingBookmarkApp; for installations
from the menu, it is extensions::TabHelper::FinishCreateBookmarkApp. This
means that for banners, OnInstall can be called after resolving the
userChoice promise, fixing the bug.

Tests are refactored and a new AppBannerManagerDesktopBrowserTest is
introduced to test the events are fired as expected after installation.

BUG=890848

Change-Id: I26a122e261ad1104af69443d9230e5458e271d24
Reviewed-on: https://chromium-review.googlesource.com/c/1256392
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596544}

chrome/browser/banners/app_banner_manager_browsertest.cc

@@ -7,7 +7,6 @@
 
 #include "base/bind.h"
 #include "base/callback.h"
-#include "base/command_line.h"
 #include "base/macros.h"
 #include "base/run_loop.h"
 #include "base/strings/string16.h"
@@ -16,6 +15,7 @@
 #include "base/test/scoped_feature_list.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "chrome/browser/banners/app_banner_manager.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_metrics.h"
 #include "chrome/browser/banners/app_banner_settings_helper.h"
@@ -26,25 +26,7 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_features.h"
-#include "chrome/common/chrome_switches.h"
-#include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
-#include "content/public/test/browser_test_utils.h"
-#include "net/base/url_util.h"
-#include "net/test/embedded_test_server/embedded_test_server.h"
-
-namespace {
-
-void ExecuteScript(Browser* browser, std::string script, bool with_gesture) {
-  content::WebContents* web_contents =
-      browser->tab_strip_model()->GetActiveWebContents();
-  if (with_gesture)
-    EXPECT_TRUE(content::ExecuteScript(web_contents, script));
-  else
-    EXPECT_TRUE(content::ExecuteScriptWithoutUserGesture(web_contents, script));
-}
-
-}  // namespace
 
 namespace banners {
 
@@ -132,8 +114,8 @@ class AppBannerManagerTest : public AppBannerManager {
     AppBannerManager::OnBannerPromptReply(std::move(controller), reply,
                                           referrer);
     if (on_banner_prompt_reply_) {
-      base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
-                                                    on_banner_prompt_reply_);
+      base::ThreadTaskRunnerHandle::Get()->PostTask(
+          FROM_HERE, std::move(on_banner_prompt_reply_));
     }
   }
 
@@ -150,49 +132,24 @@ class AppBannerManagerTest : public AppBannerManager {
   DISALLOW_COPY_AND_ASSIGN(AppBannerManagerTest);
 };
 
-class AppBannerManagerBrowserTest : public InProcessBrowserTest {
+class AppBannerManagerBrowserTest : public AppBannerManagerBrowserTestBase {
  public:
+  AppBannerManagerBrowserTest() : AppBannerManagerBrowserTestBase() {}
+
   void SetUpOnMainThread() override {
-    feature_list_.InitWithFeatures({}, {features::kExperimentalAppBanners,
-                                        features::kDesktopPWAWindowing});
     AppBannerSettingsHelper::SetTotalEngagementToTrigger(10);
     SiteEngagementScore::SetParamValuesForTesting();
-    ASSERT_TRUE(embedded_test_server()->Start());
+
+    feature_list_.InitWithFeatures({}, {features::kExperimentalAppBanners,
+                                        features::kDesktopPWAWindowing});
 
     // Make sure app banners are disabled in the browser, otherwise they will
     // interfere with the test.
     AppBannerManagerDesktop::DisableTriggeringForTesting();
-    InProcessBrowserTest::SetUpOnMainThread();
+    AppBannerManagerBrowserTestBase::SetUpOnMainThread();
   }
 
  protected:
-  // Returns a test server URL to a page with generates a banner.
-  GURL GetBannerURL() {
-    return embedded_test_server()->GetURL("/banners/manifest_test_page.html");
-  }
-
-  // Returns a test server URL with "action" = |value| set in the query string.
-  GURL GetBannerURLWithAction(const std::string& action) {
-    GURL url = GetBannerURL();
-    return net::AppendQueryParameter(url, "action", action);
-  }
-
-  // Returns a test server URL with |manifest_url| injected as the manifest tag.
-  GURL GetBannerURLWithManifest(const std::string& manifest_url) {
-    GURL url = GetBannerURL();
-    return net::AppendQueryParameter(
-        url, "manifest", embedded_test_server()->GetURL(manifest_url).spec());
-  }
-
-  // Returns a test server URL with |manifest_url| injected as the manifest tag
-  // and |key| = |value| in the query string.
-  GURL GetBannerURLWithManifestAndQuery(const std::string& manifest_url,
-                                        const std::string& key,
-                                        const std::string& value) {
-    GURL url = GetBannerURLWithManifest(manifest_url);
-    return net::AppendQueryParameter(url, key, value);
-  }
-
   std::unique_ptr<AppBannerManagerTest> CreateAppBannerManager(
       Browser* browser) {
     content::WebContents* web_contents =
@@ -317,6 +274,8 @@ class AppBannerManagerBrowserTest : public InProcessBrowserTest {
 
  private:
   base::test::ScopedFeatureList feature_list_;
+
+  DISALLOW_COPY_AND_ASSIGN(AppBannerManagerBrowserTest);
 };
 
 IN_PROC_BROWSER_TEST_F(AppBannerManagerBrowserTest, WebAppBannerCreated) {
@@ -614,8 +573,8 @@ IN_PROC_BROWSER_TEST_F(AppBannerManagerBrowserTest,
   // Now let the page call prompt with a gesture. The banner should be shown.
   TriggerBannerFlow(
       browser(), manager.get(),
-      base::BindOnce(&ExecuteScript, browser(), "callStashedPrompt();",
-                     true /* with_gesture */),
+      base::BindOnce(&AppBannerManagerBrowserTest::ExecuteScript, browser(),
+                     "callStashedPrompt();", true /* with_gesture */),
       true /* expected_will_show */, State::COMPLETE);
 
   histograms.ExpectTotalCount(banners::kMinutesHistogram, 1);
@@ -656,8 +615,8 @@ IN_PROC_BROWSER_TEST_F(AppBannerManagerBrowserTest,
   // Trigger prompt() and expect the banner to be shown.
   TriggerBannerFlow(
       browser(), manager.get(),
-      base::BindOnce(&ExecuteScript, browser(), "callStashedPrompt();",
-                     true /* with_gesture */),
+      base::BindOnce(&AppBannerManagerBrowserTest::ExecuteScript, browser(),
+                     "callStashedPrompt();", true /* with_gesture */),
       true /* expected_will_show */, State::COMPLETE);
 
   histograms.ExpectTotalCount(banners::kMinutesHistogram, 1);
@@ -686,8 +645,8 @@ IN_PROC_BROWSER_TEST_F(AppBannerManagerBrowserTest,
   // Call prompt to show the banner.
   TriggerBannerFlow(
       browser(), manager.get(),
-      base::BindOnce(&ExecuteScript, browser(), "callStashedPrompt();",
-                     true /* with_gesture */),
+      base::BindOnce(&AppBannerManagerBrowserTest::ExecuteScript, browser(),
+                     "callStashedPrompt();", true /* with_gesture */),
       true /* expected_will_show */, State::COMPLETE);
 
   // Dismiss the banner.
@@ -700,8 +659,8 @@ IN_PROC_BROWSER_TEST_F(AppBannerManagerBrowserTest,
   // Call prompt again to show the banner again.
   TriggerBannerFlow(
       browser(), manager.get(),
-      base::BindOnce(&ExecuteScript, browser(), "callStashedPrompt();",
-                     true /* with_gesture */),
+      base::BindOnce(&AppBannerManagerBrowserTest::ExecuteScript, browser(),
+                     "callStashedPrompt();", true /* with_gesture */),
       true /* expected_will_show */, State::COMPLETE);
 
   histograms.ExpectTotalCount(banners::kMinutesHistogram, 1);

chrome/browser/banners/app_banner_manager_browsertest_base.cc

+// Copyright 2018 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/banners/app_banner_manager_browsertest_base.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/test/browser_test_utils.h"
+#include "net/base/url_util.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
+
+AppBannerManagerBrowserTestBase::AppBannerManagerBrowserTestBase() = default;
+
+AppBannerManagerBrowserTestBase::~AppBannerManagerBrowserTestBase() = default;
+
+void AppBannerManagerBrowserTestBase::SetUpOnMainThread() {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  InProcessBrowserTest::SetUpOnMainThread();
+}
+
+GURL AppBannerManagerBrowserTestBase::GetBannerURL() {
+  return embedded_test_server()->GetURL("/banners/manifest_test_page.html");
+}
+
+// static
+void AppBannerManagerBrowserTestBase::ExecuteScript(Browser* browser,
+                                                    const std::string& script,
+                                                    bool with_gesture) {
+  content::WebContents* web_contents =
+      browser->tab_strip_model()->GetActiveWebContents();
+  if (with_gesture)
+    EXPECT_TRUE(content::ExecuteScript(web_contents, script));
+  else
+    EXPECT_TRUE(content::ExecuteScriptWithoutUserGesture(web_contents, script));
+}
+
+GURL AppBannerManagerBrowserTestBase::GetBannerURLWithAction(
+    const std::string& action) {
+  GURL url = GetBannerURL();
+  return net::AppendQueryParameter(url, "action", action);
+}
+
+GURL AppBannerManagerBrowserTestBase::GetBannerURLWithManifest(
+    const std::string& manifest_url) {
+  GURL url = GetBannerURL();
+  return net::AppendQueryParameter(
+      url, "manifest", embedded_test_server()->GetURL(manifest_url).spec());
+}
+
+GURL AppBannerManagerBrowserTestBase::GetBannerURLWithManifestAndQuery(
+    const std::string& manifest_url,
+    const std::string& key,
+    const std::string& value) {
+  GURL url = GetBannerURLWithManifest(manifest_url);
+  return net::AppendQueryParameter(url, key, value);
+}

chrome/browser/banners/app_banner_manager_browsertest_base.h

+// Copyright 2018 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.
+
+#ifndef CHROME_BROWSER_BANNERS_APP_BANNER_MANAGER_BROWSERTEST_BASE_H_
+#define CHROME_BROWSER_BANNERS_APP_BANNER_MANAGER_BROWSERTEST_BASE_H_
+
+#include <string>
+
+#include "base/macros.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "url/gurl.h"
+
+class Browser;
+
+// Common base class for browser tests exercising AppBannerManager. Contains
+// methods for generating test URLs that trigger app banners.
+class AppBannerManagerBrowserTestBase : public InProcessBrowserTest {
+ public:
+  AppBannerManagerBrowserTestBase();
+  ~AppBannerManagerBrowserTestBase() override;
+  void SetUpOnMainThread() override;
+
+ protected:
+  // Executes JavaScript in |script| in the active WebContents of |browser|,
+  // possibly with a user gesture depending on |with_gesture|.
+  static void ExecuteScript(Browser* browser,
+                            const std::string& script,
+                            bool with_gesture);
+
+  // Returns a test server URL to a page with generates a banner.
+  GURL GetBannerURL();
+
+  // Returns a test server URL with "action" = |value| set in the query string.
+  GURL GetBannerURLWithAction(const std::string& action);
+
+  // Returns a test server URL with |manifest_url| injected as the manifest tag.
+  GURL GetBannerURLWithManifest(const std::string& manifest_url);
+
+  // Returns a test server URL with |manifest_url| injected as the manifest tag
+  // and |key| = |value| in the query string.
+  GURL GetBannerURLWithManifestAndQuery(const std::string& manifest_url,
+                                        const std::string& key,
+                                        const std::string& value);
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(AppBannerManagerBrowserTestBase);
+};
+
+#endif  // CHROME_BROWSER_BANNERS_APP_BANNER_MANAGER_BROWSERTEST_BASE_H_

chrome/browser/banners/app_banner_manager_desktop.cc

@@ -57,6 +57,9 @@ void AppBannerManagerDesktop::DidFinishCreatingBookmarkApp(
     SendBannerAccepted();
     AppBannerSettingsHelper::RecordBannerInstallEvent(
         contents, GetAppIdentifier(), AppBannerSettingsHelper::WEB);
+
+    // OnInstall must be called last since it resets Mojo bindings.
+    OnInstall(false /* is_native app */, blink::kWebDisplayModeStandalone);
     return;
   }
 

chrome/browser/banners/app_banner_manager_desktop.h

@@ -35,13 +35,15 @@ class AppBannerManagerDesktop
  protected:
   explicit AppBannerManagerDesktop(content::WebContents* web_contents);
 
- private:
-  friend class content::WebContentsUserData<AppBannerManagerDesktop>;
-
   // AppBannerManager overrides.
   void DidFinishCreatingBookmarkApp(
       const extensions::Extension* extension,
       const WebApplicationInfo& web_app_info) override;
+
+ private:
+  friend class content::WebContentsUserData<AppBannerManagerDesktop>;
+
+  // AppBannerManager overrides.
   bool IsWebAppConsideredInstalled(content::WebContents* web_contents,
                                    const GURL& validated_url,
                                    const GURL& start_url,

chrome/browser/banners/app_banner_manager_desktop_browsertest.cc

+// Copyright 2018 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 <memory>
+#include <utility>
+
+#include "base/callback.h"
+#include "base/macros.h"
+#include "base/run_loop.h"
+#include "base/strings/string16.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/threading/thread_task_runner_handle.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_dialogs.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.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"
+#include "extensions/common/extension.h"
+
+namespace {
+
+class FakeAppBannerManagerDesktop : public banners::AppBannerManagerDesktop {
+ public:
+  explicit FakeAppBannerManagerDesktop(content::WebContents* web_contents)
+      : AppBannerManagerDesktop(web_contents) {}
+
+  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()));
+  }
+
+  // Configures a callback to be invoked when the app banner flow finishes.
+  void PrepareDone(base::OnceClosure on_done) { on_done_ = std::move(on_done); }
+
+  State state() { return AppBannerManager::state(); }
+
+ protected:
+  void OnFinished() {
+    if (on_done_) {
+      base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
+                                                    std::move(on_done_));
+    }
+  }
+
+  void DidFinishCreatingBookmarkApp(
+      const extensions::Extension* extension,
+      const WebApplicationInfo& web_app_info) override {
+    AppBannerManagerDesktop::DidFinishCreatingBookmarkApp(extension,
+                                                          web_app_info);
+    OnFinished();
+  }
+
+  void UpdateState(AppBannerManager::State state) override {
+    AppBannerManager::UpdateState(state);
+
+    if (state == AppBannerManager::State::PENDING_ENGAGEMENT ||
+        state == AppBannerManager::State::PENDING_PROMPT) {
+      OnFinished();
+    }
+  }
+
+ private:
+  base::OnceClosure on_done_;
+
+  DISALLOW_COPY_AND_ASSIGN(FakeAppBannerManagerDesktop);
+};
+
+}  // anonymous namespace
+
+using State = banners::AppBannerManager::State;
+
+class AppBannerManagerDesktopBrowserTest
+    : public AppBannerManagerBrowserTestBase {
+ public:
+  AppBannerManagerDesktopBrowserTest() : AppBannerManagerBrowserTestBase() {}
+
+  void SetUpOnMainThread() override {
+    // Trigger banners instantly.
+    AppBannerSettingsHelper::SetTotalEngagementToTrigger(0);
+    chrome::SetAutoAcceptPWAInstallDialogForTesting(true);
+
+    feature_list_.InitWithFeatures(
+        {features::kExperimentalAppBanners, features::kDesktopPWAWindowing},
+        {});
+
+    AppBannerManagerBrowserTestBase::SetUpOnMainThread();
+  }
+
+  void TearDown() override {
+    chrome::SetAutoAcceptPWAInstallDialogForTesting(false);
+  }
+
+ private:
+  base::test::ScopedFeatureList feature_list_;
+
+  DISALLOW_COPY_AND_ASSIGN(AppBannerManagerDesktopBrowserTest);
+};
+
+IN_PROC_BROWSER_TEST_F(AppBannerManagerDesktopBrowserTest,
+                       WebAppBannerResolvesUserChoice) {
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  auto* manager =
+      FakeAppBannerManagerDesktop::CreateForWebContents(web_contents);
+
+  {
+    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());
+  }
+
+  {
+    // Trigger the installation prompt and wait for installation to occur.
+    base::RunLoop run_loop;
+    manager->PrepareDone(run_loop.QuitClosure());
+    ExecuteScript(browser(), "callStashedPrompt();", true /* with_gesture */);
+    run_loop.Run();
+    EXPECT_EQ(State::COMPLETE, manager->state());
+  }
+
+  // Ensure that the userChoice promise resolves.
+  const base::string16 title = base::ASCIIToUTF16("Got userChoice: accepted");
+  content::TitleWatcher watcher(web_contents, title);
+  EXPECT_EQ(title, watcher.WaitAndGetTitle());
+}
+
+IN_PROC_BROWSER_TEST_F(AppBannerManagerDesktopBrowserTest,
+                       WebAppBannerFiresAppInstalled) {
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  auto* manager =
+      FakeAppBannerManagerDesktop::CreateForWebContents(web_contents);
+
+  {
+    base::RunLoop run_loop;
+    manager->PrepareDone(run_loop.QuitClosure());
+
+    ui_test_utils::NavigateToURL(
+        browser(), GetBannerURLWithAction("verify_appinstalled_stash_event"));
+    run_loop.Run();
+    EXPECT_EQ(State::PENDING_PROMPT, manager->state());
+  }
+
+  {
+    // Trigger the installation prompt and wait for installation to occur.
+    base::RunLoop run_loop;
+    manager->PrepareDone(run_loop.QuitClosure());
+    ExecuteScript(browser(), "callStashedPrompt();", true /* with_gesture */);
+    run_loop.Run();
+    EXPECT_EQ(State::COMPLETE, manager->state());
+  }
+
+  // Ensure that the appinstalled event fires.
+  const base::string16 title =
+      base::ASCIIToUTF16("Got appinstalled: listener, attr");
+  content::TitleWatcher watcher(web_contents, title);
+  EXPECT_EQ(title, watcher.WaitAndGetTitle());
+}

chrome/browser/extensions/bookmark_app_helper.cc

@@ -59,7 +59,6 @@
 #include "extensions/common/url_pattern.h"
 #include "net/base/load_flags.h"
 #include "net/url_request/url_request.h"
-#include "third_party/blink/public/common/manifest/web_display_mode.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 
 #if defined(OS_MACOSX)
@@ -588,25 +587,24 @@ void BookmarkAppHelper::FinishInstallation(const Extension* extension) {
       AppBannerSettingsHelper::APP_BANNER_EVENT_DID_ADD_TO_HOMESCREEN,
       base::Time::Now());
 
-  Browser* browser = chrome::FindBrowserWithWebContents(contents_);
-  // If there is no browser, it means that the app is being installed in the
-  // background. We skip some steps in this case.
-  const bool silent_install = !browser;
-
-  if (!silent_install && banners::AppBannerManagerDesktop::IsEnabled() &&
-      web_app_info_.open_as_window) {
-    banners::AppBannerManagerDesktop::FromWebContents(contents_)->OnInstall(
-        false /* is_native app */, blink::kWebDisplayModeStandalone);
-  }
-
 #if !defined(OS_CHROMEOS)
   // Pin the app to the relevant launcher depending on the OS.
   Profile* current_profile = profile_->GetOriginalProfile();
 #endif  // !defined(OS_CHROMEOS)
 
+  // If there is no browser, it means that the app is being installed in the
+  // background. We skip some steps in this case.
+  const bool silent_install =
+      (chrome::FindBrowserWithWebContents(contents_) == nullptr);
+
 // On Mac, shortcuts are automatically created for hosted apps when they are
 // installed, so there is no need to create them again.
-#if !defined(OS_MACOSX)
+#if defined(OS_MACOSX)
+  if (!silent_install && !base::CommandLine::ForCurrentProcess()->HasSwitch(
+                             ::switches::kDisableHostedAppShimCreation)) {
+    web_app::RevealAppShimInFinderForApp(current_profile, extension);
+  }
+#else
   if (create_shortcuts_) {
 #if !defined(OS_CHROMEOS)
     web_app::ShortcutLocations creation_locations;
@@ -638,13 +636,6 @@ void BookmarkAppHelper::FinishInstallation(const Extension* extension) {
   }
 #endif  // !defined(OS_MACOSX)
 
-#if defined(OS_MACOSX)
-  if (!silent_install && !base::CommandLine::ForCurrentProcess()->HasSwitch(
-                             ::switches::kDisableHostedAppShimCreation)) {
-    web_app::RevealAppShimInFinderForApp(current_profile, extension);
-  }
-#endif
-
   callback_.Run(extension, web_app_info_);
 }
 

chrome/browser/extensions/tab_helper.cc

@@ -9,6 +9,7 @@
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
+#include "chrome/browser/banners/app_banner_manager_desktop.h"
 #include "chrome/browser/extensions/activity_log/activity_log.h"
 #include "chrome/browser/extensions/api/bookmark_manager_private/bookmark_manager_private_api.h"
 #include "chrome/browser/extensions/api/declarative_content/chrome_content_rules_registry.h"
@@ -60,6 +61,7 @@
 #include "extensions/common/manifest_handlers/icons_handler.h"
 #include "services/service_manager/public/cpp/interface_provider.h"
 #include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
+#include "third_party/blink/public/common/manifest/web_display_mode.h"
 #include "url/url_constants.h"
 
 using content::NavigationController;
@@ -187,6 +189,14 @@ void TabHelper::InvokeForContentRulesRegistries(const Func& func) {
 void TabHelper::FinishCreateBookmarkApp(
     const Extension* extension,
     const WebApplicationInfo& web_app_info) {
+  // Send the 'appinstalled' event and ensure any beforeinstallpromptevent
+  // cannot trigger installation again.
+  if (banners::AppBannerManagerDesktop::IsEnabled() &&
+      web_app_info.open_as_window) {
+    banners::AppBannerManagerDesktop::FromWebContents(web_contents())
+        ->OnInstall(false /* is_native app */,
+                    blink::kWebDisplayModeStandalone);
+  }
   pending_web_app_action_ = NONE;
 }
 

chrome/test/BUILD.gn

@@ -508,6 +508,9 @@ test("browser_tests") {
       "../browser/autofill/form_structure_browsertest.cc",
       "../browser/background_fetch/background_fetch_browsertest.cc",
       "../browser/banners/app_banner_manager_browsertest.cc",
+      "../browser/banners/app_banner_manager_browsertest_base.cc",
+      "../browser/banners/app_banner_manager_browsertest_base.h",
+      "../browser/banners/app_banner_manager_desktop_browsertest.cc",
       "../browser/bitmap_fetcher/bitmap_fetcher_browsertest.cc",
       "../browser/browser_encoding_browsertest.cc",
       "../browser/browsing_data/browsing_data_cache_storage_helper_browsertest.cc",

chrome/test/data/banners/main.js

@@ -4,6 +4,7 @@
 
 const Action = {
   VERIFY_APPINSTALLED: 'verify_appinstalled',
+  VERIFY_APPINSTALLED_STASH_EVENT: 'verify_appinstalled_stash_event',
   VERIFY_PROMPT_APPINSTALLED: 'verify_prompt_appinstalled',
   VERIFY_BEFOREINSTALLPROMPT: 'verify_beforeinstallprompt',
   CALL_PROMPT_DELAYED: 'call_prompt_delayed',
@@ -76,6 +77,10 @@ function addPromptListener(action) {
     e.preventDefault();
 
     switch (action) {
+      case Action.VERIFY_APPINSTALLED_STASH_EVENT:
+        stashedEvent = e;
+        verifyEvents('appinstalled');
+        break;
       case Action.CALL_PROMPT_DELAYED:
         setTimeout(callPrompt, 0, e);
         break;
@@ -133,6 +138,7 @@ function initialize() {
       addPromptListener(Action.STASH_EVENT);
       addClickListener(action);
       break;
+    case Action.VERIFY_APPINSTALLED_STASH_EVENT:
     case Action.CALL_PROMPT_DELAYED:
     case Action.CALL_PROMPT_IN_HANDLER:
     case Action.CALL_PROMPT_NO_USERCHOICE: