Implement deleteItem and openItem on OfflinePageDownloadBridge.

components/offline_pages/downloads/download_ui_adapter.cc

Index: components/offline_pages/downloads/download_ui_adapter.cc
diff --git a/components/offline_pages/downloads/download_ui_adapter.cc b/components/offline_pages/downloads/download_ui_adapter.cc
index 776fd8cd79b691e167cd48adfc895c7e9d622056..aa8e9efc1ee3142d8712299c0e597156c5fd6410 100644
--- a/components/offline_pages/downloads/download_ui_adapter.cc
+++ b/components/offline_pages/downloads/download_ui_adapter.cc
@@ -95,6 +95,24 @@ const DownloadUIItem*
   return (*it).second.get();
 }
 
+void DownloadUIAdapter::DeleteItem(const std::string& guid) {
+  // TODO(dimich): Also remove pending request from RequestQueue.
+  model_->GetAllPages(

[fgorski, 2016/08/09 04:00:50]

On an off chance that you didn't consider it yet. What was the disadvantage of
putting offline_id, id or item_id on the download UI item? This would save you
going back to the DB and bulk reading all offline pages every time user is
interacting with UI and expects it to be responsive.

I'd consider that and ability to internally distinguish between requests and
offline pages. This is so that this method looks up in only one place and knows
where to go from there.

[Dmitry Titov, 2016/08/12 04:01:39]

Done, by adding ItemInfo internal struct of whihc DownloadUIItem is a field, and
additional info is added to ItemInfo. This way, there is no leakage of stuff
into DownloadUIItem.

+      base::Bind(&DownloadUIAdapter::OnDeleteItemByGuid,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 guid));
+}
+
+std::string DownloadUIAdapter::GetOfflineUrlByGuid(
+    const std::string& guid) const {
+  std::string offline_url;
+  // TODO(dimich): when requests are also in the cache, filter them out.
+  DownloadUIItemsMap::const_iterator it = items_.find(guid);
+  if (it != items_.end())
+    offline_url = (*it).second->offline_url;

[fgorski, 2016/08/09 04:00:50]

How about dropping the internal variable and doing this?
  return it->second->offline_url;
return std::string();

[Dmitry Titov, 2016/08/12 04:01:39]

Done.

+  return offline_url;
+}
+
 void DownloadUIAdapter::LoadCache() {
   DCHECK(!is_loaded_);
   // TODO(dimich): Add fetching from RequestQueue as well.
@@ -153,6 +171,35 @@ void DownloadUIAdapter::OnOfflinePagesChanged(
   }
 }
 
+void DownloadUIAdapter::OnDeleteItemByGuid(

[fgorski, 2016/08/09 04:00:50]

this method needs git cl format. Please run it.

[Dmitry Titov, 2016/08/12 04:01:39]

Done.

+    const MultipleOfflinePageItemResult& pages, const std::string& guid) {
+    OfflinePageItem* page = FindPageByGuid(pages, guid);
+    if (!page)
+      return;
+    std::vector<int64_t> page_ids;
+    page_ids.push_back(page.offline_id);
+    // TODO(dimich): This should be ExpirePages(...Now()..) when Expire is
+    // firing Observer method. The resulting Observer notification will update
+    // local cache.
+    model_->DeletePagesByOfflineId(
+        page_ids,
+        base::Bind(&DownloadUIAdapter::OnDeletePagesDone,
+                 weak_ptr_factory_.GetWeakPtr()));
+}
+
+void DownloadUIAdapter::OnDeletePagesDone(bool success) {
+  // Do nothing here.

[fgorski, 2016/08/09 04:00:50]

TODO: Add UMA for Download UI actions.

[Dmitry Titov, 2016/08/12 04:01:39]

Done.

+}
+
+OfflinePageItem* DownloadUIAdapter::FindPageByGuid(
+    const MultipleOfflinePageItemResult& pages, const std::string& guid) {
+  for (const auto& page : pages) {
+    if (IsVisibleInUI(page) && page.client_id.id == guid)
+      return page;
+  }
+  return nullptr;
+}
+
 bool DownloadUIAdapter::IsVisibleInUI(const OfflinePageItem& page) {
   // TODO(dimich): set up the right filter here.
   return page.client_id.name_space == kAsyncNamespace &&