Commit e8d81d2c authored by Mathieu Perreault's avatar Mathieu Perreault Committed by Commit Bot

[NTP] Move the ThumbnailSource to always run on the UI thread.

Some members were being created on UI thread and used on IO thread which is
definitely not recommended.

Bug: 843205
Change-Id: Id1d6f0e66e8e11347e4ba4ef1a60d0559952d28d
Reviewed-on: https://chromium-review.googlesource.com/1108137
Commit-Queue: Mathieu Perreault <mathp@chromium.org>
Reviewed-by: default avatarMarc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568868}
parent 05040610
......@@ -11,6 +11,7 @@
#include "chrome/browser/thumbnails/thumbnail_service.h"
#include "chrome/browser/thumbnails/thumbnail_service_factory.h"
#include "chrome/common/url_constants.h"
#include "content/public/browser/browser_thread.h"
#include "net/traffic_annotation/network_traffic_annotation.h"
#include "net/url_request/url_request.h"
#include "url/gurl.h"
......@@ -19,12 +20,10 @@
// StartDataRequest.
const char kUrlDelimiter[] = "?fb=";
// Set ThumbnailService now as Profile isn't thread safe.
ThumbnailSource::ThumbnailSource(Profile* profile, bool capture_thumbnails)
: thumbnail_service_(ThumbnailServiceFactory::GetForProfile(profile)),
capture_thumbnails_(capture_thumbnails),
image_data_fetcher_(profile->GetRequestContext()),
weak_ptr_factory_(this) {}
image_data_fetcher_(profile->GetRequestContext()) {}
ThumbnailSource::~ThumbnailSource() = default;
......@@ -37,6 +36,7 @@ void ThumbnailSource::StartDataRequest(
const std::string& path,
const content::ResourceRequestInfo::WebContentsGetter& wc_getter,
const content::URLDataSource::GotDataCallback& callback) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
GURL page_url;
GURL fallback_thumbnail_url;
ExtractPageAndThumbnailUrls(path, &page_url, &fallback_thumbnail_url);
......@@ -97,22 +97,11 @@ void ThumbnailSource::StartDataRequest(
std::string ThumbnailSource::GetMimeType(const std::string&) const {
// We need to explicitly return a mime type, otherwise if the user tries to
// drag the image they get no extension.
// TODO(treib): This isn't correct for remote thumbnails (in
// SendFetchedUrlImage), which are usually jpeg.
// NOTE: This isn't correct for remote thumbnails (in SendFetchedUrlImage),
// which are usually jpeg. However it seems to work fine.
return "image/png";
}
scoped_refptr<base::SingleThreadTaskRunner>
ThumbnailSource::TaskRunnerForRequestPath(const std::string& path) const {
// TopSites can be accessed from the IO thread. Otherwise, the URLs should be
// accessed on the UI thread.
// TODO(treib): |thumbnail_service_| is assumed to be non-null in other
// places, so probably this check isn't necessary?
return thumbnail_service_.get()
? nullptr
: content::URLDataSource::TaskRunnerForRequestPath(path);
}
bool ThumbnailSource::AllowCaching() const {
return false;
}
......
......@@ -10,7 +10,6 @@
#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "base/memory/weak_ptr.h"
#include "base/single_thread_task_runner.h"
#include "components/image_fetcher/core/image_data_fetcher.h"
#include "content/public/browser/url_data_source.h"
......@@ -39,8 +38,6 @@ class ThumbnailSource : public content::URLDataSource {
const content::ResourceRequestInfo::WebContentsGetter& wc_getter,
const content::URLDataSource::GotDataCallback& callback) override;
std::string GetMimeType(const std::string& path) const override;
scoped_refptr<base::SingleThreadTaskRunner> TaskRunnerForRequestPath(
const std::string& path) const override;
bool AllowCaching() const override;
bool ShouldServiceRequest(const GURL& url,
content::ResourceContext* resource_context,
......@@ -73,7 +70,7 @@ class ThumbnailSource : public content::URLDataSource {
image_fetcher::ImageDataFetcher image_data_fetcher_;
base::WeakPtrFactory<ThumbnailSource> weak_ptr_factory_;
base::WeakPtrFactory<ThumbnailSource> weak_ptr_factory_{this};
DISALLOW_COPY_AND_ASSIGN(ThumbnailSource);
};
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment