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

Unified Diff: chrome/browser/safe_browsing/incident_reporting/download_metadata_manager.cc

Issue 703463006: Fix DCHECK when shutting down safe browsing DownloadMetadataManager. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 6 years, 1 month 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: chrome/browser/safe_browsing/incident_reporting/download_metadata_manager.cc
diff --git a/chrome/browser/safe_browsing/incident_reporting/download_metadata_manager.cc b/chrome/browser/safe_browsing/incident_reporting/download_metadata_manager.cc
index 431bd4331f03acf4dc687f9b533e304ee27eb45d..c4b184f0797d36cbf4beaf5ad5fd3224767f7f90 100644
--- a/chrome/browser/safe_browsing/incident_reporting/download_metadata_manager.cc
+++ b/chrome/browser/safe_browsing/incident_reporting/download_metadata_manager.cc
@@ -293,8 +293,12 @@ DownloadMetadataManager::DownloadMetadataManager(
}
DownloadMetadataManager::~DownloadMetadataManager() {
- // All download managers must have gone down prior to this.
- DCHECK(contexts_.empty());
+ // Destruction may have taken place before managers have gone down.
+ for (const auto& value : contexts_) {
+ value.first->RemoveObserver(this);
+ value.second->Detach();
+ }
+ contexts_.clear();
}
void DownloadMetadataManager::AddDownloadManager(
@@ -505,11 +509,17 @@ void DownloadMetadataManager::ManagerContext::OnDownloadDestroyed(
}
DownloadMetadataManager::ManagerContext::~ManagerContext() {
- // All downloads must have been destroyed prior to deleting the context and
- // all recorded operations and callbacks must have been handled.
+ // A context should not be deleted while waiting for a load to complete.
DCHECK(pending_items_.empty());
- DCHECK_EQ(item_, static_cast<content::DownloadItem*>(nullptr));
DCHECK(get_details_callbacks_.empty());
+
+ // The context may have detached while still observing the item of interest
+ // since a DownloadManager announces that it's going down before it destroyes
+ // its items.
+ if (item_) {
+ item_->RemoveObserver(this);
+ item_ = nullptr;
+ }
}
void DownloadMetadataManager::ManagerContext::ClearPendingItems() {

Powered by Google App Engine
This is Rietveld 408576698