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

Unified Diff: components/offline_items_collection/core/offline_content_aggregator.cc

Issue 2757773002: Support providers with two namespaces (Closed)
Patch Set: Address comments 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: components/offline_items_collection/core/offline_content_aggregator.cc
diff --git a/components/offline_items_collection/core/offline_content_aggregator.cc b/components/offline_items_collection/core/offline_content_aggregator.cc
index b856247ff88a4933217b644f62c5371a39031075..acbfd5a130b4ed2daf7b2eeb38d6e895071695ef 100644
--- a/components/offline_items_collection/core/offline_content_aggregator.cc
+++ b/components/offline_items_collection/core/offline_content_aggregator.cc
@@ -39,19 +39,28 @@ void OfflineContentAggregator::RegisterProvider(
// Validate that this is the first OfflineContentProvider registered that is
// associated with |name_space|.
DCHECK(providers_.find(name_space) == providers_.end());
- DCHECK(pending_actions_.find(provider) == pending_actions_.end());
+
+ // Only set up the connection to the provider if the provider isn't associated
+ // with any other namespace.
+ if (!MapContainsValue(providers_, provider))
+ provider->AddObserver(this);
providers_[name_space] = provider;
- provider->AddObserver(this);
}
void OfflineContentAggregator::UnregisterProvider(
const std::string& name_space) {
- auto it = providers_.find(name_space);
+ auto provider_it = providers_.find(name_space);
+
+ OfflineContentProvider* provider = provider_it->second;
+ providers_.erase(provider_it);
- it->second->RemoveObserver(this);
- pending_actions_.erase(it->second);
- providers_.erase(it);
+ // Only clean up the connection to the provider if the provider isn't
+ // associated with any other namespace.
+ if (!MapContainsValue(providers_, provider)) {
+ provider->RemoveObserver(this);
+ pending_actions_.erase(provider);
+ }
}
bool OfflineContentAggregator::AreItemsAvailable() {
@@ -119,12 +128,17 @@ const OfflineItem* OfflineContentAggregator::GetItemById(const ContentId& id) {
OfflineContentProvider::OfflineItemList
OfflineContentAggregator::GetAllItems() {
+ // Create a set of unique providers to iterate over.
+ std::set<OfflineContentProvider*> providers;
+ for (auto provider_it : providers_)
+ providers.insert(provider_it.second);
+
OfflineItemList items;
- for (auto& it : providers_) {
- if (!it.second->AreItemsAvailable())
+ for (auto* provider : providers) {
+ if (!provider->AreItemsAvailable())
continue;
- OfflineItemList provider_items = it.second->GetAllItems();
+ OfflineItemList provider_items = provider->GetAllItems();
items.insert(items.end(), provider_items.begin(), provider_items.end());
}

Powered by Google App Engine
This is Rietveld 408576698