sync: Use transport mode for WEB_APPS with SplitSettingsSync

Chrome OS SplitSettingsSync has a separate control for syncing OS data
types that isn't controlled by the browser's sync-the-feature. Change
ModelType::WEB_APPS to run using the transport layer when
kSplitSettingsSync is enabled and the OS sync feature is on.

Manual test:
Run chrome --enable-features=SplitSettingsSync,SyncManualStartChromeOS,
  DesktopPWAsWithoutExtensions,DesktopPWAsUSS
Add an account
Enable OS sync in the first-run dialog
Verify chrome://sync-internals shows Web Apps are syncing, even though
browser sync is off.

Also added automated tests to sync_integration_tests.

Bug: 1013466, 1031549
Change-Id: I2cdb4ccf6b4027ce45044d945f02913482ad239d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1970290
Commit-Queue: Marc Treib <treib@chromium.org>
Auto-Submit: James Cook <jamescook@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#725457}

chrome/browser/sync/chrome_sync_client.cc

@@ -378,12 +378,7 @@ ChromeSyncClient::CreateDataTypeControllers(syncer::SyncService* sync_service) {
       base::FeatureList::IsEnabled(features::kDesktopPWAsUSS) &&
       web_app::WebAppProvider::Get(profile_)) {
     if (!disabled_types.Has(syncer::WEB_APPS)) {
-      // TODO(https://crbug.com/1031549): Run in transport-mode on Chrome OS
-      // with SplitSettingsSync.
-      controllers.push_back(std::make_unique<syncer::ModelTypeController>(
-          syncer::WEB_APPS,
-          std::make_unique<syncer::ForwardingModelTypeControllerDelegate>(
-              GetControllerDelegateForModelType(syncer::WEB_APPS).get())));
+      controllers.push_back(CreateWebAppsModelTypeController(sync_service));
     }
   }
 #endif  // BUILDFLAG(ENABLE_EXTENSIONS)
@@ -730,6 +725,29 @@ ChromeSyncClient::CreateAppSettingsModelTypeController(
           profile_, syncer::APP_SETTINGS),
       GetDumpStackClosure(), profile_);
 }
+
+std::unique_ptr<syncer::ModelTypeController>
+ChromeSyncClient::CreateWebAppsModelTypeController(
+    syncer::SyncService* sync_service) {
+  syncer::ModelTypeControllerDelegate* delegate =
+      GetControllerDelegateForModelType(syncer::WEB_APPS).get();
+#if defined(OS_CHROMEOS)
+  if (chromeos::features::IsSplitSettingsSyncEnabled()) {
+    // Use the same delegate in full-sync and transport-only modes.
+    return std::make_unique<OsSyncModelTypeController>(
+        syncer::WEB_APPS,
+        /*delegate_for_full_sync_mode=*/
+        std::make_unique<ForwardingModelTypeControllerDelegate>(delegate),
+        /*delegate_for_transport_mode=*/
+        std::make_unique<ForwardingModelTypeControllerDelegate>(delegate),
+        profile_->GetPrefs(), sync_service);
+  }
+  // Fall through.
+#endif
+  return std::make_unique<syncer::ModelTypeController>(
+      syncer::WEB_APPS,
+      std::make_unique<ForwardingModelTypeControllerDelegate>(delegate));
+}
 #endif  // BUILDFLAG(ENABLE_EXTENSIONS)
 
 }  // namespace browser_sync

chrome/browser/sync/chrome_sync_client.h

@@ -75,6 +75,10 @@ class ChromeSyncClient : public browser_sync::BrowserSyncClient {
   // Creates the ModelTypeController for syncer::APP_SETTINGS.
   std::unique_ptr<syncer::ModelTypeController>
   CreateAppSettingsModelTypeController(syncer::SyncService* sync_service);
+
+  // Creates the ModelTypeController for syncer::WEB_APPS.
+  std::unique_ptr<syncer::ModelTypeController> CreateWebAppsModelTypeController(
+      syncer::SyncService* sync_service);
 #endif  // BUILDFLAG(ENABLE_EXTENSIONS)
 
   Profile* const profile_;

chrome/browser/sync/test/integration/single_client_standalone_transport_sync_test.cc

@@ -21,6 +21,7 @@
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/sync/test/integration/os_sync_test.h"
+#include "chrome/common/chrome_features.h"
 #include "chromeos/constants/chromeos_features.h"
 #include "components/browser_sync/browser_sync_switches.h"
 #endif
@@ -297,9 +298,11 @@ IN_PROC_BROWSER_TEST_F(SingleClientStandaloneTransportSyncTest,
 class SingleClientStandaloneTransportOsSyncTest : public OsSyncTest {
  public:
   SingleClientStandaloneTransportOsSyncTest() : OsSyncTest(SINGLE_CLIENT) {
-    // Enable the Wi-Fi type and don't auto-start browser sync.
+    // Don't auto-start browser sync. Enable in-development types.
     scoped_features_.InitWithFeatures(
-        {switches::kSyncWifiConfigurations, switches::kSyncManualStartChromeOS},
+        {switches::kSyncManualStartChromeOS,
+         features::kDesktopPWAsWithoutExtensions, features::kDesktopPWAsUSS,
+         switches::kSyncWifiConfigurations},
         {});
   }
   ~SingleClientStandaloneTransportOsSyncTest() override = default;
@@ -335,7 +338,7 @@ IN_PROC_BROWSER_TEST_F(SingleClientStandaloneTransportOsSyncTest,
   EXPECT_TRUE(active_types.Has(syncer::OS_PREFERENCES));
   EXPECT_TRUE(active_types.Has(syncer::OS_PRIORITY_PREFERENCES));
   EXPECT_TRUE(active_types.Has(syncer::PRINTERS));
-  // TODO(jamescook): WEB_APPS
+  EXPECT_TRUE(active_types.Has(syncer::WEB_APPS));
   EXPECT_TRUE(active_types.Has(syncer::WIFI_CONFIGURATIONS));
 
   // Verify that a few browser non-transport-mode types are not active.
@@ -371,7 +374,7 @@ IN_PROC_BROWSER_TEST_F(SingleClientStandaloneTransportOsSyncTest,
   EXPECT_FALSE(active_types.Has(syncer::OS_PREFERENCES));
   EXPECT_FALSE(active_types.Has(syncer::OS_PRIORITY_PREFERENCES));
   EXPECT_FALSE(active_types.Has(syncer::PRINTERS));
-  // TODO(jamescook): WEB_APPS
+  EXPECT_FALSE(active_types.Has(syncer::WEB_APPS));
   EXPECT_FALSE(active_types.Has(syncer::WIFI_CONFIGURATIONS));
 
   // Browser non-transport-mode types are active.

chrome/browser/sync/test/integration/single_client_web_apps_sync_test.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/sync/test/integration/sync_test.h"
+#include "chrome/common/chrome_features.h"
+#include "components/sync/base/model_type.h"
+#include "components/sync/base/user_selectable_type.h"
+#include "components/sync/driver/profile_sync_service.h"
+#include "components/sync/driver/sync_user_settings.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+#if defined(OS_CHROMEOS)
+#include "chrome/browser/sync/test/integration/os_sync_test.h"
+#include "chromeos/constants/chromeos_features.h"
+#endif
+
+using syncer::UserSelectableType;
+using syncer::UserSelectableTypeSet;
+
+namespace {
+
+#if defined(OS_CHROMEOS)
+// Chrome OS syncs apps as an OS type.
+class SingleClientWebAppsOsSyncTest : public OsSyncTest {
+ public:
+  SingleClientWebAppsOsSyncTest() : OsSyncTest(SINGLE_CLIENT) {
+    features_.InitWithFeatures(
+        {features::kDesktopPWAsWithoutExtensions, features::kDesktopPWAsUSS},
+        {});
+  }
+  ~SingleClientWebAppsOsSyncTest() override = default;
+
+ private:
+  base::test::ScopedFeatureList features_;
+};
+
+IN_PROC_BROWSER_TEST_F(SingleClientWebAppsOsSyncTest,
+                       DisablingOsSyncFeatureDisablesDataType) {
+  ASSERT_TRUE(chromeos::features::IsSplitSettingsSyncEnabled());
+  ASSERT_TRUE(SetupSync());
+  syncer::ProfileSyncService* service = GetSyncService(0);
+  syncer::SyncUserSettings* settings = service->GetUserSettings();
+
+  EXPECT_TRUE(settings->GetOsSyncFeatureEnabled());
+  EXPECT_TRUE(service->GetActiveDataTypes().Has(syncer::WEB_APPS));
+
+  settings->SetOsSyncFeatureEnabled(false);
+  EXPECT_FALSE(settings->GetOsSyncFeatureEnabled());
+  EXPECT_FALSE(service->GetActiveDataTypes().Has(syncer::WEB_APPS));
+}
+
+#else   // !defined(OS_CHROMEOS)
+
+// See also TwoClientWebAppsSyncTest.
+class SingleClientWebAppsSyncTest : public SyncTest {
+ public:
+  SingleClientWebAppsSyncTest() : SyncTest(SINGLE_CLIENT) {
+    features_.InitWithFeatures(
+        {features::kDesktopPWAsWithoutExtensions, features::kDesktopPWAsUSS},
+        {});
+  }
+  ~SingleClientWebAppsSyncTest() override = default;
+
+ private:
+  base::test::ScopedFeatureList features_;
+};
+
+IN_PROC_BROWSER_TEST_F(SingleClientWebAppsSyncTest,
+                       DisablingSelectedTypeDisablesModelType) {
+  ASSERT_TRUE(SetupSync());
+  syncer::ProfileSyncService* service = GetSyncService(0);
+  syncer::SyncUserSettings* settings = service->GetUserSettings();
+  ASSERT_TRUE(settings->GetSelectedTypes().Has(UserSelectableType::kApps));
+  EXPECT_TRUE(service->GetActiveDataTypes().Has(syncer::WEB_APPS));
+
+  settings->SetSelectedTypes(false, UserSelectableTypeSet());
+  ASSERT_FALSE(settings->GetSelectedTypes().Has(UserSelectableType::kApps));
+  EXPECT_FALSE(service->GetActiveDataTypes().Has(syncer::WEB_APPS));
+}
+#endif  // defined(OS_CHROMEOS)
+
+}  // namespace

chrome/test/BUILD.gn

@@ -6125,6 +6125,7 @@ if (!is_android && !is_fuchsia) {
       "../browser/sync/test/integration/single_client_user_consents_sync_test.cc",
       "../browser/sync/test/integration/single_client_user_events_sync_test.cc",
       "../browser/sync/test/integration/single_client_wallet_sync_test.cc",
+      "../browser/sync/test/integration/single_client_web_apps_sync_test.cc",
       "../browser/sync/test/integration/sync_auth_test.cc",
       "../browser/sync/test/integration/sync_errors_test.cc",
       "../browser/sync/test/integration/sync_exponential_backoff_test.cc",

components/sync/driver/profile_sync_service.cc

@@ -1399,10 +1399,9 @@ ModelTypeSet ProfileSyncService::GetModelTypesForTransportOnlyMode() const {
 #if defined(OS_CHROMEOS)
   // Chrome OS system types are not tied to browser sync-the-feature.
   if (chromeos::features::IsSplitSettingsSyncEnabled()) {
-    // TODO(jamescook): WEB_APPS
     allowed_types.PutAll({APP_LIST, APP_SETTINGS, APPS, ARC_PACKAGE,
                           OS_PREFERENCES, OS_PRIORITY_PREFERENCES, PRINTERS,
-                          WIFI_CONFIGURATIONS});
+                          WEB_APPS, WIFI_CONFIGURATIONS});
   }
 #endif  // defined(OS_CHROMEOS)