Suppress PWA install promotion if related Chrome app is already installed

This CL hides the omnibox PWA install icon for installable sites if the
related_applications field lists a Chrome app that's already installed.

Example manifest field:
related_applications: [{
  "platform": "chrome_web_store",
  "id": "<extension id>"
}]

Bug: 949430
Change-Id: Ic0a5a7992811b008a1cceb382255ca1ecad35fbf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1630078
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664627}

chrome/browser/banners/app_banner_manager.cc

@@ -258,6 +258,18 @@ bool AppBannerManager::CheckIfInstalled() {
                                      manifest_.start_url, manifest_url_);
 }
 
+bool AppBannerManager::ShouldDeferToRelatedApplication() const {
+  for (const auto& related_app : manifest_.related_applications) {
+    if (manifest_.prefer_related_applications &&
+        IsSupportedAppPlatform(related_app.platform.string())) {
+      return true;
+    }
+    if (IsRelatedAppInstalled(related_app))
+      return true;
+  }
+  return false;
+}
+
 std::string AppBannerManager::GetAppIdentifier() {
   DCHECK(!manifest_.IsEmpty());
   return manifest_.start_url.spec();
@@ -354,7 +366,7 @@ void AppBannerManager::OnDidPerformInstallableWebAppCheck(
     return;
   }
 
-  if (manifest_.prefer_related_applications) {
+  if (ShouldDeferToRelatedApplication()) {
     SetInstallableWebAppCheckResult(
         InstallableWebAppCheckResult::kByUserRequest);
     Stop(PREFER_RELATED_APPLICATIONS);

chrome/browser/banners/app_banner_manager.h

@@ -175,6 +175,10 @@ class AppBannerManager : public content::WebContentsObserver,
   // Returns whether the site is already installed as a web app.
   bool CheckIfInstalled();
 
+  // Returns whether the site would prefer a related application be installed
+  // instead of the PWA or a related application is already installed.
+  bool ShouldDeferToRelatedApplication() const;
+
   // Return a string identifying this app for metrics.
   virtual std::string GetAppIdentifier();
 
@@ -195,6 +199,14 @@ class AppBannerManager : public content::WebContentsObserver,
   // Returns true if the kBypassAppBannerEngagementChecks flag is set.
   bool ShouldBypassEngagementChecks() const;
 
+  // Returns whether installation of apps from |platform| is supported on the
+  // current device.
+  virtual bool IsSupportedAppPlatform(const base::string16& platform) const = 0;
+
+  // Returns whether |related_app| is already installed.
+  virtual bool IsRelatedAppInstalled(
+      const blink::Manifest::RelatedApplication& related_app) const = 0;
+
   // Returns true if the web app at |start_url| has already been installed, or
   // should be considered installed. On Android, we rely on a heuristic that
   // may yield false negatives or false positives (crbug.com/786268).

chrome/browser/banners/app_banner_manager_android.cc

@@ -386,6 +386,18 @@ void AppBannerManagerAndroid::HideAmbientBadge() {
     infobar_service->RemoveInfoBar(ambient_badge_infobar);
 }
 
+bool AppBannerManagerAndroid::IsSupportedAppPlatform(
+    const base::string16& platform) const {
+  // TODO(https://crbug.com/949430): Implement for Android apps.
+  return false;
+}
+
+bool AppBannerManagerAndroid::IsRelatedAppInstalled(
+    const blink::Manifest::RelatedApplication& related_app) const {
+  // TODO(https://crbug.com/949430): Implement for Android apps.
+  return false;
+}
+
 base::WeakPtr<AppBannerManager> AppBannerManagerAndroid::GetWeakPtr() {
   return weak_factory_.GetWeakPtr();
 }

chrome/browser/banners/app_banner_manager_android.h

@@ -110,6 +110,9 @@ class AppBannerManagerAndroid
   void MaybeShowAmbientBadge() override;
   base::WeakPtr<AppBannerManager> GetWeakPtr() override;
   void InvalidateWeakPtrs() override;
+  bool IsSupportedAppPlatform(const base::string16& platform) const override;
+  bool IsRelatedAppInstalled(
+      const blink::Manifest::RelatedApplication& related_app) const override;
 
  private:
   friend class content::WebContentsUserData<AppBannerManagerAndroid>;

chrome/browser/banners/app_banner_manager_browsertest.cc

@@ -117,6 +117,18 @@ class AppBannerManagerTest : public AppBannerManager {
 
   void InvalidateWeakPtrs() override { weak_factory_.InvalidateWeakPtrs(); }
 
+  bool IsSupportedAppPlatform(const base::string16& platform) const override {
+    return base::EqualsASCII(platform, "chrome_web_store");
+  }
+
+  bool IsRelatedAppInstalled(
+      const blink::Manifest::RelatedApplication& related_app) const override {
+    // Corresponds to the id listed in manifest_listing_related_chrome_app.json.
+    return base::EqualsASCII(related_app.platform.string(),
+                             "chrome_web_store") &&
+           base::EqualsASCII(related_app.id.string(), "installed-extension-id");
+  }
+
  private:
   base::Closure on_done_;
 
@@ -468,18 +480,45 @@ IN_PROC_BROWSER_TEST_F(AppBannerManagerBrowserTest, WebAppBannerReprompt) {
                                 SHOWING_WEB_APP_BANNER, 1);
 }
 
-IN_PROC_BROWSER_TEST_F(AppBannerManagerBrowserTest, PreferRelatedApplications) {
+IN_PROC_BROWSER_TEST_F(AppBannerManagerBrowserTest, PreferRelatedAppUnknown) {
+  std::unique_ptr<AppBannerManagerTest> manager(
+      CreateAppBannerManager(browser()));
+
+  GURL test_url = embedded_test_server()->GetURL(
+      "/banners/manifest_test_page.html?manifest="
+      "manifest_prefer_related_apps_unknown.json");
+  TriggerBannerFlowWithNavigation(browser(), manager.get(), test_url,
+                                  false /* expected_will_show */,
+                                  State::PENDING_ENGAGEMENT);
+}
+
+IN_PROC_BROWSER_TEST_F(AppBannerManagerBrowserTest, PreferRelatedChromeApp) {
   std::unique_ptr<AppBannerManagerTest> manager(
       CreateAppBannerManager(browser()));
   base::HistogramTester histograms;
 
   GURL test_url = embedded_test_server()->GetURL(
       "/banners/manifest_test_page.html?manifest="
-      "manifest_prefer_related_apps_empty.json");
+      "manifest_prefer_related_chrome_app.json");
   TriggerBannerFlowWithNavigation(browser(), manager.get(), test_url,
                                   false /* expected_will_show */,
                                   State::COMPLETE);
+  histograms.ExpectUniqueSample(banners::kInstallableStatusCodeHistogram,
+                                PREFER_RELATED_APPLICATIONS, 1);
+}
 
+IN_PROC_BROWSER_TEST_F(AppBannerManagerBrowserTest,
+                       ListedRelatedChromeAppInstalled) {
+  std::unique_ptr<AppBannerManagerTest> manager(
+      CreateAppBannerManager(browser()));
+  base::HistogramTester histograms;
+
+  GURL test_url = embedded_test_server()->GetURL(
+      "/banners/manifest_test_page.html?manifest="
+      "manifest_listing_related_chrome_app.json");
+  TriggerBannerFlowWithNavigation(browser(), manager.get(), test_url,
+                                  false /* expected_will_show */,
+                                  State::COMPLETE);
   histograms.ExpectUniqueSample(banners::kInstallableStatusCodeHistogram,
                                 PREFER_RELATED_APPLICATIONS, 1);
 }

chrome/browser/banners/app_banner_manager_desktop.cc

@@ -7,6 +7,7 @@
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/feature_list.h"
+#include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "chrome/browser/banners/app_banner_metrics.h"
 #include "chrome/browser/banners/app_banner_settings_helper.h"
@@ -17,10 +18,15 @@
 #include "chrome/browser/web_applications/extensions/bookmark_app_util.h"
 #include "chrome/browser/web_applications/web_app_provider.h"
 #include "chrome/common/chrome_features.h"
+#include "extensions/browser/extension_registry.h"
 #include "extensions/common/constants.h"
 
 namespace {
 
+// Platform values defined in:
+// https://github.com/w3c/manifest/wiki/Platforms
+const char kPlatformChromeWebStore[] = "chrome_web_store";
+
 bool gDisableTriggeringForTesting = false;
 
 }  // namespace
@@ -39,7 +45,9 @@ void AppBannerManagerDesktop::DisableTriggeringForTesting() {
 
 AppBannerManagerDesktop::AppBannerManagerDesktop(
     content::WebContents* web_contents)
-    : AppBannerManager(web_contents) { }
+    : AppBannerManager(web_contents),
+      extension_registry_(extensions::ExtensionRegistry::Get(
+          web_contents->GetBrowserContext())) {}
 
 AppBannerManagerDesktop::~AppBannerManagerDesktop() { }
 
@@ -51,6 +59,28 @@ void AppBannerManagerDesktop::InvalidateWeakPtrs() {
   weak_factory_.InvalidateWeakPtrs();
 }
 
+bool AppBannerManagerDesktop::IsSupportedAppPlatform(
+    const base::string16& platform) const {
+  return base::EqualsASCII(platform, kPlatformChromeWebStore);
+
+  // TODO(https://crbug.com/949430): Implement for ARC apps on Chrome OS.
+}
+
+bool AppBannerManagerDesktop::IsRelatedAppInstalled(
+    const blink::Manifest::RelatedApplication& related_app) const {
+  const base::string16& platform = related_app.platform.string();
+  if (!base::EqualsASCII(platform, kPlatformChromeWebStore))
+    return false;
+
+  std::string id = base::UTF16ToUTF8(related_app.id.string());
+  return !id.empty() &&
+
+         extension_registry_->GetExtensionById(
+             id, extensions::ExtensionRegistry::ENABLED) != nullptr;
+
+  // TODO(https://crbug.com/949430): Implement for ARC apps on Chrome OS.
+}
+
 bool AppBannerManagerDesktop::IsWebAppConsideredInstalled(
     content::WebContents* web_contents,
     const GURL& validated_url,

chrome/browser/banners/app_banner_manager_desktop.h

@@ -12,6 +12,10 @@
 #include "chrome/browser/banners/app_banner_manager.h"
 #include "content/public/browser/web_contents_user_data.h"
 
+namespace extensions {
+class ExtensionRegistry;
+}
+
 namespace web_app {
 enum class InstallResultCode;
 }
@@ -37,6 +41,9 @@ class AppBannerManagerDesktop
   // AppBannerManager overrides.
   base::WeakPtr<AppBannerManager> GetWeakPtr() override;
   void InvalidateWeakPtrs() override;
+  bool IsSupportedAppPlatform(const base::string16& platform) const override;
+  bool IsRelatedAppInstalled(
+      const blink::Manifest::RelatedApplication& related_app) const override;
 
   // Called when the web app install initiated by a banner has completed.
   virtual void DidFinishCreatingWebApp(const web_app::AppId& app_id,
@@ -65,6 +72,7 @@ class AppBannerManagerDesktop
 
   void CreateWebApp(WebappInstallSource install_source);
 
+  extensions::ExtensionRegistry* extension_registry_;
   base::WeakPtrFactory<AppBannerManagerDesktop> weak_factory_{this};
 
   WEB_CONTENTS_USER_DATA_KEY_DECL();

chrome/browser/ui/views/page_action/pwa_install_view_browsertest.cc

@@ -9,6 +9,7 @@
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
 #include "chrome/browser/banners/test_app_banner_manager_desktop.h"
+#include "chrome/browser/extensions/extension_browsertest.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_dialogs.h"
@@ -25,10 +26,12 @@
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/common/referrer.h"
 #include "extensions/browser/extension_system.h"
+#include "net/test/embedded_test_server/http_request.h"
+#include "net/test/embedded_test_server/http_response.h"
 #include "services/network/public/cpp/network_switches.h"
 #include "ui/gfx/color_utils.h"
 
-class PwaInstallViewBrowserTest : public InProcessBrowserTest {
+class PwaInstallViewBrowserTest : public extensions::ExtensionBrowserTest {
  public:
   PwaInstallViewBrowserTest()
       : https_server_(net::EmbeddedTestServer::TYPE_HTTPS) {}
@@ -39,13 +42,16 @@ class PwaInstallViewBrowserTest : public InProcessBrowserTest {
         features::kDesktopPWAsOmniboxInstall);
 
     https_server_.AddDefaultHandlers(GetChromeTestDataDir());
+    https_server_.RegisterRequestHandler(
+        base::BindRepeating(&PwaInstallViewBrowserTest::RequestInterceptor,
+                            base::Unretained(this)));
     ASSERT_TRUE(https_server_.Start());
 
-    InProcessBrowserTest::SetUp();
+    extensions::ExtensionBrowserTest::SetUp();
   }
 
   void SetUpCommandLine(base::CommandLine* command_line) override {
-    InProcessBrowserTest::SetUpCommandLine(command_line);
+    extensions::ExtensionBrowserTest::SetUpCommandLine(command_line);
 
     command_line->AppendSwitchASCII(
         network::switches::kUnsafelyTreatInsecureOriginAsSecure,
@@ -53,6 +59,8 @@ class PwaInstallViewBrowserTest : public InProcessBrowserTest {
   }
 
   void SetUpOnMainThread() override {
+    extensions::ExtensionBrowserTest::SetUpOnMainThread();
+
     pwa_install_view_ =
         BrowserView::GetBrowserViewForBrowser(browser())
             ->toolbar_button_provider()
@@ -66,6 +74,17 @@ class PwaInstallViewBrowserTest : public InProcessBrowserTest {
             web_contents_);
   }
 
+  std::unique_ptr<net::test_server::HttpResponse> RequestInterceptor(
+      const net::test_server::HttpRequest& request) {
+    if (request.relative_url != intercept_request_path_)
+      return nullptr;
+    auto http_response =
+        std::make_unique<net::test_server::BasicHttpResponse>();
+    http_response->set_code(net::HTTP_FOUND);
+    http_response->set_content(intercept_request_response_);
+    return std::move(http_response);
+  }
+
   content::WebContents* GetCurrentTab() {
     return browser()->tab_strip_model()->GetActiveWebContents();
   }
@@ -157,7 +176,10 @@ class PwaInstallViewBrowserTest : public InProcessBrowserTest {
 
  protected:
   base::test::ScopedFeatureList scoped_feature_list_;
+
   net::EmbeddedTestServer https_server_;
+  std::string intercept_request_path_;
+  std::string intercept_request_response_;
 
   PageActionIconView* pwa_install_view_ = nullptr;
   content::WebContents* web_contents_ = nullptr;
@@ -327,19 +349,69 @@ IN_PROC_BROWSER_TEST_F(PwaInstallViewBrowserTest, BouncedInstallIgnored) {
   TestInstallBounce(base::TimeDelta::FromMinutes(70), 0);
 }
 
-// Omnibox install button shouldn't show for a PWA that prefers a related app
-// over the web app.
-IN_PROC_BROWSER_TEST_F(PwaInstallViewBrowserTest,
-                       SuppressForPreferredRelatedApp) {
+// Omnibox install promotion should show if there are no viable related apps
+// even if prefer_related_applications is true.
+IN_PROC_BROWSER_TEST_F(PwaInstallViewBrowserTest, PreferRelatedAppUnknown) {
+  NavigateToURL(
+      https_server_.GetURL("/banners/manifest_test_page.html?manifest="
+                           "manifest_prefer_related_apps_unknown.json"));
+  ASSERT_TRUE(app_banner_manager_->WaitForInstallableCheck());
+
+  EXPECT_TRUE(pwa_install_view_->GetVisible());
+}
+
+// Omnibox install promotion should not show if prefer_related_applications is
+// false but a related Chrome app is installed.
+IN_PROC_BROWSER_TEST_F(PwaInstallViewBrowserTest, PreferRelatedChromeApp) {
   NavigateToURL(
       https_server_.GetURL("/banners/manifest_test_page.html?manifest="
-                           "manifest_prefer_related_apps_empty.json"));
+                           "manifest_prefer_related_chrome_app.json"));
   ASSERT_TRUE(app_banner_manager_->WaitForInstallableCheck());
+
   EXPECT_FALSE(pwa_install_view_->GetVisible());
+  EXPECT_TRUE(base::EqualsASCII(
+      banners::AppBannerManager::GetInstallableWebAppName(web_contents_),
+      "Manifest prefer related chrome app"));
+}
 
-  // Site should still be considered installable even if it's not promoted in
-  // the omnibox.
+// Omnibox install promotion should not show if prefer_related_applications is
+// true and a Chrome app listed as related.
+IN_PROC_BROWSER_TEST_F(PwaInstallViewBrowserTest,
+                       ListedRelatedChromeAppInstalled) {
+  const extensions::Extension* extension =
+      LoadExtension(test_data_dir_.AppendASCII("app"));
+
+  intercept_request_path_ = "/banners/manifest_listing_related_chrome_app.json";
+  intercept_request_response_ = R"(
+    {
+      "name": "Manifest listing related chrome app",
+      "icons": [
+        {
+          "src": "/banners/image-512px.png",
+          "sizes": "512x512",
+          "type": "image/png"
+        }
+      ],
+      "scope": ".",
+      "start_url": ".",
+      "display": "standalone",
+      "prefer_related_applications": false,
+      "related_applications": [{
+        "platform": "chrome_web_store",
+        "id": ")";
+  intercept_request_response_ += extension->id();
+  intercept_request_response_ += R"(",
+        "comment": "This is the id of test/data/extensions/app"
+      }]
+    }
+  )";
+
+  NavigateToURL(https_server_.GetURL(
+      "/banners/manifest_test_page.html?manifest=" + intercept_request_path_));
+  ASSERT_TRUE(app_banner_manager_->WaitForInstallableCheck());
+
+  EXPECT_FALSE(pwa_install_view_->GetVisible());
   EXPECT_TRUE(base::EqualsASCII(
       banners::AppBannerManager::GetInstallableWebAppName(web_contents_),
-      "Manifest prefer related apps empty"));
+      "Manifest listing related chrome app"));
 }

chrome/test/data/banners/manifest_listing_related_chrome_app.json

+{
+  "name": "Manifest listing related chrome app",
+  "icons": [
+    {
+      "src": "/banners/image-512px.png",
+      "sizes": "512x512",
+      "type": "image/png"
+    }
+  ],
+  "scope": ".",
+  "start_url": ".",
+  "display": "standalone",
+  "prefer_related_applications": false,
+  "related_applications": [{
+    "platform": "chrome_web_store",
+    "id": "installed-extension-id"
+  }]
+}

chrome/test/data/banners/manifest_prefer_related_apps_empty.json → chrome/test/data/banners/manifest_prefer_related_apps_unknown.json

 {
-  "name": "Manifest prefer related apps empty",
+  "name": "Manifest prefer related apps unknown",
   "icons": [
     {
       "src": "/banners/image-512px.png",
@@ -10,5 +10,9 @@
   "scope": ".",
   "start_url": ".",
   "display": "standalone",
-  "prefer_related_applications": true
+  "prefer_related_applications": true,
+  "related_applications": [{
+    "platform": "unknown platform",
+    "id": "abcdef"
+  }]
 }

chrome/test/data/banners/manifest_prefer_related_chrome_app.json

+{
+  "name": "Manifest prefer related chrome app",
+  "icons": [
+    {
+      "src": "/banners/image-512px.png",
+      "sizes": "512x512",
+      "type": "image/png"
+    }
+  ],
+  "scope": ".",
+  "start_url": ".",
+  "display": "standalone",
+  "prefer_related_applications": true,
+  "related_applications": [{
+    "platform": "chrome_web_store",
+    "id": "some id that's not installed"
+  }]
+}