Commit db519299 authored by rockot's avatar rockot Committed by Commit bot

Reland: Factor Chrome details out of update manifest fetching.

Relanding of https://codereview.chromium.org/465543004
with a fix for official builds.

This establishes a ManifestFetchDataDelegate for use
by ExtensionDownloader and ManifestFetchData, and moves
ManifestFetchData into //extensions/browser/updater.

The delegate provides implementation details for
update manifest fetching, including brand code,
boilerplate query parameters, and ping data.

Chrome's implementation has knowledge of the
Google brand, the metrics service, and Omaha-specific
query parameters necessary for CRX item queries.

BUG=398671
TBR=isherman@chromium.org

Review URL: https://codereview.chromium.org/475423002

Cr-Commit-Position: refs/heads/master@{#295142}
parent 3c85191d
......@@ -16,6 +16,7 @@
#include "base/version.h"
#include "chrome/browser/extensions/crx_installer.h"
#include "chrome/browser/extensions/external_provider_impl.h"
#include "chrome/browser/extensions/updater/chrome_extension_downloader_factory.h"
#include "chrome/browser/extensions/updater/extension_downloader.h"
#include "chrome/common/extensions/extension_constants.h"
#include "content/public/browser/notification_details.h"
......
......@@ -41,17 +41,15 @@
#include "chrome/browser/extensions/permissions_updater.h"
#include "chrome/browser/extensions/shared_module_service.h"
#include "chrome/browser/extensions/unpacked_installer.h"
#include "chrome/browser/extensions/updater/chrome_extension_downloader_factory.h"
#include "chrome/browser/extensions/updater/extension_cache.h"
#include "chrome/browser/extensions/updater/extension_downloader.h"
#include "chrome/browser/extensions/updater/extension_updater.h"
#include "chrome/browser/google/google_brand.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/signin/profile_identity_provider.h"
#include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
#include "chrome/browser/signin/signin_manager_factory.h"
#include "chrome/browser/ui/webui/extensions/extension_icon_source.h"
#include "chrome/browser/ui/webui/favicon_source.h"
#include "chrome/browser/ui/webui/ntp/thumbnail_source.h"
#include "chrome/browser/ui/webui/signin/login_ui_service_factory.h"
#include "chrome/browser/ui/webui/theme_source.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/common/crash_keys.h"
......@@ -60,7 +58,6 @@
#include "chrome/common/extensions/manifest_url_handler.h"
#include "chrome/common/url_constants.h"
#include "components/crx_file/id_util.h"
#include "components/signin/core/browser/signin_manager.h"
#include "components/startup_metric_utils/startup_metric_utils.h"
#include "content/public/browser/devtools_agent_host.h"
#include "content/public/browser/notification_service.h"
......@@ -95,8 +92,6 @@ using content::BrowserThread;
using content::DevToolsAgentHost;
using extensions::CrxInstaller;
using extensions::Extension;
using extensions::ExtensionDownloader;
using extensions::ExtensionDownloaderDelegate;
using extensions::ExtensionIdSet;
using extensions::ExtensionInfo;
using extensions::ExtensionRegistry;
......@@ -119,13 +114,6 @@ namespace {
// Wait this many seconds after an extensions becomes idle before updating it.
const int kUpdateIdleDelay = 5;
scoped_ptr<IdentityProvider> CreateWebstoreIdentityProvider(Profile* profile) {
return make_scoped_ptr<IdentityProvider>(new ProfileIdentityProvider(
SigninManagerFactory::GetForProfile(profile),
ProfileOAuth2TokenServiceFactory::GetForProfile(profile),
LoginUIServiceFactory::GetForProfile(profile)));
}
} // namespace
// ExtensionService.
......@@ -319,8 +307,8 @@ ExtensionService::ExtensionService(Profile* profile,
profile,
update_frequency,
extensions::ExtensionCache::GetInstance(),
base::Bind(&ExtensionService::CreateExtensionDownloader,
base::Unretained(this))));
base::Bind(ChromeExtensionDownloaderFactory::CreateForProfile,
profile)));
}
component_loader_.reset(
......@@ -584,18 +572,6 @@ bool ExtensionService::UpdateExtension(const std::string& id,
return true;
}
scoped_ptr<ExtensionDownloader> ExtensionService::CreateExtensionDownloader(
ExtensionDownloaderDelegate* delegate) {
scoped_ptr<ExtensionDownloader> downloader;
scoped_ptr<IdentityProvider> identity_provider =
CreateWebstoreIdentityProvider(profile_);
downloader.reset(new ExtensionDownloader(
delegate,
profile_->GetRequestContext()));
downloader->SetWebstoreIdentityProvider(identity_provider.Pass());
return downloader.Pass();
}
void ExtensionService::ReloadExtensionImpl(
// "transient" because the process of reloading may cause the reference
// to become invalid. Instead, use |extension_id|, a copy.
......
......@@ -31,7 +31,6 @@
#include "chrome/browser/extensions/shared_user_script_master.h"
#include "chrome/browser/extensions/state_store_notification_observer.h"
#include "chrome/browser/extensions/unpacked_installer.h"
#include "chrome/browser/extensions/updater/manifest_fetch_data.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/ui/webui/extensions/extension_icon_source.h"
......
// Copyright 2014 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/extensions/updater/chrome_extension_downloader_factory.h"
#include "chrome/browser/extensions/updater/extension_downloader.h"
#include "chrome/browser/google/google_brand.h"
#include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/signin/profile_identity_provider.h"
#include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
#include "chrome/browser/signin/signin_manager_factory.h"
#include "chrome/browser/ui/webui/signin/login_ui_service_factory.h"
#include "components/omaha_query_params/omaha_query_params.h"
#include "components/signin/core/browser/signin_manager.h"
#include "google_apis/gaia/identity_provider.h"
using extensions::ExtensionDownloader;
using extensions::ExtensionDownloaderDelegate;
using omaha_query_params::OmahaQueryParams;
scoped_ptr<ExtensionDownloader>
ChromeExtensionDownloaderFactory::CreateForRequestContext(
net::URLRequestContextGetter* request_context,
ExtensionDownloaderDelegate* delegate) {
scoped_ptr<ExtensionDownloader> downloader(
new ExtensionDownloader(delegate, request_context));
#if defined(GOOGLE_CHROME_BUILD)
std::string brand;
google_brand::GetBrand(&brand);
if (!brand.empty() && !google_brand::IsOrganic(brand))
downloader->set_brand_code(brand);
#endif // defined(GOOGLE_CHROME_BUILD)
downloader->set_manifest_query_params(
OmahaQueryParams::Get(OmahaQueryParams::CRX));
downloader->set_ping_enabled_domain("google.com");
downloader->set_enable_extra_update_metrics(
ChromeMetricsServiceAccessor::IsMetricsReportingEnabled());
return downloader.Pass();
}
scoped_ptr<ExtensionDownloader>
ChromeExtensionDownloaderFactory::CreateForProfile(
Profile* profile,
ExtensionDownloaderDelegate* delegate) {
scoped_ptr<IdentityProvider> identity_provider(new ProfileIdentityProvider(
SigninManagerFactory::GetForProfile(profile),
ProfileOAuth2TokenServiceFactory::GetForProfile(profile),
LoginUIServiceFactory::GetForProfile(profile)));
scoped_ptr<ExtensionDownloader> downloader =
CreateForRequestContext(profile->GetRequestContext(), delegate);
downloader->SetWebstoreIdentityProvider(identity_provider.Pass());
return downloader.Pass();
}
// Copyright 2014 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.
#ifndef CHROME_BROWSER_EXTENSIONS_UPDATER_CHROME_EXTENSION_DOWNLOADER_FACTORY_H_
#define CHROME_BROWSER_EXTENSIONS_UPDATER_CHROME_EXTENSION_DOWNLOADER_FACTORY_H_
#include "base/memory/scoped_ptr.h"
class Profile;
namespace extensions {
class ExtensionDownloader;
class ExtensionDownloaderDelegate;
}
namespace net {
class URLRequestContextGetter;
}
// This provides a simple static interface for constructing an
// ExtensionDownloader suitable for use from within Chrome.
class ChromeExtensionDownloaderFactory {
public:
// Creates a downloader with the given request context. No profile identity
// is associated with this downloader.
static scoped_ptr<extensions::ExtensionDownloader> CreateForRequestContext(
net::URLRequestContextGetter* request_context,
extensions::ExtensionDownloaderDelegate* delegate);
// Creates a downloader for a given Profile. This downloader will be able
// to authenticate as the signed-in user in the event that it's asked to
// fetch a protected download.
static scoped_ptr<extensions::ExtensionDownloader> CreateForProfile(
Profile* profile,
extensions::ExtensionDownloaderDelegate* delegate);
};
#endif // CHROME_BROWSER_EXTENSIONS_UPDATER_CHROME_EXTENSION_DOWNLOADER_FACTORY_H_
......@@ -187,6 +187,7 @@ ExtensionDownloader::ExtensionDownloader(
base::Bind(&ExtensionDownloader::CreateExtensionFetcher,
base::Unretained(this))),
extension_cache_(NULL),
enable_extra_update_metrics_(false),
weak_ptr_factory_(this) {
DCHECK(delegate_);
DCHECK(request_context_.get());
......@@ -274,9 +275,8 @@ void ExtensionDownloader::StartBlacklistUpdate(
// Note: it is very important that we use the https version of the update
// url here to avoid DNS hijacking of the blacklist, which is not validated
// by a public key signature like .crx files are.
scoped_ptr<ManifestFetchData> blacklist_fetch(
new ManifestFetchData(extension_urls::GetWebstoreUpdateUrl(),
request_id));
scoped_ptr<ManifestFetchData> blacklist_fetch(CreateManifestFetchData(
extension_urls::GetWebstoreUpdateUrl(), request_id));
DCHECK(blacklist_fetch->base_url().SchemeIsSecure());
blacklist_fetch->AddExtension(kBlacklistAppID,
version,
......@@ -353,10 +353,10 @@ bool ExtensionDownloader::AddExtensionData(
std::vector<GURL> update_urls;
update_urls.push_back(update_url);
// If UMA is enabled, also add to ManifestFetchData for the
// If metrics are enabled, also add to ManifestFetchData for the
// webstore update URL.
if (!extension_urls::IsWebstoreUpdateUrl(update_url) &&
ChromeMetricsServiceAccessor::IsMetricsReportingEnabled()) {
enable_extra_update_metrics_) {
update_urls.push_back(extension_urls::GetWebstoreUpdateUrl());
}
......@@ -394,7 +394,7 @@ bool ExtensionDownloader::AddExtensionData(
// Otherwise add a new element to the list, if the list doesn't exist or
// if its last element is already full.
linked_ptr<ManifestFetchData> fetch(
new ManifestFetchData(update_urls[i], request_id));
CreateManifestFetchData(update_urls[i], request_id));
fetches_preparing_[std::make_pair(request_id, update_urls[i])].
push_back(fetch);
added = fetch->AddExtension(id, version.GetString(),
......@@ -948,4 +948,19 @@ void ExtensionDownloader::OnGetTokenFailure(
extension_fetcher_->Start();
}
ManifestFetchData* ExtensionDownloader::CreateManifestFetchData(
const GURL& update_url,
int request_id) {
ManifestFetchData::PingMode ping_mode = ManifestFetchData::NO_PING;
if (update_url.DomainIs(ping_enabled_domain_.c_str())) {
if (enable_extra_update_metrics_) {
ping_mode = ManifestFetchData::PING_WITH_METRICS;
} else {
ping_mode = ManifestFetchData::PING;
}
}
return new ManifestFetchData(
update_url, request_id, brand_code_, manifest_query_params_, ping_mode);
}
} // namespace extensions
......@@ -19,8 +19,8 @@
#include "base/memory/weak_ptr.h"
#include "base/version.h"
#include "chrome/browser/extensions/updater/extension_downloader_delegate.h"
#include "chrome/browser/extensions/updater/manifest_fetch_data.h"
#include "chrome/browser/extensions/updater/request_queue.h"
#include "extensions/browser/updater/manifest_fetch_data.h"
#include "extensions/common/extension.h"
#include "extensions/common/update_manifest.h"
#include "google_apis/gaia/oauth2_token_service.h"
......@@ -100,6 +100,22 @@ class ExtensionDownloader
void SetWebstoreIdentityProvider(
scoped_ptr<IdentityProvider> identity_provider);
void set_brand_code(const std::string& brand_code) {
brand_code_ = brand_code;
}
void set_manifest_query_params(const std::string& params) {
manifest_query_params_ = params;
}
void set_ping_enabled_domain(const std::string& domain) {
ping_enabled_domain_ = domain;
}
void set_enable_extra_update_metrics(bool enable) {
enable_extra_update_metrics_ = enable;
}
// These are needed for unit testing, to help identify the correct mock
// URLFetcher objects.
static const int kManifestFetcherId = 1;
......@@ -246,6 +262,9 @@ class ExtensionDownloader
virtual void OnGetTokenFailure(const OAuth2TokenService::Request* request,
const GoogleServiceAuthError& error) OVERRIDE;
ManifestFetchData* CreateManifestFetchData(const GURL& update_url,
int request_id);
// The delegate that receives the crx files downloaded by the
// ExtensionDownloader, and that fills in optional ping and update url data.
ExtensionDownloaderDelegate* delegate_;
......@@ -290,6 +309,20 @@ class ExtensionDownloader
// A pending token fetch request.
scoped_ptr<OAuth2TokenService::Request> access_token_request_;
// Brand code to include with manifest fetch queries if sending ping data.
std::string brand_code_;
// Baseline parameters to include with manifest fetch queries.
std::string manifest_query_params_;
// Domain to enable ping data. Ping data will be sent with manifest fetches
// to update URLs which match this domain. Defaults to empty (no domain).
std::string ping_enabled_domain_;
// Indicates whether or not extra metrics should be included with ping data.
// Defaults to |false|.
bool enable_extra_update_metrics_;
// Used to create WeakPtrs to |this|.
base::WeakPtrFactory<ExtensionDownloader> weak_ptr_factory_;
......
......@@ -9,7 +9,7 @@
#include <string>
#include "base/time/time.h"
#include "chrome/browser/extensions/updater/manifest_fetch_data.h"
#include "extensions/browser/updater/manifest_fetch_data.h"
class GURL;
......
......@@ -22,10 +22,10 @@
#include "base/timer/timer.h"
#include "chrome/browser/extensions/updater/extension_downloader.h"
#include "chrome/browser/extensions/updater/extension_downloader_delegate.h"
#include "chrome/browser/extensions/updater/manifest_fetch_data.h"
#include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_registrar.h"
#include "extensions/browser/extension_registry_observer.h"
#include "extensions/browser/updater/manifest_fetch_data.h"
#include "url/gurl.h"
class ExtensionServiceInterface;
......
......@@ -9,8 +9,8 @@
#include "base/basictypes.h"
#include "base/callback.h"
#include "base/memory/scoped_ptr.h"
#include "chrome/browser/extensions/updater/manifest_fetch_data.h"
#include "content/public/browser/utility_process_host_client.h"
#include "extensions/browser/updater/manifest_fetch_data.h"
#include "extensions/common/update_manifest.h"
namespace extensions {
......
......@@ -13,6 +13,7 @@
#include "chrome/browser/metrics/metrics_reporting_state.h"
#include "chrome/browser/metrics/metrics_service_accessor.h"
class ChromeExtensionDownloaderFactory;
class PrefService;
class Profile;
......@@ -28,8 +29,6 @@ void RegisterSwReporterComponent(ComponentUpdateService* cus,
}
namespace extensions {
class ExtensionDownloader;
class ManifestFetchData;
class MetricsPrivateGetIsCrashReportingEnabledFunction;
}
......@@ -54,11 +53,10 @@ class ChromeMetricsServiceAccessor : public MetricsServiceAccessor {
component_updater::ComponentUpdateService* cus,
PrefService* prefs);
friend bool prerender::IsOmniboxEnabled(Profile* profile);
friend class ChromeExtensionDownloaderFactory;
friend class ChromeRenderMessageFilter;
friend class ::CrashesDOMHandler;
friend class DataReductionProxyChromeSettings;
friend class extensions::ExtensionDownloader;
friend class extensions::ManifestFetchData;
friend class extensions::MetricsPrivateGetIsCrashReportingEnabledFunction;
friend class ::FlashDOMHandler;
friend class system_logs::ChromeInternalLogSource;
......
......@@ -795,6 +795,8 @@
'browser/extensions/token_cache/token_cache_service_factory.h',
'browser/extensions/unpacked_installer.cc',
'browser/extensions/unpacked_installer.h',
'browser/extensions/updater/chrome_extension_downloader_factory.cc',
'browser/extensions/updater/chrome_extension_downloader_factory.h',
'browser/extensions/updater/extension_cache.cc',
'browser/extensions/updater/extension_cache.h',
'browser/extensions/updater/extension_downloader.cc',
......@@ -803,8 +805,6 @@
'browser/extensions/updater/extension_downloader_delegate.h',
'browser/extensions/updater/extension_updater.cc',
'browser/extensions/updater/extension_updater.h',
'browser/extensions/updater/manifest_fetch_data.cc',
'browser/extensions/updater/manifest_fetch_data.h',
'browser/extensions/updater/request_queue.h',
'browser/extensions/updater/request_queue_impl.h',
'browser/extensions/updater/safe_manifest_parser.cc',
......
......@@ -394,6 +394,8 @@ source_set("browser") {
"suggest_permission_util.h",
"uninstall_reason.h",
"update_observer.h",
"updater/manifest_fetch_data.cc",
"updater/manifest_fetch_data.h",
"url_request_util.cc",
"url_request_util.h",
"value_store/leveldb_value_store.cc",
......
......@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "chrome/browser/extensions/updater/manifest_fetch_data.h"
#include "extensions/browser/updater/manifest_fetch_data.h"
#include <vector>
......@@ -10,9 +10,7 @@
#include "base/metrics/histogram.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_util.h"
#include "chrome/browser/google/google_brand.h"
#include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
#include "components/omaha_query_params/omaha_query_params.h"
#include "base/strings/stringprintf.h"
#include "net/base/escape.h"
namespace {
......@@ -25,13 +23,18 @@ const int kExtensionsManifestMaxURLSize = 2000;
namespace extensions {
ManifestFetchData::ManifestFetchData(const GURL& update_url, int request_id)
ManifestFetchData::ManifestFetchData(const GURL& update_url,
int request_id,
const std::string& brand_code,
const std::string& base_query_params,
PingMode ping_mode)
: base_url_(update_url),
full_url_(update_url) {
std::string query = full_url_.has_query() ?
full_url_.query() + "&" : std::string();
query += omaha_query_params::OmahaQueryParams::Get(
omaha_query_params::OmahaQueryParams::CRX);
full_url_(update_url),
brand_code_(brand_code),
ping_mode_(ping_mode) {
std::string query =
full_url_.has_query() ? full_url_.query() + "&" : std::string();
query += base_query_params;
GURL::Replacements replacements;
replacements.SetQueryStr(query);
full_url_ = full_url_.ReplaceComponents(replacements);
......@@ -39,7 +42,8 @@ ManifestFetchData::ManifestFetchData(const GURL& update_url, int request_id)
request_ids_.insert(request_id);
}
ManifestFetchData::~ManifestFetchData() {}
ManifestFetchData::~ManifestFetchData() {
}
// The format for request parameters in update checks is:
//
......@@ -49,7 +53,7 @@ ManifestFetchData::~ManifestFetchData() {}
//
// id=EXTENSION_ID&v=VERSION&uc
//
// Additionally, we may include the parameter ping=PING_DATA where PING_DATA
// Provide ping data with the parameter ping=PING_DATA where PING_DATA
// looks like r=DAYS or a=DAYS for extensions in the Chrome extensions gallery.
// ('r' refers to 'roll call' ie installation, and 'a' refers to 'active').
// These values will each be present at most once every 24 hours, and indicate
......@@ -97,22 +101,17 @@ bool ManifestFetchData::AddExtension(const std::string& id,
}
// Append brand code, rollcall and active ping parameters.
if (base_url_.DomainIs("google.com")) {
#if defined(GOOGLE_CHROME_BUILD)
std::string brand;
google_brand::GetBrand(&brand);
if (!brand.empty() && !google_brand::IsOrganic(brand))
parts.push_back("brand=" + brand);
#endif
if (ping_mode_ != NO_PING) {
if (!brand_code_.empty())
parts.push_back(base::StringPrintf("brand=%s", brand_code_.c_str()));
std::string ping_value;
pings_[id] = PingData(0, 0, false);
if (ping_data) {
if (ping_data->rollcall_days == kNeverPinged ||
ping_data->rollcall_days > 0) {
ping_value += "r=" + base::IntToString(ping_data->rollcall_days);
if (ChromeMetricsServiceAccessor::IsMetricsReportingEnabled()) {
if (ping_mode_ == PING_WITH_METRICS) {
ping_value += "&e=" + std::string(ping_data->is_enabled ? "1" : "0");
}
pings_[id].rollcall_days = ping_data->rollcall_days;
......
......@@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef CHROME_BROWSER_EXTENSIONS_UPDATER_MANIFEST_FETCH_DATA_H_
#define CHROME_BROWSER_EXTENSIONS_UPDATER_MANIFEST_FETCH_DATA_H_
#ifndef EXTENSIONS_BROWSER_UPDATER_MANIFEST_FETCH_DATA_H_
#define EXTENSIONS_BROWSER_UPDATER_MANIFEST_FETCH_DATA_H_
#include <map>
#include <set>
......@@ -22,6 +22,18 @@ class ManifestFetchData {
public:
static const int kNeverPinged = -1;
// What ping mode this fetch should use.
enum PingMode {
// No ping, no extra metrics.
NO_PING,
// Ping without extra metrics.
PING,
// Ping with extra metrics.
PING_WITH_METRICS,
};
// Each ping type is sent at most once per day.
enum PingType {
// Used for counting total installs of an extension/app/theme.
......@@ -46,7 +58,11 @@ class ManifestFetchData {
: rollcall_days(rollcall), active_days(active), is_enabled(enabled) {}
};
ManifestFetchData(const GURL& update_url, int request_id);
ManifestFetchData(const GURL& update_url,
int request_id,
const std::string& brand_code,
const std::string& base_query_params,
PingMode ping_mode);
~ManifestFetchData();
// Returns true if this extension information was successfully added. If the
......@@ -100,6 +116,14 @@ class ManifestFetchData {
// one ManifestFetchData.
std::set<int> request_ids_;
// The brand code to include with manifest fetch queries, if non-empty and
// |ping_mode_| >= PING.
const std::string brand_code_;
// The ping mode for this fetch. This determines whether or not ping data
// (and possibly extra metrics) will be included in the fetch query.
const PingMode ping_mode_;
// The set of extension IDs for which this fetch forced a CRX update.
std::set<std::string> forced_updates_;
......@@ -108,4 +132,4 @@ class ManifestFetchData {
} // namespace extensions
#endif // CHROME_BROWSER_EXTENSIONS_UPDATER_MANIFEST_FETCH_DATA_H_
#endif // EXTENSIONS_BROWSER_UPDATER_MANIFEST_FETCH_DATA_H_
......@@ -667,6 +667,8 @@
'browser/suggest_permission_util.h',
'browser/uninstall_reason.h',
'browser/update_observer.h',
'browser/updater/manifest_fetch_data.cc',
'browser/updater/manifest_fetch_data.h',
'browser/url_request_util.cc',
'browser/url_request_util.h',
'browser/value_store/leveldb_value_store.cc',
......
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