Remove NTP dependency on //content/...

chrome/browser/android/ntp/most_visited_sites.cc

 // Copyright 2013 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.
 
 #include "chrome/browser/android/ntp/most_visited_sites.h"
 
 #include <utility>
 
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "components/history/core/browser/top_sites.h"
 #include "components/ntp_tiles/pref_names.h"
 #include "components/ntp_tiles/switches.h"
 #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;
 using suggestions::SuggestionsService;
 
 namespace {
 
 // Identifiers for the various tile sources.
 const char kHistogramClientName[] = "client";
 const char kHistogramServerName[] = "server";
@@ skipping 15 matching lines @@
     NONE,
     // The item displays a site's actual favicon or touch icon.
     ICON_REAL,
     // The item displays a color derived from the site's favicon or touch icon.
     ICON_COLOR,
     // The item displays a default gray box in place of an icon.
     ICON_DEFAULT,
     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))
     bitmap = gfx::JPEGCodec::Decode(image->front(), image->size());
   return bitmap;
 }
 
 // Log an event for a given |histogram| at a given element |position|. This
 // routine exists because regular histogram macros are cached thus can't be used
 // if the name of the histogram will change at a given call site.
@@ skipping 82 matching lines @@
 
 MostVisitedSites::Suggestion::Suggestion() : provider_index(-1) {}
 
 MostVisitedSites::Suggestion::~Suggestion() {}
 
 MostVisitedSites::Suggestion::Suggestion(Suggestion&&) = default;
 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,
     net::URLRequestContextGetter* download_context,
     const base::FilePath& popular_sites_directory,
     scoped_refptr<history::TopSites> top_sites,
     SuggestionsService* suggestions,
     MostVisitedSitesSupervisor* supervisor)
     : prefs_(prefs),
       template_url_service_(template_url_service),
       variations_service_(variations_service),
       download_context_(download_context),
       popular_sites_directory_(popular_sites_directory),
       top_sites_(top_sites),
       suggestions_service_(suggestions),
       supervisor_(supervisor),
       observer_(nullptr),
       num_sites_(0),
       received_most_visited_sites_(false),
       received_popular_sites_(false),
       recorded_uma_(false),
       scoped_observer_(this),
       mv_source_(SUGGESTIONS_SERVICE),
+      blocking_pool_(std::move(blocking_pool)),
       weak_ptr_factory_(this) {
   supervisor_->SetObserver(this);
 }
 
 MostVisitedSites::~MostVisitedSites() {
   supervisor_->SetObserver(nullptr);
 }
 
 void MostVisitedSites::SetMostVisitedURLsObserver(Observer* observer,
                                                   int num_sites) {
   DCHECK(observer);
   observer_ = observer;
   num_sites_ = num_sites;
 
   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))));
   } else {
     received_popular_sites_ = true;
   }
 
   // TODO(treib): Can |top_sites_| ever be null? If not, remove these checks.
   if (top_sites_) {
     // TopSites updates itself after a delay. To ensure up-to-date results,
@@ skipping 11 matching lines @@
 
   // Immediately build the current suggestions, getting personal suggestions
   // from the SuggestionsService's cache or, if that is empty, from TopSites.
   BuildCurrentSuggestions();
   // Also start a request for fresh suggestions.
   suggestions_service_->FetchSuggestionsData();
 }
 
 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));
 }
 
 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.
     if (top_sites_)
       top_sites_->AddForcedURL(url, base::Time::Now());
     // Also fetch a remote thumbnail if possible. PopularSites or the
     // SuggestionsService can supply a thumbnail download URL.
     if (popular_sites_) {
       const std::vector<PopularSites::Site>& sites = popular_sites_->sites();
       auto it = std::find_if(
@@ skipping 12 matching lines @@
                           weak_ptr_factory_.GetWeakPtr(), false, callback));
     }
   }
   OnObtainedThumbnail(true, callback, url, bitmap.get());
 }
 
 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);
 }
 
 void MostVisitedSites::AddOrRemoveBlacklistedUrl(const GURL& url,
                                                  bool add_url) {
   // Always blacklist in the local TopSites.
   if (top_sites_) {
     if (add_url)
       top_sites_->AddBlacklistedURL(url);
     else
@@ skipping 342 matching lines @@
 
 void MostVisitedSites::TopSitesLoaded(TopSites* top_sites) {}
 
 void MostVisitedSites::TopSitesChanged(TopSites* top_sites,
                                        ChangeReason change_reason) {
   if (mv_source_ == TOP_SITES) {
     // The displayed suggestions are invalidated.
     InitiateTopSitesQuery();
   }
 }