Only show Last N Pages in the UI when the corresponding tab is visible.

components/offline_pages/core/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/core/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/core/background/request_coordinator.h"
 #include "components/offline_pages/core/background/save_page_request.h"
 #include "components/offline_pages/core/client_namespace_constants.h"
 #include "components/offline_pages/core/client_policy_controller.h"
 #include "components/offline_pages/core/downloads/download_ui_item.h"
 #include "components/offline_pages/core/offline_page_model.h"
 
 namespace offline_pages {
 
-DownloadUIAdapter::ItemInfo::ItemInfo(const OfflinePageItem& page)
+DownloadUIAdapter::ItemInfo::ItemInfo(Delegate* delegate,
+                                      const OfflinePageItem& page)
     : ui_item(base::MakeUnique<DownloadUIItem>(page)),
       is_request(false),
       offline_id(page.offline_id),
       client_id(page.client_id),
-      temporarily_hidden(false) {}
+      temporarily_hidden(delegate->IsTemporarilyHiddenInUI(page.client_id)) {}
 
-DownloadUIAdapter::ItemInfo::ItemInfo(const SavePageRequest& request)
+DownloadUIAdapter::ItemInfo::ItemInfo(Delegate* delegate,
+                                      const SavePageRequest& request)
     : ui_item(base::MakeUnique<DownloadUIItem>(request)),
       is_request(true),
       offline_id(request.request_id()),
       client_id(request.client_id()),
-      temporarily_hidden(false) {}
+      temporarily_hidden(
+          delegate->IsTemporarilyHiddenInUI(request.client_id())) {}
 
 DownloadUIAdapter::ItemInfo::~ItemInfo() {}
 
 DownloadUIAdapter::DownloadUIAdapter(OfflinePageModel* model,
                                      RequestCoordinator* request_coordinator,
                                      std::unique_ptr<Delegate> delegate)
     : model_(model),
       request_coordinator_(request_coordinator),
       delegate_(std::move(delegate)),
       state_(State::NOT_LOADED),
@@ skipping 34 matching lines @@
 void DownloadUIAdapter::OfflinePageModelLoaded(OfflinePageModel* model) {
   // This signal is not used here.
 }
 
 void DownloadUIAdapter::OfflinePageAdded(OfflinePageModel* model,
                                          const OfflinePageItem& added_page) {
   DCHECK(model == model_);
   if (!delegate_->IsVisibleInUI(added_page.client_id))
     return;
 
-  AddItemHelper(base::MakeUnique<ItemInfo>(added_page));
+  AddItemHelper(base::MakeUnique<ItemInfo>(delegate_.get(), added_page));
 }
 
 void DownloadUIAdapter::OfflinePageDeleted(int64_t offline_id,
                                            const ClientId& client_id) {
   if (!delegate_->IsVisibleInUI(client_id))
     return;
   DeleteItemHelper(client_id.id);
 }
 
 // RequestCoordinator::Observer
 void DownloadUIAdapter::OnAdded(const SavePageRequest& added_request) {
   if (!delegate_->IsVisibleInUI(added_request.client_id()))
     return;
 
-  AddItemHelper(base::MakeUnique<ItemInfo>(added_request));
+  AddItemHelper(base::MakeUnique<ItemInfo>(delegate_.get(), added_request));
 }
 
 // RequestCoordinator::Observer
 void DownloadUIAdapter::OnCompleted(
     const SavePageRequest& request,
     RequestNotifier::BackgroundSavePageResult status) {
   if (!delegate_->IsVisibleInUI(request.client_id()))
     return;
 
   // If request completed successfully, report ItemUpdated when a page is added
   // to the model. If the request failed, tell UI that the item is gone.
   if (status == RequestNotifier::BackgroundSavePageResult::SUCCESS)
     return;
   DeleteItemHelper(request.client_id().id);
 }
 
 // RequestCoordinator::Observer
 void DownloadUIAdapter::OnChanged(const SavePageRequest& request) {
   if (!delegate_->IsVisibleInUI(request.client_id()))
     return;
 
   std::string guid = request.client_id().id;
-  items_[guid] = base::MakeUnique<ItemInfo>(request);
+  items_[guid] = base::MakeUnique<ItemInfo>(delegate_.get(), request);
 
   if (state_ != State::LOADED)
     return;
 
   const DownloadUIItem& download_ui_item = *(items_[guid]->ui_item);
   for (Observer& observer : observers_)
     observer.ItemUpdated(download_ui_item);
 }
 
 void DownloadUIAdapter::TemporaryHiddenStatusChanged(
@@ skipping 42 matching lines @@
     return;
 
   std::vector<int64_t> page_ids;
   page_ids.push_back(it->second->offline_id);
   model_->DeletePagesByOfflineId(
       page_ids, base::Bind(&DownloadUIAdapter::OnDeletePagesDone,
                            weak_ptr_factory_.GetWeakPtr()));
 }
 
 int64_t DownloadUIAdapter::GetOfflineIdByGuid(const std::string& guid) const {
+  if (deleting_item_ && deleting_item_->ui_item->guid == guid)
+    return deleting_item_->offline_id;
+
   DownloadUIItems::const_iterator it = items_.find(guid);
   if (it != items_.end())
     return it->second->offline_id;
   return 0;
 }
 
 // Note that several LoadCache calls may be issued before the async GetAllPages
 // comes back.
 void DownloadUIAdapter::LoadCache() {
   state_ = State::LOADING_PAGES;
@@ skipping 14 matching lines @@
     const MultipleOfflinePageItemResult& pages) {
   // If multiple observers register quickly, the cache might be already loaded
   // by the previous LoadCache call. At the same time, if all observers already
   // left, there is no reason to populate the cache.
   if (state_ != State::LOADING_PAGES)
     return;
   for (const auto& page : pages) {
     if (delegate_->IsVisibleInUI(page.client_id)) {
       std::string guid = page.client_id.id;
       DCHECK(items_.find(guid) == items_.end());
-      std::unique_ptr<ItemInfo> item = base::MakeUnique<ItemInfo>(page);
-      item->temporarily_hidden =
-          delegate_->IsTemporarilyHiddenInUI(page.client_id);
+      std::unique_ptr<ItemInfo> item =
+          base::MakeUnique<ItemInfo>(delegate_.get(), page);
       items_[guid] = std::move(item);
     }
   }
   model_->AddObserver(this);
 
   state_ = State::LOADING_REQUESTS;
   request_coordinator_->GetAllRequests(base::Bind(
       &DownloadUIAdapter::OnRequestsLoaded, weak_ptr_factory_.GetWeakPtr()));
 }
 
 void DownloadUIAdapter::OnRequestsLoaded(
     std::vector<std::unique_ptr<SavePageRequest>> requests) {
   // If multiple observers register quickly, the cache might be already loaded
   // by the previous LoadCache call. At the same time, if all observers already
   // left, there is no reason to populate the cache.
   if (state_ != State::LOADING_REQUESTS)
     return;
 
   for (const auto& request : requests) {
     if (delegate_->IsVisibleInUI(request->client_id())) {
       std::string guid = request->client_id().id;
       DCHECK(items_.find(guid) == items_.end());
       std::unique_ptr<ItemInfo> item =
-          base::MakeUnique<ItemInfo>(*request.get());
-      item->temporarily_hidden =
-          delegate_->IsTemporarilyHiddenInUI(request->client_id());
+          base::MakeUnique<ItemInfo>(delegate_.get(), *request.get());
       items_[guid] = std::move(item);
     }
   }
   request_coordinator_->AddObserver(this);
 
   state_ = State::LOADED;
   for (Observer& observer : observers_)
     observer.ItemsLoaded();
 }
 
@@ skipping 16 matching lines @@
   // notification and when page is added, fire 'updated' instead of 'added'.
   bool request_to_page_transition =
       (it != items_.end() && it->second->is_request && !item_info->is_request);
 
   items_[guid] = std::move(item_info);
 
   if (state_ != State::LOADED)
     return;
 
   DownloadUIItem* download_ui_item = items_[guid]->ui_item.get();
+
   if (request_to_page_transition) {
     download_ui_item->download_state = DownloadUIItem::DownloadState::COMPLETE;
-    for (Observer& observer : observers_)
-      observer.ItemUpdated(*download_ui_item);
+    if (!items_[guid]->temporarily_hidden) {
+      for (Observer& observer : observers_)
+        observer.ItemUpdated(*download_ui_item);
+    }
   } else {
-    for (Observer& observer : observers_)
-      observer.ItemAdded(*download_ui_item);
+    if (!items_[guid]->temporarily_hidden) {
+      for (Observer& observer : observers_)
+        observer.ItemAdded(*download_ui_item);
+    }
   }
 }
 
 void DownloadUIAdapter::DeleteItemHelper(const std::string& guid) {
-  DownloadUIItems::const_iterator it = items_.find(guid);
+  DownloadUIItems::iterator it = items_.find(guid);
   if (it == items_.end())
     return;
+  DCHECK(deleting_item_ == nullptr);
+  deleting_item_ = std::move(it->second);

[vitaliii, 2017/02/15 10:12:16]

This deleting_item_ approach feels a bit weird.
Since it is used only for obtaining offline_id from GUID, why not providing
offline_id in the ItemDeleted notification?

[dewittj, 2017/02/15 19:49:45]

I'm thinking of a future scenario where we stop using tab ID as the GUID and
provide a helper for getting the tab ID from a GUID.  Then that would also need
to be passed to ItemDeleted.  Perhaps it should just pass the whole ItemInfo
struct back.

Dmitry, any thoughts about this?

   items_.erase(it);
 
-  if (state_ != State::LOADED)
-    return;
+  if (state_ == State::LOADED) {
+    for (Observer& observer : observers_)
+      observer.ItemDeleted(guid);
+  }
 
-  for (Observer& observer : observers_)
-    observer.ItemDeleted(guid);
+  deleting_item_.reset();
 }
 
 }  // namespace offline_pages