Create TemplateUrlData to base::Dictionary utility functions

components/search_engines/template_url_prepopulate_data_unittest.cc

 // Copyright 2014 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 "components/search_engines/template_url_prepopulate_data.h"
 
 #include <stddef.h>
 
 #include <memory>
 #include <utility>
 
 #include "base/command_line.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/macros.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/google/core/browser/google_switches.h"
 #include "components/pref_registry/testing_pref_service_syncable.h"
 #include "components/search_engines/prepopulated_engines.h"
 #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"
 
 using base::ASCIIToUTF16;
 
 namespace {
 
 SearchEngineType GetEngineType(const std::string& url) {
   TemplateURLData data;
   data.SetURL(url);
@@ skipping 114 matching lines @@
   EXPECT_EQ(ASCIIToUTF16("foo"), t_urls[0]->short_name());
   EXPECT_EQ(ASCIIToUTF16("fook"), t_urls[0]->keyword());
   EXPECT_EQ("foo.com", GetHostFromTemplateURLData(*t_urls[0]));
   EXPECT_EQ("foi.com", t_urls[0]->favicon_url.host());
   EXPECT_EQ(1u, t_urls[0]->input_encodings.size());
   EXPECT_EQ(1001, t_urls[0]->prepopulate_id);
   EXPECT_TRUE(t_urls[0]->suggestions_url.empty());
   EXPECT_TRUE(t_urls[0]->instant_url.empty());
   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}");
   base::ListValue* alternate_urls = new base::ListValue;
   alternate_urls->AppendString("http://foo.com/alternate?q={searchTerms}");
   entry->Set("alternate_urls", alternate_urls);
   entry->SetString("search_terms_replacement_key", "espv");
   overrides = new base::ListValue;
   overrides->Append(entry->CreateDeepCopy());
   prefs_.SetUserPref(prefs::kSearchProviderOverrides, overrides);
@@ skipping 183 matching lines @@
       switches::kGoogleBaseURL, "http://www.foo.com/");
   EXPECT_EQ(SEARCH_ENGINE_GOOGLE, GetEngineType(foo_url));
 }
 
 TEST_F(TemplateURLPrepopulateDataTest, GetEngineTypeForAllPrepopulatedEngines) {
   using PrepopulatedEngine = TemplateURLPrepopulateData::PrepopulatedEngine;
   const std::vector<const PrepopulatedEngine*> all_engines =
       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()));
   }
 }