Adding status info to DownloadUIItem and piping it through.

components/offline_pages/core/downloads/download_ui_adapter_unittest.cc

Index: components/offline_pages/core/downloads/download_ui_adapter_unittest.cc
diff --git a/components/offline_pages/core/downloads/download_ui_adapter_unittest.cc b/components/offline_pages/core/downloads/download_ui_adapter_unittest.cc
index c65dc37d075320fa2d36345da55abcef813f3af7..f69153205aa23b2196065b70158756624ca5edd2 100644
--- a/components/offline_pages/core/downloads/download_ui_adapter_unittest.cc
+++ b/components/offline_pages/core/downloads/download_ui_adapter_unittest.cc
@@ -16,10 +16,13 @@
 #include "base/files/file_path.h"
 #include "base/run_loop.h"
 #include "base/single_thread_task_runner.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/test_mock_time_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
+#include "components/offline_pages/core/background/offliner_stub.h"
+#include "components/offline_pages/core/background/request_coordinator_stub_taco.h"
 #include "components/offline_pages/core/client_namespace_constants.h"
 #include "components/offline_pages/core/client_policy_controller.h"
 #include "components/offline_pages/core/stub_offline_page_model.h"
@@ -31,7 +34,6 @@ namespace {
 // Constants for a test OfflinePageItem.
 static const int kTestOfflineId1 = 1;
 static const int kTestOfflineId2 = 2;
-static const int kTestOfflineId3 = 3;
 static const char kTestUrl[] = "http://foo.com/bar.mhtml";
 static const char kTestGuid1[] = "cccccccc-cccc-4ccc-0ccc-ccccccccccc1";
 static const char kTestGuid2[] = "cccccccc-cccc-4ccc-0ccc-ccccccccccc2";
@@ -47,23 +49,39 @@ static const base::Time kTestCreationTime = base::Time::Now();
 static const base::string16 kTestTitle = base::ASCIIToUTF16("test title");
 }  // namespace
 
+// Mock DownloadUIAdapter::Delegate
+class DownloadUIAdapterDelegate : public DownloadUIAdapter::Delegate {
+ public:
+  DownloadUIAdapterDelegate() {}
+  ~DownloadUIAdapterDelegate() override {}
+
+  // DownloadUIAdapter::Delegate
+  bool IsVisibleInUI(const ClientId& client_id) override { return is_visible; }
+  bool IsTemporarilyHiddenInUI(const ClientId& client_id) override {
+    return is_temporarily_hidden;
+  }
+
+  bool is_visible = true;
+  bool is_temporarily_hidden = false;
+};
+
 // Mock OfflinePageModel for testing the SavePage calls.
 class MockOfflinePageModel : public StubOfflinePageModel {
  public:
   MockOfflinePageModel(base::TestMockTimeTaskRunner* task_runner)
       : observer_(nullptr),
         task_runner_(task_runner),
-        policy_controller_(new ClientPolicyController()) {
-    adapter.reset(new DownloadUIAdapter(this));
-    // Add one page.
+        policy_controller_(new ClientPolicyController()) {}
+
+  ~MockOfflinePageModel() override {}
+
+  void AddInitialPage() {
     OfflinePageItem page(GURL(kTestUrl), kTestOfflineId1, kTestClientId1,
                          kTestFilePath, kFileSize, kTestCreationTime);
     page.title = kTestTitle;
     pages[kTestOfflineId1] = page;
   }
 
-  ~MockOfflinePageModel() override {}
-
   // OfflinePageModel overrides.
   void AddObserver(Observer* observer) override {
     EXPECT_TRUE(observer != nullptr);
@@ -111,9 +129,6 @@ class MockOfflinePageModel : public StubOfflinePageModel {
     return policy_controller_.get();
   }
 
-  // Normally, OfflinePageModel owns this adapter, so lets test it this way.
-  std::unique_ptr<DownloadUIAdapter> adapter;
-
   std::map<int64_t, OfflinePageItem> pages;
 
  private:
@@ -125,6 +140,11 @@ class MockOfflinePageModel : public StubOfflinePageModel {
   DISALLOW_COPY_AND_ASSIGN(MockOfflinePageModel);
 };
 
+// Creates mock versions for OfflinePageModel, RequestCoordinator and their
+// dependencies, then passes them to DownloadUIAdapter for testing.
+// Note that initially the OfflienPageModel is not "loaded". PumpLoop() will
+// load it, firing ItemsLoaded callback to the Adapter. Hence some tests
+// start from PumpLoop() right away if they don't need to test this.
 class DownloadUIAdapterTest : public testing::Test,
                               public DownloadUIAdapter::Observer {
  public:
@@ -144,22 +164,49 @@ class DownloadUIAdapterTest : public testing::Test,
   // queue.
   void PumpLoop();
 
+  int64_t AddRequest(const GURL& url, const ClientId& client_id);
+
+  RequestCoordinator* request_coordinator() {
+    return request_coordinator_taco_->request_coordinator();
+  }
+
   bool items_loaded;
   std::vector<std::string> added_guids, updated_guids, deleted_guids;
   std::unique_ptr<MockOfflinePageModel> model;
+  DownloadUIAdapterDelegate* adapter_delegate;
+  std::unique_ptr<DownloadUIAdapter> adapter;
+  OfflinerStub* offliner_stub;
 
  private:
+  std::unique_ptr<RequestCoordinatorStubTaco> request_coordinator_taco_;
   scoped_refptr<base::TestMockTimeTaskRunner> task_runner_;
+  base::ThreadTaskRunnerHandle task_runner_handle_;
 };
 
 DownloadUIAdapterTest::DownloadUIAdapterTest()
-    : items_loaded(false), task_runner_(new base::TestMockTimeTaskRunner) {}
+    : items_loaded(false),
+      task_runner_(new base::TestMockTimeTaskRunner),
+      task_runner_handle_(task_runner_) {}
 
 DownloadUIAdapterTest::~DownloadUIAdapterTest() {}
 
 void DownloadUIAdapterTest::SetUp() {
-  model.reset(new MockOfflinePageModel(task_runner_.get()));
-  model->adapter->AddObserver(this);
+  model = base::MakeUnique<MockOfflinePageModel>(task_runner_.get());
+  std::unique_ptr<DownloadUIAdapterDelegate> delegate =
+      base::MakeUnique<DownloadUIAdapterDelegate>();
+  adapter_delegate = delegate.get();
+  request_coordinator_taco_ = base::MakeUnique<RequestCoordinatorStubTaco>();
+
+  std::unique_ptr<OfflinerStub> offliner = base::MakeUnique<OfflinerStub>();
+  offliner_stub = offliner.get();
+  request_coordinator_taco_->SetOffliner(std::move(offliner));
+
+  request_coordinator_taco_->CreateRequestCoordinator();
+  adapter = base::MakeUnique<DownloadUIAdapter>(
+      model.get(), request_coordinator_taco_->request_coordinator(),
+      std::move(delegate));
+
+  adapter->AddObserver(this);
 }
 
 void DownloadUIAdapterTest::ItemsLoaded() {
@@ -182,22 +229,33 @@ void DownloadUIAdapterTest::PumpLoop() {
   task_runner_->RunUntilIdle();
 }
 
+int64_t DownloadUIAdapterTest::AddRequest(const GURL& url,
+                                          const ClientId& client_id) {
+  return request_coordinator()->SavePageLater(
+      url, client_id, /* user_requested */ true,
+      RequestCoordinator::RequestAvailability::ENABLED_FOR_OFFLINER);
+}
+
 TEST_F(DownloadUIAdapterTest, InitialLoad) {
-  EXPECT_NE(nullptr, model->adapter);
+  EXPECT_NE(nullptr, adapter.get());
+  model->AddInitialPage();
   EXPECT_FALSE(items_loaded);
   PumpLoop();
   EXPECT_TRUE(items_loaded);
-  const DownloadUIItem* item = model->adapter->GetItem(kTestGuid1);
+  const DownloadUIItem* item = adapter->GetItem(kTestGuid1);
   EXPECT_NE(nullptr, item);
 }
 
 TEST_F(DownloadUIAdapterTest, InitialItemConversion) {
+  model->AddInitialPage();
   EXPECT_EQ(1UL, model->pages.size());
   EXPECT_EQ(kTestGuid1, model->pages[kTestOfflineId1].client_id.id);
   PumpLoop();
-  const DownloadUIItem* item = model->adapter->GetItem(kTestGuid1);
+  const DownloadUIItem* item = adapter->GetItem(kTestGuid1);
   EXPECT_EQ(kTestGuid1, item->guid);
   EXPECT_EQ(kTestUrl, item->url.spec());
+  EXPECT_EQ(DownloadUIItem::DownloadState::COMPLETE, item->download_state);
+  EXPECT_EQ(0, item->download_progress_bytes);
   EXPECT_EQ(kTestFilePath, item->target_path);
   EXPECT_EQ(kTestCreationTime, item->start_time);
   EXPECT_EQ(kFileSize, item->total_bytes);
@@ -205,6 +263,7 @@ TEST_F(DownloadUIAdapterTest, InitialItemConversion) {
 }
 
 TEST_F(DownloadUIAdapterTest, ItemDeletedAdded) {
+  model->AddInitialPage();
   PumpLoop();
   // Add page, notify adapter.
   OfflinePageItem page(GURL(kTestUrl), kTestOfflineId2, kTestClientId2,
@@ -223,26 +282,54 @@ TEST_F(DownloadUIAdapterTest, ItemDeletedAdded) {
   EXPECT_EQ(kTestGuid2, deleted_guids[1]);
 }
 
-TEST_F(DownloadUIAdapterTest, ItemWithWrongNamespace) {
+TEST_F(DownloadUIAdapterTest, NotVisibleItem) {
+  model->AddInitialPage();
   PumpLoop();
+  adapter_delegate->is_visible = false;
   OfflinePageItem page1(
       GURL(kTestUrl), kTestOfflineId2, kTestClientIdOtherNamespace,
       base::FilePath(kTestFilePath), kFileSize, kTestCreationTime);
   model->AddPageAndNotifyAdapter(page1);
   PumpLoop();
-  // Should not add the page with wrong namespace.
+  // Should not add the page.
   EXPECT_EQ(0UL, added_guids.size());
+}
 
-  OfflinePageItem page2(GURL(kTestUrl), kTestOfflineId3, kTestClientIdOtherGuid,
-                        base::FilePath(kTestFilePath), kFileSize,
-                        kTestCreationTime);
-  model->AddPageAndNotifyAdapter(page2);
+TEST_F(DownloadUIAdapterTest, TemporarilyNotVisibleItem) {
+  adapter_delegate->is_temporarily_hidden = true;
+  model->AddInitialPage();
   PumpLoop();
-  // Should not add the page with wrong guid.
+  // Initial Item should be invisible in the collection now.
+  EXPECT_EQ(nullptr, adapter->GetItem(kTestGuid1));
+  EXPECT_EQ(0UL, adapter->GetAllItems().size());
   EXPECT_EQ(0UL, added_guids.size());
+  EXPECT_EQ(0UL, deleted_guids.size());
+
+  adapter_delegate->is_temporarily_hidden = false;
+  // Notify adapter about visibility change for the clientId of initial page.
+  adapter->TemporaryHiddenStatusChanged(kTestClientId1);
+  PumpLoop();
+
+  // There should be OnAdded simulated.
+  EXPECT_EQ(1UL, added_guids.size());
+  EXPECT_EQ(0UL, deleted_guids.size());
+  // Also the item should be visible in the collection of items now.
+  EXPECT_NE(nullptr, adapter->GetItem(kTestGuid1));
+  EXPECT_EQ(1UL, adapter->GetAllItems().size());
+
+  // Switch visibility back to hidden
+  adapter_delegate->is_temporarily_hidden = true;
+  adapter->TemporaryHiddenStatusChanged(kTestClientId1);
+  // There should be OnDeleted fired.
+  EXPECT_EQ(1UL, added_guids.size());
+  EXPECT_EQ(1UL, deleted_guids.size());
+  // Also the item should be visible in the collection of items now.
+  EXPECT_EQ(nullptr, adapter->GetItem(kTestGuid1));
+  EXPECT_EQ(0UL, adapter->GetAllItems().size());
 }
 
 TEST_F(DownloadUIAdapterTest, ItemAdded) {
+  model->AddInitialPage();
   PumpLoop();
   // Clear the initial page and replace it with updated one.
   model->pages.clear();
@@ -261,16 +348,95 @@ TEST_F(DownloadUIAdapterTest, ItemAdded) {
 }
 
 TEST_F(DownloadUIAdapterTest, NoHangingLoad) {
-  EXPECT_NE(nullptr, model->adapter);
+  model->AddInitialPage();
+  EXPECT_NE(nullptr, adapter.get());
   EXPECT_FALSE(items_loaded);
   // Removal of last observer causes cache unload of not-yet-loaded cache.
-  model->adapter->RemoveObserver(this);
+  adapter->RemoveObserver(this);
   // This will complete async fetch of items, but...
   PumpLoop();
   // items should not be loaded when there is no observers!
   EXPECT_FALSE(items_loaded);
   // This should not crash.
-  model->adapter->AddObserver(this);
+  adapter->AddObserver(this);
+}
+
+TEST_F(DownloadUIAdapterTest, LoadExistingRequest) {
+  AddRequest(GURL(kTestUrl), kTestClientId1);
+  PumpLoop();
+  EXPECT_TRUE(items_loaded);
+  const DownloadUIItem* item = adapter->GetItem(kTestGuid1);
+  EXPECT_NE(nullptr, item);
+}
+
+TEST_F(DownloadUIAdapterTest, AddRequest) {
+  PumpLoop();
+  EXPECT_TRUE(items_loaded);
+  EXPECT_EQ(0UL, added_guids.size());
+  AddRequest(GURL(kTestUrl), kTestClientId1);
+  PumpLoop();
+  EXPECT_EQ(1UL, added_guids.size());
+  EXPECT_EQ(kTestClientId1.id, added_guids[0]);
+  const DownloadUIItem* item = adapter->GetItem(kTestGuid1);
+  EXPECT_NE(nullptr, item);
+}
+
+TEST_F(DownloadUIAdapterTest, RemoveRequest) {
+  int64_t id = AddRequest(GURL(kTestUrl), kTestClientId1);
+  PumpLoop();
+  // No added requests, the initial one is loaded.
+  EXPECT_EQ(0UL, added_guids.size());
+  EXPECT_NE(nullptr, adapter->GetItem(kTestGuid1));
+  EXPECT_EQ(0UL, deleted_guids.size());
+
+  std::vector<int64_t> requests_to_remove = {id};
+  request_coordinator()->RemoveRequests(
+      requests_to_remove,
+      base::Bind(
+          [](int64_t id, const MultipleItemStatuses& statuses) {
+            EXPECT_EQ(1UL, statuses.size());
+            EXPECT_EQ(id, statuses[0].first);
+            EXPECT_EQ(ItemActionStatus::SUCCESS, statuses[0].second);
+          },
+          id));
+  PumpLoop();
+
+  EXPECT_EQ(0UL, added_guids.size());
+  EXPECT_EQ(1UL, deleted_guids.size());
+  EXPECT_EQ(kTestClientId1.id, deleted_guids[0]);
+  EXPECT_EQ(nullptr, adapter->GetItem(kTestGuid1));
+}
+
+TEST_F(DownloadUIAdapterTest, RequestBecomesPage) {
+  // This will cause requests to be 'offlined' all the way and removed.
+  offliner_stub->enable_callback(true);
+  AddRequest(GURL(kTestUrl), kTestClientId1);
+  PumpLoop();
+
+  const DownloadUIItem* item = adapter->GetItem(kTestGuid1);
+  EXPECT_NE(nullptr, item);
+  // The item is still IN_PROGRESS, since we did not delete it when
+  // request is competed successfully, waiting for the page with the
+  // same client_id to come in.
+  EXPECT_EQ(DownloadUIItem::DownloadState::IN_PROGRESS, item->download_state);
+  // Add a new saved page with the same client id.
+  // This simulates what happens when the request is completed.
+  // It should not fire and OnAdded or OnDeleted, just OnUpdated.
+  OfflinePageItem page(GURL(kTestUrl), kTestOfflineId1, kTestClientId1,
+                       base::FilePath(kTestFilePath), kFileSize,
+                       kTestCreationTime);
+  model->AddPageAndNotifyAdapter(page);
+  PumpLoop();
+
+  // No added or deleted items, the one existing item should be 'updated'.
+  EXPECT_EQ(0UL, added_guids.size());
+  EXPECT_EQ(0UL, deleted_guids.size());
+
+  EXPECT_GE(updated_guids.size(), 1UL);
+  std::string last_updated_guid = updated_guids[updated_guids.size() - 1];
+  item = adapter->GetItem(last_updated_guid);
+  EXPECT_NE(nullptr, item);
+  EXPECT_EQ(DownloadUIItem::DownloadState::COMPLETE, item->download_state);
 }
 
 }  // namespace offline_pages