Added AddDownload communication path and observers

chrome/browser/background_fetch/background_fetch_client_impl.cc

Index: chrome/browser/background_fetch/background_fetch_client_impl.cc
diff --git a/chrome/browser/background_fetch/background_fetch_client_impl.cc b/chrome/browser/background_fetch/background_fetch_client_impl.cc
index ae1cef10842c192251d0a538b4c239882a75446b..101fa58868e55f6de9f27f4a6f182acb48390c5e 100644
--- a/chrome/browser/background_fetch/background_fetch_client_impl.cc
+++ b/chrome/browser/background_fetch/background_fetch_client_impl.cc
@@ -60,6 +60,30 @@ void BackgroundFetchClientImpl::Shutdown() {
 void BackgroundFetchClientImpl::SetDelegate(
     content::BackgroundFetchClient::Delegate* delegate) {
   delegate_ = delegate;
+
+  // If there are any previously registered observers, notify them now.
+  for (auto& observer : observers_)
+    observer.OnItemsAvailable(this);
+}
+
+void BackgroundFetchClientImpl::AddDownload(const std::string& registration_id,
+                                            const std::string& title,
+                                            int64_t total_download_size) {
+  // Construct an OfflineItem to pass to the Observers.
+  offline_items_collection::OfflineItem item(

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

nit: drop offline_items_collection:: (you have a `using` statement above)

[harkness, 2017/04/21 14:55:20]

Done.

+      ContentId(namespace_, registration_id));
+  item.title = title;
+  item.is_transient = true;
+  item.is_openable = false;
+  item.is_resumable = true;
+  item.percent_completed = 0;
+  item.total_size_bytes = total_download_size;

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

Should we set page_url or original_url for the origin attribution?

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

Need to set |is_off_the_record|?

[harkness, 2017/04/21 14:55:19]

Done.

[harkness, 2017/04/21 14:55:19]

hmm, I thought I had put a TODO in here to do a more thorough construction, but
I'll just go ahead and do the ones you pointed out and leave TODOs for
individual ones I'm not sure how to set.

+
+  offline_items_collection::OfflineContentProvider::OfflineItemList added_items;

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

nit: drop offline_items_collection:: (you have a `using` statement above)

[harkness, 2017/04/21 14:55:19]

Done.

+  added_items.push_back(item);
+
+  for (auto& observer : observers_)
+    observer.OnItemsAdded(added_items);
 }
 
 void BackgroundFetchClientImpl::CancelDownload(const ContentId& content_id) {
@@ -102,14 +126,30 @@ void BackgroundFetchClientImpl::GetVisualsForItem(
 }
 
 void BackgroundFetchClientImpl::AddObserver(Observer* observer) {
-  // TODO(harkness): Add a path for the OfflineContentProvider to observe
-  // changes in available BackgroundFetch items.
+  DCHECK(observer);
+  if (observers_.HasObserver(observer))
+    return;
+
+  observers_.AddObserver(observer);
+
+  // If the Delegate has been set then it is assumed to be ready for queries.
+  if (delegate_)
+    observer->OnItemsAvailable(this);
 }
 
-void BackgroundFetchClientImpl::RemoveObserver(Observer* observer) {}
+void BackgroundFetchClientImpl::RemoveObserver(Observer* observer) {
+  DCHECK(observer);
+  if (!observers_.HasObserver(observer))
+    return;
+
+  observers_.RemoveObserver(observer);

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

No need to check HasObserver() first, it's safe to remove observers not in the
list.

[harkness, 2017/04/21 14:55:19]

Done.

[Peter Beverloo, 2017/04/24 14:39:14]

Doesn't seem to be done?

+}
 
 bool BackgroundFetchClientImpl::AreItemsAvailable() {
-  // TODO(harkness): Follow up with dtrainor about what action to take for this.
+  // If there is a delegate, it is assumed to be ready for query.
+  if (delegate_)
+    return true;
+
   return false;

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

return delegate_;

[harkness, 2017/04/21 14:55:19]

Done.

 }