desktop-pwas: Check policy app install state when checking for promotability

When a user has a web app installed then uninstalled via policy we leave
behind some policy install info in their prefs. This info is used by
CanCreateWebApp() to determine whether there is an app already installed
by policy.
This causes a bug where users cannot install web apps after they've been
removed from policy.

This CL updates the check to also check whether the app is actually
installed and not just check for the presence of the pref data.

Bug: 1090182
Change-Id: I736118cec4e0b1ff34ae335965dc7b8816c20837
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2224417
Auto-Submit: Alan Cutter <alancutter@chromium.org>
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#774395}

chrome/browser/banners/app_banner_manager_desktop.cc

@@ -157,17 +157,16 @@ bool AppBannerManagerDesktop::IsExternallyInstalledWebApp() {
     // Use manifest as source of truth if available.
     web_app::AppId manifest_app_id =
         web_app::GenerateAppIdFromURL(manifest_.start_url);
-    return registrar().HasExternalApp(manifest_app_id);
+    // TODO(crbug.com/1090182): Make HasExternalApp imply IsLocallyInstalled.
+    return registrar().IsLocallyInstalled(manifest_app_id) &&
+           registrar().HasExternalApp(manifest_app_id);
   }
-  // Check URL wouldn't collide with an external app's install URL.
-  const GURL& url = web_contents()->GetLastCommittedURL();
-  if (registrar().LookupExternalAppId(url).has_value())
-    return true;
   // Check an app created for this page wouldn't collide with any external app.
-  web_app::AppId possible_app_id = web_app::GenerateAppIdFromURL(url);
-  if (registrar().HasExternalApp(possible_app_id))
-    return true;
-  return false;
+  web_app::AppId possible_app_id =
+      web_app::GenerateAppIdFromURL(web_contents()->GetLastCommittedURL());
+  // TODO(crbug.com/1090182): Make HasExternalApp imply IsLocallyInstalled.
+  return registrar().IsLocallyInstalled(possible_app_id) &&
+         registrar().HasExternalApp(possible_app_id);
 }
 
 bool AppBannerManagerDesktop::ShouldAllowWebAppReplacementInstall() {

chrome/browser/banners/app_banner_manager_desktop_browsertest.cc

@@ -27,8 +27,10 @@
 #include "chrome/browser/ui/web_applications/test/web_app_browsertest_util.h"
 #include "chrome/browser/ui/web_applications/web_app_dialog_utils.h"
 #include "chrome/browser/web_applications/components/external_install_options.h"
+#include "chrome/browser/web_applications/components/pending_app_manager.h"
 #include "chrome/browser/web_applications/components/web_app_constants.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
+#include "chrome/browser/web_applications/components/web_app_provider_base.h"
 #include "chrome/common/chrome_features.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/browser/web_contents.h"
@@ -260,12 +262,14 @@ IN_PROC_BROWSER_TEST_F(AppBannerManagerDesktopBrowserTest,
       TestAppBannerManagerDesktop::FromWebContents(
           browser()->tab_strip_model()->GetActiveWebContents());
 
+  // Install web app by policy.
   web_app::ExternalInstallOptions options =
       web_app::CreateInstallOptions(GetBannerURL());
   options.install_source = web_app::ExternalInstallSource::kExternalPolicy;
   options.user_display_mode = web_app::DisplayMode::kBrowser;
   web_app::PendingAppManagerInstall(browser()->profile(), options);
 
+  // Run promotability check.
   {
     base::RunLoop run_loop;
     manager->PrepareDone(run_loop.QuitClosure());
@@ -280,4 +284,48 @@ IN_PROC_BROWSER_TEST_F(AppBannerManagerDesktopBrowserTest,
   EXPECT_FALSE(manager->IsPromptAvailableForTesting());
 }
 
+IN_PROC_BROWSER_TEST_F(AppBannerManagerDesktopBrowserTest,
+                       PolicyAppUninstalled_Prompt) {
+  TestAppBannerManagerDesktop* manager =
+      TestAppBannerManagerDesktop::FromWebContents(
+          browser()->tab_strip_model()->GetActiveWebContents());
+  Profile* profile = browser()->profile();
+
+  // Install web app by policy.
+  web_app::ExternalInstallOptions options =
+      web_app::CreateInstallOptions(GetBannerURL());
+  options.install_source = web_app::ExternalInstallSource::kExternalPolicy;
+  options.user_display_mode = web_app::DisplayMode::kBrowser;
+  web_app::PendingAppManagerInstall(profile, options);
+
+  // Uninstall web app by policy.
+  {
+    base::RunLoop run_loop;
+    web_app::WebAppProviderBase::GetProviderBase(profile)
+        ->pending_app_manager()
+        .UninstallApps({GetBannerURL()},
+                       web_app::ExternalInstallSource::kExternalPolicy,
+                       base::BindLambdaForTesting(
+                           [&run_loop](const GURL& app_url, bool succeeded) {
+                             EXPECT_TRUE(succeeded);
+                             run_loop.Quit();
+                           }));
+    run_loop.Run();
+  }
+
+  // Run promotability check.
+  {
+    base::RunLoop run_loop;
+    manager->PrepareDone(run_loop.QuitClosure());
+
+    ui_test_utils::NavigateToURL(browser(), GetBannerURL());
+    run_loop.Run();
+    EXPECT_EQ(State::PENDING_PROMPT, manager->state());
+  }
+
+  EXPECT_EQ(AppBannerManager::InstallableWebAppCheckResult::kPromotable,
+            manager->GetInstallableWebAppCheckResultForTesting());
+  EXPECT_TRUE(manager->IsPromptAvailableForTesting());
+}
+
 }  // namespace banners