Remove VisitedLink dependency on rest of chrome

chrome/browser/visitedlink/visitedlink_master.cc

Index: chrome/browser/visitedlink/visitedlink_master.cc
diff --git a/chrome/browser/visitedlink/visitedlink_master.cc b/chrome/browser/visitedlink/visitedlink_master.cc
index ec3c9e611c2de66ba8f2e9e89d3495010ede105c..51a00fde81266ad18b08717196d60155552c1894 100644
--- a/chrome/browser/visitedlink/visitedlink_master.cc
+++ b/chrome/browser/visitedlink/visitedlink_master.cc
@@ -24,11 +24,11 @@
 #include "base/rand_util.h"
 #include "base/string_util.h"
 #include "base/threading/thread_restrictions.h"
-#include "chrome/browser/history/history.h"
-#include "chrome/browser/history/history_service_factory.h"
-#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/visitedlink/visitedlink_delegate.h"
 #include "chrome/browser/visitedlink/visitedlink_event_listener.h"
+#include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
+#include "googleurl/src/gurl.h"
 
 using content::BrowserThread;
 using file_util::ScopedFILE;
@@ -138,7 +138,7 @@ void AsyncClose(FILE** file) {
 // which case it notifies the builder via DisownMaster(). The builder will
 // delete itself once rebuilding is complete, and not execute any callback.
 class VisitedLinkMaster::TableBuilder
-    : public HistoryService::URLEnumerator,
+    : public VisitedLinkDelegate::URLEnumerator,
       public base::RefCountedThreadSafe<TableBuilder> {
  public:
   TableBuilder(VisitedLinkMaster* master,
@@ -150,8 +150,8 @@ class VisitedLinkMaster::TableBuilder
   // table will be being rebuilt simultaneously on the other thread.
   void DisownMaster();
 
-  // HistoryService::URLEnumerator
-  virtual void OnURL(const history::URLRow& url_row);
+  // VisitedLinkDelegate::URLEnumerator
+  virtual void OnURL(const GURL& url);
   virtual void OnComplete(bool succeed);
 
  private:
@@ -174,30 +174,34 @@ class VisitedLinkMaster::TableBuilder
 
   // Stores the fingerprints we computed on the background thread.
   VisitedLinkCommon::Fingerprints fingerprints_;
+
+  DISALLOW_COPY_AND_ASSIGN(TableBuilder);
 };
 
 // VisitedLinkMaster ----------------------------------------------------------
 
-VisitedLinkMaster::VisitedLinkMaster(Profile* profile)
-    : profile_(profile) {
-  listener_.reset(new VisitedLinkEventListener(profile));
-  DCHECK(listener_.get());
+VisitedLinkMaster::VisitedLinkMaster(content::BrowserContext* browser_context,
+                                     VisitedLinkDelegate* delegate)
+    : browser_context_(browser_context),
+      delegate_(delegate),
+      listener_(new VisitedLinkEventListener(
+          ALLOW_THIS_IN_INITIALIZER_LIST(this), browser_context)) {
   InitMembers();
 }
 
 VisitedLinkMaster::VisitedLinkMaster(Listener* listener,
-                                     HistoryService* history_service,
+                                     VisitedLinkDelegate* delegate,
                                      bool suppress_rebuild,
                                      const FilePath& filename,
                                      int32 default_table_size)
-    : profile_(NULL) {
+    : browser_context_(NULL),
+      delegate_(delegate) {
   listener_.reset(listener);
   DCHECK(listener_.get());
   InitMembers();
 
   database_name_override_ = filename;
   table_size_override_ = default_table_size;
-  history_service_override_ = history_service;
   suppress_rebuild_ = suppress_rebuild;
 }
 
@@ -220,7 +224,6 @@ void VisitedLinkMaster::InitMembers() {
   shared_memory_serial_ = 0;
   used_items_ = 0;
   table_size_override_ = 0;
-  history_service_override_ = NULL;
   suppress_rebuild_ = false;
   sequence_token_ = BrowserThread::GetBlockingPool()->GetSequenceToken();
 
@@ -241,7 +244,9 @@ bool VisitedLinkMaster::Init() {
 
 VisitedLinkMaster::Hash VisitedLinkMaster::TryToAddURL(const GURL& url) {
   // Extra check that we are not incognito. This should not happen.
-  if (profile_ && profile_->IsOffTheRecord()) {
+  // TODO(boliu): Move this check to HistoryService when IsOffTheRecord is
+  // removed from BrowserContext.
+  if (browser_context_ && browser_context_->IsOffTheRecord()) {
     NOTREACHED();
     return null_hash_;
   }
@@ -320,10 +325,12 @@ void VisitedLinkMaster::DeleteAllURLs() {
   listener_->Reset();
 }
 
-void VisitedLinkMaster::DeleteURLs(const history::URLRows& rows) {
-  typedef std::set<GURL>::const_iterator SetIterator;
+VisitedLinkDelegate* VisitedLinkMaster::GetDelegate() {
+  return delegate_;
+}
 
-  if (rows.empty())
+void VisitedLinkMaster::DeleteURLs(URLIterator* urls) {
+  if (!urls->HasNextURL())
     return;
 
   listener_->Reset();
@@ -331,9 +338,8 @@ void VisitedLinkMaster::DeleteURLs(const history::URLRows& rows) {
   if (table_builder_) {
     // A rebuild is in progress, save this deletion in the temporary list so
     // it can be added once rebuild is complete.
-    for (history::URLRows::const_iterator i = rows.begin(); i != rows.end();
-         ++i) {
-      const GURL& url(i->url());
+    while (urls->HasNextURL()) {
+      const GURL url(urls->NextURL());
       if (!url.is_valid())
         continue;
 
@@ -357,9 +363,8 @@ void VisitedLinkMaster::DeleteURLs(const history::URLRows& rows) {
 
   // Compute the deleted URLs' fingerprints and delete them
   std::set<Fingerprint> deleted_fingerprints;
-  for (history::URLRows::const_iterator i = rows.begin(); i != rows.end();
-       ++i) {
-    const GURL& url(i->url());
+  while (urls->HasNextURL()) {
+    const GURL url(urls->NextURL());

[brettw, 2013/01/02 20:23:40]

If you change the interface as suggested above, and you keep the & here to avoid
the copy?

[boliu, 2013/01/02 21:36:10]

Joth replied before I did but after I uploaded a new patch. I changed it to
GURL& here, but did not update the Iterator interface.

Is there a style you prefer?

     if (!url.is_valid())
       continue;
     deleted_fingerprints.insert(
@@ -581,7 +586,7 @@ bool VisitedLinkMaster::InitFromScratch(bool suppress_rebuild) {
   // to disk. We don't want to save explicitly here, since the rebuild may
   // not complete, leaving us with an empty but valid visited link database.
   // In the future, we won't know we need to try rebuilding again.
-  return RebuildTableFromHistory();
+  return RebuildTableFromDelegate();
 }
 
 bool VisitedLinkMaster::ReadFileHeader(FILE* file,
@@ -641,10 +646,10 @@ bool VisitedLinkMaster::GetDatabaseFileName(FilePath* filename) {
     return true;
   }
 
-  if (!profile_ || profile_->GetPath().empty())
+  if (!browser_context_ || browser_context_->GetPath().empty())
     return false;
 
-  FilePath profile_dir = profile_->GetPath();
+  FilePath profile_dir = browser_context_->GetPath();
   *filename = profile_dir.Append(FILE_PATH_LITERAL("Visited Links"));
   return true;
 }
@@ -810,23 +815,8 @@ uint32 VisitedLinkMaster::NewTableSizeForCount(int32 item_count) const {
 }
 
 // See the TableBuilder definition in the header file for how this works.
-bool VisitedLinkMaster::RebuildTableFromHistory() {
+bool VisitedLinkMaster::RebuildTableFromDelegate() {
   DCHECK(!table_builder_);
-  if (table_builder_)
-    return false;
-
-  HistoryService* history_service = history_service_override_;
-  if (!history_service && profile_) {
-    history_service =
-        HistoryServiceFactory::GetForProfile(profile_,
-                                             Profile::EXPLICIT_ACCESS);
-  }
-
-  if (!history_service) {
-    DLOG(WARNING) << "Attempted to rebuild visited link table, but couldn't "
-                     "obtain a HistoryService.";
-    return false;
-  }
 
   // TODO(brettw) make sure we have reasonable salt!
   table_builder_ = new TableBuilder(this, salt_);
@@ -834,7 +824,7 @@ bool VisitedLinkMaster::RebuildTableFromHistory() {
   // Make sure the table builder stays live during the call, even if the
   // master is deleted. This is balanced in TableBuilder::OnCompleteMainThread.
   table_builder_->AddRef();
-  history_service->IterateURLs(table_builder_);
+  delegate_->RebuildTable(table_builder_);
   return true;
 }
 
@@ -958,8 +948,7 @@ void VisitedLinkMaster::TableBuilder::DisownMaster() {
   master_ = NULL;
 }
 
-void VisitedLinkMaster::TableBuilder::OnURL(const history::URLRow& url_row) {
-  const GURL& url(url_row.url());
+void VisitedLinkMaster::TableBuilder::OnURL(const GURL& url) {
   if (!url.is_empty()) {
     fingerprints_.push_back(VisitedLinkMaster::ComputeURLFingerprint(
         url.spec().data(), url.spec().length(), salt_));