Added AddDownload communication path and observers

chrome/browser/background_fetch/background_fetch_client_impl.cc

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <sstream>
 #include <string>
 
 #include "chrome/browser/background_fetch/background_fetch_client_impl.h"
 
 #include "chrome/browser/offline_items_collection/offline_content_aggregator_factory.h"
@@ skipping 42 matching lines @@
   aggregator->UnregisterProvider(namespace_);
 
   // If the profile is an incognito profile, clean up all data.
   if (delegate_ && namespace_ == kBackgroundFetchNamespace)
     delegate_->CleanupAllTasks();
 }
 
 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) {
   DCHECK_EQ(content_id.name_space, namespace_);
 
   if (!delegate_)
     return;
 
   delegate_->CancelDownload(content_id.id);
 }
@@ skipping 22 matching lines @@
 }
 
 void BackgroundFetchClientImpl::GetVisualsForItem(
     const ContentId& id,
     const VisualsCallback& callback) {
   // TODO(harkness): Update with icons.
   callback.Run(id, nullptr);
 }
 
 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.

 }
 
 void BackgroundFetchClientImpl::RemoveItem(const ContentId& content_id) {
   // Not applicable for Background Fetch, since offline items don't exist in the
   // offline content system beyond the duration of the download.
 }
 
 const OfflineItem* BackgroundFetchClientImpl::GetItemById(
     const ContentId& content_id) {
   // TODO(harkness): Follow up with dtrainor about what action to take for this.
   return nullptr;
 }
 
 OfflineContentProvider::OfflineItemList
 BackgroundFetchClientImpl::GetAllItems() {
   // TODO(harkness): Follow up with dtrainor about what action to take for this.
   return OfflineContentProvider::OfflineItemList();
 }