API to provide status of save page reqeusts to API.

components/offline_pages/background/request_coordinator_unittest.cc

Index: components/offline_pages/background/request_coordinator_unittest.cc
diff --git a/components/offline_pages/background/request_coordinator_unittest.cc b/components/offline_pages/background/request_coordinator_unittest.cc
index ca13ae5a8da5ee00e1dd58fb1b74b8943e341a4a..225e1b4be71057a016919b1c48e42ef75df86f18 100644
--- a/components/offline_pages/background/request_coordinator_unittest.cc
+++ b/components/offline_pages/background/request_coordinator_unittest.cc
@@ -30,7 +30,12 @@ namespace offline_pages {
 namespace {
 // put test constants here
 const GURL kUrl("http://universe.com/everything");
-const ClientId kClientId("bookmark", "42");
+const GURL kUrl2("http://universe.com/nothing");
+const std::string kClientNamespace("bookmark");
+const std::string kId1("42");
+const std::string kId2("life*universe+everything");
+const ClientId kClientId(kClientNamespace, kId1);
+const ClientId kClientId2(kClientNamespace, kId2);
 const int kRequestId(1);
 const long kTestTimeoutSeconds = 1;
 const long kTestTimeBudgetSeconds = 200;
@@ -38,6 +43,7 @@ const int kBatteryPercentageHigh = 75;
 const bool kPowerRequired = true;
 const bool kUserRequested = true;
 const int kAttemptCount = 1;
+const int kAttemptCountTooHigh = 5;
 }  // namespace
 
 class SchedulerStub : public Scheduler {
@@ -138,7 +144,7 @@ class RequestCoordinatorTest
     return coordinator_->is_busy();
   }
 
-  // Empty callback function
+  // Empty callback function.
   void EmptyCallbackFunction(bool result) {
   }
 
@@ -147,7 +153,7 @@ class RequestCoordinatorTest
     waiter_.Signal();
   }
 
-  // Callback for Add requests
+  // Callback for Add requests.
   void AddRequestDone(RequestQueue::AddRequestResult result,
                       const SavePageRequest& request);
 
@@ -155,6 +161,9 @@ class RequestCoordinatorTest
   void GetRequestsDone(RequestQueue::GetRequestsResult result,
                        const std::vector<SavePageRequest>& requests);
 
+  // Callback for getting request statuses.
+  void GetRequestStatusesDone(const std::vector<RequestStatus>& statuses);
+
   void SendOfflinerDoneCallback(const SavePageRequest& request,
                                 Offliner::RequestStatus status);
 
@@ -166,6 +175,10 @@ class RequestCoordinatorTest
     return last_requests_;
   }
 
+  const std::vector<RequestStatus>& last_statuses() const {
+    return last_statuses_;
+  }
+
   void EnableOfflinerCallback(bool enable) {
     offliner_->enable_callback(enable);
   }
@@ -195,6 +208,7 @@ class RequestCoordinatorTest
  private:
   RequestQueue::GetRequestsResult last_get_requests_result_;
   std::vector<SavePageRequest> last_requests_;
+  std::vector<RequestStatus> last_statuses_;
   scoped_refptr<base::TestMockTimeTaskRunner> task_runner_;
   base::ThreadTaskRunnerHandle task_runner_handle_;
   std::unique_ptr<RequestCoordinator> coordinator_;
@@ -238,6 +252,12 @@ void RequestCoordinatorTest::GetRequestsDone(
   last_requests_ = requests;
 }
 
+void RequestCoordinatorTest::GetRequestStatusesDone(
+    const std::vector<RequestStatus>& statuses)  {
+  last_statuses_ = statuses;
+  waiter_.Signal();
+}
+
 void RequestCoordinatorTest::AddRequestDone(
     RequestQueue::AddRequestResult result,
     const SavePageRequest& request) {}
@@ -516,9 +536,8 @@ TEST_F(RequestCoordinatorTest, PrerendererTimeout) {
             last_offlining_status());
 }
 
-
 TEST_F(RequestCoordinatorTest, TimeBudgetExceeded) {
-  // Build a request to use with the pre-renderer, and put it on the queue.
+  // Build two requests to use with the pre-renderer, and put it on the queue.
   offline_pages::SavePageRequest request1(
       kRequestId, kUrl, kClientId, base::Time::Now(), kUserRequested);
   offline_pages::SavePageRequest request2(
@@ -529,7 +548,7 @@ TEST_F(RequestCoordinatorTest, TimeBudgetExceeded) {
       base::Bind(&RequestCoordinatorTest::AddRequestDone,
                  base::Unretained(this)));
     coordinator()->queue()->AddRequest(
-      request1,
+        request1,  // TODO: This is a bug, should be request2, fix it later.

[fgorski, 2016/08/02 04:18:12]

nit: TODO(whom)

[Pete Williamson, 2016/08/03 00:24:32]

Done.

       base::Bind(&RequestCoordinatorTest::AddRequestDone,
                  base::Unretained(this)));
   PumpLoop();
@@ -563,4 +582,40 @@ TEST_F(RequestCoordinatorTest, TimeBudgetExceeded) {
   EXPECT_EQ(1UL, last_requests().size());
 }
 
+TEST_F(RequestCoordinatorTest, GetRequestStatuses) {
+  // Add two requests to the queue.
+  offline_pages::SavePageRequest request1(
+      kRequestId, kUrl, kClientId, base::Time::Now(), kUserRequested);
+  offline_pages::SavePageRequest request2(
+      kRequestId + 1, kUrl2, kClientId2, base::Time::Now(), kUserRequested);
+  // TODO(petewil): Use SetRequestStatus instead once it is available.
+  request2.set_attempt_count(kAttemptCountTooHigh);
+  coordinator()->queue()->AddRequest(
+      request1,
+      base::Bind(&RequestCoordinatorTest::AddRequestDone,
+                 base::Unretained(this)));
+    coordinator()->queue()->AddRequest(
+      request2,
+      base::Bind(&RequestCoordinatorTest::AddRequestDone,
+                 base::Unretained(this)));
+  PumpLoop();
+
+  // Start the async status fetching.
+  coordinator()->GetRequestStatuses(
+      kClientNamespace,
+      base::Bind(&RequestCoordinatorTest::GetRequestStatusesDone,
+                 base::Unretained(this)));
+  PumpLoop();
+
+  // Wait for async get to finish.
+  WaitForCallback();
+  PumpLoop();
+
+  // Check that the statuses found in the callback match what we expect.
+  EXPECT_EQ(2UL, last_statuses().size());
+  EXPECT_EQ(SavePageRequest::Status::PENDING, last_statuses().at(0).status);
+  EXPECT_EQ(kId1, last_statuses().at(0).client_id);
+

[fgorski, 2016/08/02 04:18:12]

nit: remove empty line

Question: Do you intend to verify the second request some other time, or is this
an omission? (please add a TODO in former case, or fix the code in the latter.)

[Pete Williamson, 2016/08/03 00:24:32]

Done.

+}
+
 }  // namespace offline_pages