Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1103)

Unified Diff: content/browser/background_fetch/background_fetch_job_controller.cc

Issue 2727253002: Added DownloadItem::Observer to JobController. (Closed)
Patch Set: Added TODO Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: content/browser/background_fetch/background_fetch_job_controller.cc
diff --git a/content/browser/background_fetch/background_fetch_job_controller.cc b/content/browser/background_fetch/background_fetch_job_controller.cc
index 51712c74b24189258ffdc5cf3f6b8f09ef0d6593..e6b96f72d26484649f7d52fc3534b570d5947c01 100644
--- a/content/browser/background_fetch/background_fetch_job_controller.cc
+++ b/content/browser/background_fetch/background_fetch_job_controller.cc
@@ -24,15 +24,27 @@ BackgroundFetchJobController::BackgroundFetchJobController(
std::unique_ptr<BackgroundFetchJobData> job_data)
: browser_context_(browser_context),
storage_partition_(storage_partition),
- job_data_(std::move(job_data)) {}
+ job_data_(std::move(job_data)),
+ weak_ptr_factory_(this) {}
BackgroundFetchJobController::~BackgroundFetchJobController() = default;
void BackgroundFetchJobController::Shutdown() {
- // TODO(harkness): Remove any observers.
+ DownloadManager* manager =
+ BrowserContext::GetDownloadManager(browser_context_);
+ // For any downloads in progress, remove the observer.
+ for (auto& guid_map_entry : download_guid_map_) {
+ DownloadItem* item = manager->GetDownloadByGuid(guid_map_entry.first);
+ // TODO(harkness): Follow up with DownloadManager team about whether this
+ // should be a DCHECK.
+ if (item)
+ item->RemoveObserver(this);
+ }
+ download_guid_map_.clear();
+
// TODO(harkness): Write final status to the DataManager. After this call, the
// |job_data_| is no longer valid.
- job_data_.reset(nullptr);
+ job_data_.reset();
}
void BackgroundFetchJobController::StartProcessing() {
@@ -42,22 +54,71 @@ void BackgroundFetchJobController::StartProcessing() {
job_data_->GetNextBackgroundFetchRequestInfo();
ProcessRequest(fetch_request);
- // Currently this only supports jobs of size 1.
- // TODO(crbug.com/692544)
- DCHECK(!job_data_->HasRequestsRemaining());
+ // Currently, this processes a single request at a time.
+}
+
+void BackgroundFetchJobController::DownloadStarted(
+ const std::string& request_guid,
+ DownloadItem* item,
+ DownloadInterruptReason reason) {
+ // Start observing the DownloadItem. No need to store the pointer because it
+ // can be retrieved from the DownloadManager.
+ item->AddObserver(this);
+ download_guid_map_[item->GetGuid()] = request_guid;
+ job_data_->SetRequestDownloadGuid(request_guid, item->GetGuid());
+}
+
+void BackgroundFetchJobController::OnDownloadUpdated(DownloadItem* item) {
+ DCHECK(download_guid_map_.find(item->GetGuid()) != download_guid_map_.end());
+
+ bool requests_remaining = false;
+ switch (item->GetState()) {
+ case DownloadItem::DownloadState::CANCELLED:
+ // TODO(harkness): Expand the "complete" flag on the RequestInfo to
+ // include error states.
+ case DownloadItem::DownloadState::COMPLETE:
+ requests_remaining = job_data_->BackgroundFetchRequestInfoComplete(
+ download_guid_map_[item->GetGuid()]);
+ // TODO(harkness): Tell the notification service to update the download
+ // notification.
+
+ if (requests_remaining) {
+ ProcessRequest(job_data_->GetNextBackgroundFetchRequestInfo());
+ } else if (job_data_->IsComplete()) {
+ // TODO(harkness): Return the data to the context.
+ }
+ break;
+ case DownloadItem::DownloadState::INTERRUPTED:
+ // TODO(harkness): Just update the notification that it is paused.
+ break;
+ case DownloadItem::DownloadState::IN_PROGRESS:
+ // TODO(harkness): If the download was previously paused, this should now
+ // unpause the notification.
+ break;
+ case DownloadItem::DownloadState::MAX_DOWNLOAD_STATE:
+ // TODO(harkness): Consider how we want to handle errors here.
+ break;
+ }
+}
+
+void BackgroundFetchJobController::OnDownloadDestroyed(DownloadItem* item) {
+ // This can be called as part of Shutdown. Make sure the observers are
+ // removed.
+ item->RemoveObserver(this);
}
void BackgroundFetchJobController::ProcessRequest(
const BackgroundFetchRequestInfo& fetch_request) {
- // TODO(harkness): Start the observer watching for this download.
// TODO(harkness): Check if the download is already in progress or completed.
- // Send the fetch request to the DownloadManager.
+ // Set up the download parameters and the OnStartedCallback.
std::unique_ptr<DownloadUrlParameters> params(
base::MakeUnique<DownloadUrlParameters>(
fetch_request.url(), storage_partition_->GetURLRequestContext()));
- // TODO(harkness): Currently this is the only place the browser_context is
- // used. Evaluate whether we can just pass in the download manager explicitly.
+ params->set_callback(
+ base::Bind(&BackgroundFetchJobController::DownloadStarted,
+ weak_ptr_factory_.GetWeakPtr(), fetch_request.guid()));
+
BrowserContext::GetDownloadManager(browser_context_)
->DownloadUrl(std::move(params));
}

Powered by Google App Engine
This is Rietveld 408576698