Implement deleteItem and openItem on OfflinePageDownloadBridge.

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"
@@ skipping 77 matching lines @@
 }
 
 const DownloadUIItem*
     DownloadUIAdapter::GetItem(const std::string& guid) const {
   DownloadUIItemsMap::const_iterator it = items_.find(guid);
   if (it == items_.end())
     return nullptr;
   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.
   model_->AddObserver(this);
   model_->GetAllPages(
       base::Bind(&DownloadUIAdapter::OnOfflinePagesLoaded,
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
 void DownloadUIAdapter::ClearCache() {
@@ skipping 38 matching lines @@
     added_guids.push_back(guid);
   }
   // Only one added page
   CHECK(added_guids.size() <= 1);
   for (auto& guid : added_guids) {
     const DownloadUIItem& item = *(items_.find(guid)->second.get());
     FOR_EACH_OBSERVER(Observer, observers_, ItemAdded(item));
   }
 }
 
+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 &&
          base::IsValidGUID(page.client_id.id);
 }
 
 }  // namespace offline_pages