[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: I04e65a4a060a6e91c0f7d32bd44be51492e2fd4e
Reviewed-on: https://chromium-review.googlesource.com/c/1282503
Commit-Queue: calamity <calamity@chromium.org>
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603800}

chrome/browser/web_applications/bookmark_apps/system_web_app_manager.cc

@@ -30,12 +30,12 @@ namespace {
 PendingAppManager::AppInfo CreateAppInfoForSystemApp(const GURL& url) {
   DCHECK_EQ(content::kChromeUIScheme, url.scheme());
   return {
-      url,
-      LaunchContainer::kWindow,
-      InstallSource::kSystemInstalled,
+      url, LaunchContainer::kWindow, InstallSource::kSystemInstalled,
       false /* create_shortcuts */,
       PendingAppManager::AppInfo::kDefaultOverridePreviousUserUninstall,
       true /* bypass_service_worker_check */,
+      // TODO(calamity): Design a less heavy-handed update condition.
+      true /* always_update */,
   };
 }
 

chrome/browser/web_applications/components/pending_app_manager.cc

@@ -17,6 +17,7 @@ const bool PendingAppManager::AppInfo::kDefaultCreateShortcuts = true;
 const bool PendingAppManager::AppInfo::kDefaultOverridePreviousUserUninstall =
     false;
 const bool PendingAppManager::AppInfo::kDefaultBypassServiceWorkerCheck = false;
+const bool PendingAppManager::AppInfo::kDefaultAlwaysUpdate = false;
 const bool PendingAppManager::AppInfo::kDefaultRequireManifest = false;
 
 PendingAppManager::AppInfo::AppInfo(GURL url,
@@ -25,6 +26,7 @@ PendingAppManager::AppInfo::AppInfo(GURL url,
                                     bool create_shortcuts,
                                     bool override_previous_user_uninstall,
                                     bool bypass_service_worker_check,
+                                    bool always_update,
                                     bool require_manifest)
     : url(std::move(url)),
       launch_container(launch_container),
@@ -32,6 +34,7 @@ PendingAppManager::AppInfo::AppInfo(GURL url,
       create_shortcuts(create_shortcuts),
       override_previous_user_uninstall(override_previous_user_uninstall),
       bypass_service_worker_check(bypass_service_worker_check),
+      always_update(always_update),
       require_manifest(require_manifest) {}
 
 PendingAppManager::AppInfo::AppInfo(PendingAppManager::AppInfo&& other) =

chrome/browser/web_applications/components/pending_app_manager.h

@@ -40,6 +40,7 @@ class PendingAppManager {
     static const bool kDefaultCreateShortcuts;
     static const bool kDefaultOverridePreviousUserUninstall;
     static const bool kDefaultBypassServiceWorkerCheck;
+    static const bool kDefaultAlwaysUpdate;
     static const bool kDefaultRequireManifest;
 
     AppInfo(GURL url,
@@ -49,6 +50,7 @@ class PendingAppManager {
             bool override_previous_user_uninstall =
                 kDefaultOverridePreviousUserUninstall,
             bool bypass_service_worker_check = kDefaultBypassServiceWorkerCheck,
+            bool always_update = kDefaultAlwaysUpdate,
             bool require_manifest = kDefaultRequireManifest);
     AppInfo(AppInfo&& other);
     ~AppInfo();
@@ -61,6 +63,9 @@ class PendingAppManager {
     const LaunchContainer launch_container;
     const InstallSource install_source;
     const bool create_shortcuts;
+
+    // Whether the app should be reinstalled even if the user has previously
+    // uninstalled it.
     const bool override_previous_user_uninstall;
 
     // This must only be used by pre-installed default or system apps that are
@@ -68,6 +73,9 @@ class PendingAppManager {
     // programmatically.
     const bool bypass_service_worker_check;
 
+    // Whether the app should be reinstalled even if it is already installed.
+    const bool always_update;
+
     // This should be used for installing all default apps so that good metadata
     // is ensured.
     const bool require_manifest;

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,27 +192,31 @@ void PendingBookmarkAppManager::MaybeStartNextInstallation() {
         }
       }
     }
-
-    current_task_and_callback_ = std::move(front);
-
-    CreateWebContentsIfNecessary();
-    Observe(web_contents_.get());
-
-    content::NavigationController::LoadURLParams load_params(
-        current_task_and_callback_->task->app_info().url);
-    load_params.transition_type = ui::PAGE_TRANSITION_GENERATED;
-    web_contents_->GetController().LoadURLWithParams(load_params);
-    timer_->Start(
-        FROM_HERE,
-        base::TimeDelta::FromSeconds(kSecondsToWaitForWebContentsLoad),
-        base::BindOnce(&PendingBookmarkAppManager::OnWebContentsLoadTimedOut,
-                       weak_ptr_factory_.GetWeakPtr()));
+    StartInstallationTask(std::move(front));
     return;
   }
 
   web_contents_.reset();
 }
 
+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());
+
+  content::NavigationController::LoadURLParams load_params(
+      current_task_and_callback_->task->app_info().url);
+  load_params.transition_type = ui::PAGE_TRANSITION_GENERATED;
+  web_contents_->GetController().LoadURLWithParams(load_params);
+  timer_->Start(
+      FROM_HERE, base::TimeDelta::FromSeconds(kSecondsToWaitForWebContentsLoad),
+      base::BindOnce(&PendingBookmarkAppManager::OnWebContentsLoadTimedOut,
+                     weak_ptr_factory_.GetWeakPtr()));
+}
+
 void PendingBookmarkAppManager::CreateWebContentsIfNecessary() {
   if (web_contents_)
     return;

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

@@ -30,24 +30,26 @@ class PendingBookmarkAppManagerBrowserTest : public InProcessBrowserTest {
  protected:
   void InstallApp(const GURL& url,
                   bool bypass_service_worker_check = false,
+                  bool always_update = false,
                   bool require_manifest = false) {
     base::RunLoop run_loop;
     web_app::WebAppProvider::Get(browser()->profile())
         ->pending_app_manager()
-        .Install(web_app::PendingAppManager::AppInfo(
-                     url, web_app::LaunchContainer::kWindow,
-                     web_app::InstallSource::kInternal,
-                     false /* create_shortcuts */,  // Avoid creating real
-                                                    // shortcuts in tests.
-                     web_app::PendingAppManager::AppInfo::
-                         kDefaultOverridePreviousUserUninstall,
-                     bypass_service_worker_check, require_manifest),
-                 base::BindLambdaForTesting(
-                     [this, &run_loop](const GURL& provided_url,
-                                       web_app::InstallResultCode code) {
-                       result_code_ = code;
-                       run_loop.QuitClosure().Run();
-                     }));
+        .Install(
+            web_app::PendingAppManager::AppInfo(
+                url, web_app::LaunchContainer::kWindow,
+                web_app::InstallSource::kInternal,
+                false /* create_shortcuts */,  // Avoid creating real
+                                               // shortcuts in tests.
+                web_app::PendingAppManager::AppInfo::
+                    kDefaultOverridePreviousUserUninstall,
+                bypass_service_worker_check, always_update, require_manifest),
+            base::BindLambdaForTesting(
+                [this, &run_loop](const GURL& provided_url,
+                                  web_app::InstallResultCode code) {
+                  result_code_ = code;
+                  run_loop.QuitClosure().Run();
+                }));
     run_loop.Run();
     ASSERT_TRUE(result_code_.has_value());
   }
@@ -122,6 +124,33 @@ 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"));
+    InstallApp(url, false /* bypass_service_worker_check */,
+               true /* always_update */);
+    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"));
+    InstallApp(url, false /* bypass_service_worker_check */,
+               true /* always_update */);
+    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,
@@ -154,7 +183,7 @@ IN_PROC_BROWSER_TEST_F(PendingBookmarkAppManagerBrowserTest,
   GURL url(
       embedded_test_server()->GetURL("/banners/no_manifest_test_page.html"));
   InstallApp(url, false /* bypass_service_worker_check */,
-             true /* require_manifest */);
+             false /* always_update */, true /* require_manifest */);
   EXPECT_EQ(web_app::InstallResultCode::kFailedUnknownReason,
             result_code_.value());
   base::Optional<std::string> id =

chrome/browser/web_applications/extensions/pending_bookmark_app_manager_unittest.cc

@@ -569,6 +569,46 @@ 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 = []() {
+    return web_app::PendingAppManager::AppInfo(
+        GURL(kFooWebAppUrl), web_app::LaunchContainer::kWindow,
+        web_app::InstallSource::kExternalPolicy,
+        web_app::PendingAppManager::AppInfo::kDefaultCreateShortcuts,
+        web_app::PendingAppManager::AppInfo::
+            kDefaultOverridePreviousUserUninstall,
+        web_app::PendingAppManager::AppInfo::kDefaultBypassServiceWorkerCheck,
+        true /* always_update */);
+  };
+  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(