Support providers with two namespaces

components/offline_items_collection/core/offline_content_aggregator.cc

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..4e4bc425a9aab0607104a7087eb58196d7469923 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 namespaces.

[fgorski, 2017/03/17 04:15:15]

nit: would singular namespace work here?

[David Trainor- moved to gerrit, 2017/03/22 00:12:45]

Done.

+  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 namespaces.
+  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());
   }