Revert "[sync::test] Add two client integration test for custom passphrase"

This reverts commit 48c7a842.

Reason for revert: Broke compile step
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20ASan%20LSan%20Builder/60281


Original change's description:
> [sync::test] Add two client integration test for custom passphrase
> 
> Add a Sync integration test which exercises custom passphrase flows in
> an end-to-end manner with two clients. The test ensures that two clients
> can sync data properly in the presence of custom passphrase-based
> encryption and different key derivation methods.
> 
> Refactor SetScryptFeatureState() from the single-client test to a scoped
> feature toggler that resides in encryption_helper, because the
> functionality now needs to also be used from the two-client test.
> 
> Bug: 894148
> Change-Id: Ia211b221f89a99092d3b19690ecae6bb4fa77d02
> Reviewed-on: https://chromium-review.googlesource.com/c/1277648
> Commit-Queue: David Davidović <davidovic@google.com>
> Reviewed-by: vitaliii <vitaliii@chromium.org>
> Reviewed-by: Marc Treib <treib@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#599929}

TBR=treib@chromium.org,vitaliii@chromium.org,davidovic@google.com

Change-Id: I8913698d352dc65ea9da27b052425a53394ff879
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 894148
Reviewed-on: https://chromium-review.googlesource.com/c/1283030Reviewed-by: Christos Froussios <cfroussios@chromium.org>
Commit-Queue: Christos Froussios <cfroussios@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599931}

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

@@ -9,9 +9,7 @@
 #include "chrome/browser/sync/test/integration/encryption_helper.h"
 #include "components/browser_sync/profile_sync_service.h"
 #include "components/sync/base/passphrase_enums.h"
-#include "components/sync/base/sync_base_switches.h"
 #include "components/sync/base/system_encryptor.h"
-#include "components/sync/engine/sync_engine_switches.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace encryption_helper {
@@ -176,24 +174,3 @@ std::string PassphraseRequiredStateChecker::GetDebugMessage() const {
   return "Waiting until decryption passphrase is " +
          std::string(desired_state_ ? "required" : "not required");
 }
-
-ScopedScryptFeatureToggler::ScopedScryptFeatureToggler(
-    bool force_disabled,
-    bool use_for_new_passphrases) {
-  std::vector<base::Feature> enabled_features;
-  std::vector<base::Feature> disabled_features;
-  if (force_disabled) {
-    enabled_features.push_back(
-        switches::kSyncForceDisableScryptForCustomPassphrase);
-  } else {
-    disabled_features.push_back(
-        switches::kSyncForceDisableScryptForCustomPassphrase);
-  }
-  if (use_for_new_passphrases) {
-    enabled_features.push_back(switches::kSyncUseScryptForNewCustomPassphrases);
-  } else {
-    disabled_features.push_back(
-        switches::kSyncUseScryptForNewCustomPassphrases);
-  }
-  feature_list_.InitWithFeatures(enabled_features, disabled_features);
-}

chrome/browser/sync/test/integration/encryption_helper.h

@@ -7,7 +7,6 @@
 
 #include <string>
 
-#include "base/test/scoped_feature_list.h"
 #include "chrome/browser/sync/test/integration/single_client_status_change_checker.h"
 #include "components/sync/base/cryptographer.h"
 #include "components/sync/protocol/nigori_specifics.pb.h"
@@ -77,12 +76,4 @@ class PassphraseRequiredStateChecker : public SingleClientStatusChangeChecker {
   bool desired_state_;
 };
 
-class ScopedScryptFeatureToggler {
- public:
-  ScopedScryptFeatureToggler(bool force_disabled, bool use_for_new_passphrases);
-
- private:
-  base::test::ScopedFeatureList feature_list_;
-};
-
 #endif  // CHROME_BROWSER_SYNC_TEST_INTEGRATION_ENCRYPTION_HELPER_H_

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

@@ -8,7 +8,9 @@
 #include "components/browser_sync/profile_sync_service.h"
 #include "components/sync/base/cryptographer.h"
 #include "components/sync/base/passphrase_enums.h"
+#include "components/sync/base/sync_base_switches.h"
 #include "components/sync/base/system_encryptor.h"
+#include "components/sync/engine/sync_engine_switches.h"
 
 namespace {
 
@@ -160,6 +162,27 @@ class SingleClientCustomPassphraseSyncTest : public SyncTest {
     return cryptographer;
   }
 
+  void SetScryptFeaturesState(bool force_disabled,
+                              bool use_for_new_passphrases) {
+    std::vector<base::Feature> enabled_features;
+    std::vector<base::Feature> disabled_features;
+    if (force_disabled) {
+      enabled_features.push_back(
+          switches::kSyncForceDisableScryptForCustomPassphrase);
+    } else {
+      disabled_features.push_back(
+          switches::kSyncForceDisableScryptForCustomPassphrase);
+    }
+    if (use_for_new_passphrases) {
+      enabled_features.push_back(
+          switches::kSyncUseScryptForNewCustomPassphrases);
+    } else {
+      disabled_features.push_back(
+          switches::kSyncUseScryptForNewCustomPassphrases);
+    }
+    feature_list_.InitWithFeatures(enabled_features, disabled_features);
+  }
+
   void InjectEncryptedServerBookmark(const std::string& title,
                                      const GURL& url,
                                      const KeyParams& key_params) {
@@ -172,6 +195,7 @@ class SingleClientCustomPassphraseSyncTest : public SyncTest {
 
  private:
   SystemEncryptor system_encryptor_;
+  base::test::ScopedFeatureList feature_list_;
 
   DISALLOW_COPY_AND_ASSIGN(SingleClientCustomPassphraseSyncTest);
 };
@@ -195,8 +219,8 @@ IN_PROC_BROWSER_TEST_F(SingleClientCustomPassphraseSyncTest,
 
 IN_PROC_BROWSER_TEST_F(SingleClientCustomPassphraseSyncTest,
                        CommitsEncryptedDataUsingPbkdf2WhenScryptDisabled) {
-  ScopedScryptFeatureToggler toggler(/*force_disabled=*/false,
-                                     /*use_for_new_passphrases=*/false);
+  SetScryptFeaturesState(/*force_disabled=*/false,
+                         /*use_for_new_passphrases=*/false);
   SetEncryptionPassphraseForClient(/*index=*/0, "hunter2");
   ASSERT_TRUE(SetupSync());
   ASSERT_TRUE(AddURL(/*profile=*/0, "PBKDF2 encrypted",
@@ -214,8 +238,8 @@ IN_PROC_BROWSER_TEST_F(SingleClientCustomPassphraseSyncTest,
 
 IN_PROC_BROWSER_TEST_F(SingleClientCustomPassphraseSyncTest,
                        CommitsEncryptedDataUsingScryptWhenScryptEnabled) {
-  ScopedScryptFeatureToggler toggler(/*force_disabled=*/false,
-                                     /*use_for_new_passphrases=*/true);
+  SetScryptFeaturesState(/*force_disabled=*/false,
+                         /*use_for_new_passphrases=*/true);
   SetEncryptionPassphraseForClient(/*index=*/0, "hunter2");
   ASSERT_TRUE(SetupSync());
 
@@ -250,8 +274,8 @@ IN_PROC_BROWSER_TEST_F(SingleClientCustomPassphraseSyncTest,
 
 IN_PROC_BROWSER_TEST_F(SingleClientCustomPassphraseSyncTest,
                        CanDecryptScryptKeyEncryptedDataWhenScryptNotDisabled) {
-  ScopedScryptFeatureToggler toggler(/*force_disabled=*/false,
-                                     /*use_for_new_passphrases_=*/false);
+  SetScryptFeaturesState(/*force_disabled=*/false,
+                         /*used_for_new_passphrases_=*/false);
   KeyParams key_params = {
       KeyDerivationParams::CreateForScrypt("someConstantSalt"), "hunter2"};
   InjectEncryptedServerBookmark("scypt-encrypted bookmark",
@@ -277,8 +301,8 @@ IN_PROC_BROWSER_TEST_F(SingleClientCustomPassphraseSyncTest,
                                 key_params);
   // Can only set feature state now because creating a Nigori and injecting an
   // encrypted bookmark both require key derivation using scrypt.
-  ScopedScryptFeatureToggler toggler(/*force_disabled=*/true,
-                                     /*use_for_new_passphrases_=*/false);
+  SetScryptFeaturesState(/*force_disabled=*/true,
+                         /*used_for_new_passphrases_=*/false);
   SetNigoriInFakeServer(GetFakeServer(), nigori);
   SetDecryptionPassphraseForClient(/*index=*/0, "hunter2");
 
@@ -289,8 +313,8 @@ IN_PROC_BROWSER_TEST_F(SingleClientCustomPassphraseSyncTest,
 
 IN_PROC_BROWSER_TEST_F(SingleClientCustomPassphraseSyncTest,
                        DoesNotLeakUnencryptedData) {
-  ScopedScryptFeatureToggler toggler(/*force_disabled=*/false,
-                                     /*use_for_new_passphrases=*/false);
+  SetScryptFeaturesState(/*force_disabled=*/false,
+                         /*use_for_new_passphrases=*/false);
   SetEncryptionPassphraseForClient(/*index=*/0, "hunter2");
   DatatypeCommitCountingFakeServerObserver observer(GetFakeServer());
   ASSERT_TRUE(SetupSync());
@@ -314,8 +338,8 @@ IN_PROC_BROWSER_TEST_F(SingleClientCustomPassphraseSyncTest,
 
 IN_PROC_BROWSER_TEST_F(SingleClientCustomPassphraseSyncTest,
                        ReencryptsDataWhenPassphraseIsSet) {
-  ScopedScryptFeatureToggler toggler(/*force_disabled=*/false,
-                                     /*use_for_new_passphrases=*/false);
+  SetScryptFeaturesState(/*force_disabled=*/false,
+                         /*use_for_new_passphrases=*/false);
   ASSERT_TRUE(SetupSync());
   ASSERT_TRUE(WaitForNigori(PassphraseType::KEYSTORE_PASSPHRASE));
   ASSERT_TRUE(AddURL(/*profile=*/0, "Re-encryption is great",

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

-// Copyright 2018 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/bookmarks_helper.h"
-#include "chrome/browser/sync/test/integration/encryption_helper.h"
-#include "chrome/browser/sync/test/integration/profile_sync_service_harness.h"
-#include "chrome/browser/sync/test/integration/sync_test.h"
-#include "components/sync/base/sync_base_switches.h"
-#include "components/sync/engine/sync_engine_switches.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace {
-
-static const int kEncryptingClientId = 0;
-static const int kDecryptingClientId = 1;
-
-using bookmarks_helper::AddURL;
-using bookmarks_helper::AllModelsMatchVerifier;
-
-// These tests consider the client as a black-box; they are not concerned with
-// whether the data is committed to the server correctly encrypted. Rather, they
-// test the end-to-end behavior of two clients when a custom passphrase is set,
-// i.e. whether the second client can see data that was committed by the first
-// client. To test proper encryption behavior, a separate single-client test is
-// used.
-class TwoClientCustomPassphraseSyncTest : public SyncTest {
- public:
-  TwoClientCustomPassphraseSyncTest() : SyncTest(TWO_CLIENT) {}
-  ~TwoClientCustomPassphraseSyncTest() override {}
-
-  bool WaitForBookmarksToMatchVerifier() {
-    return BookmarksMatchVerifierChecker().Wait();
-  }
-
-  bool WaitForPassphraseRequiredState(int index, bool desired_state) {
-    return PassphraseRequiredStateChecker(GetSyncService(index), desired_state)
-        .Wait();
-  }
-
-  void AddTestBookmarksToClient(int index) {
-    ASSERT_TRUE(AddURL(index, 0, "What are you syncing about?",
-                       GURL("https://google.com/synced-bookmark-1")));
-    ASSERT_TRUE(AddURL(index, 1, "Test bookmark",
-                       GURL("https://google.com/synced-bookmark-2")));
-  }
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(TwoClientCustomPassphraseSyncTest);
-};
-
-IN_PROC_BROWSER_TEST_F(TwoClientCustomPassphraseSyncTest,
-                       DecryptionFailsWhenIncorrectPassphraseProvided) {
-  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(AllModelsMatchVerifier());
-
-  GetSyncService(kEncryptingClientId)
-      ->SetEncryptionPassphrase("hunter2",
-                                syncer::SyncService::PassphraseType::EXPLICIT);
-  ASSERT_TRUE(WaitForPassphraseRequiredState(kDecryptingClientId,
-                                             /*desired_state=*/true));
-  EXPECT_FALSE(GetSyncService(kDecryptingClientId)
-                   ->SetDecryptionPassphrase("incorrect passphrase"));
-  EXPECT_TRUE(
-      GetSyncService(kDecryptingClientId)->IsPassphraseRequiredForDecryption());
-}
-
-IN_PROC_BROWSER_TEST_F(TwoClientCustomPassphraseSyncTest, ClientsCanSyncData) {
-  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(AllModelsMatchVerifier());
-
-  GetSyncService(kEncryptingClientId)
-      ->SetEncryptionPassphrase("hunter2",
-                                syncer::SyncService::PassphraseType::EXPLICIT);
-  ASSERT_TRUE(WaitForPassphraseRequiredState(kDecryptingClientId,
-                                             /*desired_state=*/true));
-  EXPECT_TRUE(
-      GetSyncService(kDecryptingClientId)->SetDecryptionPassphrase("hunter2"));
-  EXPECT_TRUE(WaitForPassphraseRequiredState(kDecryptingClientId,
-                                             /*desired_state=*/false));
-  AddTestBookmarksToClient(kEncryptingClientId);
-
-  ASSERT_TRUE(
-      GetClient(kEncryptingClientId)
-          ->AwaitMutualSyncCycleCompletion(GetClient(kDecryptingClientId)));
-  EXPECT_TRUE(WaitForBookmarksToMatchVerifier());
-}
-
-IN_PROC_BROWSER_TEST_F(TwoClientCustomPassphraseSyncTest,
-                       ClientsCanSyncDataWhenScryptEncryptionNotEnabled) {
-  ScopedScryptFeatureToggler toggler(/*force_disabled=*/false,
-                                     /*use_for_new_passphrases=*/false);
-  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(AllModelsMatchVerifier());
-
-  GetSyncService(kEncryptingClientId)
-      ->SetEncryptionPassphrase("hunter2",
-                                syncer::SyncService::PassphraseType::EXPLICIT);
-  ASSERT_TRUE(WaitForPassphraseRequiredState(kDecryptingClientId,
-                                             /*desired_state=*/true));
-  EXPECT_TRUE(
-      GetSyncService(kDecryptingClientId)->SetDecryptionPassphrase("hunter2"));
-  EXPECT_TRUE(WaitForPassphraseRequiredState(kDecryptingClientId,
-                                             /*desired_state=*/false));
-  AddTestBookmarksToClient(kEncryptingClientId);
-
-  ASSERT_TRUE(
-      GetClient(kEncryptingClientId)
-          ->AwaitMutualSyncCycleCompletion(GetClient(kDecryptingClientId)));
-  EXPECT_TRUE(WaitForBookmarksToMatchVerifier());
-}
-
-IN_PROC_BROWSER_TEST_F(TwoClientCustomPassphraseSyncTest,
-                       ClientsCanSyncDataWhenScryptEncryptionEnabledInBoth) {
-  ScopedScryptFeatureToggler toggler(/*force_disabled=*/false,
-                                     /*use_for_new_passphrases=*/true);
-  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(AllModelsMatchVerifier());
-
-  GetSyncService(kEncryptingClientId)
-      ->SetEncryptionPassphrase("hunter2",
-                                syncer::SyncService::PassphraseType::EXPLICIT);
-  ASSERT_TRUE(WaitForPassphraseRequiredState(kDecryptingClientId,
-                                             /*desired_state=*/true));
-  EXPECT_TRUE(
-      GetSyncService(kDecryptingClientId)->SetDecryptionPassphrase("hunter2"));
-  EXPECT_TRUE(WaitForPassphraseRequiredState(kDecryptingClientId,
-                                             /*desired_state=*/false));
-  AddTestBookmarksToClient(kEncryptingClientId);
-
-  ASSERT_TRUE(
-      GetClient(kEncryptingClientId)
-          ->AwaitMutualSyncCycleCompletion(GetClient(kDecryptingClientId)));
-  EXPECT_TRUE(WaitForBookmarksToMatchVerifier());
-}
-
-IN_PROC_BROWSER_TEST_F(TwoClientCustomPassphraseSyncTest,
-                       ClientsCanSyncDataWhenScryptEncryptionEnabledInOne) {
-  ScopedScryptFeatureToggler toggler(/*force_disabled=*/false,
-                                     /*use_for_new_passphrases=*/false);
-  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(AllModelsMatchVerifier());
-
-  {
-    ScopedScryptFeatureToggler temporary_toggler(
-        /*force_disabled=*/false, /*use_for_new_passphrases=*/true);
-    GetSyncService(kEncryptingClientId)
-        ->SetEncryptionPassphrase(
-            "hunter2", syncer::SyncService::PassphraseType::EXPLICIT);
-  }
-  ASSERT_TRUE(WaitForPassphraseRequiredState(kDecryptingClientId,
-                                             /*desired_state=*/true));
-  EXPECT_TRUE(
-      GetSyncService(kDecryptingClientId)->SetDecryptionPassphrase("hunter2"));
-  EXPECT_TRUE(WaitForPassphraseRequiredState(kDecryptingClientId,
-                                             /*desired_state=*/false));
-  AddTestBookmarksToClient(kEncryptingClientId);
-
-  ASSERT_TRUE(
-      GetClient(kEncryptingClientId)
-          ->AwaitMutualSyncCycleCompletion(GetClient(kDecryptingClientId)));
-  EXPECT_TRUE(WaitForBookmarksToMatchVerifier());
-}
-
-}  // namespace

chrome/test/BUILD.gn

@@ -5362,7 +5362,6 @@ if (!is_android && !is_fuchsia) {
       "../browser/sync/test/integration/two_client_autocomplete_sync_test.cc",
       "../browser/sync/test/integration/two_client_autofill_sync_test.cc",
       "../browser/sync/test/integration/two_client_bookmarks_sync_test.cc",
-      "../browser/sync/test/integration/two_client_custom_passphrase_sync_test.cc",
       "../browser/sync/test/integration/two_client_dictionary_sync_test.cc",
       "../browser/sync/test/integration/two_client_extension_settings_and_app_settings_sync_test.cc",
       "../browser/sync/test/integration/two_client_extensions_sync_test.cc",