Add support for OfflinePagesDownloadBridge.

components/offline_pages/downloads/download_ui_adapter.cc

+// Copyright 2016 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 "components/offline_pages/downloads/download_ui_adapter.h"
+
+#include "base/bind.h"
+#include "base/guid.h"
+#include "base/logging.h"
+#include "base/memory/ptr_util.h"
+#include "base/threading/thread_task_runner_handle.h"
+#include "components/offline_pages/client_namespace_constants.h"
+#include "components/offline_pages/downloads/download_ui_item.h"
+#include "components/offline_pages/offline_page_model.h"
+
+namespace offline_pages {
+
+DownloadUIAdapter::DownloadUIAdapter(OfflinePageModel* model)
+    : model_(model),
+      is_loaded_(false),
+      weak_ptr_factory_(this) {
+}
+
+DownloadUIAdapter::~DownloadUIAdapter() { }
+
+void DownloadUIAdapter::AddObserver(Observer* observer) {

[fgorski, 2016/07/25 17:09:56]

Consider DCHECKing the observer.

[Dmitry Titov, 2016/07/26 00:41:24]

Done, not sure it is useful since it'll fail fast anyways.

+  if (!observers_.might_have_observers())
+    LoadCache();
+  observers_.AddObserver(observer);
+  // If the items are already loaded, post the notification right away.
+  // Don't just invoke it from here to avoid reentrancy in the client.
+  if (is_loaded_) {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::Bind(&DownloadUIAdapter::NotifyOnLoad,
+                              weak_ptr_factory_.GetWeakPtr(),
+                              base::Unretained(observer)));
+  }
+}
+
+void DownloadUIAdapter::RemoveObserver(Observer* observer) {
+  observers_.RemoveObserver(observer);
+  // Once the last observer is gone, clear cached data.
+  if (!observers_.might_have_observers())
+    ClearCache();
+}
+
+void DownloadUIAdapter::OfflinePageModelLoaded(OfflinePageModel* model) {
+  // This signal is not used here.
+}
+
+void DownloadUIAdapter::OfflinePageModelChanged(OfflinePageModel* model) {
+  DCHECK(model == model_);
+  model_->GetAllPages(
+      base::Bind(&DownloadUIAdapter::OnOfflinePagesChanged,
+                 weak_ptr_factory_.GetWeakPtr()));
+}
+
+void DownloadUIAdapter::OfflinePageDeleted(
+  int64_t offline_id, const ClientId& client_id) {
+  for(const auto& item : items_) {
+    if (item.second->guid == client_id.id) {
+      items_.erase(item.first);
+      FOR_EACH_OBSERVER(Observer, observers_, ItemDeleted(client_id.id));
+      return;
+    }
+  }
+}
+
+const DownloadUIItemsMap& DownloadUIAdapter::GetAllItems() const {
+  return items_;
+}
+
+const DownloadUIItem* DownloadUIAdapter::GetItem(std::string guid) const {
+  DownloadUIItemsMap::const_iterator it = items_.find(guid);
+  if (it == items_.end())
+    return nullptr;
+  return (*it).second.get();
+}
+
+void DownloadUIAdapter::LoadCache() {
+  DCHECK(!is_loaded_);
+  // TODO(dimich): Add fetching from RequestQueue as well.
+  model_->AddObserver(this);
+  model_->GetAllPages(
+      base::Bind(&DownloadUIAdapter::OnOfflinePagesLoaded,
+                 weak_ptr_factory_.GetWeakPtr()));
+}
+
+void DownloadUIAdapter::ClearCache() {
+  model_->RemoveObserver(this);
+  items_.clear();
+  is_loaded_ = false;
+}
+
+void DownloadUIAdapter::OnOfflinePagesLoaded(
+    const MultipleOfflinePageItemResult& pages) {
+  for (const auto& page : pages) {
+    if (IsVisibleInUI(page)) {
+      std::unique_ptr<DownloadUIItem> new_item(new DownloadUIItem(page));
+      DCHECK(items_.find(new_item->guid) == items_.end());
+      items_[new_item->guid] = std::move(new_item);
+    }
+  }
+  is_loaded_ = true;
+  FOR_EACH_OBSERVER(Observer, observers_, ItemsLoaded());
+}
+
+void DownloadUIAdapter::NotifyOnLoad(Observer* observer) {
+  if (observer && observers_.HasObserver(observer))
+    observer->ItemsLoaded();
+}
+
+void DownloadUIAdapter::OnOfflinePagesChanged(
+    const MultipleOfflinePageItemResult& pages) {
+

[fgorski, 2016/07/25 17:09:56]

nit: this empty line probably not needed.

[Dmitry Titov, 2016/07/26 00:41:24]

Done.

+  std::vector<std::string> added_guids, changed_guids;
+
+  for (const auto& page : pages) {
+    if (IsVisibleInUI(page)) {
+      std::unique_ptr<DownloadUIItem> item(new DownloadUIItem(page));
+      DownloadUIItemsMap::const_iterator it = items_.find(item->guid);

[fgorski, 2016/07/25 17:28:41]

this is not used.

[Dmitry Titov, 2016/07/26 00:41:24]

Done.

+      std::string guid = item->guid;

[fgorski, 2016/07/25 17:09:56]

this can probably be const &

[Dmitry Titov, 2016/07/26 00:41:24]

Done.

+      if (items_.find(guid) == items_.end()) {

[fgorski, 2016/07/25 17:09:56]

This code does not do the validation of duplicate GUIDs like the one above.

turning added_guids (and probably changed_guids) to a set and checking against
it allows for making it here.

[Dmitry Titov, 2016/07/26 00:41:24]

It's not clear to me what this method should enforce uniqueness of entries for
the same page here (or de-dupe anything), especially if it was added and changed
after that etc. It feels it should be done at the point where
OnOfflinePagesChanged is originating.

+        added_guids.push_back(guid);
+        items_[guid] = std::move(item);

[fgorski, 2016/07/25 17:09:56]

shouldn't this happen for both changed and added case?
After all we want to update the cache.

You should probably move it after the if statement

[Dmitry Titov, 2016/07/26 00:41:24]

Done.

+      } else {
+        changed_guids.push_back(guid);
+      }
+    }
+  }
+  for (auto& guid : added_guids) {
+    FOR_EACH_OBSERVER(Observer, observers_,
+                      ItemAdded(*((*items_.find(guid)).second.get())));

[fgorski, 2016/07/25 17:09:56]

How about the two options below for readability.

FOR_EACH_OBSERVER(Observer, observers_,
                  ItemAdded(*(items_.find(guid)->second.get()))

OR:
  for (auto& guid : added_guids) {
    const DownloadUIItem& item = *(items_.find(guid)->second.get());
    FOR_EACH_OBSERVER(Observer, observers_, ItemAdded(item));
  }

Applies below as well.

[Dmitry Titov, 2016/07/26 00:41:24]

Done.

+  }
+  for (auto& guid : changed_guids) {
+    FOR_EACH_OBSERVER(Observer, observers_,
+                      ItemUpdated(*((*items_.find(guid)).second.get())));
+  }
+}
+
+bool DownloadUIAdapter::IsVisibleInUI(const OfflinePageItem& page) {
+  // TODO(dimich): set up the right filter here.
+  return page.client_id.name_space == kAsyncNamespace &&
+         base::IsValidGUID(page.client_id.id);
+}
+
+}  // namespace offline_pages