Initial checkin of OfflineContentProvider.

components/offline_content/core/offline_content_aggregator.cc

Index: components/offline_content/core/offline_content_aggregator.cc
diff --git a/components/offline_content/core/offline_content_aggregator.cc b/components/offline_content/core/offline_content_aggregator.cc
new file mode 100644
index 0000000000000000000000000000000000000000..f7ea9bae2cbfe65daee8b050ecdfcc22383da650
--- /dev/null
+++ b/components/offline_content/core/offline_content_aggregator.cc
@@ -0,0 +1,257 @@
+// 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 <algorithm>
+#include <string>
+
+#include "base/bind.h"
+#include "base/memory/ptr_util.h"
+#include "base/threading/thread_task_runner_handle.h"
+#include "components/offline_content/core/offline_content_aggregator.h"
+#include "components/offline_content/core/offline_item.h"
+
+namespace offline_content {
+
+namespace {
+
+// Helper method to determine whether or not |observers| contains |observer|.
+bool VectorContains(
+    const std::vector<OfflineContentProvider::Observer*>& observers,
+    OfflineContentProvider::Observer* observer) {
+  const auto& it = std::find(observers.cbegin(), observers.cend(), observer);
+  return it != observers.end();
+}
+
+// Helper method to remove |observer| from |observers|.
+void RemoveFromVector(std::vector<OfflineContentProvider::Observer*>& observers,
+                      OfflineContentProvider::Observer* observer) {
+  auto it = std::find(observers.begin(), observers.end(), observer);
+  if (it != observers.end())
+    observers.erase(it);
+}
+
+}  // namespace
+
+OfflineContentAggregator::OfflineContentAggregator()
+    : sent_on_items_available_(false), weak_ptr_factory_(this) {}
+
+OfflineContentAggregator::~OfflineContentAggregator() = default;
+
+void OfflineContentAggregator::RegisterProvider(
+    const std::string& name_space,
+    OfflineContentProvider* provider) {
+  // 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(name_space) == pending_actions_.end());

[fgorski, 2017/02/14 22:17:55]

did you consider adding: DCHECK(provider); ?

[David Trainor- moved to gerrit, 2017/02/22 01:23:12]

Ah good call done!

+
+  providers_[name_space] = provider;
+  provider->AddObserver(this);
+}
+
+void OfflineContentAggregator::UnregisterProvider(
+    const std::string& name_space) {
+  auto it = providers_.find(name_space);
+  DCHECK(it != providers_.end());
+
+  it->second->RemoveObserver(this);
+
+  providers_.erase(it);
+  pending_actions_.erase(name_space);
+}
+
+bool OfflineContentAggregator::AreItemsAvailable() {
+  return sent_on_items_available_;
+}
+
+void OfflineContentAggregator::OpenItem(const ContentId& id) {
+  auto it = providers_.find(id.name_space);
+
+  if (it == providers_.end())
+    return;
+
+  pending_actions_[id.name_space].push_back(base::Bind(
+      &OfflineContentProvider::OpenItem, base::Unretained(it->second), id));

[fgorski, 2017/02/14 22:17:55]

I feel a little uncomfortable about this one.
Is there a way to do it through a weak_ptr?

I understand the obvious piece (we unregister and remove pending actions).
But what if the provider gets destroy as we process the pending actions, or
something like that?

[David Trainor- moved to gerrit, 2017/02/22 01:23:12]

Ended up checking if the entry was removed from the pending_actions_ map in the
loop.  Felt cleaner than adding a list of WeakPtrFactories, and I was hoping not
to bleed the need for a weak pointer into the interface; If we need them I'd
rather this class maintain the factories since it fully understands the
lifecycle of when the provider is registered.

Let me know if you think this is ok.  If not I can add the list of
WeakPtrFactories.

+  FlushPendingActionsIfReady(it->second);

[fgorski, 2017/02/14 22:17:55]

Did you consider alternative approach:
* run if provider ready
* add to pending actions otherwise
which could be wrapped in RunIfReady

It makes little sense to try and flush from almost every action, as chances that
nothing changed the state in between (otherwise the pending activities queue
would be flushed already).

[David Trainor- moved to gerrit, 2017/02/22 01:23:11]

Yeah that makes sense.  Will change.

+}
+
+void OfflineContentAggregator::RemoveItem(const ContentId& id) {
+  auto it = providers_.find(id.name_space);
+
+  if (it == providers_.end())
+    return;
+
+  pending_actions_[id.name_space].push_back(base::Bind(
+      &OfflineContentProvider::RemoveItem, base::Unretained(it->second), id));
+  FlushPendingActionsIfReady(it->second);
+}
+
+void OfflineContentAggregator::CancelDownload(const ContentId& id) {
+  auto it = providers_.find(id.name_space);
+
+  if (it == providers_.end())
+    return;
+
+  pending_actions_[id.name_space].push_back(
+      base::Bind(&OfflineContentProvider::CancelDownload,
+                 base::Unretained(it->second), id));
+  FlushPendingActionsIfReady(it->second);
+}
+
+void OfflineContentAggregator::PauseDownload(const ContentId& id) {
+  auto it = providers_.find(id.name_space);
+
+  if (it == providers_.end())
+    return;
+
+  pending_actions_[id.name_space].push_back(
+      base::Bind(&OfflineContentProvider::PauseDownload,
+                 base::Unretained(it->second), id));
+  FlushPendingActionsIfReady(it->second);
+}
+
+void OfflineContentAggregator::ResumeDownload(const ContentId& id) {
+  auto it = providers_.find(id.name_space);
+
+  if (it == providers_.end())
+    return;
+
+  pending_actions_[id.name_space].push_back(
+      base::Bind(&OfflineContentProvider::ResumeDownload,
+                 base::Unretained(it->second), id));
+  FlushPendingActionsIfReady(it->second);
+}
+
+OfflineItem* OfflineContentAggregator::GetItemById(const ContentId& id) {

[fgorski, 2017/02/14 22:17:55]

should this be a const OfflineItem*?

[David Trainor- moved to gerrit, 2017/02/22 01:23:12]

Done.

+  auto it = providers_.find(id.name_space);
+
+  if (it == providers_.end() || !it->second->AreItemsAvailable())
+    return nullptr;
+
+  return it->second->GetItemById(id);
+}
+
+OfflineContentProvider::OfflineItemList
+OfflineContentAggregator::GetAllItems() {
+  OfflineItemList items;
+  for (auto& it : providers_) {
+    if (!it.second->AreItemsAvailable())
+      continue;
+
+    OfflineItemList provider_items = it.second->GetAllItems();
+    items.insert(items.end(), provider_items.begin(), provider_items.end());
+  }
+
+  return items;
+}
+
+void OfflineContentAggregator::AddObserver(
+    OfflineContentProvider::Observer* observer) {
+  DCHECK(observer);
+  if (observers_.HasObserver(observer))
+    return;
+
+  observers_.AddObserver(observer);
+
+  if (sent_on_items_available_) {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE,
+        base::Bind(&OfflineContentAggregator::CheckAndNotifyItemsAvailable,
+                   weak_ptr_factory_.GetWeakPtr()));
+  }
+}
+
+void OfflineContentAggregator::RemoveObserver(
+    OfflineContentProvider::Observer* observer) {
+  DCHECK(observer);
+  if (!observers_.HasObserver(observer))
+    return;
+
+  RemoveFromVector(signaled_observers_, observer);
+  observers_.RemoveObserver(observer);
+}
+
+void OfflineContentAggregator::OnItemsAvailable(
+    OfflineContentProvider* provider) {
+  // Flush any pending actions that should be mirrored to the provider.
+  FlushPendingActionsIfReady(provider);

[fgorski, 2017/02/14 22:17:55]

Why did you choose to flush first and then inform observers?
Probably worth documenting with a comment.

[David Trainor- moved to gerrit, 2017/02/22 01:23:12]

Will document in the header.  Let me know if you want me to add something here
as well.  I wanted the data set to reflect the actions take on the data before
consumers tried to query it.

+
+  // Some observers might already be under the impression that this class was
+  // initialized.  Just treat this as an OnItemsAdded and notify those observers
+  // of the new items.
+  if (signaled_observers_.size() > 0) {
+    OfflineItemList items = provider->GetAllItems();
+    if (items.size() > 0) {
+      for (auto& observer : signaled_observers_) {
+        observer->OnItemsAdded(items);
+      }
+    }

[qinmin, 2017/02/14 07:37:20]

can we just early return here? 

[David Trainor- moved to gerrit, 2017/02/22 01:23:12]

No I don't think so :(.  We still need to notify the other observers that this
provider is initialized.

+  }
+
+  // Check if there were any observers who haven't been told that this class is
+  // initialized yet.  If so, notify them now.
+  CheckAndNotifyItemsAvailable();
+}
+
+void OfflineContentAggregator::OnItemsAdded(const OfflineItemList& items) {
+  for (auto& observer : observers_)
+    observer.OnItemsAdded(items);
+}
+
+void OfflineContentAggregator::OnItemRemoved(const ContentId& id) {
+  for (auto& observer : observers_)
+    observer.OnItemRemoved(id);
+}
+
+void OfflineContentAggregator::OnItemUpdated(const OfflineItem& item) {
+  for (auto& observer : observers_)
+    observer.OnItemUpdated(item);
+}
+
+void OfflineContentAggregator::CheckAndNotifyItemsAvailable() {
+  if (providers_.size() == 0)
+    return;
+
+  // If we haven't sent out the initialization message yet, make sure all
+  // underlying OfflineContentProviders are ready before notifying observers
+  // that we're ready to send items.
+  if (!sent_on_items_available_) {
+    for (auto& it : providers_) {
+      if (!it.second->AreItemsAvailable())
+        return;
+    }
+  }
+
+  // Notify all observers who haven't been told about the initialization that we
+  // are initialized.  Track the observers so that we don't notify them again.
+  for (auto& observer : observers_) {
+    if (!VectorContains(signaled_observers_, &observer)) {
+      observer.OnItemsAvailable(this);
+      signaled_observers_.push_back(&observer);
+    }
+  }
+
+  // Track that we've told the world that we are initialized.
+  sent_on_items_available_ = true;
+}
+
+void OfflineContentAggregator::FlushPendingActionsIfReady(
+    OfflineContentProvider* provider) {
+  if (!provider->AreItemsAvailable())
+    return;
+
+  // Find the actions that correspond to the provider.  Requires a reverse
+  // lookup.
+  for (auto& it : providers_) {
+    if (it.second == provider) {
+      // Flush all of the actions to the provider.
+      for (auto& action : pending_actions_[it.first])
+        action.Run();
+      pending_actions_[it.first].clear();
+      return;
+    }
+  }
+}
+
+}  // namespace offline_content