Skip to content

GitLab

T
tangled
Commit c6f7106f authored by sfiera's avatar sfiera Committed by Commit bot
Browse files
Options

Remove Profile dependency from PopularSites. 

Still depends on FileDownloader, but that's more difficult to work around, and it's lightweight enough that it could and probably should go into components itself.

BUG=603026

Review-Url: https://codereview.chromium.org/1919043005
Cr-Commit-Position: refs/heads/master@{#390357}
parent d8308942
Hide whitespace changes
Inline Side-by-side
Showing with 70 additions and 36 deletions
chrome/browser/android/ntp/most_visited_sites.cc
View file @ c6f7106f
......@@ -19,6 +19,7 @@
#include "chrome/browser/history/top_sites_factory.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/search/suggestions/suggestions_service_factory.h"
#include "chrome/browser/search_engines/template_url_service_factory.h"
#include "chrome/browser/supervised_user/supervised_user_service.h"
#include "chrome/browser/supervised_user/supervised_user_service_factory.h"
#include "chrome/browser/supervised_user/supervised_user_url_filter.h"
......@@ -203,7 +204,9 @@ void MostVisitedSites::SetMostVisitedURLsObserver(
if (ShouldShowPopularSites() &&
NeedPopularSites(profile_->GetPrefs(), num_sites_)) {
popular_sites_.reset(new PopularSites(
profile_,
profile_->GetPrefs(),
TemplateURLServiceFactory::GetForProfile(profile_),
profile_->GetRequestContext(),
GetPopularSitesCountry(),
GetPopularSitesVersion(),
false,
......
chrome/browser/android/ntp/popular_sites.cc
View file @ c6f7106f
......@@ -18,8 +18,6 @@
#include "base/time/time.h"
#include "base/values.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/search_engines/template_url_service_factory.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/common/chrome_switches.h"
#include "components/google/core/browser/google_util.h"
......@@ -48,17 +46,14 @@ const char kPopularSitesVersionPref[] = "popular_sites_version";
// Extract the country from the default search engine if the default search
// engine is Google.
std::string GetDefaultSearchEngineCountryCode(Profile* profile) {
DCHECK(profile);
std::string GetDefaultSearchEngineCountryCode(
const TemplateURLService* template_url_service) {
DCHECK(template_url_service);
base::CommandLine* cmd_line = base::CommandLine::ForCurrentProcess();
if (!cmd_line->HasSwitch(switches::kEnableNTPSearchEngineCountryDetection))
return std::string();
const TemplateURLService* template_url_service =
TemplateURLServiceFactory::GetForProfile(profile);
DCHECK(template_url_service);
const TemplateURL* default_provider =
template_url_service->GetDefaultSearchProvider();
// It's possible to not have a default provider in the case that the default
......@@ -93,12 +88,13 @@ std::string GetVariationsServiceCountry() {
// Google is the default search engine set. If Google is not the default search
// engine use the country provided by VariationsService. Fallback to a default
// if we can't make an educated guess.
std::string GetCountryToUse(Profile* profile,
std::string GetCountryToUse(const TemplateURLService* template_url_service,
const std::string& override_country) {
if (!override_country.empty())
return override_country;
std::string country_code = GetDefaultSearchEngineCountryCode(profile);
std::string country_code = GetDefaultSearchEngineCountryCode(
template_url_service);
if (country_code.empty())
country_code = GetVariationsServiceCountry();
......@@ -179,32 +175,44 @@ PopularSites::Site::Site(const Site& other) = default;
PopularSites::Site::~Site() {}
PopularSites::PopularSites(Profile* profile,
PopularSites::PopularSites(PrefService* prefs,
const TemplateURLService* template_url_service,
net::URLRequestContextGetter* download_context,
const std::string& override_country,
const std::string& override_version,
bool force_download,
const FinishedCallback& callback)
: PopularSites(profile,
GetCountryToUse(profile, override_country),
: PopularSites(prefs,
template_url_service,
download_context,
GetCountryToUse(template_url_service, override_country),
GetVersionToUse(override_version),
GURL(),
force_download,
callback) {}
PopularSites::PopularSites(Profile* profile,
PopularSites::PopularSites(PrefService* prefs,
const TemplateURLService* template_url_service,
net::URLRequestContextGetter* download_context,
const GURL& url,
const FinishedCallback& callback)
: PopularSites(profile, std::string(), std::string(), url, true, callback) {
}
: PopularSites(prefs,
template_url_service,
download_context,
std::string(),
std::string(),
url,
true,
callback) {}
PopularSites::~PopularSites() {}
std::string PopularSites::GetCountry() const {
return profile_->GetPrefs()->GetString(kPopularSitesCountryPref);
return prefs_->GetString(kPopularSitesCountryPref);
}
std::string PopularSites::GetVersion() const {
return profile_->GetPrefs()->GetString(kPopularSitesVersionPref);
return prefs_->GetString(kPopularSitesVersionPref);
}
// static
......@@ -215,7 +223,9 @@ void PopularSites::RegisterProfilePrefs(
user_prefs->RegisterStringPref(kPopularSitesVersionPref, std::string());
}
PopularSites::PopularSites(Profile* profile,
PopularSites::PopularSites(PrefService* prefs,
const TemplateURLService* template_url_service,
net::URLRequestContextGetter* download_context,
const std::string& country,
const std::string& version,
const GURL& override_url,
......@@ -225,10 +235,12 @@ PopularSites::PopularSites(Profile* profile,
pending_country_(country),
pending_version_(version),
local_path_(GetPopularSitesPath()),
profile_(profile),
prefs_(prefs),
template_url_service_(template_url_service),
download_context_(download_context),
weak_ptr_factory_(this) {
const base::Time last_download_time = base::Time::FromInternalValue(
profile_->GetPrefs()->GetInt64(kPopularSitesLastDownloadPref));
prefs_->GetInt64(kPopularSitesLastDownloadPref));
const base::TimeDelta time_since_last_download =
base::Time::Now() - last_download_time;
const base::TimeDelta redownload_interval =
......@@ -256,7 +268,7 @@ void PopularSites::FetchPopularSites(const GURL& url,
bool force_download,
bool is_fallback) {
downloader_.reset(new FileDownloader(
url, local_path_, force_download, profile_->GetRequestContext(),
url, local_path_, force_download, download_context_,
base::Bind(&PopularSites::OnDownloadDone, base::Unretained(this),
is_fallback)));
}
......@@ -264,13 +276,12 @@ void PopularSites::FetchPopularSites(const GURL& url,
void PopularSites::OnDownloadDone(bool is_fallback,
FileDownloader::Result result) {
downloader_.reset();
PrefService* prefs = profile_->GetPrefs();
switch (result) {
case FileDownloader::DOWNLOADED:
prefs->SetInt64(kPopularSitesLastDownloadPref,
prefs_->SetInt64(kPopularSitesLastDownloadPref,
base::Time::Now().ToInternalValue());
prefs->SetString(kPopularSitesCountryPref, pending_country_);
prefs->SetString(kPopularSitesVersionPref, pending_version_);
prefs_->SetString(kPopularSitesCountryPref, pending_country_);
prefs_->SetString(kPopularSitesVersionPref, pending_version_);
ParseSiteList(local_path_);
break;
case FileDownloader::EXISTS:
......
chrome/browser/android/ntp/popular_sites.h
View file @ c6f7106f
......@@ -25,7 +25,8 @@ namespace user_prefs {
class PrefRegistrySyncable;
}
class Profile;
class PrefService;
class TemplateURLService;
// Downloads and provides a list of suggested popular sites, for display on
// the NTP when there are not enough personalized suggestions. Caches the
......@@ -56,7 +57,9 @@ class PopularSites {
// override the baked-in default version.
// Set |force_download| to enforce re-downloading the suggestions file, even
// if it already exists on disk.
PopularSites(Profile* profile,
PopularSites(PrefService* prefs,
const TemplateURLService* template_url_service,
net::URLRequestContextGetter* download_context,
const std::string& override_country,
const std::string& override_version,
bool force_download,
......@@ -64,7 +67,9 @@ class PopularSites {
// This fetches the popular sites from a given url and is only used for
// debugging through the popular-sites-internals page.
PopularSites(Profile* profile,
PopularSites(PrefService* prefs,
const TemplateURLService* template_url_service,
net::URLRequestContextGetter* download_context,
const GURL& url,
const FinishedCallback& callback);
......@@ -83,7 +88,9 @@ class PopularSites {
user_prefs::PrefRegistrySyncable* user_prefs);
private:
PopularSites(Profile* profile,
PopularSites(PrefService* prefs,
const TemplateURLService* template_url_service,
net::URLRequestContextGetter* download_context,
const std::string& country,
const std::string& version,
const GURL& override_url,
......@@ -113,7 +120,9 @@ class PopularSites {
base::FilePath local_path_;
Profile* profile_;
PrefService* prefs_;
const TemplateURLService* template_url_service_;
net::URLRequestContextGetter* download_context_;
base::WeakPtrFactory<PopularSites> weak_ptr_factory_;
......
chrome/browser/ui/webui/popular_sites_internals_message_handler.cc
View file @ c6f7106f
......@@ -14,6 +14,7 @@
#include "base/values.h"
#include "chrome/browser/android/ntp/popular_sites.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/search_engines/template_url_service_factory.h"
#include "components/url_formatter/url_fixer.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/web_ui.h"
......@@ -55,8 +56,12 @@ void PopularSitesInternalsMessageHandler::HandleRegisterForEvents(
std::string country;
std::string version;
Profile* profile = Profile::FromWebUI(web_ui());
popular_sites_.reset(new PopularSites(
Profile::FromWebUI(web_ui()), country, version, false,
profile->GetPrefs(),
TemplateURLServiceFactory::GetForProfile(profile),
profile->GetRequestContext(),
country, version, false,
base::Bind(&PopularSitesInternalsMessageHandler::OnPopularSitesAvailable,
base::Unretained(this), false)));
}
......@@ -64,6 +69,7 @@ void PopularSitesInternalsMessageHandler::HandleRegisterForEvents(
void PopularSitesInternalsMessageHandler::HandleDownload(
const base::ListValue* args) {
DCHECK_EQ(3u, args->GetSize());
Profile* profile = Profile::FromWebUI(web_ui());
auto callback =
base::Bind(&PopularSitesInternalsMessageHandler::OnPopularSitesAvailable,
base::Unretained(this), true);
......@@ -72,7 +78,9 @@ void PopularSitesInternalsMessageHandler::HandleDownload(
args->GetString(0, &url);
if (!url.empty()) {
popular_sites_.reset(new PopularSites(
Profile::FromWebUI(web_ui()),
profile->GetPrefs(),
TemplateURLServiceFactory::GetForProfile(profile),
profile->GetRequestContext(),
url_formatter::FixupURL(url, std::string()), callback));
return;
}
......@@ -80,8 +88,11 @@ void PopularSitesInternalsMessageHandler::HandleDownload(
args->GetString(1, &country);
std::string version;
args->GetString(2, &version);
popular_sites_.reset(new PopularSites(Profile::FromWebUI(web_ui()), country,
version, true, callback));
popular_sites_.reset(new PopularSites(
profile->GetPrefs(),
TemplateURLServiceFactory::GetForProfile(profile),
profile->GetRequestContext(),
country, version, true, callback));
}
void PopularSitesInternalsMessageHandler::HandleViewJson(
......
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