[System PWAs] Make System PWAs update on Chrome launch.

As a temporary measure, make System PWAs update on every Chrome launch.
This allows developers to see changes when they update manifests. This
will be replaced with a lighter, more robust system before consumer
launch.

Bug: 836128
Change-Id: I82febcd9b0f7ae5f8be401bdedbe4846008562f4
Reviewed-on: https://chromium-review.googlesource.com/c/1325595Reviewed-by: Alan Cutter <alancutter@chromium.org>
Commit-Queue: calamity <calamity@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607933}

chrome/browser/web_applications/bookmark_apps/system_web_app_manager.cc

@@ -34,6 +34,7 @@ PendingAppManager::AppInfo CreateAppInfoForSystemApp(const GURL& url) {
                                                InstallSource::kSystemInstalled);
   app_info.create_shortcuts = false;
   app_info.bypass_service_worker_check = true;
+  app_info.always_update = true;
   return app_info;
 }
 

chrome/browser/web_applications/bookmark_apps/system_web_app_manager_unittest.cc

@@ -42,6 +42,7 @@ PendingAppManager::AppInfo GetWindowedAppInfo() {
                                   InstallSource::kSystemInstalled);
   info.create_shortcuts = false;
   info.bypass_service_worker_check = true;
+  info.always_update = true;
   return info;
 }
 

chrome/browser/web_applications/components/pending_app_manager.cc

@@ -33,11 +33,12 @@ bool PendingAppManager::AppInfo::operator==(
     const PendingAppManager::AppInfo& other) const {
   return std::tie(url, launch_container, install_source, create_shortcuts,
                   override_previous_user_uninstall, bypass_service_worker_check,
-                  require_manifest) ==
+                  require_manifest, always_update) ==
          std::tie(other.url, other.launch_container, other.install_source,
                   other.create_shortcuts,
                   other.override_previous_user_uninstall,
-                  other.bypass_service_worker_check, other.require_manifest);
+                  other.bypass_service_worker_check, other.require_manifest,
+                  other.always_update);
 }
 
 PendingAppManager::PendingAppManager() = default;

chrome/browser/web_applications/components/pending_app_manager.h

@@ -65,6 +65,9 @@ class PendingAppManager {
     // This should be used for installing all default apps so that good metadata
     // is ensured.
     bool require_manifest = false;
+
+    // Whether the app should be reinstalled even if it is already installed.
+    bool always_update = false;
   };
 
   PendingAppManager();

chrome/browser/web_applications/extensions/pending_bookmark_app_manager.cc

@@ -141,9 +141,6 @@ void PendingBookmarkAppManager::SetTimerForTesting(
   timer_ = std::move(timer);
 }
 
-// Returns (as the base::Optional part) whether or not there is already a known
-// extension for the given ID. The bool inside the base::Optional is, when
-// known, whether the extension is installed (true) or uninstalled (false).
 base::Optional<bool> PendingBookmarkAppManager::IsExtensionPresentAndInstalled(
     const std::string& extension_id) {
   if (ExtensionRegistry::Get(profile_)->GetExtensionById(
@@ -169,6 +166,12 @@ void PendingBookmarkAppManager::MaybeStartNextInstallation() {
 
     const web_app::PendingAppManager::AppInfo& app_info =
         front->task->app_info();
+
+    if (app_info.always_update) {
+      StartInstallationTask(std::move(front));
+      return;
+    }
+
     base::Optional<std::string> extension_id =
         extension_ids_map_.LookupExtensionId(app_info.url);
 
@@ -189,8 +192,17 @@ void PendingBookmarkAppManager::MaybeStartNextInstallation() {
         }
       }
     }
+    StartInstallationTask(std::move(front));
+    return;
+  }
+
+  web_contents_.reset();
+}
 
-    current_task_and_callback_ = std::move(front);
+void PendingBookmarkAppManager::StartInstallationTask(
+    std::unique_ptr<TaskAndCallback> task) {
+  DCHECK(!current_task_and_callback_);
+  current_task_and_callback_ = std::move(task);
 
   CreateWebContentsIfNecessary();
   Observe(web_contents_.get());
@@ -200,14 +212,9 @@ void PendingBookmarkAppManager::MaybeStartNextInstallation() {
   load_params.transition_type = ui::PAGE_TRANSITION_GENERATED;
   web_contents_->GetController().LoadURLWithParams(load_params);
   timer_->Start(
-        FROM_HERE,
-        base::TimeDelta::FromSeconds(kSecondsToWaitForWebContentsLoad),
+      FROM_HERE, base::TimeDelta::FromSeconds(kSecondsToWaitForWebContentsLoad),
       base::BindOnce(&PendingBookmarkAppManager::OnWebContentsLoadTimedOut,
                      weak_ptr_factory_.GetWeakPtr()));
-    return;
-  }
-
-  web_contents_.reset();
 }
 
 void PendingBookmarkAppManager::CreateWebContentsIfNecessary() {

chrome/browser/web_applications/extensions/pending_bookmark_app_manager.h

@@ -62,11 +62,17 @@ class PendingBookmarkAppManager final : public web_app::PendingAppManager,
  private:
   struct TaskAndCallback;
 
+  // Returns (as the base::Optional part) whether or not there is already a
+  // known extension for the given ID. The bool inside the base::Optional is,
+  // when known, whether the extension is installed (true) or uninstalled
+  // (false).
   base::Optional<bool> IsExtensionPresentAndInstalled(
       const std::string& extension_id);
 
   void MaybeStartNextInstallation();
 
+  void StartInstallationTask(std::unique_ptr<TaskAndCallback> task);
+
   void CreateWebContentsIfNecessary();
 
   void OnInstalled(BookmarkAppInstallationTask::Result result);

chrome/browser/web_applications/extensions/pending_bookmark_app_manager_browsertest.cc

@@ -123,6 +123,37 @@ IN_PROC_BROWSER_TEST_F(PendingBookmarkAppManagerBrowserTest,
   EXPECT_FALSE(app);
 }
 
+IN_PROC_BROWSER_TEST_F(PendingBookmarkAppManagerBrowserTest, AlwaysUpdate) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(features::kDesktopPWAWindowing);
+  ASSERT_TRUE(embedded_test_server()->Start());
+  {
+    GURL url(embedded_test_server()->GetURL(
+        "/banners/"
+        "manifest_test_page.html?manifest=manifest_short_name_only.json"));
+    web_app::PendingAppManager::AppInfo app_info = CreateAppInfo(url);
+    app_info.always_update = true;
+    InstallApp(std::move(app_info));
+
+    const extensions::Extension* app =
+        extensions::util::GetInstalledPwaForUrl(browser()->profile(), url);
+    EXPECT_TRUE(app);
+    EXPECT_EQ("Manifest", app->name());
+  }
+  {
+    GURL url(
+        embedded_test_server()->GetURL("/banners/manifest_test_page.html"));
+    web_app::PendingAppManager::AppInfo app_info = CreateAppInfo(url);
+    app_info.always_update = true;
+    InstallApp(std::move(app_info));
+
+    const extensions::Extension* app =
+        extensions::util::GetInstalledPwaForUrl(browser()->profile(), url);
+    EXPECT_TRUE(app);
+    EXPECT_EQ("Manifest test app", app->name());
+  }
+}
+
 // Test that adding a manifest that points to a chrome:// URL does not actually
 // install a bookmark app that points to a chrome:// URL.
 IN_PROC_BROWSER_TEST_F(PendingBookmarkAppManagerBrowserTest,

chrome/browser/web_applications/extensions/pending_bookmark_app_manager_unittest.cc

@@ -575,6 +575,43 @@ TEST_F(PendingBookmarkAppManagerTest, Install_ConcurrentCallsSameApp) {
   EXPECT_EQ(GURL(kFooWebAppUrl), install_callback_url());
 }
 
+TEST_F(PendingBookmarkAppManagerTest, Install_AlwaysUpdate) {
+  auto pending_app_manager = GetPendingBookmarkAppManagerWithTestFactories();
+
+  auto get_always_update_info = []() {
+    web_app::PendingAppManager::AppInfo info(
+        GURL(kFooWebAppUrl), web_app::LaunchContainer::kWindow,
+        web_app::InstallSource::kExternalPolicy);
+    info.always_update = true;
+    return info;
+  };
+  pending_app_manager->Install(
+      get_always_update_info(),
+      base::BindOnce(&PendingBookmarkAppManagerTest::InstallCallback,
+                     base::Unretained(this)));
+
+  base::RunLoop().RunUntilIdle();
+  SuccessfullyLoad(GURL(kFooWebAppUrl));
+
+  EXPECT_EQ(1u, installation_task_run_count());
+  EXPECT_TRUE(app_installed());
+  EXPECT_EQ(GURL(kFooWebAppUrl), install_callback_url());
+  ResetResults();
+
+  pending_app_manager->Install(
+      get_always_update_info(),
+      base::BindOnce(&PendingBookmarkAppManagerTest::InstallCallback,
+                     base::Unretained(this)));
+
+  base::RunLoop().RunUntilIdle();
+  SuccessfullyLoad(GURL(kFooWebAppUrl));
+
+  // The app is reinstalled even though it is already installed.
+  EXPECT_EQ(1u, installation_task_run_count());
+  EXPECT_TRUE(app_installed());
+  EXPECT_EQ(GURL(kFooWebAppUrl), install_callback_url());
+}
+
 TEST_F(PendingBookmarkAppManagerTest, Install_FailsLoadIncorrectURL) {
   auto pending_app_manager = GetPendingBookmarkAppManagerWithTestFactories();
   pending_app_manager->Install(