Remove NTP dependency on //content/...

chrome/browser/android/ntp/most_visited_sites.cc

Index: chrome/browser/android/ntp/most_visited_sites.cc
diff --git a/chrome/browser/android/ntp/most_visited_sites.cc b/chrome/browser/android/ntp/most_visited_sites.cc
index 372090f08bd8bfc924ab982ee9b9716f3ee777cc..c3f859394b7a5d151daea5d0595b6fa24e5d6289 100644
--- a/chrome/browser/android/ntp/most_visited_sites.cc
+++ b/chrome/browser/android/ntp/most_visited_sites.cc
@@ -22,12 +22,10 @@
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_service.h"
 #include "components/variations/variations_associated_data.h"
-#include "content/public/browser/browser_thread.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/gfx/codec/jpeg_codec.h"
 #include "url/gurl.h"
 
-using content::BrowserThread;
 using history::TopSites;
 using suggestions::ChromeSuggestion;
 using suggestions::SuggestionsProfile;
@@ -63,10 +61,10 @@ enum MostVisitedTileType {
     NUM_TILE_TYPES,
 };
 
+// May only be called from blocking thread pool.
 std::unique_ptr<SkBitmap> MaybeFetchLocalThumbnail(
     const GURL& url,
     const scoped_refptr<TopSites>& top_sites) {
-  DCHECK_CURRENTLY_ON(BrowserThread::DB);
   scoped_refptr<base::RefCountedMemory> image;
   std::unique_ptr<SkBitmap> bitmap;
   if (top_sites && top_sites->GetPageThumbnail(url, false, &image))
@@ -169,6 +167,7 @@ MostVisitedSites::Suggestion&
 MostVisitedSites::Suggestion::operator=(Suggestion&&) = default;
 
 MostVisitedSites::MostVisitedSites(
+    scoped_refptr<base::SequencedWorkerPool> blocking_pool,
     PrefService* prefs,
     const TemplateURLService* template_url_service,
     variations::VariationsService* variations_service,
@@ -192,6 +191,7 @@ MostVisitedSites::MostVisitedSites(
       recorded_uma_(false),
       scoped_observer_(this),
       mv_source_(SUGGESTIONS_SERVICE),
+      blocking_pool_(std::move(blocking_pool)),
       weak_ptr_factory_(this) {
   supervisor_->SetObserver(this);
 }
@@ -209,8 +209,8 @@ void MostVisitedSites::SetMostVisitedURLsObserver(Observer* observer,
   if (ShouldShowPopularSites() &&
       NeedPopularSites(prefs_, num_sites_)) {
     popular_sites_.reset(new PopularSites(
-        prefs_, template_url_service_, variations_service_, download_context_,
-        popular_sites_directory_, GetPopularSitesCountry(),
+        blocking_pool_, prefs_, template_url_service_, variations_service_,
+        download_context_, popular_sites_directory_, GetPopularSitesCountry(),
         GetPopularSitesVersion(), false,
         base::Bind(&MostVisitedSites::OnPopularSitesAvailable,
                    base::Unretained(this))));
@@ -242,12 +242,14 @@ void MostVisitedSites::SetMostVisitedURLsObserver(Observer* observer,
 
 void MostVisitedSites::GetURLThumbnail(const GURL& url,
                                        const ThumbnailCallback& callback) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK(thread_checker_.CalledOnValidThread());
 
-  // TODO(treib): Move this to the blocking pool? Doesn't seem related to DB.
-  BrowserThread::PostTaskAndReplyWithResult(
-      BrowserThread::DB, FROM_HERE,
-      base::Bind(&MaybeFetchLocalThumbnail, url, top_sites_),
+  base::PostTaskAndReplyWithResult(
+      blocking_pool_

[blundell, 2016/05/25 09:00:24]

Usually I would suggest making behavioral changes in separate CLs from
dependency injection changes for separation of concerns. Something to note for
the future. In this case, please add the behavioral change to the CL
description.

+          ->GetTaskRunnerWithShutdownBehavior(

[Bernhard Bauer, 2016/05/25 17:02:49]

Sorry to barge in again here -- this creates a new TaskRunner every time, which
is kind of unnecessary. You can just get a TaskRunner in the constructor and
store that in a member instead of the SWP.

(Also, just for the sake of completeness: the TaskRunner here does not guarantee
that tasks will run sequentially; a SequencedTaskRunner would do that. That's
probably fine in this case, but just something to be aware of.)

[sfiera, 2016/05/27 10:37:42]

OK, done. Though that's in addition to the SWP, which is needed for
PopularSites, instead of instead of.
Seems like it's probably preferable to support parallel fetch of thumbnails this
way, but it's called from Java code that I don't know.

+              base::SequencedWorkerPool::CONTINUE_ON_SHUTDOWN)
+          .get(),
+      FROM_HERE, base::Bind(&MaybeFetchLocalThumbnail, url, top_sites_),
       base::Bind(&MostVisitedSites::OnLocalThumbnailFetched,
                  weak_ptr_factory_.GetWeakPtr(), url, callback));
 }
@@ -256,7 +258,7 @@ void MostVisitedSites::OnLocalThumbnailFetched(
     const GURL& url,
     const ThumbnailCallback& callback,
     std::unique_ptr<SkBitmap> bitmap) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK(thread_checker_.CalledOnValidThread());
   if (!bitmap.get()) {
     // A thumbnail is not locally available for |url|. Make sure it is put in
     // the list to be fetched at the next visit to this site.
@@ -289,7 +291,7 @@ void MostVisitedSites::OnObtainedThumbnail(bool is_local_thumbnail,
                                            const ThumbnailCallback& callback,
                                            const GURL& url,
                                            const SkBitmap* bitmap) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK(thread_checker_.CalledOnValidThread());
   callback.Run(is_local_thumbnail, bitmap);
 }