Added AddDownload communication path and observers

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 19eaa574efe84bda5c98a87a569c7a2172fb004a..1ab86a3703e036c3c57329de6f069e69acf815f3 100644
--- a/components/offline_items_collection/core/offline_content_aggregator.cc
+++ b/components/offline_items_collection/core/offline_content_aggregator.cc
@@ -42,10 +42,14 @@ void OfflineContentAggregator::RegisterProvider(
 
   // Only set up the connection to the provider if the provider isn't associated
   // with any other namespace.
+  bool need_add_observer = false;

[harkness, 2017/04/20 12:25:16]

dtrainor: This is ugly, but I threw it in there to get the tests to not crash. 

Should the OfflineContentAggregator be able to support a synchronous
OnItemsAvailable call which is triggered from the AddObserver? I saw that in the
OfflineContentAggregator's AddObserver, you're doing an async
CheckAndNotifyItemsAvailable() call, but for the BFCI, we're making the
simplifying assumption that as long as the delegate is set, it's ready, so we
shouldn't need the call to be async.

[Peter Beverloo, 2017/04/20 16:27:55]

There's a race condition there. If the OCA would send us resume/click/other
events before we're done initializing, they might be dropped on the floor. (I
presume the OCA doesn't queue events until a provider is available.)

Perhaps we should:
    a) Always register the delegate..
    b) Have //content send us an ContentAvailable signal of sorts when stuff is
initialized. This would call OnItemsAvailable.
    c) Queue events in the BGFetchClientImpl and only send them to the delegate
when the signal has been received.

That said, I agree that this is a footgun in the OCA API and would still suggest
this fix :).

[David Trainor- moved to gerrit, 2017/04/25 03:24:42]

It actually does queue them up and flush them once you signal you're ready! :D
Making it synchronous actually has more of an impact on the caller than on
anything else.  I wanted to try to have a consistent behavior for how the API
would work for anyone trying to register an observer.  Ideally it'd just all be
asynchronous since we need to support that anyway.  If we do this, could you add
a TODO in the bgfetch client code to make it asynchronous when possible?  I
think you can probably get away with it for now without causing issues.

Do you immediately call OnItemAdded right after you build and register your
delegate?

   if (!MapContainsValue(providers_, provider))
-    provider->AddObserver(this);
+    need_add_observer = true;
 
   providers_[name_space] = provider;
+
+  if (need_add_observer)
+    provider->AddObserver(this);
 }
 
 void OfflineContentAggregator::UnregisterProvider(