WebApp: Rename InstallWebAppFromSync to InstallBookmarkAppFromSync.

1) Rename InstallManager::InstallWebAppFromSync to InstallBookmarkAppFromSync.

2) Move the check we introduced here:
https://chromium-review.googlesource.com/c/chromium/src/+/1981239
to WebAppInstallManager::InstallBookmarkAppFromSync() so web apps team owns it.

3) Demote DCHECK(provider) to if(provider) to avoid crashes for legacy
or bizarre profiles with bookmark apps data in them.
If you try to run chromeos-on-linux binary for a profile data dir created
by chrome-linux binary, you get the "bizarre" profile.

This CL doesn't introduce any behavior changes.

We will implement migration of bookmark apps arriving from the sync
server in follow up CLs under crbug.com/1020037 task.

Bug: 1020037
Change-Id: Ic7861734b6bbcb9fe228eb513782251ba8b4ff32
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2060220Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#742867}

chrome/browser/extensions/extension_sync_service.cc

@@ -9,7 +9,6 @@
 
 #include "base/auto_reset.h"
 #include "base/bind_helpers.h"
-#include "base/feature_list.h"
 #include "base/one_shot_event.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/extensions/extension_service.h"
@@ -22,7 +21,6 @@
 #include "chrome/browser/web_applications/components/install_manager.h"
 #include "chrome/browser/web_applications/components/web_app_provider_base.h"
 #include "chrome/common/buildflags.h"
-#include "chrome/common/chrome_features.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/extensions/sync_helper.h"
 #include "chrome/common/web_application_info.h"
@@ -507,12 +505,6 @@ void ExtensionSyncService::ApplyBookmarkAppSyncData(
     const ExtensionSyncData& extension_sync_data) {
   DCHECK(extension_sync_data.is_app());
 
-  if (base::FeatureList::IsEnabled(features::kDesktopPWAsWithoutExtensions)) {
-    // If new Web Applications system is enabled, any legacy sync-initiated
-    // installation requests are ignored.
-    return;
-  }
-
   // Process bookmark app sync if necessary.
   GURL bookmark_app_url(extension_sync_data.bookmark_app_url());
   if (!bookmark_app_url.is_valid() ||
@@ -543,10 +535,12 @@ void ExtensionSyncService::ApplyBookmarkAppSyncData(
   }
 
   auto* provider = web_app::WebAppProviderBase::GetProviderBase(profile_);
-  DCHECK(provider);
-
-  provider->install_manager().InstallWebAppFromSync(
+  // Legacy profiles containing server-side bookmark apps data must be excluded
+  // from sync if the web apps system is disabled for such a profile.
+  if (provider) {
+    provider->install_manager().InstallBookmarkAppFromSync(
         extension_sync_data.id(), std::move(web_app_info), base::DoNothing());
+  }
 }
 
 void ExtensionSyncService::SetSyncStartFlareForTesting(

chrome/browser/web_applications/components/install_manager.h

@@ -118,11 +118,11 @@ class InstallManager {
                                        WebappInstallSource install_source,
                                        OnceInstallCallback callback) = 0;
 
-  // For the old ExtensionSyncService-based system only:
-  // Starts background installation or an update of a web app from the sync
+  // For backward compatibility with ExtensionSyncService-based system:
+  // Starts background installation or an update of a bookmark app from the sync
   // system. |web_application_info| contains received sync data. Icons will be
   // downloaded from the icon URLs provided in |web_application_info|.
-  virtual void InstallWebAppFromSync(
+  virtual void InstallBookmarkAppFromSync(
       const AppId& app_id,
       std::unique_ptr<WebApplicationInfo> web_application_info,
       OnceInstallCallback callback) = 0;

chrome/browser/web_applications/extensions/install_manager_bookmark_app_unittest.cc

@@ -621,7 +621,7 @@ TEST_F(InstallManagerBookmarkAppTest, CreateWebAppFromInfo) {
                    .empty());
 }
 
-TEST_F(InstallManagerBookmarkAppTest, InstallWebAppFromSync) {
+TEST_F(InstallManagerBookmarkAppTest, InstallBookmarkAppFromSync) {
   CreateEmptyDataRetriever();
 
   EXPECT_EQ(0u, registry()->enabled_extensions().size());
@@ -647,7 +647,7 @@ TEST_F(InstallManagerBookmarkAppTest, InstallWebAppFromSync) {
   {
     base::RunLoop run_loop;
 
-    provider->install_manager().InstallWebAppFromSync(
+    provider->install_manager().InstallBookmarkAppFromSync(
         app_id, std::move(web_app_info),
         base::BindLambdaForTesting([&](const web_app::AppId& installed_app_id,
                                        web_app::InstallResultCode code) {
@@ -693,7 +693,7 @@ TEST_F(InstallManagerBookmarkAppTest, InstallWebAppFromSync) {
   {
     base::RunLoop run_loop;
 
-    provider->install_manager().InstallWebAppFromSync(
+    provider->install_manager().InstallBookmarkAppFromSync(
         app_id, std::move(web_app_info2),
         base::BindLambdaForTesting([&](const web_app::AppId& installed_app_id,
                                        web_app::InstallResultCode code) {

chrome/browser/web_applications/web_app_install_manager.cc

@@ -8,6 +8,7 @@
 
 #include "base/bind.h"
 #include "base/callback.h"
+#include "base/feature_list.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/profiles/profile.h"
@@ -18,6 +19,7 @@
 #include "chrome/browser/web_applications/components/web_app_utils.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/browser/web_applications/web_app_install_task.h"
+#include "chrome/common/chrome_features.h"
 #include "chrome/common/web_application_info.h"
 #include "content/public/browser/web_contents.h"
 
@@ -114,10 +116,19 @@ void WebAppInstallManager::InstallWebAppWithParams(
   tasks_.insert(std::move(task));
 }
 
-void WebAppInstallManager::InstallWebAppFromSync(
+void WebAppInstallManager::InstallBookmarkAppFromSync(
     const AppId& app_id,
     std::unique_ptr<WebApplicationInfo> web_application_info,
     OnceInstallCallback callback) {
+  if (base::FeatureList::IsEnabled(features::kDesktopPWAsWithoutExtensions)) {
+    // If new Web Applications system is enabled, any legacy sync-initiated
+    // installation requests are ignored for now.
+    // TODO(crbug.com/1020037): If app_id is not installed, migrate bookmark app
+    // to new web app: Enqueue installation as if the user had requested to
+    // install it.
+    return;
+  }
+
   // Skip sync update if app exists.
   // All manifest fields will be set locally via update (see crbug.com/926083)
   // so we must not sync them in order to avoid a device-to-device sync war.

chrome/browser/web_applications/web_app_install_manager.h

@@ -60,8 +60,7 @@ class WebAppInstallManager final : public InstallManager,
                                const InstallParams& install_params,
                                WebappInstallSource install_source,
                                OnceInstallCallback callback) override;
-  // For the old ExtensionSyncService-based system only:
-  void InstallWebAppFromSync(
+  void InstallBookmarkAppFromSync(
       const AppId& app_id,
       std::unique_ptr<WebApplicationInfo> web_application_info,
       OnceInstallCallback callback) override;

chrome/browser/web_applications/web_app_install_manager_unittest.cc

@@ -365,8 +365,10 @@ class WebAppInstallManagerTest : public WebAppTest {
   TestFileUtils* file_utils_ = nullptr;
 };
 
+// TODO(crbug.com/1020037): Move this test out into
+// install_manager_bookmark_app_unittest.cc.
 TEST_F(WebAppInstallManagerTest,
-       InstallWebAppFromSync_TwoConcurrentInstallsAreRunInOrder) {
+       InstallBookmarkAppFromSync_TwoConcurrentInstallsAreRunInOrder) {
   InitEmptyRegistrar();
 
   const GURL url1{"https://example.com/path"};
@@ -435,7 +437,7 @@ TEST_F(WebAppInstallManagerTest,
   EXPECT_FALSE(install_manager().has_web_contents_for_testing());
 
   // Enqueue a request to install the 1st app.
-  install_manager().InstallWebAppFromSync(
+  install_manager().InstallBookmarkAppFromSync(
       app1_id, CreateWebAppInfo(url1),
       base::BindLambdaForTesting(
           [&](const AppId& installed_app_id, InstallResultCode code) {
@@ -451,7 +453,7 @@ TEST_F(WebAppInstallManagerTest,
 
   // Immediately enqueue a request to install the 2nd app, WebContents is not
   // ready.
-  install_manager().InstallWebAppFromSync(
+  install_manager().InstallBookmarkAppFromSync(
       app2_id, CreateWebAppInfo(url2),
       base::BindLambdaForTesting(
           [&](const AppId& installed_app_id, InstallResultCode code) {
@@ -486,8 +488,10 @@ TEST_F(WebAppInstallManagerTest,
   EXPECT_EQ(expected_event_order, event_order);
 }
 
+// TODO(crbug.com/1020037): Move this test out into
+// install_manager_bookmark_app_unittest.cc.
 TEST_F(WebAppInstallManagerTest,
-       InstallWebAppFromSync_InstallManagerDestroyed) {
+       InstallBookmarkAppFromSync_InstallManagerDestroyed) {
   InitEmptyRegistrar();
 
   const GURL app_url("https://example.com/path");
@@ -515,7 +519,7 @@ TEST_F(WebAppInstallManagerTest,
         return std::unique_ptr<WebAppDataRetriever>(std::move(data_retriever));
       }));
 
-  install_manager().InstallWebAppFromSync(
+  install_manager().InstallBookmarkAppFromSync(
       app_id, CreateWebAppInfo(app_url),
       base::BindLambdaForTesting(
           [](const AppId& installed_app_id, InstallResultCode code) {