Create TemplateUrlData to base::Dictionary utility functions

components/search_engines/template_url_prepopulate_data_unittest.cc

Index: components/search_engines/template_url_prepopulate_data_unittest.cc
diff --git a/components/search_engines/template_url_prepopulate_data_unittest.cc b/components/search_engines/template_url_prepopulate_data_unittest.cc
index e57a940cf8947923a3ddc2deaa1f7e8a6dd5cb0d..c1e57b751d6d24fc5fca6edb11d4e957e8df1f12 100644
--- a/components/search_engines/template_url_prepopulate_data_unittest.cc
+++ b/components/search_engines/template_url_prepopulate_data_unittest.cc
@@ -19,6 +19,7 @@
 #include "components/search_engines/search_engines_pref_names.h"
 #include "components/search_engines/search_terms_data.h"
 #include "components/search_engines/template_url.h"
+#include "components/search_engines/template_url_data_util.h"
 #include "components/search_engines/template_url_service.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -154,6 +155,11 @@ TEST_F(TemplateURLPrepopulateDataTest, ProvidersFromPrefs) {
   EXPECT_EQ(0u, t_urls[0]->alternate_urls.size());
   EXPECT_TRUE(t_urls[0]->search_terms_replacement_key.empty());
 

[Peter Kasting, 2016/11/20 07:57:50]

Nit: Why this blank line?

[Alexander Yashkin, 2016/11/20 20:24:05]

Deleted.

+  EXPECT_TRUE(t_urls[0]->show_in_default_list);
+  EXPECT_TRUE(t_urls[0]->safe_for_autoreplace);
+  EXPECT_TRUE(t_urls[0]->date_created.is_null());
+  EXPECT_TRUE(t_urls[0]->last_modified.is_null());

[Peter Kasting, 2016/11/20 07:57:50]

Is it important to check these?  If so, should we be checking them below as well
(around line 190)?  Or do they not really have anything to do with what this
particular test is trying to verify?  (For example, if we're just checking the
default values of these fields, and that's tested by another unittest, then
having these here doesn't really increase test coverage, it just means more
places to fix if we change something.)

[Alexander Yashkin, 2016/11/20 20:24:05]

I saw that fields safe_for_autoreplace, date_created and last_modified are
initialized with non default values for TemplateURLData initialized from
prepopulated engines. So I decided to check this in tests too.
They are not tested anywhere, also I added check for their values below in
TemplateURLPrepopulateDataTest.ProvidersFromPrepopulated

[Peter Kasting, 2016/11/21 02:17:07]

Seems fine, but note that if you follow my suggestion elsewhere to make this
functionality set the two time fields to Now() that the EXPECTs on these fields
should probably just disappear from both these places.

+
   // Test the optional settings too.
   entry->SetString("suggest_url", "http://foo.com/suggest?q={searchTerms}");
   entry->SetString("instant_url", "http://foo.com/instant?q={searchTerms}");
@@ -357,8 +363,7 @@ TEST_F(TemplateURLPrepopulateDataTest, GetEngineTypeForAllPrepopulatedEngines) {
       TemplateURLPrepopulateData::GetAllPrepopulatedEngines();
   for (const PrepopulatedEngine* engine : all_engines) {
     std::unique_ptr<TemplateURLData> data =
-        TemplateURLPrepopulateData::MakeTemplateURLDataFromPrepopulatedEngine(
-            *engine);
+        TemplateURLDataFromPrepopulatedEngine(*engine);
     EXPECT_EQ(engine->type,
               TemplateURL(*data).GetEngineType(SearchTermsData()));
   }