Added collecting of data to be fed to the JTL interpreter.

chrome/browser/profile_resetter/automatic_profile_resetter_delegate_unittest.cc

Index: chrome/browser/profile_resetter/automatic_profile_resetter_delegate_unittest.cc
diff --git a/chrome/browser/profile_resetter/automatic_profile_resetter_delegate_unittest.cc b/chrome/browser/profile_resetter/automatic_profile_resetter_delegate_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..f1fa930e21e0cd82c6c269c02b86290f7983a21e
--- /dev/null
+++ b/chrome/browser/profile_resetter/automatic_profile_resetter_delegate_unittest.cc
@@ -0,0 +1,408 @@
+// Copyright 2013 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 "chrome/browser/profile_resetter/automatic_profile_resetter_delegate.h"
+
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/prefs/pref_service.h"
+#include "base/run_loop.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/test/values_test_util.h"
+#include "base/values.h"
+#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/search_engines/template_url_service.h"
+#include "chrome/browser/search_engines/template_url_service_factory.h"
+#include "chrome/browser/search_engines/template_url_service_test_util.h"
+#include "chrome/common/pref_names.h"
+#include "chrome/test/base/testing_pref_service_syncable.h"
+#include "chrome/test/base/testing_profile.h"
+#include "content/public/browser/notification_service.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+#if defined(OS_WIN)
+#include "chrome/browser/enumerate_modules_model_win.h"
+#endif
+
+using base::UTF8ToUTF16;

[Peter Kasting, 2013/10/15 01:31:28]

This is unnecessary (you already qualify the two calls)

[engedy, 2013/10/15 22:13:07]

Done.

+
+namespace {
+
+// Keep this value in sync with prefs::kDefaultSearchProviderURL and friends.
+const char kDefaultSearchProviderPrefix[] = "default_search_provider";

[Peter Kasting, 2013/10/15 01:31:28]

(1) For any of these constants used in just one place, consider just inlining
the constant there (naming it in this case doesn't buy us a ton of readability).

(2) Otherwise consider scoping these constants within the classes below if
possible so they're even more narrowly-scoped than "whole file"

[engedy, 2013/10/15 22:13:07]

Done.

+
+const char kTestSearchURL[] = "http://example.com/search?q={searchTerms}";
+const char kTestSuggestURL[] = "http://example.com/suggest?q={searchTerms}";
+const char kTestInstantURL[] = "http://example.com/instant?q={searchTerms}";
+const char kTestImageURL[] = "http://example.com/image?q={searchTerms}";
+const char kTestSearchURLPostParams[] = "search-post-params";
+const char kTestSuggestURLPostParams[] = "suggest-post-params";
+const char kTestInstantURLPostParams[] = "instant-post-params";
+const char kTestImageURLPostParams[] = "image-post-params";
+
+const char kTestIconURL[] = "http://example.com/favicon.ico";
+const char kTestNewTabURL[] = "http://example.com/newtab.html";
+const char kTestAlternateURL[] = "http://example.com/s?q={searchTerms}";
+
+const char kTestName[] = "name";
+const char kTestKeyword[] = "keyword";
+const char kTestSearchTermReplacementKey[] = "key";
+const char kTestPrepopulateId[] = "2";
+const char kTestEncoding[] = "UTF-8";
+
+// Test fixtures -------------------------------------------------------------
+
+class GenericTestBase : public testing::Test {

[Peter Kasting, 2013/10/15 01:31:28]

I'm surprised there isn't already some sort of "TestWithTestingProfile" or
similar base class like this available.

[engedy, 2013/10/15 22:13:07]

I might have missed something, but could not find any that did not have lots of
superfluous parts.

+ protected:
+  GenericTestBase() {}
+
+  virtual void SetUp() { profile_.reset(new TestingProfile()); }
+
+  TestingProfile* profile() { return profile_.get(); }
+
+ private:
+  content::TestBrowserThreadBundle thread_bundle_;
+  scoped_ptr<TestingProfile> profile_;
+
+  DISALLOW_COPY_AND_ASSIGN(GenericTestBase);
+};
+
+class TemplateURLSpecificTestBase : public testing::Test {
+ protected:
+  TemplateURLSpecificTestBase() {}
+
+  virtual void SetUp() { test_util_.SetUp(); }
+
+  virtual void TearDown() { test_util_.TearDown(); }
+
+  TestingProfile* profile() { return test_util_.profile(); }
+
+  TemplateURL* CreateTestTemplateURL() {

[Peter Kasting, 2013/10/15 01:31:28]

Nit: Again, consider returning scoped_ptr.

Another option, since this is only called by one place, is to inline this code
at that one place.  It can always be factored back out later if someone else
wants it.

[engedy, 2013/10/15 22:13:07]

Done.

+    TemplateURLData data;
+
+    data.SetURL(kTestSearchURL);
+    data.suggestions_url = kTestSuggestURL;
+    data.instant_url = kTestInstantURL;
+    data.image_url = kTestImageURL;
+    data.search_url_post_params = kTestSearchURLPostParams;
+    data.suggestions_url_post_params = kTestSuggestURLPostParams;
+    data.instant_url_post_params = kTestInstantURLPostParams;
+    data.image_url_post_params = kTestImageURLPostParams;
+
+    data.favicon_url = GURL(kTestIconURL);
+    data.new_tab_url = kTestNewTabURL;
+    data.alternate_urls.push_back(kTestAlternateURL);
+
+    data.short_name = base::UTF8ToUTF16(kTestName);
+    data.SetKeyword(base::UTF8ToUTF16(kTestKeyword));
+    data.search_terms_replacement_key = kTestSearchTermReplacementKey;
+    EXPECT_TRUE(base::StringToInt(kTestPrepopulateId, &data.prepopulate_id));
+    data.input_encodings.push_back(kTestEncoding);
+    data.safe_for_autoreplace = true;
+
+    return new TemplateURL(profile(), data);
+  }
+
+  TemplateURLServiceTestUtil test_util_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TemplateURLSpecificTestBase);
+};
+
+template <class BaseTestFixture>
+class ResetterDelegateMixin : public BaseTestFixture {
+ protected:
+  ResetterDelegateMixin() {}
+
+  virtual void SetUp() OVERRIDE {
+    BaseTestFixture::SetUp();
+    resetter_delegate_.reset(
+        new AutomaticProfileResetterDelegateImpl(BaseTestFixture::profile()));
+  }
+
+  virtual void TearDown() OVERRIDE {
+    resetter_delegate_.reset();
+    BaseTestFixture::TearDown();
+  }
+
+  AutomaticProfileResetterDelegate* resetter_delegate() {
+    return resetter_delegate_.get();
+  }
+
+ private:
+  scoped_ptr<AutomaticProfileResetterDelegate> resetter_delegate_;
+
+  DISALLOW_COPY_AND_ASSIGN(ResetterDelegateMixin);
+};
+
+typedef ResetterDelegateMixin<GenericTestBase>
+    AutomaticProfileResetterDelegateTest;
+
+typedef ResetterDelegateMixin<TemplateURLSpecificTestBase>
+    AutomaticProfileResetterDelegateTestTemplateURLs;
+
+// Helper classes and functions ----------------------------------------------
+
+// Verifies that the |details| of a search engine as provided by the delegate
+// are correct in comparison to the |expected_details| coming from the Prefs.
+void VerifyDetails(const base::DictionaryValue& expected_details,
+                   const base::DictionaryValue& details) {
+  const char* keys_to_verify[] = {

[Peter Kasting, 2013/10/15 01:31:28]

Again, consider listing the differences rather than the similarities?

[engedy, 2013/10/15 22:13:07]

Done.

+      "search_url",              "search_terms_replacement_key",
+      "suggest_url",             "instant_url",
+      "image_url",               "new_tab_url",
+      "icon_url",                "search_url_post_params",
+      "suggest_url_post_params", "instant_url_post_params",
+      "image_url_post_params",   "name",
+      "keyword",                 "encodings",
+      "prepopulate_id",          "alternate_urls"};
+
+  for (size_t i = 0; i < arraysize(keys_to_verify); ++i) {
+    SCOPED_TRACE(testing::Message() << "Key: " << keys_to_verify[i]);
+    const base::Value* expected_value;
+    const base::Value* actual_value;
+    ASSERT_TRUE(expected_details.Get(keys_to_verify[i], &expected_value));
+    ASSERT_TRUE(details.Get(keys_to_verify[i], &actual_value));
+    EXPECT_TRUE(actual_value->Equals(expected_value));
+  }
+}
+
+class MockCallbackTarget {
+ public:
+  MockCallbackTarget() {}
+
+  MOCK_CONST_METHOD0(Run, void(void));
+
+  base::Closure CreateClosure() {
+    return base::Closure(
+        base::Bind(&MockCallbackTarget::Run, base::Unretained(this)));
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MockCallbackTarget);
+};
+
+// Tests ---------------------------------------------------------------------
+
+TEST_F(AutomaticProfileResetterDelegateTest,
+       TriggerAndWaitOnModuleEnumeration) {
+  testing::StrictMock<MockCallbackTarget> mock_target;
+
+  // Expect ready_callback to be called just after the modules have been
+  // enumerated. Fail if it is not called, or called too early.
+  resetter_delegate()->RequestCallbackWhenLoadedModulesAreEnumerated(
+      mock_target.CreateClosure());
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_CALL(mock_target, Run());
+  resetter_delegate()->EnumerateLoadedModulesIfNeeded();
+  base::RunLoop().RunUntilIdle();
+
+  testing::Mock::VerifyAndClearExpectations(&mock_target);
+
+  // Expect ready_callback to be posted immediately when the modules have
+  // already been enumerated.
+  EXPECT_CALL(mock_target, Run());
+  resetter_delegate()->RequestCallbackWhenLoadedModulesAreEnumerated(
+      mock_target.CreateClosure());
+  base::RunLoop().RunUntilIdle();
+
+#if defined(OS_WIN)
+  testing::Mock::VerifyAndClearExpectations(&mock_target);
+
+  // Expect ready_callback to be posted immediately even when the modules had
+  // already been enumerated when the delegate was constructed.
+  scoped_ptr<AutomaticProfileResetterDelegate> late_resetter_delegate(
+      new AutomaticProfileResetterDelegateImpl(profile()));
+
+  EXPECT_CALL(mock_target, Run());
+  late_resetter_delegate->RequestCallbackWhenLoadedModulesAreEnumerated(
+      mock_target.CreateClosure());
+  base::RunLoop().RunUntilIdle();
+#endif
+}
+
+TEST_F(AutomaticProfileResetterDelegateTest, GetLoadedModuleNameDigests) {
+  resetter_delegate()->EnumerateLoadedModulesIfNeeded();
+  base::RunLoop().RunUntilIdle();
+  scoped_ptr<base::ListValue> module_name_digests(
+      resetter_delegate()->GetLoadedModuleNameDigests());
+
+  // Just verify that each element looks like an MD5 hash in hexadecimal, and
+  // also that we have at least one element on Win.
+  ASSERT_TRUE(module_name_digests);
+  for (base::ListValue::const_iterator it = module_name_digests->begin();
+       it != module_name_digests->end();
+       ++it) {
+    std::string digest_hex;
+    std::vector<uint8> digest_raw;
+
+    ASSERT_TRUE((*it)->GetAsString(&digest_hex));
+    ASSERT_TRUE(base::HexStringToBytes(digest_hex, &digest_raw));
+    EXPECT_EQ(16u, digest_raw.size());
+  }
+#if defined(OS_WIN)
+  EXPECT_LE(1u, module_name_digests->GetSize());
+#endif
+}
+
+TEST_F(AutomaticProfileResetterDelegateTestTemplateURLs,
+       LoadAndWaitOnTemplateURLService) {
+  testing::StrictMock<MockCallbackTarget> mock_target;
+
+  // Expect ready_callback to be called just after the template URL service gets
+  // initialized. Fail if it is not called, or called too early.
+  resetter_delegate()->RequestCallbackWhenTemplateURLServiceIsLoaded(
+      mock_target.CreateClosure());
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_CALL(mock_target, Run());
+  resetter_delegate()->LoadTemplateURLServiceIfNeeded();
+  base::RunLoop().RunUntilIdle();
+
+  testing::Mock::VerifyAndClearExpectations(&mock_target);
+
+  // Expect ready_callback to be posted immediately when the template URL
+  // service is already initialized.
+  EXPECT_CALL(mock_target, Run());
+  resetter_delegate()->RequestCallbackWhenTemplateURLServiceIsLoaded(
+      mock_target.CreateClosure());
+  base::RunLoop().RunUntilIdle();
+
+  testing::Mock::VerifyAndClearExpectations(&mock_target);
+
+  // Expect ready_callback to be posted immediately even when the template URL
+  // service had already been initialized when the delegate was constructed.
+  scoped_ptr<AutomaticProfileResetterDelegate> late_resetter_delegate(
+      new AutomaticProfileResetterDelegateImpl(profile()));
+
+  EXPECT_CALL(mock_target, Run());
+  late_resetter_delegate->RequestCallbackWhenTemplateURLServiceIsLoaded(
+      mock_target.CreateClosure());
+  base::RunLoop().RunUntilIdle();
+}
+
+TEST_F(AutomaticProfileResetterDelegateTestTemplateURLs,
+       GetDefaultSearchProviderDetails) {
+  TemplateURLService* template_url_service = test_util_.model();
+  test_util_.VerifyLoad();
+
+  // Create a custom search provider, and make it the default. Note that this
+  // will update all data related to the default search provider in Prefs.
+  TemplateURL* custom_dsp = CreateTestTemplateURL();
+  template_url_service->Add(custom_dsp);
+  template_url_service->SetDefaultSearchProvider(custom_dsp);
+
+  scoped_ptr<base::DictionaryValue> dsp_details(
+      resetter_delegate()->GetDefaultSearchProviderDetails());
+
+  // Verify above details against the user preferences that have been stored by
+  // TemplateURLService. We leverage on the fact that the names for all these
+  // preferences start with the same prefix. However, as preferences are stored
+  // (and normally can only be accessed) without path expansion, this trickery
+  // below is needed.

[Peter Kasting, 2013/10/15 01:31:28]

Nit: This comment is just sort of confusing... it's not really clear to me what
below constitutes "trickery".

[engedy, 2013/10/15 22:13:07]

Clarified comment.

+  PrefService* prefs = profile()->GetPrefs();
+  ASSERT_TRUE(prefs);
+  scoped_ptr<base::DictionaryValue> pref_values_with_path_expansion(
+      prefs->GetPreferenceValues());
+  base::DictionaryValue* expected_dsp_details;
+  ASSERT_TRUE(pref_values_with_path_expansion->GetDictionary(
+      kDefaultSearchProviderPrefix, &expected_dsp_details));
+  VerifyDetails(*expected_dsp_details, *dsp_details);
+}
+
+TEST_F(AutomaticProfileResetterDelegateTestTemplateURLs,
+       IsDefaultSearchProviderManaged) {
+  test_util_.VerifyLoad();
+
+  EXPECT_FALSE(resetter_delegate()->IsDefaultSearchProviderManaged());
+
+  // Enable having a default search provider, and also set one from policy.
+  test_util_.SetManagedDefaultSearchPreferences(
+      true, kTestName, kTestKeyword, kTestSearchURL, "", "", "", "", "");

[Peter Kasting, 2013/10/15 01:31:28]

Nit: "" -> std::string() where possible (many places)

[engedy, 2013/10/15 22:13:07]

Done.

+
+  EXPECT_TRUE(resetter_delegate()->IsDefaultSearchProviderManaged());
+  scoped_ptr<base::DictionaryValue> dsp_details(
+      resetter_delegate()->GetDefaultSearchProviderDetails());
+  base::ExpectDictStringValue(kTestSearchURL, *dsp_details, "search_url");
+
+  // Disable having a default search provider, but nevertheless, set one from
+  // policy.
+  test_util_.RemoveManagedDefaultSearchPreferences();
+  test_util_.SetManagedDefaultSearchPreferences(
+      false, kTestName, kTestKeyword, kTestSearchURL, "", "", "", "", "");
+
+  EXPECT_TRUE(resetter_delegate()->IsDefaultSearchProviderManaged());
+  EXPECT_EQ(NULL, resetter_delegate()->GetDefaultSearchProviderDetails());
+
+  // Disable having a default search provider, and set an invalid one from
+  // policy.
+  test_util_.RemoveManagedDefaultSearchPreferences();
+  test_util_.SetManagedDefaultSearchPreferences(
+      true, "", "", "", "", "", "", "", "");
+
+  EXPECT_TRUE(resetter_delegate()->IsDefaultSearchProviderManaged());
+  EXPECT_EQ(NULL, resetter_delegate()->GetDefaultSearchProviderDetails());
+}
+
+TEST_F(AutomaticProfileResetterDelegateTestTemplateURLs,
+       DISABLED_IsDefaultSearchProviderManagedSubtle) {
+  test_util_.VerifyLoad();
+
+  // Only set a single policy to disable having a default search provider.
+  TestingPrefServiceSyncable* pref_service = profile()->GetTestingPrefService();
+  pref_service->SetManagedPref(prefs::kDefaultSearchProviderEnabled,
+                               new base::FundamentalValue(false));
+  test_util_.model()->Observe(
+      chrome::NOTIFICATION_DEFAULT_SEARCH_POLICY_CHANGED,
+      content::NotificationService::AllSources(),
+      content::NotificationService::NoDetails());
+
+  EXPECT_TRUE(resetter_delegate()->IsDefaultSearchProviderManaged());
+  EXPECT_EQ(NULL, resetter_delegate()->GetDefaultSearchProviderDetails());
+}
+
+TEST_F(AutomaticProfileResetterDelegateTestTemplateURLs,
+       GetPrepopulatedSearchProvidersDetails) {
+  TemplateURLService* template_url_service = test_util_.model();
+  test_util_.VerifyLoad();
+
+  scoped_ptr<base::ListValue> prepopulated_search_engines_details(
+      resetter_delegate()->GetPrepopulatedSearchProvidersDetails());
+
+  // Do the same kind of verification as for GetDefaultSearchEngineDetails:
+  // subsequently set each pre-populated engine as the default, so we can verify
+  // that the details returned by the delegate about one particular engine are
+  // correct in comparison to what has been stored to the Prefs.
+  std::vector<TemplateURL*> prepopulated_engines =
+      template_url_service->GetTemplateURLs();
+
+  ASSERT_EQ(prepopulated_engines.size(),
+            prepopulated_search_engines_details->GetSize());
+
+  // This assumes that the order of engines are preserved.

[Peter Kasting, 2013/10/15 01:31:28]

Are preserved where?  In the prefs?  That seems like a questionable assumption.

[engedy, 2013/10/15 22:13:07]

Replaced with a more solid assumption, and clarified wording.

+  for (size_t i = 0; i < prepopulated_engines.size(); ++i) {
+    template_url_service->SetDefaultSearchProvider(prepopulated_engines[i]);
+
+    PrefService* prefs = profile()->GetPrefs();
+    ASSERT_TRUE(prefs);
+    scoped_ptr<base::DictionaryValue> pref_values_with_path_expansion(
+        prefs->GetPreferenceValues());
+    base::DictionaryValue* expected_dsp_details;
+    ASSERT_TRUE(pref_values_with_path_expansion->GetDictionary(
+        kDefaultSearchProviderPrefix, &expected_dsp_details));
+
+    base::DictionaryValue* details;
+    ASSERT_TRUE(
+        prepopulated_search_engines_details->GetDictionary(i, &details));
+
+    SCOPED_TRACE(testing::Message() << "Details: " << *details);
+    VerifyDetails(*expected_dsp_details, *details);
+  }
+}
+
+}  // namespace