Remove dependency on visitedlink from history

chrome/browser/history/history_service.cc

 // Copyright (c) 2012 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.
 
 // The history system runs on a background thread so that potentially slow
 // database operations don't delay the browser. This backend processing is
 // represented by HistoryBackend. The HistoryService's job is to dispatch to
 // that thread.
 //
 // Main thread                       History thread
@@ skipping 30 matching lines @@
 #include "chrome/common/url_constants.h"
 #include "components/dom_distiller/core/url_constants.h"
 #include "components/history/core/browser/download_row.h"
 #include "components/history/core/browser/history_client.h"
 #include "components/history/core/browser/history_database_params.h"
 #include "components/history/core/browser/history_service_observer.h"
 #include "components/history/core/browser/history_types.h"
 #include "components/history/core/browser/in_memory_database.h"
 #include "components/history/core/browser/keyword_search_term.h"
 #include "components/history/core/browser/visit_database.h"
+#include "components/history/core/browser/visit_delegate.h"
 #include "components/history/core/browser/visit_filter.h"
 #include "components/history/core/browser/web_history_service.h"
 #include "components/history/core/common/thumbnail_score.h"
-#include "components/visitedlink/browser/visitedlink_master.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/download_item.h"
 #include "sync/api/sync_error_factory.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 
 using base::Time;
 using history::HistoryBackend;
 using history::KeywordID;
 
 namespace {
@@ skipping 16 matching lines @@
                            const history::QueryURLResult* result) {
   callback.Run(result->success, result->row, result->visits);
 }
 
 void RunWithVisibleVisitCountToHostResult(
     const HistoryService::GetVisibleVisitCountToHostCallback& callback,
     const history::VisibleVisitCountToHostResult* result) {
   callback.Run(result->success, result->count, result->first_visit);
 }
 
-// Extract history::URLRows into GURLs for VisitedLinkMaster.
-class URLIteratorFromURLRows
-    : public visitedlink::VisitedLinkMaster::URLIterator {
- public:
-  explicit URLIteratorFromURLRows(const history::URLRows& url_rows)
-      : itr_(url_rows.begin()),
-        end_(url_rows.end()) {
-  }
-
-  const GURL& NextURL() override { return (itr_++)->url(); }
-
-  bool HasNextURL() const override { return itr_ != end_; }
-
- private:
-  history::URLRows::const_iterator itr_;
-  history::URLRows::const_iterator end_;
-
-  DISALLOW_COPY_AND_ASSIGN(URLIteratorFromURLRows);
-};
-
 // Callback from WebHistoryService::ExpireWebHistory().
 void ExpireWebHistoryComplete(bool success) {
   // Ignore the result.
   //
   // TODO(davidben): ExpireLocalAndRemoteHistoryBetween callback should not fire
   // until this completes.
 }
 
 }  // namespace
 
@@ skipping 92 matching lines @@
  private:
   const base::WeakPtr<HistoryService> history_service_;
   const scoped_refptr<base::SequencedTaskRunner> service_task_runner_;
 };
 
 // The history thread is intentionally not a BrowserThread because the
 // sync integration unit tests depend on being able to create more than one
 // history thread.
 HistoryService::HistoryService()
     : thread_(new base::Thread(kHistoryThreadName)),
-      history_client_(NULL),
+      history_client_(nullptr),
       backend_loaded_(false),
       no_db_(false),
       weak_ptr_factory_(this) {
 }
 
 HistoryService::HistoryService(
-    history::HistoryClient* history_client, Profile* profile)
+    history::HistoryClient* history_client,
+    scoped_ptr<history::VisitDelegate> visit_delegate)
     : thread_(new base::Thread(kHistoryThreadName)),
+      visit_delegate_(visit_delegate.Pass()),
       history_client_(history_client),
-      visitedlink_master_(new visitedlink::VisitedLinkMaster(
-          profile, this, true)),
       backend_loaded_(false),
       no_db_(false),
       weak_ptr_factory_(this) {
 }
 
 HistoryService::~HistoryService() {
   DCHECK(thread_checker_.CalledOnValidThread());
   // Shutdown the backend. This does nothing if Cleanup was already invoked.
   Cleanup();
 }
 
 bool HistoryService::BackendLoaded() {
   DCHECK(thread_checker_.CalledOnValidThread());
   return backend_loaded_;
 }
 
 void HistoryService::ClearCachedDataForContextID(
     history::ContextID context_id) {
   DCHECK(thread_) << "History service being called after cleanup";
   DCHECK(thread_checker_.CalledOnValidThread());
   ScheduleTask(PRIORITY_NORMAL,
                base::Bind(&HistoryBackend::ClearCachedDataForContextID,
                           history_backend_.get(), context_id));
 }
 
 history::URLDatabase* HistoryService::InMemoryDatabase() {
   DCHECK(thread_checker_.CalledOnValidThread());
-  return in_memory_backend_ ? in_memory_backend_->db() : NULL;
+  return in_memory_backend_ ? in_memory_backend_->db() : nullptr;
 }
 
 bool HistoryService::GetTypedCountForURL(const GURL& url, int* typed_count) {
   DCHECK(thread_checker_.CalledOnValidThread());
   history::URLRow url_row;
   if (!GetRowForURL(url, &url_row))
     return false;
   *typed_count = url_row.typed_count();
   return true;
 }
@@ skipping 130 matching lines @@
       history::HistoryAddPageArgs(url, time, context_id, nav_entry_id, referrer,
                                   redirects, transition, visit_source,
                                   did_replace_entry));
 }
 
 void HistoryService::AddPage(const GURL& url,
                              base::Time time,
                              history::VisitSource visit_source) {
   DCHECK(thread_checker_.CalledOnValidThread());
   AddPage(
-      history::HistoryAddPageArgs(url, time, NULL, 0, GURL(),
+      history::HistoryAddPageArgs(url, time, nullptr, 0, GURL(),
                                   history::RedirectList(),
                                   ui::PAGE_TRANSITION_LINK,
                                   visit_source, false));
 }
 
 void HistoryService::AddPage(const history::HistoryAddPageArgs& add_page_args) {
   DCHECK(thread_) << "History service being called after cleanup";
   DCHECK(thread_checker_.CalledOnValidThread());
 
   // Filter out unwanted URLs. We don't add auto-subframe URLs. They are a
   // large part of history (think iframes for ads) and we never display them in
   // history UI. We will still add manual subframes, which are ones the user
   // has clicked on to get.
   if (!CanAddURL(add_page_args.url))
     return;
 
-  // Add link & all redirects to visited link list.
-  if (visitedlink_master_) {
-    visitedlink_master_->AddURL(add_page_args.url);
-
+  // Inform VisitedDelegate of all links and redirects.
+  if (visit_delegate_) {
     if (!add_page_args.redirects.empty()) {
       // We should not be asked to add a page in the middle of a redirect chain.
-      DCHECK_EQ(add_page_args.url,
-                add_page_args.redirects[add_page_args.redirects.size() - 1]);
+      DCHECK_EQ(add_page_args.url, add_page_args.redirects.back());
 
       // We need the !redirects.empty() condition above since size_t is unsigned
       // and will wrap around when we subtract one from a 0 size.

[droger, 2015/02/02 09:39:21]

This comment seems outdated.
I am also not convinced that the new code is equivalent to the old one:
add_page_args.url was always added, but now it's only added if the redirects are
empty.

[sdefresne, 2015/02/04 18:01:54]

The DCHECK() verify that the last element of add_page_args.redirect is
add_page_args.url. The old code was first sending add_page_args.url and then
every elements of add_page_args.redirect except the last when the array was not
empty. The new code always sends all the element of the array, so the same
elements will be sent, just in a different order (and the order is not important
in the delegate treat the array as a set).

Removed the comment.

-      for (size_t i = 0; i < add_page_args.redirects.size() - 1; i++)
-        visitedlink_master_->AddURL(add_page_args.redirects[i]);
+      visit_delegate_->AddURLs(add_page_args.redirects);
+    } else {
+      visit_delegate_->AddURL(add_page_args.url);
     }
   }
 
   ScheduleTask(PRIORITY_NORMAL,
                base::Bind(&HistoryBackend::AddPage, history_backend_.get(),
                           add_page_args));
 }
 
 void HistoryService::AddPageNoVisitForBookmark(const GURL& url,
                                                const base::string16& title) {
@@ skipping 33 matching lines @@
                                         int typed_count,
                                         Time last_visit,
                                         bool hidden,
                                         history::VisitSource visit_source) {
   DCHECK(thread_) << "History service being called after cleanup";
   DCHECK(thread_checker_.CalledOnValidThread());
   // Filter out unwanted URLs.
   if (!CanAddURL(url))
     return;
 
-  // Add to the visited links system.
-  if (visitedlink_master_)
-    visitedlink_master_->AddURL(url);
+  // Inform VisitDelegate of the URL.
+  if (visit_delegate_)
+    visit_delegate_->AddURL(url);
 
   history::URLRow row(url);
   row.set_title(title);
   row.set_visit_count(visit_count);
   row.set_typed_count(typed_count);
   row.set_last_visit(last_visit);
   row.set_hidden(hidden);
 
   history::URLRows rows;
   rows.push_back(row);
 
   ScheduleTask(PRIORITY_NORMAL,
                base::Bind(&HistoryBackend::AddPagesWithDetails,
                           history_backend_.get(), rows, visit_source));
 }
 
 void HistoryService::AddPagesWithDetails(const history::URLRows& info,
                                          history::VisitSource visit_source) {
   DCHECK(thread_) << "History service being called after cleanup";
   DCHECK(thread_checker_.CalledOnValidThread());
-  // Add to the visited links system.
-  if (visitedlink_master_) {
+
+  // Inform the VisitDelegate of the URLs
+  if (!info.empty() && visit_delegate_) {
     std::vector<GURL> urls;
     urls.reserve(info.size());
-    for (history::URLRows::const_iterator i = info.begin(); i != info.end();
-         ++i)
-      urls.push_back(i->url());
-
-    visitedlink_master_->AddURLs(urls);
+    for (const auto& row : info)
+      urls.push_back(row.url());
+    visit_delegate_->AddURLs(urls);
   }
 
   ScheduleTask(PRIORITY_NORMAL,
                base::Bind(&HistoryBackend::AddPagesWithDetails,
                           history_backend_.get(), info, visit_source));
 }
 
 base::CancelableTaskTracker::TaskId HistoryService::GetFavicons(
     const std::vector<GURL>& icon_urls,
     int icon_types,
@@ skipping 212 matching lines @@
 }
 
 // Handle queries for a list of all downloads in the history database's
 // 'downloads' table.
 void HistoryService::QueryDownloads(
     const DownloadQueryCallback& callback) {
   DCHECK(thread_) << "History service being called after cleanup";
   DCHECK(thread_checker_.CalledOnValidThread());
   std::vector<history::DownloadRow>* rows =
     new std::vector<history::DownloadRow>();
-  scoped_ptr<std::vector<history::DownloadRow> > scoped_rows(rows);
+  scoped_ptr<std::vector<history::DownloadRow>> scoped_rows(rows);
   // Beware! The first Bind() does not simply |scoped_rows.get()| because
   // base::Passed(&scoped_rows) nullifies |scoped_rows|, and compilers do not
   // guarantee that the first Bind's arguments are evaluated before the second
   // Bind's arguments.
   thread_->message_loop_proxy()->PostTaskAndReply(
       FROM_HERE,
       base::Bind(&HistoryBackend::QueryDownloads, history_backend_.get(), rows),
       base::Bind(callback, base::Passed(&scoped_rows)));
 }
 
@@ skipping 165 matching lines @@
     // reference from the history thread, ensuring everything works properly.
     //
     // TODO(ajwong): Cleanup HistoryBackend lifetime issues.
     //     See http://crbug.com/99767.
     history_backend_->AddRef();
     base::Closure closing_task =
         base::Bind(&HistoryBackend::Closing, history_backend_.get());
     ScheduleTask(PRIORITY_NORMAL, closing_task);
     closing_task.Reset();
     HistoryBackend* raw_ptr = history_backend_.get();
-    history_backend_ = NULL;
+    history_backend_ = nullptr;
     thread_->message_loop()->ReleaseSoon(FROM_HERE, raw_ptr);
   }
 
   // Delete the thread, which joins with the background thread. We defensively
-  // NULL the pointer before deleting it in case somebody tries to use it
+  // nullptr the pointer before deleting it in case somebody tries to use it
   // during shutdown, but this shouldn't happen.
   base::Thread* thread = thread_;
-  thread_ = NULL;
+  thread_ = nullptr;
   delete thread;
 }
 
-void HistoryService::RebuildTable(
-    const scoped_refptr<URLEnumerator>& enumerator) {
-  DCHECK(thread_) << "History service being called after cleanup";
-  DCHECK(thread_checker_.CalledOnValidThread());
-  ScheduleTask(PRIORITY_NORMAL, base::Bind(&HistoryBackend::IterateURLs,
-                                           history_backend_.get(), enumerator));
-}
-
 bool HistoryService::Init(
     bool no_db,
     PrefService* prefs,
     const history::HistoryDatabaseParams& history_database_params) {
   DCHECK(thread_) << "History service being called after cleanup";
   DCHECK(thread_checker_.CalledOnValidThread());
   base::Thread::Options options;
   options.timer_slack = base::TIMER_SLACK_MAXIMUM;
   if (!thread_->StartWithOptions(options)) {
     Cleanup();
@@ skipping 16 matching lines @@
   scoped_refptr<HistoryBackend> backend(new HistoryBackend(
       history_dir_, new BackendDelegate(weak_ptr_factory_.GetWeakPtr(),
                                         base::ThreadTaskRunnerHandle::Get()),
       history_client_));
   history_backend_.swap(backend);
 
   ScheduleTask(PRIORITY_UI,
                base::Bind(&HistoryBackend::Init, history_backend_.get(),
                           languages, no_db_, history_database_params));
 
-  if (visitedlink_master_) {
-    bool result = visitedlink_master_->Init();
-    DCHECK(result);
-  }
+  if (visit_delegate_ && !visit_delegate_->Init(this))
+    return false;
 
   return true;
 }
 
 void HistoryService::ScheduleAutocomplete(const base::Callback<
     void(history::HistoryBackend*, history::URLDatabase*)>& callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
   ScheduleTask(PRIORITY_UI, base::Bind(&HistoryBackend::ScheduleAutocomplete,
                                        history_backend_.get(), callback));
 }
@@ skipping 205 matching lines @@
 }
 
 void HistoryService::NotifyURLsDeleted(bool all_history,
                                        bool expired,
                                        const history::URLRows& deleted_rows,
                                        const std::set<GURL>& favicon_urls) {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (!thread_)
     return;
 
-  // Update the visited link system for deleted URLs. We will update the
-  // visited link system for added URLs as soon as we get the add
-  // notification (we don't have to wait for the backend, which allows us to
-  // be faster to update the state).
+  // Inform the VisitDelegate of the deleted URLs. We will inform the delegate
+  // of added URLs as soon as we get the add notification (we don't have to wait
+  // for the backend, which allows us to be faster to update the state).
   //
   // For deleted URLs, we don't typically know what will be deleted since
   // delete notifications are by time. We would also like to be more
   // respectful of privacy and never tell the user something is gone when it
   // isn't. Therefore, we update the delete URLs after the fact.
-  if (visitedlink_master_) {
+  if (visit_delegate_) {
     if (all_history) {
-      visitedlink_master_->DeleteAllURLs();
+      visit_delegate_->DeleteAllURLs();
     } else {
-      URLIteratorFromURLRows iterator(deleted_rows);
-      visitedlink_master_->DeleteURLs(&iterator);
+      std::vector<GURL> urls;
+      urls.reserve(deleted_rows.size());
+      for (const auto& row : deleted_rows)
+        urls.push_back(row.url());
+      visit_delegate_->DeleteURLs(urls);
     }
   }
 
   FOR_EACH_OBSERVER(
       history::HistoryServiceObserver, observers_,
       OnURLsDeleted(this, all_history, expired, deleted_rows, favicon_urls));
 }
 
 void HistoryService::NotifyHistoryServiceLoaded() {
   DCHECK(thread_checker_.CalledOnValidThread());
@@ skipping 27 matching lines @@
     const HistoryService::OnFaviconChangedCallback& callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
   return favicon_changed_callback_list_.Add(callback);
 }
 
 void HistoryService::NotifyFaviconChanged(
     const std::set<GURL>& changed_favicons) {
   DCHECK(thread_checker_.CalledOnValidThread());
   favicon_changed_callback_list_.Notify(changed_favicons);
 }