Commit 0816757e authored by Evan Stade's avatar Evan Stade Committed by Commit Bot

[Chrome OS] Fixes for download shutdown path.

a) if a dangerous download is cancelled, don't try to update it (with
a potentially half-destroyed profile). Like other notification types,
just close and return.

b) make the download service depend on the NotificationDisplayService.

Bug: 810302, 810738, 578868
Change-Id: Ibc3080510e128954e233485606d8de520b72a97c
Reviewed-on: https://chromium-review.googlesource.com/956087
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: default avatarDavid Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542010}
parent ade9f572
......@@ -6,6 +6,7 @@
#include "chrome/browser/download/download_core_service_impl.h"
#include "chrome/browser/history/history_service_factory.h"
#include "chrome/browser/notifications/notification_display_service_factory.h"
#include "chrome/browser/profiles/incognito_helpers.h"
#include "chrome/browser/profiles/profile.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h"
......@@ -27,6 +28,7 @@ DownloadCoreServiceFactory::DownloadCoreServiceFactory()
"DownloadCoreService",
BrowserContextDependencyManager::GetInstance()) {
DependsOn(HistoryServiceFactory::GetInstance());
DependsOn(NotificationDisplayServiceFactory::GetInstance());
}
DownloadCoreServiceFactory::~DownloadCoreServiceFactory() {}
......
......@@ -321,6 +321,9 @@ std::string DownloadItemNotification::GetNotificationId() const {
}
void DownloadItemNotification::CloseNotification() {
if (closed_)
return;
NotificationDisplayServiceFactory::GetForProfile(profile())->Close(
NotificationHandler::Type::TRANSIENT, GetNotificationId());
}
......@@ -348,6 +351,17 @@ void DownloadItemNotification::UpdateNotificationData(bool display,
bool bump_priority) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
if (item_->GetState() == download::DownloadItem::CANCELLED) {
// Confirms that a download is cancelled by user action.
DCHECK(item_->GetLastReason() ==
download::DOWNLOAD_INTERRUPT_REASON_USER_CANCELED ||
item_->GetLastReason() ==
download::DOWNLOAD_INTERRUPT_REASON_USER_SHUTDOWN);
CloseNotification();
return;
}
DownloadItemModel model(item_);
DownloadCommands command(item_);
......@@ -382,14 +396,9 @@ void DownloadItemNotification::UpdateNotificationData(bool display,
notification_->set_progress(100);
break;
case download::DownloadItem::CANCELLED:
// Confirms that a download is cancelled by user action.
DCHECK(item_->GetLastReason() ==
download::DOWNLOAD_INTERRUPT_REASON_USER_CANCELED ||
item_->GetLastReason() ==
download::DOWNLOAD_INTERRUPT_REASON_USER_SHUTDOWN);
CloseNotification();
return; // Skips the remaining since the notification has closed.
// Handled above.
NOTREACHED();
return;
case download::DownloadItem::INTERRUPTED:
// Shows a notifiation as progress type once so the visible content will
// be updated. (same as the case of type = COMPLETE)
......
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