Add a Profile* member to TemplateURL. This makes some invocations of ReplaceSearchTerms() a bit le…

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

Index: chrome/browser/sync/test/integration/search_engines_helper.cc
===================================================================
--- chrome/browser/sync/test/integration/search_engines_helper.cc	(revision 132905)
+++ chrome/browser/sync/test/integration/search_engines_helper.cc	(working copy)
@@ -18,18 +18,8 @@
 #include "chrome/browser/sync/test/integration/sync_datatype_helper.h"
 #include "chrome/browser/sync/test/integration/sync_test.h"
 
-using sync_datatype_helper::test;
+namespace {
 
-namespace search_engines_helper {
-
-TemplateURLService* GetServiceForProfile(int index) {
-  return TemplateURLServiceFactory::GetForProfile(test()->GetProfile(index));
-}
-
-TemplateURLService* GetVerifierService() {
-  return TemplateURLServiceFactory::GetForProfile(test()->verifier());
-}
-
 GUIDToTURLMap CreateGUIDToTURLMap(TemplateURLService* service) {
   CHECK(service);
 
@@ -67,43 +57,11 @@
   return result;
 }
 
-bool ServiceMatchesVerifier(int profile) {
-  TemplateURLService* verifier = GetVerifierService();
-  TemplateURLService* other = GetServiceForProfile(profile);
-
-  CHECK(verifier);
-  CHECK(other);
-
-  TemplateURLService::TemplateURLVector verifier_turls =
-      verifier->GetTemplateURLs();
-  if (verifier_turls.size() != other->GetTemplateURLs().size()) {
-    LOG(ERROR) << "Verifier and other service have a different count of TURLs: "
-               << verifier_turls.size() << " vs "
-               << other->GetTemplateURLs().size() << " respectively.";
-    return false;
-  }
-
-  for (size_t i = 0; i < verifier_turls.size(); ++i) {
-    const TemplateURL* verifier_turl = verifier_turls.at(i);
-    CHECK(verifier_turl);
-    const TemplateURL* other_turl = other->GetTemplateURLForKeyword(
-        verifier_turl->keyword());
-
-    if (!other_turl) {
-      LOG(ERROR) << "The other service did not contain a TURL with keyword: "
-                 << verifier_turl->keyword();
-      return false;
-    }
-    if (!TURLsMatch(verifier_turl, other_turl))
-      return false;
-  }
-
-  return true;
-}
-
 bool ServicesMatch(int profile_a, int profile_b) {
-  TemplateURLService* service_a = GetServiceForProfile(profile_a);
-  TemplateURLService* service_b = GetServiceForProfile(profile_b);
+  TemplateURLService* service_a =
+      search_engines_helper::GetServiceForProfile(profile_a);
+  TemplateURLService* service_b =
+      search_engines_helper::GetServiceForProfile(profile_b);
   CHECK(service_a);
   CHECK(service_b);
 
@@ -145,14 +103,69 @@
   return true;
 }
 
+// Convenience helper for consistently generating the same keyword for a given
+// seed.
+string16 CreateKeyword(int seed) {
+  return ASCIIToUTF16(base::StringPrintf("test%d", seed));
+}
+
+}  // namespace
+
+namespace search_engines_helper {
+
+TemplateURLService* GetServiceForProfile(int profile_index) {
+  return TemplateURLServiceFactory::GetForProfile(
+      sync_datatype_helper::test()->GetProfile(profile_index));
+}
+
+TemplateURLService* GetVerifierService() {
+  return TemplateURLServiceFactory::GetForProfile(
+      sync_datatype_helper::test()->verifier());
+}
+
+bool ServiceMatchesVerifier(int profile_index) {
+  TemplateURLService* verifier = GetVerifierService();
+  TemplateURLService* other = GetServiceForProfile(profile_index);
+
+  CHECK(verifier);
+  CHECK(other);
+
+  TemplateURLService::TemplateURLVector verifier_turls =
+      verifier->GetTemplateURLs();
+  if (verifier_turls.size() != other->GetTemplateURLs().size()) {
+    LOG(ERROR) << "Verifier and other service have a different count of TURLs: "
+               << verifier_turls.size() << " vs "
+               << other->GetTemplateURLs().size() << " respectively.";
+    return false;
+  }
+
+  for (size_t i = 0; i < verifier_turls.size(); ++i) {
+    const TemplateURL* verifier_turl = verifier_turls.at(i);
+    CHECK(verifier_turl);
+    const TemplateURL* other_turl = other->GetTemplateURLForKeyword(
+        verifier_turl->keyword());
+
+    if (!other_turl) {
+      LOG(ERROR) << "The other service did not contain a TURL with keyword: "
+                 << verifier_turl->keyword();
+      return false;
+    }
+    if (!TURLsMatch(verifier_turl, other_turl))
+      return false;
+  }
+
+  return true;
+}
+
 bool AllServicesMatch() {
   // Use 0 as the baseline.
-  if (test()->use_verifier() && !ServiceMatchesVerifier(0)) {
+  if (sync_datatype_helper::test()->use_verifier() &&
+      !ServiceMatchesVerifier(0)) {
     LOG(ERROR) << "TemplateURLService 0 does not match verifier.";
     return false;
   }
 
-  for (int it = 1; it < test()->num_clients(); ++it) {
+  for (int it = 1; it < sync_datatype_helper::test()->num_clients(); ++it) {
     if (!ServicesMatch(0, it)) {
       LOG(ERROR) << "TemplateURLService " << it << " does not match with "
                  << "service 0.";
@@ -162,25 +175,21 @@
   return true;
 }
 
-// Convenience helper for consistently generating the same keyword for a given
-// seed.
-string16 CreateKeyword(int seed) {
-  return ASCIIToUTF16(base::StringPrintf("test%d", seed));
-}
-
-TemplateURL* CreateTestTemplateURL(int seed) {
-  return CreateTestTemplateURL(seed, CreateKeyword(seed),
+TemplateURL* CreateTestTemplateURL(Profile* profile, int seed) {
+  return CreateTestTemplateURL(profile, seed, CreateKeyword(seed),
                                base::StringPrintf("0000-0000-0000-%04d", seed));
 }
 
-TemplateURL* CreateTestTemplateURL(int seed,
+TemplateURL* CreateTestTemplateURL(Profile* profile,
+                                   int seed,
                                    const string16& keyword,
                                    const std::string& sync_guid) {
-  return CreateTestTemplateURL(seed, keyword,
+  return CreateTestTemplateURL(profile, seed, keyword,
       base::StringPrintf("http://www.test%d.com/", seed), sync_guid);
 }
 
-TemplateURL* CreateTestTemplateURL(int seed,
+TemplateURL* CreateTestTemplateURL(Profile* profile,
+                                   int seed,
                                    const string16& keyword,
                                    const std::string& url,
                                    const std::string& sync_guid) {
@@ -194,29 +203,30 @@
   data.last_modified = base::Time::FromTimeT(100);
   data.prepopulate_id = 999999;
   data.sync_guid = sync_guid;
-  return new TemplateURL(data);
+  return new TemplateURL(profile, data);
 }
 
-void AddSearchEngine(int profile, int seed) {
-  GetServiceForProfile(profile)->Add(CreateTestTemplateURL(seed));
-  if (test()->use_verifier())
-    GetVerifierService()->Add(CreateTestTemplateURL(seed));
+void AddSearchEngine(int profile_index, int seed) {
+  Profile* profile = sync_datatype_helper::test()->GetProfile(profile_index);
+  TemplateURLServiceFactory::GetForProfile(profile)->Add(
+      CreateTestTemplateURL(profile, seed));
+  if (sync_datatype_helper::test()->use_verifier())
+    GetVerifierService()->Add(CreateTestTemplateURL(profile, seed));
 }
 
-void EditSearchEngine(int profile,
+void EditSearchEngine(int profile_index,
                       const string16& keyword,
                       const string16& short_name,
                       const string16& new_keyword,
                       const std::string& url) {
   DCHECK(!url.empty());
-  const TemplateURL* turl =
-      GetServiceForProfile(profile)->GetTemplateURLForKeyword(keyword);
+  TemplateURLService* service = GetServiceForProfile(profile_index);
+  const TemplateURL* turl = service->GetTemplateURLForKeyword(keyword);
   EXPECT_TRUE(turl);
   ASSERT_FALSE(new_keyword.empty());
-  GetServiceForProfile(profile)->ResetTemplateURL(turl, short_name, new_keyword,
-                                                  url);
+  service->ResetTemplateURL(turl, short_name, new_keyword, url);
   // Make sure we do the same on the verifier.
-  if (test()->use_verifier()) {
+  if (sync_datatype_helper::test()->use_verifier()) {
     const TemplateURL* verifier_turl =
         GetVerifierService()->GetTemplateURLForKeyword(keyword);
     EXPECT_TRUE(verifier_turl);
@@ -225,13 +235,14 @@
   }
 }
 
-void DeleteSearchEngineByKeyword(int profile, const string16& keyword) {
-  const TemplateURL* turl =
-      GetServiceForProfile(profile)->GetTemplateURLForKeyword(keyword);
+void DeleteSearchEngineBySeed(int profile_index, int seed) {
+  TemplateURLService* service = GetServiceForProfile(profile_index);
+  string16 keyword(CreateKeyword(seed));
+  const TemplateURL* turl = service->GetTemplateURLForKeyword(keyword);
   EXPECT_TRUE(turl);
-  GetServiceForProfile(profile)->Remove(turl);
+  service->Remove(turl);
   // Make sure we do the same on the verifier.
-  if (test()->use_verifier()) {
+  if (sync_datatype_helper::test()->use_verifier()) {
     const TemplateURL* verifier_turl =
         GetVerifierService()->GetTemplateURLForKeyword(keyword);
     EXPECT_TRUE(verifier_turl);
@@ -239,18 +250,14 @@
   }
 }
 
-void DeleteSearchEngineBySeed(int profile, int seed) {
-  DeleteSearchEngineByKeyword(profile, CreateKeyword(seed));
-}
-
-void ChangeDefaultSearchProvider(int profile, int seed) {
-  TemplateURLService* service = GetServiceForProfile(profile);
+void ChangeDefaultSearchProvider(int profile_index, int seed) {
+  TemplateURLService* service = GetServiceForProfile(profile_index);
   ASSERT_TRUE(service);
   const TemplateURL* turl =
       service->GetTemplateURLForKeyword(CreateKeyword(seed));
   ASSERT_TRUE(turl);
   service->SetDefaultSearchProvider(turl);
-  if (test()->use_verifier()) {
+  if (sync_datatype_helper::test()->use_verifier()) {

[Andrew T Wilson (Slow), 2012/04/23 16:36:36]

This is fine to land, but does it really help readability to explicitly use
namespacing everywhere in this file rather than putting a single
using::sync_datatype_helper::test; at the top?

[Peter Kasting, 2012/04/23 18:14:26]

I'm not sure, honestly.

The reason I changed this was because I was really confused reading the code
originally and kept looking for some sort of file-scoped or class-scoped member
named test().  It was only after a bit of searching that I noticed the using
statement.

OTOH, omitting it makes the code more verbose, which is also bad.

I'm not sure in the end which consequence is worse.

     const TemplateURL* verifier_turl =
         GetVerifierService()->GetTemplateURLForKeyword(CreateKeyword(seed));
     ASSERT_TRUE(verifier_turl);