Test fixes and clean up in preparation for moving DSE to Prefs.

chrome/browser/ui/search_engines/keyword_editor_controller_unittest.cc

Index: chrome/browser/ui/search_engines/keyword_editor_controller_unittest.cc
diff --git a/chrome/browser/ui/search_engines/keyword_editor_controller_unittest.cc b/chrome/browser/ui/search_engines/keyword_editor_controller_unittest.cc
index f13247e7527459c2e2072ac0272bb61b626c874d..43f2f71d053ffb99660b0e6f4df3a96cee41b88a 100644
--- a/chrome/browser/ui/search_engines/keyword_editor_controller_unittest.cc
+++ b/chrome/browser/ui/search_engines/keyword_editor_controller_unittest.cc
@@ -2,20 +2,16 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/message_loop/message_loop.h"
+#include "base/compiler_specific.h"
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
-#include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search_engines/template_url.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/browser/ui/search_engines/keyword_editor_controller.h"
 #include "chrome/browser/ui/search_engines/template_url_table_model.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/gtest/include/gtest/gtest.h"
 #include "ui/base/models/table_model_observer.h"
 
@@ -31,11 +27,35 @@ static const base::string16 kB1(ASCIIToUTF16("b1"));
 class KeywordEditorControllerTest : public testing::Test,
                                     public ui::TableModelObserver {
  public:
-  // Initializes all of the state.
-  void Init(bool simulate_load_failure);
+  KeywordEditorControllerTest()
+      : simulate_load_failure_(false),
+        model_changed_count_(0),
+        items_changed_count_(0),
+        added_count_(0),
+        removed_count_(0) {}
+
+  explicit KeywordEditorControllerTest(bool simulate_load_failure)
+      : simulate_load_failure_(simulate_load_failure),
+        model_changed_count_(0),
+        items_changed_count_(0),
+        added_count_(0),
+        removed_count_(0) {}
+
+  virtual void SetUp() OVERRIDE {
+    util_.SetUp();
+
+    if (simulate_load_failure_)
+      util_.model()->OnWebDataServiceRequestDone(0, NULL);
+    else
+      util_.ChangeModelToLoadState();
+
+    controller_.reset(new KeywordEditorController(util_.profile()));
+    controller_->table_model()->SetObserver(this);
+  }
 
-  virtual void SetUp() {
-    Init(false);
+  virtual void TearDown() OVERRIDE {
+    controller_.reset();
+    util_.TearDown();
   }
 
   virtual void OnModelChanged() OVERRIDE {
@@ -69,35 +89,29 @@ class KeywordEditorControllerTest : public testing::Test,
   }
 
   void SimulateDefaultSearchIsManaged(const std::string& url) {
-    ASSERT_FALSE(url.empty());
-    TestingPrefServiceSyncable* service = profile_->GetTestingPrefService();
-    service->SetManagedPref(prefs::kDefaultSearchProviderEnabled,
-                            new base::FundamentalValue(true));
-    service->SetManagedPref(prefs::kDefaultSearchProviderSearchURL,
-                            new base::StringValue(url));
-    service->SetManagedPref(prefs::kDefaultSearchProviderName,
-                            new base::StringValue("managed"));
-    service->SetManagedPref(prefs::kDefaultSearchProviderKeyword,
-                            new base::StringValue("managed"));
-    // Clear the IDs that are not specified via policy.
-    service->SetManagedPref(prefs::kDefaultSearchProviderID,
-                            new base::StringValue(std::string()));
-    service->SetManagedPref(prefs::kDefaultSearchProviderPrepopulateID,
-                            new base::StringValue(std::string()));
-    model_->Observe(chrome::NOTIFICATION_DEFAULT_SEARCH_POLICY_CHANGED,
-                    content::NotificationService::AllSources(),
-                    content::NotificationService::NoDetails());
+    util_.SetManagedDefaultSearchPreferences(true,
+                                             "managed",
+                                             "managed",
+                                             url,
+                                             std::string(),
+                                             std::string(),
+                                             std::string(),
+                                             std::string(),
+                                             std::string());
   }
 
-  TemplateURLTableModel* table_model() const {
-    return controller_->table_model();
-  }
+  TemplateURLTableModel* table_model() { return controller_->table_model(); }
+  KeywordEditorController* controller() { return controller_.get(); }
+  TemplateURLServiceTestUtil* util() { return &util_; }
 
- protected:
-  base::MessageLoopForUI message_loop_;
-  scoped_ptr<TestingProfile> profile_;
+  int items_changed_count() { return items_changed_count_; }

[Peter Kasting, 2014/05/06 21:49:28]

Nit: These next three can be const.

The ones above can be const iff their returned pointers could also be const.  (I
didn't look at the callers to see.)

+  int added_count() { return added_count_; }
+  int removed_count() { return removed_count_; }
+
+ private:
   scoped_ptr<KeywordEditorController> controller_;
-  TemplateURLService* model_;
+  TemplateURLServiceTestUtil util_;
+  bool simulate_load_failure_;
 
   int model_changed_count_;
   int items_changed_count_;
@@ -105,27 +119,16 @@ class KeywordEditorControllerTest : public testing::Test,
   int removed_count_;
 };
 
-void KeywordEditorControllerTest::Init(bool simulate_load_failure) {
-  ClearChangeCount();
-
-  // If init is called twice, make sure that the controller is destroyed before
-  // the profile is.
-  controller_.reset();
-  profile_.reset(new TestingProfile());
-  TemplateURLServiceFactory::GetInstance()->SetTestingFactoryAndUse(
-      profile_.get(), &TemplateURLServiceFactory::BuildInstanceFor);
-
-  model_ = TemplateURLServiceFactory::GetForProfile(profile_.get());
-  if (simulate_load_failure)
-    model_->OnWebDataServiceRequestDone(0, NULL);
-
-  controller_.reset(new KeywordEditorController(profile_.get()));
-  controller_->table_model()->SetObserver(this);
-}
+class KeywordEditorControllerNoWebDataTest
+    : public KeywordEditorControllerTest {
+ public:
+  KeywordEditorControllerNoWebDataTest() : KeywordEditorControllerTest(true) {}
+};
 
 // Tests adding a TemplateURL.
 TEST_F(KeywordEditorControllerTest, Add) {
-  controller_->AddTemplateURL(kA, kB, "http://c");
+  int original_row_count = table_model()->RowCount();
+  controller()->AddTemplateURL(kA, kB, "http://c");
 
   // Verify the observer was notified.
   VerifyChangeCount(0, 0, 1, 0);
@@ -133,13 +136,11 @@ TEST_F(KeywordEditorControllerTest, Add) {
     return;
 
   // Verify the TableModel has the new data.
-  ASSERT_EQ(1, table_model()->RowCount());
+  ASSERT_EQ(original_row_count + 1, table_model()->RowCount());
 
-  // Verify the TemplateURLService has the new entry.
-  ASSERT_EQ(1U, model_->GetTemplateURLs().size());
-
-  // Verify the entry is what we added.
-  const TemplateURL* turl = model_->GetTemplateURLs()[0];
+  // Verify the TemplateURLService has the new data.
+  const TemplateURL* turl = util()->model()->GetTemplateURLForKeyword(kB);
+  ASSERT_TRUE(turl);
   EXPECT_EQ(ASCIIToUTF16("a"), turl->short_name());
   EXPECT_EQ(ASCIIToUTF16("b"), turl->keyword());
   EXPECT_EQ("http://c", turl->url());
@@ -147,12 +148,12 @@ TEST_F(KeywordEditorControllerTest, Add) {
 
 // Tests modifying a TemplateURL.
 TEST_F(KeywordEditorControllerTest, Modify) {
-  controller_->AddTemplateURL(kA, kB, "http://c");
+  controller()->AddTemplateURL(kA, kB, "http://c");
   ClearChangeCount();
 
   // Modify the entry.
-  TemplateURL* turl = model_->GetTemplateURLs()[0];
-  controller_->ModifyTemplateURL(turl, kA1, kB1, "http://c1");
+  TemplateURL* turl = util()->model()->GetTemplateURLs()[0];
+  controller()->ModifyTemplateURL(turl, kA1, kB1, "http://c1");
 
   // Make sure it was updated appropriately.
   VerifyChangeCount(0, 1, 0, 0);
@@ -163,98 +164,98 @@ TEST_F(KeywordEditorControllerTest, Modify) {
 
 // Tests making a TemplateURL the default search provider.
 TEST_F(KeywordEditorControllerTest, MakeDefault) {
-  controller_->AddTemplateURL(kA, kB, "http://c{searchTerms}");
+  int index = controller()->AddTemplateURL(kA, kB, "http://c{searchTerms}");
   ClearChangeCount();
 
-  const TemplateURL* turl = model_->GetTemplateURLs()[0];
-  int new_default = controller_->MakeDefaultTemplateURL(0);
-  EXPECT_EQ(0, new_default);
+  const TemplateURL* turl = util()->model()->GetTemplateURLForKeyword(kB);
+  int new_default = controller()->MakeDefaultTemplateURL(index);
+  EXPECT_EQ(index, new_default);
   // Making an item the default sends a handful of changes. Which are sent isn't
   // important, what is important is 'something' is sent.
-  ASSERT_TRUE(items_changed_count_ > 0 || added_count_ > 0 ||
-              removed_count_ > 0);
-  ASSERT_TRUE(model_->GetDefaultSearchProvider() == turl);
+  ASSERT_TRUE(items_changed_count() > 0 || added_count() > 0 ||
+              removed_count() > 0);
+  ASSERT_TRUE(util()->model()->GetDefaultSearchProvider() == turl);
 
   // Making it default a second time should fail.
-  new_default = controller_->MakeDefaultTemplateURL(0);
+  new_default = controller()->MakeDefaultTemplateURL(index);
   EXPECT_EQ(-1, new_default);
 }
 
 // Tests that a TemplateURL can't be made the default if the default search
 // provider is managed via policy.
 TEST_F(KeywordEditorControllerTest, CannotSetDefaultWhileManaged) {
-  controller_->AddTemplateURL(kA, kB, "http://c{searchTerms}");
-  controller_->AddTemplateURL(kA1, kB1, "http://d{searchTerms}");
+  controller()->AddTemplateURL(kA, kB, "http://c{searchTerms}");
+  controller()->AddTemplateURL(kA1, kB1, "http://d{searchTerms}");
   ClearChangeCount();
 
   const TemplateURL* turl1 =
-      model_->GetTemplateURLForKeyword(ASCIIToUTF16("b"));
+      util()->model()->GetTemplateURLForKeyword(ASCIIToUTF16("b"));
   ASSERT_TRUE(turl1 != NULL);
   const TemplateURL* turl2 =
-      model_->GetTemplateURLForKeyword(ASCIIToUTF16("b1"));
+      util()->model()->GetTemplateURLForKeyword(ASCIIToUTF16("b1"));
   ASSERT_TRUE(turl2 != NULL);
 
-  EXPECT_TRUE(controller_->CanMakeDefault(turl1));
-  EXPECT_TRUE(controller_->CanMakeDefault(turl2));
+  EXPECT_TRUE(controller()->CanMakeDefault(turl1));
+  EXPECT_TRUE(controller()->CanMakeDefault(turl2));
 
   SimulateDefaultSearchIsManaged(turl2->url());
-  EXPECT_TRUE(model_->is_default_search_managed());
+  EXPECT_TRUE(util()->model()->is_default_search_managed());
 
-  EXPECT_FALSE(controller_->CanMakeDefault(turl1));
-  EXPECT_FALSE(controller_->CanMakeDefault(turl2));
+  EXPECT_FALSE(controller()->CanMakeDefault(turl1));
+  EXPECT_FALSE(controller()->CanMakeDefault(turl2));
 }
 
 // Tests that a TemplateURL can't be edited if it is the managed default search
 // provider.
 TEST_F(KeywordEditorControllerTest, EditManagedDefault) {
-  controller_->AddTemplateURL(kA, kB, "http://c{searchTerms}");
-  controller_->AddTemplateURL(kA1, kB1, "http://d{searchTerms}");
+  controller()->AddTemplateURL(kA, kB, "http://c{searchTerms}");
+  controller()->AddTemplateURL(kA1, kB1, "http://d{searchTerms}");
   ClearChangeCount();
 
   const TemplateURL* turl1 =
-      model_->GetTemplateURLForKeyword(ASCIIToUTF16("b"));
+      util()->model()->GetTemplateURLForKeyword(ASCIIToUTF16("b"));
   ASSERT_TRUE(turl1 != NULL);
   const TemplateURL* turl2 =
-      model_->GetTemplateURLForKeyword(ASCIIToUTF16("b1"));
+      util()->model()->GetTemplateURLForKeyword(ASCIIToUTF16("b1"));
   ASSERT_TRUE(turl2 != NULL);
 
-  EXPECT_TRUE(controller_->CanEdit(turl1));
-  EXPECT_TRUE(controller_->CanEdit(turl2));
+  EXPECT_TRUE(controller()->CanEdit(turl1));
+  EXPECT_TRUE(controller()->CanEdit(turl2));
 
   // Simulate setting a managed default.  This will add another template URL to
   // the model.
   SimulateDefaultSearchIsManaged(turl2->url());
-  EXPECT_TRUE(model_->is_default_search_managed());
-  EXPECT_TRUE(controller_->CanEdit(turl1));
-  EXPECT_TRUE(controller_->CanEdit(turl2));
-  EXPECT_FALSE(controller_->CanEdit(model_->GetDefaultSearchProvider()));
+  EXPECT_TRUE(util()->model()->is_default_search_managed());
+  EXPECT_TRUE(controller()->CanEdit(turl1));
+  EXPECT_TRUE(controller()->CanEdit(turl2));
+  EXPECT_FALSE(
+      controller()->CanEdit(util()->model()->GetDefaultSearchProvider()));
 }
 
-TEST_F(KeywordEditorControllerTest, MakeDefaultNoWebData) {
-  // Simulate a failure to load Web Data.
-  Init(true);
-
-  controller_->AddTemplateURL(kA, kB, "http://c{searchTerms}");
+TEST_F(KeywordEditorControllerNoWebDataTest, MakeDefaultNoWebData) {
+  int index = controller()->AddTemplateURL(kA, kB, "http://c{searchTerms}");
   ClearChangeCount();
 
   // This should not result in a crash.
-  int new_default = controller_->MakeDefaultTemplateURL(0);
-  EXPECT_EQ(0, new_default);
+  int new_default = controller()->MakeDefaultTemplateURL(index);
+  EXPECT_EQ(index, new_default);
 }
 
 // Mutates the TemplateURLService and make sure table model is updating
 // appropriately.
 TEST_F(KeywordEditorControllerTest, MutateTemplateURLService) {
+  int original_row_count = table_model()->RowCount();
+
   TemplateURLData data;
   data.short_name = ASCIIToUTF16("b");
   data.SetKeyword(ASCIIToUTF16("a"));
-  TemplateURL* turl = new TemplateURL(profile_.get(), data);
-  model_->Add(turl);
+  TemplateURL* turl = new TemplateURL(util()->profile(), data);
+  util()->model()->Add(turl);
 
   // Table model should have updated.
   VerifyChangeCount(1, 0, 0, 0);
 
   // And should contain the newly added TemplateURL.
-  ASSERT_EQ(1, table_model()->RowCount());
-  ASSERT_EQ(0, table_model()->IndexOfTemplateURL(turl));
+  ASSERT_EQ(original_row_count + 1, table_model()->RowCount());
+  ASSERT_GE(table_model()->IndexOfTemplateURL(turl), 0);
 }