WebApps: Ignore manifest updating for not-locally-installed web apps

Web apps that sync across devices are not considered locally installed for
non-Chrome OS devices. This CL ensures we don't accidentally install them
as part of manifest updating.

Bug: 926083
Change-Id: I34ba3610b04a3cf62e6c7bd362bed2fd770d7b55
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1831737
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Auto-Submit: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Alexey Baskakov <loyso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#701440}

chrome/browser/web_applications/components/app_registry_controller.h

@@ -36,6 +36,10 @@ class AppRegistryController {
   virtual void SetAppLaunchContainer(const AppId& app_id,
                                      LaunchContainer launch_container) = 0;
 
+  virtual void SetAppIsLocallyInstalledForTesting(
+      const AppId& app_id,
+      bool is_locally_installed) = 0;
+
   // Safe downcast:
   virtual WebAppSyncBridge* AsWebAppSyncBridge() = 0;
 

chrome/browser/web_applications/components/manifest_update_manager.cc

@@ -36,7 +36,7 @@ void ManifestUpdateManager::MaybeUpdate(const GURL& url,
   if (!base::FeatureList::IsEnabled(features::kDesktopPWAsLocalUpdating))
     return;
 
-  if (app_id.empty()) {
+  if (app_id.empty() || !registrar_->IsLocallyInstalled(app_id)) {
     NotifyResult(url, ManifestUpdateResult::kNoAppInScope);
     return;
   }

chrome/browser/web_applications/components/manifest_update_manager_browsertest.cc

@@ -14,6 +14,7 @@
 #include "chrome/browser/installable/installable_metrics.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
+#include "chrome/browser/web_applications/components/app_registry_controller.h"
 #include "chrome/browser/web_applications/components/install_finalizer.h"
 #include "chrome/browser/web_applications/components/install_manager.h"
 #include "chrome/browser/web_applications/components/web_app_provider_base.h"
@@ -138,8 +139,6 @@ class ManifestUpdateManagerBrowserTest : public InProcessBrowserTest {
     GURL app_url = GetAppURL();
     ui_test_utils::NavigateToURL(browser(), app_url);
 
-    auto* provider = WebAppProviderBase::GetProviderBase(browser()->profile());
-
     AppId app_id;
     base::RunLoop run_loop;
     InstallManager::InstallParams params;
@@ -148,7 +147,7 @@ class ManifestUpdateManagerBrowserTest : public InProcessBrowserTest {
     params.add_to_quick_launch_bar = false;
     params.bypass_service_worker_check = true;
     params.require_manifest = false;
-    provider->install_manager().InstallWebAppWithParams(
+    GetProvider().install_manager().InstallWebAppWithParams(
         browser()->tab_strip_model()->GetActiveWebContents(), params,
         WebappInstallSource::OMNIBOX_INSTALL_ICON,
         base::BindLambdaForTesting(
@@ -173,6 +172,10 @@ class ManifestUpdateManagerBrowserTest : public InProcessBrowserTest {
     return std::move(awaiter).AwaitNextResult();
   }
 
+  WebAppProviderBase& GetProvider() {
+    return *WebAppProviderBase::GetProviderBase(browser()->profile());
+  }
+
  private:
   base::test::ScopedFeatureList scoped_feature_list_;
 
@@ -245,9 +248,7 @@ IN_PROC_BROWSER_TEST_F(ManifestUpdateManagerBrowserTest,
   GURL url = GetAppURL();
   UpdateCheckResultAwaiter awaiter(browser(), url);
   ui_test_utils::NavigateToURL(browser(), url);
-  WebAppProviderBase::GetProviderBase(browser()->profile())
-      ->install_finalizer()
-      .UninstallWebApp(app_id, base::DoNothing());
+  GetProvider().install_finalizer().UninstallWebApp(app_id, base::DoNothing());
   EXPECT_EQ(std::move(awaiter).AwaitNextResult(),
             ManifestUpdateResult::kAppUninstalled);
 }
@@ -370,6 +371,30 @@ IN_PROC_BROWSER_TEST_F(ManifestUpdateManagerBrowserTest,
             ManifestUpdateResult::kAppDataInvalid);
 }
 
+IN_PROC_BROWSER_TEST_F(ManifestUpdateManagerBrowserTest,
+                       CheckIgnoresNonLocalApps) {
+  const char* manifest_template = R"(
+    {
+      "name": "Test app name",
+      "start_url": ".",
+      "scope": "/",
+      "display": "standalone",
+      "icons": $1,
+      "theme_color": "$2"
+    }
+  )";
+  OverrideManifest(manifest_template, {kInstallableIconList, "blue"});
+  AppId app_id = InstallWebApp();
+
+  GetProvider().registry_controller().SetAppIsLocallyInstalledForTesting(app_id,
+                                                                         false);
+  EXPECT_FALSE(GetProvider().registrar().IsLocallyInstalled(app_id));
+
+  OverrideManifest(manifest_template, {kInstallableIconList, "red"});
+  EXPECT_EQ(GetResultAfterPageLoad(GetAppURL(), &app_id),
+            ManifestUpdateResult::kNoAppInScope);
+}
+
 IN_PROC_BROWSER_TEST_F(ManifestUpdateManagerBrowserTest,
                        CheckFindsThemeColorChange) {
   const char* manifest_template = R"(

chrome/browser/web_applications/extensions/bookmark_app_registry_controller.cc

@@ -9,6 +9,7 @@
 #include "base/one_shot_event.h"
 #include "chrome/browser/extensions/launch_util.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/web_applications/extensions/bookmark_app_util.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
@@ -55,6 +56,13 @@ void BookmarkAppRegistryController::SetAppLaunchContainer(
   }
 }
 
+void BookmarkAppRegistryController::SetAppIsLocallyInstalledForTesting(
+    const web_app::AppId& app_id,
+    bool is_locally_installed) {
+  SetBookmarkAppIsLocallyInstalled(profile(), GetExtension(app_id),
+                                   is_locally_installed);
+}
+
 web_app::WebAppSyncBridge* BookmarkAppRegistryController::AsWebAppSyncBridge() {
   return nullptr;
 }

chrome/browser/web_applications/extensions/bookmark_app_registry_controller.h

@@ -23,6 +23,8 @@ class BookmarkAppRegistryController : public web_app::AppRegistryController {
   void SetAppLaunchContainer(
       const web_app::AppId& app_id,
       web_app::LaunchContainer launch_container) override;
+  void SetAppIsLocallyInstalledForTesting(const web_app::AppId& app_id,
+                                          bool is_locally_installed) override;
   web_app::WebAppSyncBridge* AsWebAppSyncBridge() override;
 
  private:

chrome/browser/web_applications/web_app_sync_bridge.cc

@@ -132,6 +132,15 @@ void WebAppSyncBridge::SetAppLaunchContainer(const AppId& app_id,
     web_app->SetLaunchContainer(launch_container);
 }
 
+void WebAppSyncBridge::SetAppIsLocallyInstalledForTesting(
+    const AppId& app_id,
+    bool is_locally_installed) {
+  ScopedRegistryUpdate update(this);
+  WebApp* web_app = update->UpdateApp(app_id);
+  if (web_app)
+    web_app->SetIsLocallyInstalled(is_locally_installed);
+}
+
 WebAppSyncBridge* WebAppSyncBridge::AsWebAppSyncBridge() {
   return this;
 }

chrome/browser/web_applications/web_app_sync_bridge.h

@@ -58,6 +58,8 @@ class WebAppSyncBridge : public AppRegistryController,
   void Init(base::OnceClosure callback) override;
   void SetAppLaunchContainer(const AppId& app_id,
                              LaunchContainer launch_container) override;
+  void SetAppIsLocallyInstalledForTesting(const AppId& app_id,
+                                          bool is_locally_installed) override;
   WebAppSyncBridge* AsWebAppSyncBridge() override;
 
  private: