[sync] Refactor search engines' integration tests

This patch migrates the helper libraries to avoid using the concept of
a 'seed', which was hard to grasp, and instead migrates tests to more
directly use keywords.

Change-Id: Ibcdae092baade651daca059850ac31f276f47564
Bug: None
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2495461Reviewed-by: Maksim Moskvitin <mmoskvitin@google.com>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821819}

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

@@ -9,6 +9,10 @@
 #include <vector>
 
 #include "base/bind.h"
+#include "base/guid.h"
+#include "base/hash/sha1.h"
+#include "base/strings/strcat.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
@@ -34,8 +38,8 @@ GUIDToTURLMap CreateGUIDToTURLMap(TemplateURLService* service) {
 }
 
 std::string GetTURLInfoString(const TemplateURL& turl) {
-  return "TemplateURL: shortname: " + base::UTF16ToASCII(turl.short_name()) +
-         " keyword: " + base::UTF16ToASCII(turl.keyword()) +
+  return "TemplateURL: shortname: " + base::UTF16ToUTF8(turl.short_name()) +
+         " keyword: " + base::UTF16ToUTF8(turl.keyword()) +
          " url: " + turl.url();
 }
 
@@ -93,12 +97,6 @@ bool ServicesMatch(int profile_a, int profile_b, std::ostream* os) {
   return true;
 }
 
-// Convenience helper for consistently generating the same keyword for a given
-// seed.
-base::string16 CreateKeyword(int seed) {
-  return base::ASCIIToUTF16(base::StringPrintf("test%d", seed));
-}
-
 }  // namespace
 
 namespace search_engines_helper {
@@ -163,110 +161,102 @@ bool AllServicesMatch(std::ostream* os) {
   return true;
 }
 
-std::unique_ptr<TemplateURL> CreateTestTemplateURL(Profile* profile, int seed) {
-  return CreateTestTemplateURL(profile, seed, CreateKeyword(seed),
-                               base::StringPrintf("0000-0000-0000-%04d", seed));
+TemplateURLBuilder::TemplateURLBuilder(const std::string& keyword) {
+  data_.SetShortName(base::UTF8ToUTF16(keyword));
+  data_.SetKeyword(base::UTF8ToUTF16(keyword));
+  data_.SetURL(base::StringPrintf("http://www.test-%s.com/", keyword.c_str()));
+  data_.favicon_url = GURL("http://favicon.url");
+  data_.safe_for_autoreplace = true;
+  data_.date_created = base::Time::FromTimeT(100);
+  data_.last_modified = base::Time::FromTimeT(100);
+  data_.prepopulate_id = 999999;
+
+  // Produce a GUID deterministically from |keyword|.
+  std::string hex_encoded_hash = base::HexEncode(
+      base::SHA1HashSpan(base::as_bytes(base::make_span(keyword))));
+  hex_encoded_hash.resize(12);
+  data_.sync_guid =
+      base::StrCat({"12345678-0000-4000-8000-", hex_encoded_hash});
+  DCHECK(base::IsValidGUID(data_.sync_guid));
 }
 
-std::unique_ptr<TemplateURL> CreateTestTemplateURL(
-    Profile* profile,
-    int seed,
-    const base::string16& keyword,
-    const std::string& sync_guid) {
-  return CreateTestTemplateURL(profile, seed, keyword,
-      base::StringPrintf("http://www.test%d.com/", seed), sync_guid);
-}
+TemplateURLBuilder::~TemplateURLBuilder() = default;
 
-std::unique_ptr<TemplateURL> CreateTestTemplateURL(
-    Profile* profile,
-    int seed,
-    const base::string16& keyword,
-    const std::string& url,
-    const std::string& sync_guid) {
-  TemplateURLData data;
-  data.SetShortName(CreateKeyword(seed));
-  data.SetKeyword(keyword);
-  data.SetURL(url);
-  data.favicon_url = GURL("http://favicon.url");
-  data.safe_for_autoreplace = true;
-  data.date_created = base::Time::FromTimeT(100);
-  data.last_modified = base::Time::FromTimeT(100);
-  data.prepopulate_id = 999999;
-  data.sync_guid = sync_guid;
-  return std::make_unique<TemplateURL>(data);
+std::unique_ptr<TemplateURL> TemplateURLBuilder::Build() {
+  return std::make_unique<TemplateURL>(data_);
 }
 
-void AddSearchEngine(int profile_index, int seed) {
+void AddSearchEngine(int profile_index, const std::string& keyword) {
   Profile* profile = test()->GetProfile(profile_index);
-  TemplateURLServiceFactory::GetForProfile(profile)->Add(
-      CreateTestTemplateURL(profile, seed));
+  TemplateURLBuilder builder(keyword);
+  TemplateURLServiceFactory::GetForProfile(profile)->Add(builder.Build());
   if (test()->UseVerifier())
-    GetVerifierService()->Add(CreateTestTemplateURL(profile, seed));
+    GetVerifierService()->Add(builder.Build());
 }
 
 void EditSearchEngine(int profile_index,
-                      const base::string16& keyword,
+                      const std::string& keyword,
                       const base::string16& short_name,
-                      const base::string16& new_keyword,
+                      const std::string& new_keyword,
                       const std::string& url) {
   ASSERT_FALSE(url.empty());
   TemplateURLService* service = GetServiceForBrowserContext(profile_index);
-  TemplateURL* turl = service->GetTemplateURLForKeyword(keyword);
+  TemplateURL* turl =
+      service->GetTemplateURLForKeyword(base::UTF8ToUTF16(keyword));
   EXPECT_TRUE(turl);
   ASSERT_FALSE(new_keyword.empty());
-  service->ResetTemplateURL(turl, short_name, new_keyword, url);
+  service->ResetTemplateURL(turl, short_name, base::UTF8ToUTF16(new_keyword),
+                            url);
   // Make sure we do the same on the verifier.
   if (test()->UseVerifier()) {
-    TemplateURL* verifier_turl =
-        GetVerifierService()->GetTemplateURLForKeyword(keyword);
+    TemplateURL* verifier_turl = GetVerifierService()->GetTemplateURLForKeyword(
+        base::UTF8ToUTF16(keyword));
     EXPECT_TRUE(verifier_turl);
     GetVerifierService()->ResetTemplateURL(verifier_turl, short_name,
-                                           new_keyword, url);
+                                           base::UTF8ToUTF16(new_keyword), url);
   }
 }
 
-void DeleteSearchEngineBySeed(int profile_index, int seed) {
+void DeleteSearchEngine(int profile_index, const std::string& keyword) {
   TemplateURLService* service = GetServiceForBrowserContext(profile_index);
-  base::string16 keyword(CreateKeyword(seed));
-  TemplateURL* turl = service->GetTemplateURLForKeyword(keyword);
+  TemplateURL* turl =
+      service->GetTemplateURLForKeyword(base::UTF8ToUTF16(keyword));
   EXPECT_TRUE(turl);
   service->Remove(turl);
   // Make sure we do the same on the verifier.
   if (test()->UseVerifier()) {
-    TemplateURL* verifier_turl =
-        GetVerifierService()->GetTemplateURLForKeyword(keyword);
+    TemplateURL* verifier_turl = GetVerifierService()->GetTemplateURLForKeyword(
+        base::UTF8ToUTF16(keyword));
     EXPECT_TRUE(verifier_turl);
     GetVerifierService()->Remove(verifier_turl);
   }
 }
 
-void ChangeDefaultSearchProvider(int profile_index, int seed) {
+void ChangeDefaultSearchProvider(int profile_index,
+                                 const std::string& keyword) {
   TemplateURLService* service = GetServiceForBrowserContext(profile_index);
-  TemplateURL* turl = service->GetTemplateURLForKeyword(CreateKeyword(seed));
+  TemplateURL* turl =
+      service->GetTemplateURLForKeyword(base::UTF8ToUTF16(keyword));
   ASSERT_TRUE(turl);
   service->SetUserSelectedDefaultSearchProvider(turl);
   if (test()->UseVerifier()) {
-    TemplateURL* verifier_turl =
-        GetVerifierService()->GetTemplateURLForKeyword(CreateKeyword(seed));
+    TemplateURL* verifier_turl = GetVerifierService()->GetTemplateURLForKeyword(
+        base::UTF8ToUTF16(keyword));
     ASSERT_TRUE(verifier_turl);
     GetVerifierService()->SetUserSelectedDefaultSearchProvider(verifier_turl);
   }
 }
 
-bool HasSearchEngine(int profile_index, int seed) {
-  return HasSearchEngineWithKeyword(profile_index, CreateKeyword(seed));
-}
-
-bool HasSearchEngineWithKeyword(int profile_index,
-                                const base::string16& keyword) {
+bool HasSearchEngine(int profile_index, const std::string& keyword) {
   TemplateURLService* service = GetServiceForBrowserContext(profile_index);
-  TemplateURL* turl = service->GetTemplateURLForKeyword(keyword);
+  TemplateURL* turl =
+      service->GetTemplateURLForKeyword(base::UTF8ToUTF16(keyword));
   return turl != nullptr;
 }
 
-base::string16 GetDefaultSearchEngineKeyword(int profile_index) {
+std::string GetDefaultSearchEngineKeyword(int profile_index) {
   TemplateURLService* service = GetServiceForBrowserContext(profile_index);
-  return service->GetDefaultSearchProvider()->keyword();
+  return base::UTF16ToUTF8(service->GetDefaultSearchProvider()->keyword());
 }
 
 SearchEnginesMatchChecker::SearchEnginesMatchChecker() {
@@ -289,12 +279,10 @@ void SearchEnginesMatchChecker::OnTemplateURLServiceChanged() {
   CheckExitCondition();
 }
 
-HasSearchEngineChecker::HasSearchEngineChecker(int profile_index, int seed)
-    : HasSearchEngineChecker(profile_index, CreateKeyword(seed)) {}
-
 HasSearchEngineChecker::HasSearchEngineChecker(int profile_index,
-                                               const base::string16& keyword)
-    : service_(GetServiceForBrowserContext(profile_index)), keyword_(keyword) {
+                                               const std::string& keyword)
+    : service_(GetServiceForBrowserContext(profile_index)),
+      keyword_(base::UTF8ToUTF16(keyword)) {
   observer_.Add(service_);
 }
 

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

@@ -15,9 +15,7 @@
 #include "components/search_engines/template_url_service.h"
 #include "components/search_engines/template_url_service_observer.h"
 
-class Profile;
 class TemplateURL;
-class TemplateURLService;
 
 using GUIDToTURLMap = std::map<std::string, const TemplateURL*>;
 
@@ -37,52 +35,47 @@ bool ServiceMatchesVerifier(int profile_index);
 bool AllServicesMatch();
 bool AllServicesMatch(std::ostream* os);
 
-// Create a TemplateURL with some test values based on |seed|.
-std::unique_ptr<TemplateURL> CreateTestTemplateURL(
-    Profile* profile,
-    int seed,
-    const base::string16& keyword,
-    const std::string& sync_guid);
-std::unique_ptr<TemplateURL> CreateTestTemplateURL(
-    Profile* profile,
-    int seed,
-    const base::string16& keyword,
-    const std::string& url,
-    const std::string& sync_guid);
-
-// Add a search engine based on a seed to the service at index |profile_index|
-// and the verifier if it is used.
-void AddSearchEngine(int profile_index, int seed);
+// Builder class that by default infers all fields from |keyword| and allows
+// overriding those default values.
+class TemplateURLBuilder {
+ public:
+  explicit TemplateURLBuilder(const std::string& keyword);
+  ~TemplateURLBuilder();
+
+  TemplateURLData* data() { return &data_; }
+  std::unique_ptr<TemplateURL> Build();
+
+ private:
+  TemplateURLData data_;
+};
+
+// Add a search engine based on a keyword to the service at index
+// |profile_index| and the verifier if it is used.
+void AddSearchEngine(int profile_index, const std::string& keyword);
 
 // Retrieves a search engine from the service at index |profile_index| with
 // original keyword |keyword| and changes its user-visible fields. Does the same
 // to the verifier, if it is used.
 void EditSearchEngine(int profile_index,
-                      const base::string16& keyword,
+                      const std::string& keyword,
                       const base::string16& short_name,
-                      const base::string16& new_keyword,
+                      const std::string& new_keyword,
                       const std::string& url);
 
-// Deletes a search engine from the service at index |profile_index| which was
-// generated by seed |seed|.
-void DeleteSearchEngineBySeed(int profile_index, int seed);
-
-// Change the search engine generated with |seed| in service at index
-// |profile_index| to be the new default. Does the same to the verifier, if it
-// is used.
-void ChangeDefaultSearchProvider(int profile_index, int seed);
+// Deletes a search engine from the service at index |profile_index| with
+// |keyword|.
+void DeleteSearchEngine(int profile_index, const std::string& keyword);
 
-// Returns true if the profile at |profile_index| has a search engine matching
-// the search engine generated with |seed|.
-bool HasSearchEngine(int profile_index, int seed);
+// Changes the search engine with |keyword| to be the new default for
+// |profile_index|. Does the same to the verifier, if it is used.
+void ChangeDefaultSearchProvider(int profile_index, const std::string& keyword);
 
 // Returns true if the profile at |profile_index| has a search engine matching
 // |keyword|.
-bool HasSearchEngineWithKeyword(int profile_index,
-                                const base::string16& keyword);
+bool HasSearchEngine(int profile_index, const std::string& keyword);
 
 // Returns the keyword for the default search engine at |profile_index|.
-base::string16 GetDefaultSearchEngineKeyword(int profile_index);
+std::string GetDefaultSearchEngineKeyword(int profile_index);
 
 // Checker that blocks until all services have the same search engine data.
 class SearchEnginesMatchChecker : public StatusChangeChecker,
@@ -103,12 +96,11 @@ class SearchEnginesMatchChecker : public StatusChangeChecker,
 };
 
 // Checker that blocks until |profile_index| has a search engine matching the
-// search engine generated with |seed| or |keyword|.
+// search engine generated with |keyword|.
 class HasSearchEngineChecker : public StatusChangeChecker,
                                public TemplateURLServiceObserver {
  public:
-  HasSearchEngineChecker(int profile_index, int seed);
-  HasSearchEngineChecker(int profile_index, const base::string16& keyword);
+  HasSearchEngineChecker(int profile_index, const std::string& keyword);
   ~HasSearchEngineChecker() override;
 
   // StatusChangeChecker overrides.

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

@@ -40,7 +40,8 @@ class SingleClientSearchEnginesSyncTest : public SyncTest {
 IN_PROC_BROWSER_TEST_F(SingleClientSearchEnginesSyncTest, Sanity) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
   ASSERT_TRUE(search_engines_helper::ServiceMatchesVerifier(0));
-  search_engines_helper::AddSearchEngine(0, 0);
+  search_engines_helper::AddSearchEngine(/*profile_index=*/0,
+                                         /*keyword=*/"test0");
   ASSERT_TRUE(UpdatedProgressMarkerChecker(GetSyncService(0)).Wait());
   ASSERT_TRUE(search_engines_helper::ServiceMatchesVerifier(0));
 }

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

@@ -15,8 +15,21 @@
 #include "components/search_engines/template_url_service.h"
 #include "content/public/test/browser_test.h"
 
-using base::ASCIIToUTF16;
+namespace {
+
+using search_engines_helper::AddSearchEngine;
+using search_engines_helper::AllServicesMatch;
+using search_engines_helper::ChangeDefaultSearchProvider;
+using search_engines_helper::DeleteSearchEngine;
+using search_engines_helper::EditSearchEngine;
+using search_engines_helper::GetDefaultSearchEngineKeyword;
+using search_engines_helper::GetServiceForBrowserContext;
+using search_engines_helper::GetVerifierService;
+using search_engines_helper::HasSearchEngine;
+using search_engines_helper::HasSearchEngineChecker;
 using search_engines_helper::SearchEnginesMatchChecker;
+using search_engines_helper::ServiceMatchesVerifier;
+using search_engines_helper::TemplateURLBuilder;
 
 class TwoClientSearchEnginesSyncTest : public SyncTest {
  public:
@@ -62,41 +75,37 @@ IN_PROC_BROWSER_TEST_F(TwoClientSearchEnginesSyncTest, E2E_ENABLED(Add)) {
   ResetSyncForPrimaryAccount();
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
   // TODO(crbug.com/953711): Ideally we could immediately assert
-  // search_engines_helper::AllServicesMatch(), but that's not possible today
-  // without introducing flakiness due to random GUIDs in prepopulated engines.
+  // AllServicesMatch(), but that's not possible today without introducing
+  // flakiness due to random GUIDs in prepopulated engines.
   ASSERT_TRUE(SearchEnginesMatchChecker().Wait());
 
-  // Note that a random seed is needed due to the E2E nature of the tests, and
-  // the synced data persisting in the server across tests.
-  int search_engine_seed = base::Time::Now().ToInternalValue();
-  search_engines_helper::AddSearchEngine(0, search_engine_seed);
-  ASSERT_TRUE(search_engines_helper::HasSearchEngine(0, search_engine_seed));
+  const std::string kKeyword = "test0";
+  AddSearchEngine(/*profile_index=*/0, kKeyword);
+  ASSERT_TRUE(HasSearchEngine(/*profile_index=*/0, kKeyword));
 
   ASSERT_TRUE(SearchEnginesMatchChecker().Wait());
-  ASSERT_TRUE(search_engines_helper::HasSearchEngine(1, search_engine_seed));
+  ASSERT_TRUE(HasSearchEngine(/*profile_index=*/1, kKeyword));
 }
 
 IN_PROC_BROWSER_TEST_F(TwoClientSearchEnginesSyncTest, E2E_ENABLED(Delete)) {
   ResetSyncForPrimaryAccount();
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
   // TODO(crbug.com/953711): Ideally we could immediately assert
-  // search_engines_helper::AllServicesMatch(), but that's not possible today
-  // without introducing flakiness due to random GUIDs in prepopulated engines.
+  // AllServicesMatch(), but that's not possible today without introducing
+  // flakiness due to random GUIDs in prepopulated engines.
   ASSERT_TRUE(SearchEnginesMatchChecker().Wait());
 
-  // Note that a random seed is needed due to the E2E nature of the tests, and
-  // the synced data persisting in the server across tests.
-  int search_engine_seed = base::Time::Now().ToInternalValue();
-  search_engines_helper::AddSearchEngine(0, search_engine_seed);
-  ASSERT_TRUE(search_engines_helper::HasSearchEngine(0, search_engine_seed));
+  const std::string kKeyword = "test0";
+  AddSearchEngine(/*profile_index=*/0, kKeyword);
+  ASSERT_TRUE(HasSearchEngine(/*profile_index=*/0, kKeyword));
 
   ASSERT_TRUE(SearchEnginesMatchChecker().Wait());
-  ASSERT_TRUE(search_engines_helper::HasSearchEngine(1, search_engine_seed));
+  ASSERT_TRUE(HasSearchEngine(/*profile_index=*/1, kKeyword));
 
-  search_engines_helper::DeleteSearchEngineBySeed(0, search_engine_seed);
+  DeleteSearchEngine(/*profile_index=*/0, kKeyword);
 
   ASSERT_TRUE(SearchEnginesMatchChecker().Wait());
-  ASSERT_FALSE(search_engines_helper::HasSearchEngine(1, search_engine_seed));
+  ASSERT_FALSE(HasSearchEngine(/*profile_index=*/1, kKeyword));
 }
 
 IN_PROC_BROWSER_TEST_F(TwoClientSearchEnginesSyncTest,
@@ -104,13 +113,14 @@ IN_PROC_BROWSER_TEST_F(TwoClientSearchEnginesSyncTest,
   ResetSyncForPrimaryAccount();
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
   // TODO(crbug.com/953711): Ideally we could immediately assert
-  // search_engines_helper::AllServicesMatch(), but that's not possible today
-  // without introducing flakiness due to random GUIDs in prepopulated engines.
+  // AllServicesMatch(), but that's not possible today without introducing
+  // flakiness due to random GUIDs in prepopulated engines.
   ASSERT_TRUE(SearchEnginesMatchChecker().Wait());
 
   // Add a few entries.
-  for (int i = 0; i < 3; ++i)
-    search_engines_helper::AddSearchEngine(0, i);
+  AddSearchEngine(/*profile_index=*/0, "test0");
+  AddSearchEngine(/*profile_index=*/0, "test1");
+  AddSearchEngine(/*profile_index=*/0, "test2");
 
   ASSERT_TRUE(SearchEnginesMatchChecker().Wait());
 }
@@ -118,20 +128,21 @@ IN_PROC_BROWSER_TEST_F(TwoClientSearchEnginesSyncTest,
 IN_PROC_BROWSER_TEST_F(TwoClientSearchEnginesSyncTestWithVerifier, Duplicates) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
   // TODO(crbug.com/953711): Ideally we could immediately assert
-  // search_engines_helper::AllServicesMatch(), but that's not possible today
-  // without introducing flakiness due to random GUIDs in prepopulated engines.
+  // AllServicesMatch(), but that's not possible today without introducing
+  // flakiness due to random GUIDs in prepopulated engines.
   ASSERT_TRUE(SearchEnginesMatchChecker().Wait());
 
-  // Add two entries with the same Name and URL (but different keywords).
-  // Note that we have to change the GUID of the duplicate.
-  search_engines_helper::AddSearchEngine(0, 0);
-  Profile* profile = sync_datatype_helper::test()->GetProfile(0);
-  TemplateURLServiceFactory::GetForProfile(profile)->Add(
-      search_engines_helper::CreateTestTemplateURL(profile, 0,
-          ASCIIToUTF16("somethingelse"), "newguid"));
-  search_engines_helper::GetVerifierService()->Add(
-      search_engines_helper::CreateTestTemplateURL(profile, 0,
-          ASCIIToUTF16("somethingelse"), "newguid"));
+  // Add two entries with the same Name and URL (but different keywords). Note
+  // that we have to change the GUID of the duplicate.
+  TemplateURLBuilder builder("test0");
+  GetServiceForBrowserContext(0)->Add(builder.Build());
+  GetVerifierService()->Add(builder.Build());
+
+  builder.data()->SetKeyword(base::UTF8ToUTF16("test1"));
+  builder.data()->sync_guid = "newguid";
+  GetServiceForBrowserContext(0)->Add(builder.Build());
+  GetVerifierService()->Add(builder.Build());
+
   ASSERT_TRUE(SearchEnginesMatchChecker().Wait());
 }
 
@@ -140,18 +151,18 @@ IN_PROC_BROWSER_TEST_F(TwoClientSearchEnginesSyncTest,
   ResetSyncForPrimaryAccount();
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
   // TODO(crbug.com/953711): Ideally we could immediately assert
-  // search_engines_helper::AllServicesMatch(), but that's not possible today
-  // without introducing flakiness due to random GUIDs in prepopulated engines.
+  // AllServicesMatch(), but that's not possible today without introducing
+  // flakiness due to random GUIDs in prepopulated engines.
   ASSERT_TRUE(SearchEnginesMatchChecker().Wait());
 
-  search_engines_helper::AddSearchEngine(0, 0);
+  AddSearchEngine(/*profile_index=*/0, "test0");
 
   // Change the keyword.
   ASSERT_TRUE(SearchEnginesMatchChecker().Wait());
 
-  search_engines_helper::EditSearchEngine(0, ASCIIToUTF16("test0"),
-      ASCIIToUTF16("test0"), ASCIIToUTF16("newkeyword"),
-      "http://www.test0.com/");
+  EditSearchEngine(/*profile_index=*/0, /*keyword=*/"test0",
+                   base::UTF8ToUTF16("test0"), /*new_keyword=*/"newkeyword",
+                   "http://www.test0.com/");
 
   ASSERT_TRUE(SearchEnginesMatchChecker().Wait());
 }
@@ -160,18 +171,18 @@ IN_PROC_BROWSER_TEST_F(TwoClientSearchEnginesSyncTest, E2E_ENABLED(UpdateUrl)) {
   ResetSyncForPrimaryAccount();
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
   // TODO(crbug.com/953711): Ideally we could immediately assert
-  // search_engines_helper::AllServicesMatch(), but that's not possible today
-  // without introducing flakiness due to random GUIDs in prepopulated engines.
+  // AllServicesMatch(), but that's not possible today without introducing
+  // flakiness due to random GUIDs in prepopulated engines.
   ASSERT_TRUE(SearchEnginesMatchChecker().Wait());
 
-  search_engines_helper::AddSearchEngine(0, 0);
+  AddSearchEngine(/*profile_index=*/0, "test0");
 
   ASSERT_TRUE(SearchEnginesMatchChecker().Wait());
 
   // Change the URL.
-  search_engines_helper::EditSearchEngine(0, ASCIIToUTF16("test0"),
-      ASCIIToUTF16("test0"), ASCIIToUTF16("test0"),
-      "http://www.wikipedia.org/q=%s");
+  EditSearchEngine(/*profile_index=*/0, /*keyword=*/"test0",
+                   base::UTF8ToUTF16("test0"), /*new_keyword=*/"test0",
+                   "http://www.wikipedia.org/q=%s");
 
   ASSERT_TRUE(SearchEnginesMatchChecker().Wait());
 }
@@ -181,17 +192,17 @@ IN_PROC_BROWSER_TEST_F(TwoClientSearchEnginesSyncTest,
   ResetSyncForPrimaryAccount();
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
   // TODO(crbug.com/953711): Ideally we could immediately assert
-  // search_engines_helper::AllServicesMatch(), but that's not possible today
-  // without introducing flakiness due to random GUIDs in prepopulated engines.
+  // AllServicesMatch(), but that's not possible today without introducing
+  // flakiness due to random GUIDs in prepopulated engines.
   ASSERT_TRUE(SearchEnginesMatchChecker().Wait());
 
-  search_engines_helper::AddSearchEngine(0, 0);
+  AddSearchEngine(/*profile_index=*/0, "test0");
 
   ASSERT_TRUE(SearchEnginesMatchChecker().Wait());
 
   // Change the short name.
-  search_engines_helper::EditSearchEngine(0, ASCIIToUTF16("test0"),
-      ASCIIToUTF16("New Name"), ASCIIToUTF16("test0"), "http://www.test0.com/");
+  EditSearchEngine(/*profile_index=*/0, "test0", base::UTF8ToUTF16("New Name"),
+                   "test0", "http://www.test0.com/");
 
   ASSERT_TRUE(SearchEnginesMatchChecker().Wait());
 }
@@ -199,69 +210,70 @@ IN_PROC_BROWSER_TEST_F(TwoClientSearchEnginesSyncTest,
 IN_PROC_BROWSER_TEST_F(TwoClientSearchEnginesSyncTest, ConflictKeyword) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
   // TODO(crbug.com/953711): Ideally we could immediately assert
-  // search_engines_helper::AllServicesMatch(), but that's not possible today
-  // without introducing flakiness due to random GUIDs in prepopulated engines.
+  // AllServicesMatch(), but that's not possible today without introducing
+  // flakiness due to random GUIDs in prepopulated engines.
   ASSERT_TRUE(SearchEnginesMatchChecker().Wait());
 
   // Add a different search engine to each client, but make their keywords
   // conflict.
-  search_engines_helper::AddSearchEngine(0, 0);
-  search_engines_helper::AddSearchEngine(1, 1);
-  TemplateURLService* service =
-      search_engines_helper::GetServiceForBrowserContext(1);
-  TemplateURL* turl = service->GetTemplateURLForKeyword(ASCIIToUTF16("test1"));
+  AddSearchEngine(/*profile_index=*/0, "test0");
+  AddSearchEngine(/*profile_index=*/1, "test1");
+  TemplateURLService* service = GetServiceForBrowserContext(1);
+  TemplateURL* turl =
+      service->GetTemplateURLForKeyword(base::UTF8ToUTF16("test1"));
   EXPECT_TRUE(turl);
-  service->ResetTemplateURL(turl, turl->short_name(), ASCIIToUTF16("test0"),
-                            turl->url());
+  service->ResetTemplateURL(turl, turl->short_name(),
+                            base::UTF8ToUTF16("test0"), turl->url());
 
   ASSERT_TRUE(AwaitQuiescence());
-  ASSERT_TRUE(search_engines_helper::AllServicesMatch());
+  ASSERT_TRUE(AllServicesMatch());
 }
 
 IN_PROC_BROWSER_TEST_F(TwoClientSearchEnginesSyncTest, MergeMultiple) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
   // TODO(crbug.com/953711): Ideally we could immediately assert
-  // search_engines_helper::AllServicesMatch(), but that's not possible today
-  // without introducing flakiness due to random GUIDs in prepopulated engines.
+  // AllServicesMatch(), but that's not possible today without introducing
+  // flakiness due to random GUIDs in prepopulated engines.
   ASSERT_TRUE(SearchEnginesMatchChecker().Wait());
 
   // Set up some different search engines on each client, with some interesting
-  // conflicts.
-  // client0: { SE0, SE1, SE2 }
-  for (int i = 0; i < 3; ++i)
-    search_engines_helper::AddSearchEngine(0, i);
+  // conflicts. client0: { SE0, SE1, SE2 }
+  AddSearchEngine(/*profile_index=*/0, "test0");
+  AddSearchEngine(/*profile_index=*/0, "test1");
+  AddSearchEngine(/*profile_index=*/0, "test2");
 
   // client1: { SE0, SE2, SE3, SE0 + different URL }
-  search_engines_helper::AddSearchEngine(1, 0);
-  search_engines_helper::AddSearchEngine(1, 2);
-  search_engines_helper::AddSearchEngine(1, 3);
-  Profile* profile = sync_datatype_helper::test()->GetProfile(1);
-  TemplateURLServiceFactory::GetForProfile(profile)->Add(
-      search_engines_helper::CreateTestTemplateURL(profile, 0,
-          ASCIIToUTF16("somethingelse.com"), "http://www.somethingelse.com/",
-          "somethingelse"));
+  AddSearchEngine(/*profile_index=*/0, "test0");
+  AddSearchEngine(/*profile_index=*/0, "test2");
+  AddSearchEngine(/*profile_index=*/0, "test3");
+
+  TemplateURLBuilder builder("test0");
+  builder.data()->SetKeyword(base::UTF8ToUTF16("somethingelse.com"));
+  builder.data()->SetURL("http://www.somethingelse.com/");
+  builder.data()->sync_guid = "somethingelse";
+  GetServiceForBrowserContext(1)->Add(builder.Build());
 
   ASSERT_TRUE(AwaitQuiescence());
-  ASSERT_TRUE(search_engines_helper::AllServicesMatch());
+  ASSERT_TRUE(AllServicesMatch());
 }
 
 IN_PROC_BROWSER_TEST_F(TwoClientSearchEnginesSyncTestWithVerifier,
                        DisableSync) {
   ASSERT_TRUE(SetupSync());
   // TODO(crbug.com/953711): Ideally we could immediately assert
-  // search_engines_helper::AllServicesMatch(), but that's not possible today
-  // without introducing flakiness due to random GUIDs in prepopulated engines.
+  // AllServicesMatch(), but that's not possible today without introducing
+  // flakiness due to random GUIDs in prepopulated engines.
   ASSERT_TRUE(SearchEnginesMatchChecker().Wait());
 
   ASSERT_TRUE(GetClient(1)->DisableSyncForAllDatatypes());
-  search_engines_helper::AddSearchEngine(0, 0);
+  AddSearchEngine(/*profile_index=*/0, "test0");
   ASSERT_TRUE(UpdatedProgressMarkerChecker(GetSyncService(0)).Wait());
-  ASSERT_TRUE(search_engines_helper::ServiceMatchesVerifier(0));
-  ASSERT_FALSE(search_engines_helper::ServiceMatchesVerifier(1));
+  ASSERT_TRUE(ServiceMatchesVerifier(0));
+  ASSERT_FALSE(ServiceMatchesVerifier(1));
 
   ASSERT_TRUE(GetClient(1)->EnableSyncForRegisteredDatatypes());
   ASSERT_TRUE(AwaitQuiescence());
-  ASSERT_TRUE(search_engines_helper::AllServicesMatch());
+  ASSERT_TRUE(AllServicesMatch());
 }
 
 IN_PROC_BROWSER_TEST_F(TwoClientSearchEnginesSyncTest,
@@ -269,17 +281,17 @@ IN_PROC_BROWSER_TEST_F(TwoClientSearchEnginesSyncTest,
   ResetSyncForPrimaryAccount();
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
   // TODO(crbug.com/953711): Ideally we could immediately assert
-  // search_engines_helper::AllServicesMatch(), but that's not possible today
-  // without introducing flakiness due to random GUIDs in prepopulated engines.
+  // AllServicesMatch(), but that's not possible today without introducing
+  // flakiness due to random GUIDs in prepopulated engines.
   ASSERT_TRUE(SearchEnginesMatchChecker().Wait());
 
-  search_engines_helper::AddSearchEngine(0, 0);
+  AddSearchEngine(/*profile_index=*/0, "test0");
   ASSERT_TRUE(SearchEnginesMatchChecker().Wait());
 
   // Change the default to the new search engine, sync, and ensure that it
   // changed in the second client. AllServicesMatch does a default search
   // provider check.
-  search_engines_helper::ChangeDefaultSearchProvider(0, 0);
+  ChangeDefaultSearchProvider(/*profile_index=*/0, "test0");
   ASSERT_TRUE(SearchEnginesMatchChecker().Wait());
 }
 
@@ -290,20 +302,20 @@ IN_PROC_BROWSER_TEST_F(TwoClientSearchEnginesSyncTest,
   ResetSyncForPrimaryAccount();
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
   // TODO(crbug.com/953711): Ideally we could immediately assert
-  // search_engines_helper::AllServicesMatch(), but that's not possible today
-  // without introducing flakiness due to random GUIDs in prepopulated engines.
+  // AllServicesMatch(), but that's not possible today without introducing
+  // flakiness due to random GUIDs in prepopulated engines.
   ASSERT_TRUE(SearchEnginesMatchChecker().Wait());
 
-  search_engines_helper::AddSearchEngine(0, 0);
-  search_engines_helper::AddSearchEngine(0, 1);
+  AddSearchEngine(/*profile_index=*/0, "test0");
+  AddSearchEngine(/*profile_index=*/0, "test1");
   ASSERT_TRUE(SearchEnginesMatchChecker().Wait());
 
-  search_engines_helper::ChangeDefaultSearchProvider(0, 0);
+  ChangeDefaultSearchProvider(/*profile_index=*/0, "test0");
   ASSERT_TRUE(SearchEnginesMatchChecker().Wait());
 
   // Change the default on the first client and delete the old default.
-  search_engines_helper::ChangeDefaultSearchProvider(0, 1);
-  search_engines_helper::DeleteSearchEngineBySeed(0, 0);
+  ChangeDefaultSearchProvider(/*profile_index=*/0, "test1");
+  DeleteSearchEngine(/*profile_index=*/0, "test0");
   ASSERT_TRUE(SearchEnginesMatchChecker().Wait());
 }
 
@@ -317,12 +329,11 @@ IN_PROC_BROWSER_TEST_F(TwoClientSearchEnginesSyncTest,
                        DeleteSyncedDefaultWithoutPrefSync) {
   ASSERT_TRUE(SetupClients());
 
-  search_engines_helper::AddSearchEngine(/*profile_index=*/0, /*seed=*/0);
-  search_engines_helper::AddSearchEngine(/*profile_index=*/0, /*seed=*/1);
-  search_engines_helper::AddSearchEngine(/*profile_index=*/1, /*seed=*/0);
-  search_engines_helper::AddSearchEngine(/*profile_index=*/1, /*seed=*/1);
-  search_engines_helper::ChangeDefaultSearchProvider(/*profile_index=*/0,
-                                                     /*seed=*/0);
+  AddSearchEngine(/*profile_index=*/0, "test0");
+  AddSearchEngine(/*profile_index=*/0, "test1");
+  AddSearchEngine(/*profile_index=*/1, "test0");
+  AddSearchEngine(/*profile_index=*/1, "test1");
+  ChangeDefaultSearchProvider(/*profile_index=*/0, "test0");
 
   ASSERT_TRUE(SetupSync());
   ASSERT_TRUE(SearchEnginesMatchChecker().Wait());
@@ -332,39 +343,32 @@ IN_PROC_BROWSER_TEST_F(TwoClientSearchEnginesSyncTest,
   GetFakeServer()->SetThrottledTypes({syncer::PREFERENCES});
 
   // Rule out search engines with underscores existing at this point.
-  // Note that seed==0 corresponds to keyword "test0".
-  ASSERT_TRUE(search_engines_helper::HasSearchEngineWithKeyword(
-      /*profile_index=*/0, ASCIIToUTF16("test0")));
-  ASSERT_FALSE(search_engines_helper::HasSearchEngineWithKeyword(
-      /*profile_index=*/0, ASCIIToUTF16("test0_")));
-  ASSERT_TRUE(search_engines_helper::HasSearchEngineWithKeyword(
-      /*profile_index=*/1, ASCIIToUTF16("test0")));
-  ASSERT_FALSE(search_engines_helper::HasSearchEngineWithKeyword(
-      /*profile_index=*/1, ASCIIToUTF16("test0_")));
+  ASSERT_TRUE(HasSearchEngine(
+      /*profile_index=*/0, "test0"));
+  ASSERT_FALSE(HasSearchEngine(
+      /*profile_index=*/0, "test0_"));
+  ASSERT_TRUE(HasSearchEngine(
+      /*profile_index=*/1, "test0"));
+  ASSERT_FALSE(HasSearchEngine(
+      /*profile_index=*/1, "test0_"));
 
   // Change the default on the first client (profile index 0) and delete the old
   // default.
-  search_engines_helper::ChangeDefaultSearchProvider(/*profile_index=*/0,
-                                                     /*seed=*/1);
-  search_engines_helper::DeleteSearchEngineBySeed(/*profile_index=*/0,
-                                                  /*seed=*/0);
+  ChangeDefaultSearchProvider(/*profile_index=*/0, "test1");
+  DeleteSearchEngine(/*profile_index=*/0, "test0");
 
   // The test needs to wait until the second client (profile index 1) receives
   // the deletion. In order to do so, use the first client (profile index 0) to
-  // create a third search engine (seed 2) and wait until it gets sync-ed to the
+  // create a third search engine (test2) and wait until it gets sync-ed to the
   // second client (profile index 1).
-  search_engines_helper::AddSearchEngine(/*profile_index=*/0, /*seed=*/2);
-  ASSERT_TRUE(search_engines_helper::HasSearchEngineChecker(/*profile_index=*/1,
-                                                            /*seed=*/2)
-                  .Wait());
+  AddSearchEngine(/*profile_index=*/0, "test2");
+  ASSERT_TRUE(HasSearchEngineChecker(/*profile_index=*/1, "test2").Wait());
 
   // In the receiving end (profile index 1), the deletion cannot be honored
   // since it's the default search provider. Expect that it's preserved.
-  EXPECT_TRUE(search_engines_helper::HasSearchEngineWithKeyword(
-      /*profile_index=*/1, ASCIIToUTF16("test0")));
-  EXPECT_EQ(
-      search_engines_helper::GetDefaultSearchEngineKeyword(/*profile_index=*/1),
-      ASCIIToUTF16("test0"));
+  EXPECT_TRUE(HasSearchEngine(
+      /*profile_index=*/1, "test0"));
+  EXPECT_EQ(GetDefaultSearchEngineKeyword(/*profile_index=*/1), "test0");
 
   // The search engine that cannot be deleted should not immediately sync back
   // to profile index 0. Eventually, it likely will during reconciliation on
@@ -372,16 +376,16 @@ IN_PROC_BROWSER_TEST_F(TwoClientSearchEnginesSyncTest,
   // sending an immediate undelete or creating an underscore duplicate.
   // https://crbug.com/1022775
   //
-  // To test this, we create yet another engine (seed 3) that we wait to be
+  // To test this, we create yet another engine (test3) that we wait to be
   // synced from profile index 1 to profile index 0. Then we verify that "test0"
   // or "test0_" was not also synced back. (We used to create a duplicate
   // underscored engine, so we verify we don't do that anymore.)
-  search_engines_helper::AddSearchEngine(/*profile_index=*/1, /*seed=*/3);
-  ASSERT_TRUE(search_engines_helper::HasSearchEngineChecker(/*profile_index=*/0,
-                                                            /*seed=*/3)
-                  .Wait());
-  EXPECT_FALSE(search_engines_helper::HasSearchEngineWithKeyword(
-      /*profile_index=*/0, ASCIIToUTF16("test0")));
-  EXPECT_FALSE(search_engines_helper::HasSearchEngineWithKeyword(
-      /*profile_index=*/0, ASCIIToUTF16("test0_")));
+  AddSearchEngine(/*profile_index=*/1, "test3");
+  ASSERT_TRUE(HasSearchEngineChecker(/*profile_index=*/0, "test3").Wait());
+  EXPECT_FALSE(HasSearchEngine(
+      /*profile_index=*/0, "test0"));
+  EXPECT_FALSE(HasSearchEngine(
+      /*profile_index=*/0, "test0_"));
 }
+
+}  // namespace