Refactor search_engines tests to better reuse code

chrome/browser/search_engines/template_url_service_test_util.h

Index: chrome/browser/search_engines/template_url_service_test_util.h
diff --git a/chrome/browser/search_engines/template_url_service_test_util.h b/chrome/browser/search_engines/template_url_service_test_util.h
index a1ca5fdfce8026a0ad024982aff0d00c5512bc13..5232f9fb1aa64435d9e0a2b57c02d16974430d61 100644
--- a/chrome/browser/search_engines/template_url_service_test_util.h
+++ b/chrome/browser/search_engines/template_url_service_test_util.h
@@ -12,6 +12,7 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/strings/string16.h"
+#include "components/search_engines/template_url_data.h"
 #include "components/search_engines/template_url_service_observer.h"
 
 class GURL;
@@ -20,6 +21,16 @@ class TemplateURLService;
 class TestingProfile;
 class TestingSearchTermsData;
 
+// Sets the managed preferences for the default search provider and trigger
+// notification.

[Peter Kasting, 2016/11/30 00:50:37]

Nit: I wasn't clear which notification this was, then I saw the code didn't
explicitly fire a notification; that happens automatically due to lower layers.

Accordingly, I would just leave that detail out of this comment.  Layering-wise,
if we change that implementation detail lower down, we wouldn't want to fix the
comment here, and it doesn't seem like this function cares about enforcing that
that happens.

(Same comment below)

[Alexander Yashkin, 2016/11/30 06:01:20]

Done

+void SetManagedDefaultSearchPreferences(const TemplateURLData& managed_tdata,

[Peter Kasting, 2016/11/30 00:50:37]

Nit: "tdata" is an unusual spelling for this (it kinda mirrors "turl", which is
common, although in retrospect I'm not sure I ever should have started using
that).  Maybe "data" or "managed_data" or "turl_data"?

[Alexander Yashkin, 2016/11/30 06:01:20]

Done

+                                        bool enabled,

[Peter Kasting, 2016/11/30 00:50:37]

Nit: Do you need to comment on what, precisely, this enables?

[Alexander Yashkin, 2016/11/30 06:01:20]

Added comment:
// enabled arg enables/disables use of managed engine by DefaultSearchManager.

+                                        TestingProfile* profile);
+
+// Removes all the managed preferences for the default search provider and
+// triggers notification.
+void RemoveManagedDefaultSearchPreferences(TestingProfile* profile);
+
 class TemplateURLServiceTestUtil : public TemplateURLServiceObserver {
  public:
   TemplateURLServiceTestUtil();
@@ -55,24 +66,6 @@ class TemplateURLServiceTestUtil : public TemplateURLServiceObserver {
   // Sets the google base url.  |base_url| must be valid.
   void SetGoogleBaseURL(const GURL& base_url);
 
-  // Sets the managed preferences for the default search provider and trigger
-  // notification. If |alternate_url| is empty, uses an empty list of alternate
-  // URLs, otherwise uses a list containing a single entry.
-  void SetManagedDefaultSearchPreferences(
-      bool enabled,
-      const std::string& name,
-      const std::string& keyword,
-      const std::string& search_url,
-      const std::string& suggest_url,
-      const std::string& icon_url,
-      const std::string& encodings,
-      const std::string& alternate_url,
-      const std::string& search_terms_replacement_key);
-
-  // Removes all the managed preferences for the default search provider and
-  // triggers notification.
-  void RemoveManagedDefaultSearchPreferences();
-
   KeywordWebDataService* web_data_service() { return web_data_service_.get(); }
   TemplateURLService* model() { return model_.get(); }
   TestingProfile* profile() { return profile_.get(); }