Commit ecebf633 authored by Min Qin's avatar Min Qin Committed by Commit Bot

AndroidTelemtryService should not create DownloadManagerImpl

Creating DownloadManagerImpl on startup is very costly as it loads
a lot of downloads. So BrowserContext::GetDownloadManager() should
not be called in classes like AndroidTelemetryService, which is
created in browser startup.
This CL fixes the behavior by making AndroidTelemetryService observe
SimpleDownloadManagerCoordinator. The coordinator class is a wrapper
of the DownloadManagerImpl and will not load any history downloads
on startup.

BUG=1003160

Change-Id: Iab9a42071821e01d4f70c779113cf23a2fb534f9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1799684Reviewed-by: default avatarDaniel Rubery <drubery@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#695868}
parent a763080e
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "base/task/post_task.h" #include "base/task/post_task.h"
#include "chrome/browser/download/simple_download_manager_coordinator_factory.h"
#include "chrome/browser/history/history_service_factory.h" #include "chrome/browser/history/history_service_factory.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/safe_browsing/safe_browsing_navigation_observer_manager.h" #include "chrome/browser/safe_browsing/safe_browsing_navigation_observer_manager.h"
...@@ -78,31 +79,29 @@ AndroidTelemetryService::AndroidTelemetryService( ...@@ -78,31 +79,29 @@ AndroidTelemetryService::AndroidTelemetryService(
DCHECK(profile_); DCHECK(profile_);
DCHECK(sb_service_); DCHECK(sb_service_);
content::DownloadManager* download_manager = download::SimpleDownloadManagerCoordinator* coordinator =
BrowserContext::GetDownloadManager(profile_); SimpleDownloadManagerCoordinatorFactory::GetForKey(
if (download_manager) { profile_->GetProfileKey());
// Look for new downloads being created. coordinator->AddObserver(this);
download_manager->AddObserver(this);
}
} }
AndroidTelemetryService::~AndroidTelemetryService() { AndroidTelemetryService::~AndroidTelemetryService() {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
content::DownloadManager* download_manager = download::SimpleDownloadManagerCoordinator* coordinator =
BrowserContext::GetDownloadManager(profile_); SimpleDownloadManagerCoordinatorFactory::GetForKey(
if (download_manager) { profile_->GetProfileKey());
download_manager->RemoveObserver(this); coordinator->RemoveObserver(this);
}
} }
void AndroidTelemetryService::OnDownloadCreated( void AndroidTelemetryService::OnDownloadCreated(
content::DownloadManager* manager,
download::DownloadItem* item) { download::DownloadItem* item) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
if (!BrowserContext::GetDownloadManager(profile_)->IsManagerInitialized()) { download::SimpleDownloadManagerCoordinator* coordinator =
SimpleDownloadManagerCoordinatorFactory::GetForKey(
profile_->GetProfileKey());
if (!coordinator->has_all_history_downloads())
return; return;
}
if (!CanSendPing(item)) { if (!CanSendPing(item)) {
return; return;
......
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "chrome/browser/safe_browsing/telemetry/telemetry_service.h" #include "chrome/browser/safe_browsing/telemetry/telemetry_service.h"
#include "components/download/public/common/download_item.h" #include "components/download/public/common/download_item.h"
#include "components/download/public/common/simple_download_manager_coordinator.h"
#include "components/safe_browsing/proto/csd.pb.h" #include "components/safe_browsing/proto/csd.pb.h"
#include "content/public/browser/download_manager.h" #include "content/public/browser/download_manager.h"
...@@ -57,20 +58,18 @@ enum class ApkDownloadTelemetryOutcome { ...@@ -57,20 +58,18 @@ enum class ApkDownloadTelemetryOutcome {
// the perspective of this class: // the perspective of this class:
// 1. Downloading off-market APKs. See: go/zurkon-v1-referrer-dd // 1. Downloading off-market APKs. See: go/zurkon-v1-referrer-dd
class AndroidTelemetryService : public download::DownloadItem::Observer, class AndroidTelemetryService
public content::DownloadManager::Observer, : public download::DownloadItem::Observer,
public TelemetryService { public download::SimpleDownloadManagerCoordinator::Observer,
public TelemetryService {
public: public:
AndroidTelemetryService(SafeBrowsingService* sb_service, Profile* profile); AndroidTelemetryService(SafeBrowsingService* sb_service, Profile* profile);
~AndroidTelemetryService() override; ~AndroidTelemetryService() override;
// content::DownloadManager::Observer. // download::SimpleDownloadManagerCoordinator::Observer.
// Called when a new download is created. void OnDownloadCreated(download::DownloadItem* item) override;
void OnDownloadCreated(content::DownloadManager* manager,
download::DownloadItem* item) override;
// download::DownloadItem::Observer // download::DownloadItem::Observer.
// Called when the state of a download item changes.
void OnDownloadUpdated(download::DownloadItem* download) override; void OnDownloadUpdated(download::DownloadItem* download) override;
private: private:
......
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