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

This reverts commit 2dd14e92.

Reason for revert: This changes the meaning of existing code. E.g. the default app installer now calls with require_manifest false and always_update true, instead of require_manifest true.

Original change's description:
> [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}

TBR=calamity@chromium.org,alancutter@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 836128
Change-Id: I997eed1e5154ee1cc4f40eed55c6d2ffdadb2cf1
Reviewed-on: https://chromium-review.googlesource.com/c/1322185Reviewed-by: Ben Wells <benwells@chromium.org>
Commit-Queue: calamity <calamity@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606314}

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,7 +17,6 @@ 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,
@@ -26,7 +25,6 @@ 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),
@@ -34,7 +32,6 @@ 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,7 +40,6 @@ 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,
@@ -50,7 +49,6 @@ 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();
@@ -63,9 +61,6 @@ 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
@@ -73,9 +68,6 @@ 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,6 +141,9 @@ 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(
@@ -166,12 +169,6 @@ 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);
 
@@ -192,17 +189,8 @@ void PendingBookmarkAppManager::MaybeStartNextInstallation() {
         }
       }
     }
-    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);
+    current_task_and_callback_ = std::move(front);
 
     CreateWebContentsIfNecessary();
     Observe(web_contents_.get());
@@ -212,9 +200,14 @@ void PendingBookmarkAppManager::StartInstallationTask(
     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,17 +62,11 @@ 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,20 +30,18 @@ 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(
+        .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),
+                     bypass_service_worker_check, require_manifest),
                  base::BindLambdaForTesting(
                      [this, &run_loop](const GURL& provided_url,
                                        web_app::InstallResultCode code) {
@@ -124,33 +122,6 @@ 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,
@@ -183,7 +154,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 */,
-             false /* always_update */, true /* require_manifest */);
+             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,46 +569,6 @@ 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(