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

components/offline_pages/core/recent_tabs/recent_tabs_ui_adapter_delegate.cc

Index: components/offline_pages/core/recent_tabs/recent_tabs_ui_adapter_delegate.cc
diff --git a/components/offline_pages/core/recent_tabs/recent_tabs_ui_adapter_delegate.cc b/components/offline_pages/core/recent_tabs/recent_tabs_ui_adapter_delegate.cc
new file mode 100644
index 0000000000000000000000000000000000000000..55b39cd291969e21ca4f9f161d0aaaa3c5c6a412
--- /dev/null
+++ b/components/offline_pages/core/recent_tabs/recent_tabs_ui_adapter_delegate.cc
@@ -0,0 +1,95 @@
+// 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/recent_tabs/recent_tabs_ui_adapter_delegate.h"
+
+#include "base/bind.h"
+#include "base/threading/thread_task_runner_handle.h"
+#include "components/offline_pages/core/client_policy_controller.h"
+#include "components/offline_pages/core/offline_page_model.h"
+
+namespace offline_pages {
+
+namespace {
+const char kRecentTabsUIAdapterKey[] = "recent-tabs-download-ui-adapter";
+}  // namespace
+
+RecentTabsUIAdapterDelegate::RecentTabsUIAdapterDelegate(
+    OfflinePageModel* model)
+    : model_(model) {}
+
+RecentTabsUIAdapterDelegate::~RecentTabsUIAdapterDelegate() = default;
+
+offline_pages::DownloadUIAdapter*
+RecentTabsUIAdapterDelegate::GetOrCreateRecentTabsUIAdapter(
+    offline_pages::OfflinePageModel* offline_page_model,
+    offline_pages::RequestCoordinator* request_coordinator) {
+  offline_pages::DownloadUIAdapter* recent_tabs_ui_adapter =
+      static_cast<DownloadUIAdapter*>(
+          offline_page_model->GetUserData(kRecentTabsUIAdapterKey));
+
+  if (!recent_tabs_ui_adapter) {
+    RecentTabsUIAdapterDelegate* delegate =
+        new RecentTabsUIAdapterDelegate(offline_page_model);
+    recent_tabs_ui_adapter = new DownloadUIAdapter(
+        offline_page_model, request_coordinator,
+        std::unique_ptr<RecentTabsUIAdapterDelegate>(delegate));
+    delegate->set_ui_adapter(recent_tabs_ui_adapter);

[Dmitry Titov, 2017/02/16 23:30:41]

That is even more hair-raising :) 

How about instead of passing delegate into adapter's ctor we have
adapter::SetDelegate(std::unique_ptr<Delegate>)?

Then it would look like:

adapter = new Adapter()
delegate = new Delegate(..., adapter)
adapter->SetDelegate(delegate)

WDYT? 

[tschumann, 2017/02/17 11:30:35]

Drive-by: [started reading this in wrong context, but wanted to share a
different in opinion (now that I already looked at it) -- feel free to ignore
though)].

The RegisterTab/UnregisterTab functions seems misplaced on this class (and are
the reason why you need the delegate needs to know the ui_adapter).
My understanding is that the idea of the delegate is to customize the adaptor
(see class comment at the Delegate interface).
There are 2 smells:
-- why does the bridge call into the delegate and not the adaptor itself.
Shouldn't the delegate be considered an implementation detail?
-- having the delegate initiate an interaction to the adaptor

proposal:
-- have the bridge access the adaptor
-- Why not move the RegisterTab/UnregisterTab functions into the adaptor and let
the adaptor notify the delegate about the change?
-- no need for the delegate to know the adaptor :-)

[dewittj, 2017/02/17 22:33:38]

I think it's important to maintain the separation of concerns here:
RecentTabsUIAdapterDelegate is the only part of the system that knows about
tabs.

I rearranged things so that there are no more pointer shenanigans, please let me
know what you think.

[tschumann, 2017/02/24 10:40:14]

I think it's fine as is -- like I said this was primarily my personal opinion.
There's a light smell that you have a customizing class that needs to know about
the class it customizes. Maybe that's a case where actual inheritance would be
the better option. Or splitting out another class that knows about both (the
delegate and the adaptor).
But maybe it's just too early (this is the first implementation of the delegate,
right?) and it's best to wait and see.

At any rate, I'm fine with this solution (and don't think I should have a lot to
say in your code base anyways). Hope I didn't slow you down!

+    offline_page_model->SetUserData(kRecentTabsUIAdapterKey,
+                                    recent_tabs_ui_adapter);
+  }
+
+  return recent_tabs_ui_adapter;
+}
+
+// static
+RecentTabsUIAdapterDelegate* RecentTabsUIAdapterDelegate::FromDownloadUIAdapter(
+    DownloadUIAdapter* ui_adapter) {
+  return static_cast<RecentTabsUIAdapterDelegate*>(ui_adapter->delegate());
+}
+
+// static
+int RecentTabsUIAdapterDelegate::TabIdFromClientId(const ClientId& client_id) {
+  int tab_id = 0;
+  bool convert_result = base::StringToInt(client_id.id, &tab_id);
+  DCHECK(convert_result);
+  return tab_id;
+}
+
+bool RecentTabsUIAdapterDelegate::IsVisibleInUI(const ClientId& client_id) {
+  return IsRecentTab(client_id);
+}
+
+bool RecentTabsUIAdapterDelegate::IsTemporarilyHiddenInUI(
+    const ClientId& client_id) {
+  return active_tabs_.count(TabIdFromClientId(client_id)) == 0;
+}
+
+void RecentTabsUIAdapterDelegate::RegisterTab(int tab_id) {
+  DCHECK(ui_adapter_);
+  if (active_tabs_.count(tab_id) > 0)
+    return;
+  active_tabs_.insert(tab_id);
+  ui_adapter_->TemporaryHiddenStatusChanged(ClientIdFromTabId(tab_id));
+}
+
+void RecentTabsUIAdapterDelegate::UnregisterTab(int tab_id) {
+  DCHECK(ui_adapter_);
+  if (active_tabs_.count(tab_id) == 0)
+    return;
+  active_tabs_.erase(tab_id);
+  ui_adapter_->TemporaryHiddenStatusChanged(ClientIdFromTabId(tab_id));
+}
+
+// static
+ClientId RecentTabsUIAdapterDelegate::ClientIdFromTabId(int tab_id) {
+  return ClientId(kLastNNamespace, base::IntToString(tab_id));
+}
+
+bool RecentTabsUIAdapterDelegate::IsRecentTab(const ClientId& page) {
+  return model_->GetPolicyController()->IsShownAsRecentlyVisitedSite(
+      page.name_space);
+}
+
+}  // namespace offline_pages