Remove VisitedLink dependency on rest of chrome

chrome/browser/visitedlink/visitedlink_unittest.cc

Index: chrome/browser/visitedlink/visitedlink_unittest.cc
diff --git a/chrome/browser/visitedlink/visitedlink_unittest.cc b/chrome/browser/visitedlink/visitedlink_unittest.cc
index d13e988b4cd7518aa8f1ee9aa7830524a9cc1ba1..71aca09a1bcd69235fdead0a255831b6b071df82 100644
--- a/chrome/browser/visitedlink/visitedlink_unittest.cc
+++ b/chrome/browser/visitedlink/visitedlink_unittest.cc
@@ -13,9 +13,9 @@
 #include "base/shared_memory.h"
 #include "base/string_util.h"
 #include "base/time.h"
+#include "chrome/browser/visitedlink/visitedlink_delegate.h"
 #include "chrome/browser/visitedlink/visitedlink_event_listener.h"
 #include "chrome/browser/visitedlink/visitedlink_master.h"
-#include "chrome/browser/visitedlink/visitedlink_master_factory.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/renderer/visitedlink_slave.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
@@ -23,6 +23,7 @@
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/test/mock_render_process_host.h"
+#include "content/public/test/test_browser_context.h"
 #include "content/public/test/test_browser_thread.h"
 #include "content/public/test/test_renderer_host.h"
 #include "googleurl/src/gurl.h"
@@ -34,6 +35,8 @@ using content::RenderViewHostTester;
 
 namespace {
 
+typedef std::vector<GURL> URLs;
+
 // a nice long URL that we can append numbers to to get new URLs
 const char g_test_prefix[] =
   "http://www.google.com/products/foo/index.html?id=45028640526508376&seq=";
@@ -44,13 +47,68 @@ GURL TestURL(int i) {
   return GURL(StringPrintf("%s%d", g_test_prefix, i));
 }
 
-ProfileKeyedService* BuildVisitedLinkMaster(Profile* profile) {
-  VisitedLinkMaster* master = new VisitedLinkMaster(profile);
-  master->Init();
-  return master;
+std::vector<VisitedLinkSlave*> g_slaves;
+
+// ========================== TestVisitedLinkDelegate ==========================

[tfarina, 2013/01/08 00:16:43]

nit: what is up with this kind of comment? Can you just remove it? It's very
uncommon to have this.

+class TestVisitedLinkDelegate : public VisitedLinkDelegate {
+ public:
+  virtual bool AreEquivalentContexts(
+      content::BrowserContext* context1,
+      content::BrowserContext* context2) OVERRIDE;
+  virtual void RebuildTable(scoped_refptr<URLEnumerator> enumerator) OVERRIDE;
+
+  void AddURLForRebuild(const GURL& url);
+
+ private:
+
+  URLs rebuild_urls_;
+};
+
+bool TestVisitedLinkDelegate::AreEquivalentContexts(
+    content::BrowserContext* context1, content::BrowserContext* context2) {
+  DCHECK_EQ(context1, context2);
+  return true;  // Test only has one profile.
 }
 
-std::vector<VisitedLinkSlave*> g_slaves;
+void TestVisitedLinkDelegate::RebuildTable(scoped_refptr<URLEnumerator> enumerator) {
+  for (URLs::const_iterator itr = rebuild_urls_.begin();
+       itr != rebuild_urls_.end();
+       ++itr)
+    enumerator->OnURL(*itr);
+  enumerator->OnComplete(true);
+}
+
+void TestVisitedLinkDelegate::AddURLForRebuild(const GURL& url) {
+  rebuild_urls_.push_back(url);
+}
+// ======================== TestVisitedLinkDelegate End ========================

[tfarina, 2013/01/08 00:16:43]

nit: heh, never saw this before ;), could you remove?

+
+// ============================== TestURLIterator ==============================
+class TestURLIterator : public VisitedLinkMaster::URLIterator {
+ public:
+  explicit TestURLIterator(const URLs& urls);
+
+  virtual const GURL& NextURL() OVERRIDE;
+  virtual bool HasNextURL() const OVERRIDE;
+
+ private:
+  URLs::const_iterator iterator_;
+  URLs::const_iterator end_;
+};
+
+TestURLIterator::TestURLIterator(const URLs& urls)
+    : iterator_(urls.begin()),
+      end_(urls.end()) {
+}
+
+const GURL& TestURLIterator::NextURL() {
+  return *(iterator_++);
+}
+
+bool TestURLIterator::HasNextURL() const {
+  return iterator_ != end_;
+}
+// ============================ TestURLIterator End ============================
 
 }  // namespace
 
@@ -91,12 +149,6 @@ class VisitedLinkTest : public testing::Test {
   VisitedLinkTest()
       : ui_thread_(BrowserThread::UI, &message_loop_),
         file_thread_(BrowserThread::FILE, &message_loop_) {}
-  // Initialize the history system. This should be called before InitVisited().
-  bool InitHistory() {
-    history_service_.reset(new HistoryService);
-    return history_service_->Init(history_dir_, NULL);
-  }
-
   // Initializes the visited link objects. Pass in the size that you want a
   // freshly created table to be. 0 means use the default.
   //
@@ -105,7 +157,7 @@ class VisitedLinkTest : public testing::Test {
   bool InitVisited(int initial_size, bool suppress_rebuild) {
     // Initialize the visited link system.
     master_.reset(new VisitedLinkMaster(new TrackingVisitedLinkEventListener(),
-                                        history_service_.get(),
+                                        &delegate_,
                                         suppress_rebuild, visited_file_,
                                         initial_size));
     return master_->Init();
@@ -117,18 +169,6 @@ class VisitedLinkTest : public testing::Test {
     if (master_.get())
       master_.reset(NULL);
 
-    if (history_service_.get()) {
-      history_service_->SetOnBackendDestroyTask(MessageLoop::QuitClosure());
-      history_service_->Cleanup();
-      history_service_.reset();
-
-      // Wait for the backend class to terminate before deleting the files and
-      // moving to the next test. Note: if this never terminates, somebody is
-      // probably leaking a reference to the history backend, so it never calls
-      // our destroy task.
-      MessageLoop::current()->Run();
-    }
-
     // Wait for all pending file I/O to be completed.
     BrowserThread::GetBlockingPool()->FlushForTesting();
   }
@@ -140,7 +180,6 @@ class VisitedLinkTest : public testing::Test {
     // Clean up after our caller, who may have left the database open.
     ClearDB();
 
-    ASSERT_TRUE(InitHistory());
     ASSERT_TRUE(InitVisited(0, true));
     master_->DebugValidate();
 
@@ -202,13 +241,12 @@ class VisitedLinkTest : public testing::Test {
   FilePath visited_file_;
 
   scoped_ptr<VisitedLinkMaster> master_;
-  scoped_ptr<HistoryService> history_service_;
+  TestVisitedLinkDelegate delegate_;
 };
 
 // This test creates and reads some databases to make sure the data is
 // preserved throughout those operations.
 TEST_F(VisitedLinkTest, DatabaseIO) {
-  ASSERT_TRUE(InitHistory());
   ASSERT_TRUE(InitVisited(0, true));
 
   for (int i = 0; i < g_test_count; i++)
@@ -221,7 +259,6 @@ TEST_F(VisitedLinkTest, DatabaseIO) {
 // Checks that we can delete things properly when there are collisions.
 TEST_F(VisitedLinkTest, Delete) {
   static const int32 kInitialSize = 17;
-  ASSERT_TRUE(InitHistory());
   ASSERT_TRUE(InitVisited(kInitialSize, true));
 
   // Add a cluster from 14-17 wrapping around to 0. These will all hash to the
@@ -260,7 +297,6 @@ TEST_F(VisitedLinkTest, Delete) {
 // When we delete more than kBigDeleteThreshold we trigger different behavior
 // where the entire file is rewritten.
 TEST_F(VisitedLinkTest, BigDelete) {
-  ASSERT_TRUE(InitHistory());
   ASSERT_TRUE(InitVisited(16381, true));
 
   // Add the base set of URLs that won't be deleted.
@@ -270,21 +306,21 @@ TEST_F(VisitedLinkTest, BigDelete) {
 
   // Add more URLs than necessary to trigger this case.
   const int kTestDeleteCount = VisitedLinkMaster::kBigDeleteThreshold + 2;
-  history::URLRows urls_to_delete;
+  URLs urls_to_delete;
   for (int32 i = g_test_count; i < g_test_count + kTestDeleteCount; i++) {
     GURL url(TestURL(i));
     master_->AddURL(url);
-    urls_to_delete.push_back(history::URLRow(url));
+    urls_to_delete.push_back(url);
   }
 
-  master_->DeleteURLs(urls_to_delete);
+  TestURLIterator iterator(urls_to_delete);
+  master_->DeleteURLs(&iterator);
   master_->DebugValidate();
 
   Reload();
 }
 
 TEST_F(VisitedLinkTest, DeleteAll) {
-  ASSERT_TRUE(InitHistory());
   ASSERT_TRUE(InitVisited(0, true));
 
   {
@@ -320,7 +356,6 @@ TEST_F(VisitedLinkTest, DeleteAll) {
   }
 
   // Reopen and validate.
-  ASSERT_TRUE(InitHistory());
   ASSERT_TRUE(InitVisited(0, true));
   master_->DebugValidate();
   EXPECT_EQ(0, master_->GetUsedCount());
@@ -333,7 +368,6 @@ TEST_F(VisitedLinkTest, DeleteAll) {
 TEST_F(VisitedLinkTest, Resizing) {
   // Create a very small database.
   const int32 initial_size = 17;
-  ASSERT_TRUE(InitHistory());
   ASSERT_TRUE(InitVisited(initial_size, true));
 
   // ...and a slave
@@ -382,14 +416,11 @@ TEST_F(VisitedLinkTest, Resizing) {
 
 // Tests that if the database doesn't exist, it will be rebuilt from history.
 TEST_F(VisitedLinkTest, Rebuild) {
-  ASSERT_TRUE(InitHistory());
-
   // Add half of our URLs to history. This needs to be done before we
   // initialize the visited link DB.
   int history_count = g_test_count / 2;
   for (int i = 0; i < history_count; i++)
-    history_service_->AddPage(
-        TestURL(i), base::Time::Now(), history::SOURCE_BROWSED);
+    delegate_.AddURLForRebuild(TestURL(i));
 
   // Initialize the visited link DB. Since the visited links file doesn't exist
   // and we don't suppress history rebuilding, this will load from history.
@@ -407,9 +438,10 @@ TEST_F(VisitedLinkTest, Rebuild) {
 
   // Add one more and then delete it.
   master_->AddURL(TestURL(g_test_count));
-  history::URLRows deleted_urls;
-  deleted_urls.push_back(history::URLRow(TestURL(g_test_count)));
-  master_->DeleteURLs(deleted_urls);
+  URLs urls_to_delete;
+  urls_to_delete.push_back(TestURL(g_test_count));
+  TestURLIterator iterator(urls_to_delete);
+  master_->DeleteURLs(&iterator);
 
   // Wait for the rebuild to complete. The task will terminate the message
   // loop when the rebuild is done. There's no chance that the rebuild will
@@ -428,7 +460,6 @@ TEST_F(VisitedLinkTest, Rebuild) {
 
 // Test that importing a large number of URLs will work
 TEST_F(VisitedLinkTest, BigImport) {
-  ASSERT_TRUE(InitHistory());
   ASSERT_TRUE(InitVisited(0, false));
 
   // Before the table rebuilds, add a large number of URLs
@@ -446,7 +477,6 @@ TEST_F(VisitedLinkTest, BigImport) {
 }
 
 TEST_F(VisitedLinkTest, Listener) {
-  ASSERT_TRUE(InitHistory());
   ASSERT_TRUE(InitVisited(0, true));
 
   // Add test URLs.
@@ -455,10 +485,12 @@ TEST_F(VisitedLinkTest, Listener) {
     ASSERT_EQ(i + 1, master_->GetUsedCount());
   }
 
-  history::URLRows deleted_urls;
-  deleted_urls.push_back(history::URLRow(TestURL(0)));
   // Delete an URL.
-  master_->DeleteURLs(deleted_urls);
+  URLs urls_to_delete;
+  urls_to_delete.push_back(TestURL(0));
+  TestURLIterator iterator(urls_to_delete);
+  master_->DeleteURLs(&iterator);
+
   // ... and all of the remaining ones.
   master_->DeleteAllURLs();
 
@@ -472,6 +504,7 @@ TEST_F(VisitedLinkTest, Listener) {
   EXPECT_EQ(2, listener->reset_count());
 }
 
+// TODO(boliu): Inherit content::TestBrowserContext when componentized.
 class VisitCountingProfile : public TestingProfile {
  public:
   VisitCountingProfile()
@@ -523,7 +556,7 @@ class VisitRelayingRenderProcessHost : public MockRenderProcessHost {
   virtual bool Send(IPC::Message* msg) OVERRIDE {
     VisitCountingProfile* counting_profile =
         static_cast<VisitCountingProfile*>(
-            Profile::FromBrowserContext(GetBrowserContext()));
+            GetBrowserContext());
 
     if (msg->type() == ChromeViewMsg_VisitedLink_Add::ID) {
       PickleIterator iter(*msg);
@@ -559,6 +592,7 @@ class VisitedLinkRenderProcessHostFactory
   DISALLOW_COPY_AND_ASSIGN(VisitedLinkRenderProcessHostFactory);
 };
 
+// TODO(boliu): Inherit content::RenderViewHostTestHarness when componentized.
 class VisitedLinkEventsTest : public ChromeRenderViewHostTestHarness {
  public:
   VisitedLinkEventsTest()
@@ -567,12 +601,10 @@ class VisitedLinkEventsTest : public ChromeRenderViewHostTestHarness {
   virtual ~VisitedLinkEventsTest() {}
   virtual void SetUp() {
     browser_context_.reset(new VisitCountingProfile());
-    profile()->CreateHistoryService(true, false);
-    master_ = static_cast<VisitedLinkMaster*>(
-        VisitedLinkMasterFactory::GetInstance()->
-            SetTestingFactoryAndUse(profile(), BuildVisitedLinkMaster));
+    master_.reset(new VisitedLinkMaster(profile(), &delegate_));
+    master_->Init();
     SetRenderProcessHostFactory(&vc_rph_factory_);
-    ChromeRenderViewHostTestHarness::SetUp();
+    content::RenderViewHostTestHarness::SetUp();
   }
 
   VisitCountingProfile* profile() const {
@@ -580,7 +612,7 @@ class VisitedLinkEventsTest : public ChromeRenderViewHostTestHarness {
   }
 
   VisitedLinkMaster* master() const {
-    return master_;
+    return master_.get();
   }
 
   void WaitForCoalescense() {
@@ -596,7 +628,8 @@ class VisitedLinkEventsTest : public ChromeRenderViewHostTestHarness {
   VisitedLinkRenderProcessHostFactory vc_rph_factory_;
 
  private:
-  VisitedLinkMaster* master_;
+  TestVisitedLinkDelegate delegate_;
+  scoped_ptr<VisitedLinkMaster> master_;
   content::TestBrowserThread ui_thread_;
   content::TestBrowserThread file_thread_;
 
@@ -656,7 +689,7 @@ TEST_F(VisitedLinkEventsTest, Coalescense) {
 }
 
 TEST_F(VisitedLinkEventsTest, Basics) {
-  rvh_tester()->CreateRenderView(string16(),
+  RenderViewHostTester::For(rvh())->CreateRenderView(string16(),
                                  MSG_ROUTING_NONE,
                                  -1);
 
@@ -681,12 +714,12 @@ TEST_F(VisitedLinkEventsTest, Basics) {
 }
 
 TEST_F(VisitedLinkEventsTest, TabVisibility) {
-  rvh_tester()->CreateRenderView(string16(),
+  RenderViewHostTester::For(rvh())->CreateRenderView(string16(),
                                  MSG_ROUTING_NONE,
                                  -1);
 
   // Simulate tab becoming inactive.
-  rvh_tester()->SimulateWasHidden();
+  RenderViewHostTester::For(rvh())->SimulateWasHidden();
 
   // Add a few URLs.
   master()->AddURL(GURL("http://acidtests.org/"));
@@ -700,14 +733,14 @@ TEST_F(VisitedLinkEventsTest, TabVisibility) {
   EXPECT_EQ(0, profile()->reset_event_count());
 
   // Simulate the tab becoming active.
-  rvh_tester()->SimulateWasShown();
+  RenderViewHostTester::For(rvh())->SimulateWasShown();
 
   // We should now have 3 add events, still no reset events.
   EXPECT_EQ(1, profile()->add_event_count());
   EXPECT_EQ(0, profile()->reset_event_count());
 
   // Deactivate the tab again.
-  rvh_tester()->SimulateWasHidden();
+  RenderViewHostTester::For(rvh())->SimulateWasHidden();
 
   // Add a bunch of URLs (over 50) to exhaust the link event buffer.
   for (int i = 0; i < 100; i++)
@@ -720,7 +753,7 @@ TEST_F(VisitedLinkEventsTest, TabVisibility) {
   EXPECT_EQ(0, profile()->reset_event_count());
 
   // Activate the tab.
-  rvh_tester()->SimulateWasShown();
+  RenderViewHostTester::For(rvh())->SimulateWasShown();
 
   // We should have only one more reset event.
   EXPECT_EQ(1, profile()->add_event_count());