Rename RequestCoordinator::queue() -> queue_for_testing()

Change-Id: I586fbd98fbf5529d7144513f75fa470d1accd0e8
Reviewed-on: https://chromium-review.googlesource.com/1236462Reviewed-by: Cathy Li <chili@chromium.org>
Reviewed-by: calamity <calamity@chromium.org>
Commit-Queue: Dan H <harringtond@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593571}

chrome/browser/ui/webui/offline/offline_internals_ui_message_handler.cc

@@ -183,27 +183,22 @@ void OfflineInternalsUIMessageHandler::HandleStoredPagesCallback(
 
 void OfflineInternalsUIMessageHandler::HandleRequestQueueCallback(
     std::string callback_id,
-    offline_pages::GetRequestsResult result,
     std::vector<std::unique_ptr<offline_pages::SavePageRequest>> requests) {
   base::ListValue save_page_requests;
-  if (result == offline_pages::GetRequestsResult::SUCCESS) {
   for (const auto& request : requests) {
     auto save_page_request = std::make_unique<base::DictionaryValue>();
     save_page_request->SetString("onlineUrl", request->url().spec());
     save_page_request->SetDouble("creationTime",
                                  request->creation_time().ToJsTime());
     save_page_request->SetString("status", GetStringFromSavePageStatus());
-      save_page_request->SetString("namespace",
-                                   request->client_id().name_space);
+    save_page_request->SetString("namespace", request->client_id().name_space);
     save_page_request->SetDouble("lastAttemptTime",
                                  request->last_attempt_time().ToJsTime());
     save_page_request->SetString("id", std::to_string(request->request_id()));
-      save_page_request->SetString("originalUrl",
-                                   request->original_url().spec());
+    save_page_request->SetString("originalUrl", request->original_url().spec());
     save_page_request->SetString("requestOrigin", request->request_origin());
     save_page_requests.Append(std::move(save_page_request));
   }
-  }
   ResolveJavascriptCallback(base::Value(callback_id), save_page_requests);
 }
 
@@ -214,7 +209,7 @@ void OfflineInternalsUIMessageHandler::HandleGetRequestQueue(
   CHECK(args->GetString(0, &callback_id));
 
   if (request_coordinator_) {
-    request_coordinator_->queue()->GetRequests(base::Bind(
+    request_coordinator_->GetAllRequests(base::BindOnce(
         &OfflineInternalsUIMessageHandler::HandleRequestQueueCallback,
         weak_ptr_factory_.GetWeakPtr(), callback_id));
   } else {

chrome/browser/ui/webui/offline/offline_internals_ui_message_handler.h

@@ -95,7 +95,6 @@ class OfflineInternalsUIMessageHandler : public content::WebUIMessageHandler {
   // Callback for async GetRequests calls.
   void HandleRequestQueueCallback(
       std::string callback_id,
-      offline_pages::GetRequestsResult result,
       std::vector<std::unique_ptr<offline_pages::SavePageRequest>> requests);
 
   // Callback for DeletePage/DeleteAllPages calls.

components/offline_pages/core/background/request_coordinator.h

@@ -218,7 +218,7 @@ class RequestCoordinator : public KeyedService,
                              int64_t received_bytes) override;
 
   // Returns the request queue used for requests.  Coordinator keeps ownership.
-  RequestQueue* queue() { return queue_.get(); }
+  RequestQueue* queue_for_testing() { return queue_.get(); }
 
   // Return an unowned pointer to the Scheduler.
   Scheduler* scheduler() { return scheduler_.get(); }

components/offline_pages/core/background/request_coordinator_unittest.cc

@@ -147,7 +147,9 @@ class RequestCoordinatorTest : public testing::Test {
   RequestCoordinator* coordinator() const {
     return coordinator_taco_->request_coordinator();
   }
-
+  RequestQueue* queue() {
+    return coordinator_taco_->request_coordinator()->queue_for_testing();
+  }
   RequestCoordinatorState state() { return coordinator()->state(); }
 
   // Test processing callback function.
@@ -446,8 +448,8 @@ void RequestCoordinatorTest::SetupForOfflinerDoneCallbackTest(
   // Mark request as started and add it to the queue,
   // then wait for callback to finish.
   request->MarkAttemptStarted(base::Time::Now());
-  coordinator()->queue()->AddRequest(
-      *request, base::BindOnce(&RequestCoordinatorTest::AddRequestDone,
+  queue()->AddRequest(*request,
+                      base::BindOnce(&RequestCoordinatorTest::AddRequestDone,
                                      base::Unretained(this)));
   PumpLoop();
 
@@ -473,8 +475,8 @@ void RequestCoordinatorTest::SendOfflinerDoneCallback(
 SavePageRequest RequestCoordinatorTest::AddRequest1() {
   offline_pages::SavePageRequest request1(kRequestId1, kUrl1, kClientId1,
                                           base::Time::Now(), kUserRequested);
-  coordinator()->queue()->AddRequest(
-      request1, base::BindOnce(&RequestCoordinatorTest::AddRequestDone,
+  queue()->AddRequest(request1,
+                      base::BindOnce(&RequestCoordinatorTest::AddRequestDone,
                                      base::Unretained(this)));
   return request1;
 }
@@ -482,8 +484,8 @@ SavePageRequest RequestCoordinatorTest::AddRequest1() {
 SavePageRequest RequestCoordinatorTest::AddRequest2() {
   offline_pages::SavePageRequest request2(kRequestId2, kUrl2, kClientId2,
                                           base::Time::Now(), kUserRequested);
-  coordinator()->queue()->AddRequest(
-      request2, base::BindOnce(&RequestCoordinatorTest::AddRequestDone,
+  queue()->AddRequest(request2,
+                      base::BindOnce(&RequestCoordinatorTest::AddRequestDone,
                                      base::Unretained(this)));
   return request2;
 }
@@ -607,8 +609,8 @@ TEST_F(RequestCoordinatorTest, SavePageLater) {
                                base::Unretained(this))));
 
   // Expect that a request got placed on the queue.
-  coordinator()->queue()->GetRequests(base::BindOnce(
-      &RequestCoordinatorTest::GetRequestsDone, base::Unretained(this)));
+  queue()->GetRequests(base::BindOnce(&RequestCoordinatorTest::GetRequestsDone,
+                                      base::Unretained(this)));
 
   // Expect that the request is not added to the disabled list by default.
   EXPECT_TRUE(disabled_requests().empty());
@@ -655,8 +657,8 @@ TEST_F(RequestCoordinatorTest, SavePageLaterFailed) {
   EXPECT_NE(0, SavePageLater());
 
   // Expect that a request got placed on the queue.
-  coordinator()->queue()->GetRequests(base::BindOnce(
-      &RequestCoordinatorTest::GetRequestsDone, base::Unretained(this)));
+  queue()->GetRequests(base::BindOnce(&RequestCoordinatorTest::GetRequestsDone,
+                                      base::Unretained(this)));
 
   // Wait for callbacks to finish, both request queue and offliner.
   PumpLoop();
@@ -702,8 +704,8 @@ TEST_F(RequestCoordinatorTest, OfflinerDoneRequestSucceeded) {
   EXPECT_TRUE(processing_callback_called());
 
   // Verify the request gets removed from the queue, and wait for callbacks.
-  coordinator()->queue()->GetRequests(base::BindOnce(
-      &RequestCoordinatorTest::GetRequestsDone, base::Unretained(this)));
+  queue()->GetRequests(base::BindOnce(&RequestCoordinatorTest::GetRequestsDone,
+                                      base::Unretained(this)));
   PumpLoop();
 
   // We should not find any requests in the queue anymore.
@@ -774,8 +776,8 @@ TEST_F(RequestCoordinatorTest, OfflinerDoneRequestFailed) {
   // Busy processing 2nd request.
   EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);
 
-  coordinator()->queue()->GetRequests(base::BindOnce(
-      &RequestCoordinatorTest::GetRequestsDone, base::Unretained(this)));
+  queue()->GetRequests(base::BindOnce(&RequestCoordinatorTest::GetRequestsDone,
+                                      base::Unretained(this)));
   PumpLoop();
 
   // Now just one request in the queue since failed request removed
@@ -816,8 +818,8 @@ TEST_F(RequestCoordinatorTest, OfflinerDoneRequestFailedNoRetryFailure) {
   // Busy processing 2nd request.
   EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);
 
-  coordinator()->queue()->GetRequests(base::BindOnce(
-      &RequestCoordinatorTest::GetRequestsDone, base::Unretained(this)));
+  queue()->GetRequests(base::BindOnce(&RequestCoordinatorTest::GetRequestsDone,
+                                      base::Unretained(this)));
   PumpLoop();
 
   // Now just one request in the queue since non-retryable failure.
@@ -859,8 +861,8 @@ TEST_F(RequestCoordinatorTest, OfflinerDoneRequestFailedNoNextFailure) {
   // Not busy for NO_NEXT failure.
   EXPECT_FALSE(state() == RequestCoordinatorState::OFFLINING);
 
-  coordinator()->queue()->GetRequests(base::BindOnce(
-      &RequestCoordinatorTest::GetRequestsDone, base::Unretained(this)));
+  queue()->GetRequests(base::BindOnce(&RequestCoordinatorTest::GetRequestsDone,
+                                      base::Unretained(this)));
   PumpLoop();
 
   // Both requests still in queue.
@@ -881,8 +883,8 @@ TEST_F(RequestCoordinatorTest, OfflinerDoneForegroundCancel) {
   EXPECT_TRUE(processing_callback_called());
 
   // Verify the request is not removed from the queue, and wait for callbacks.
-  coordinator()->queue()->GetRequests(base::BindOnce(
-      &RequestCoordinatorTest::GetRequestsDone, base::Unretained(this)));
+  queue()->GetRequests(base::BindOnce(&RequestCoordinatorTest::GetRequestsDone,
+                                      base::Unretained(this)));
   PumpLoop();
 
   // Request no longer in the queue (for single attempt policy).
@@ -904,8 +906,8 @@ TEST_F(RequestCoordinatorTest, OfflinerDoneOffliningCancel) {
   EXPECT_TRUE(processing_callback_called());
 
   // Verify the request is not removed from the queue, and wait for callbacks.
-  coordinator()->queue()->GetRequests(base::BindOnce(
-      &RequestCoordinatorTest::GetRequestsDone, base::Unretained(this)));
+  queue()->GetRequests(base::BindOnce(&RequestCoordinatorTest::GetRequestsDone,
+                                      base::Unretained(this)));
   PumpLoop();
 
   // Request still in the queue.
@@ -975,8 +977,8 @@ TEST_F(RequestCoordinatorTest, SchedulerGetsLeastRestrictiveConditions) {
   AddRequest1();
   offline_pages::SavePageRequest request2(kRequestId2, kUrl2, kClientId2,
                                           base::Time::Now(), !kUserRequested);
-  coordinator()->queue()->AddRequest(
-      request2, base::BindOnce(&RequestCoordinatorTest::AddRequestDone,
+  queue()->AddRequest(request2,
+                      base::BindOnce(&RequestCoordinatorTest::AddRequestDone,
                                      base::Unretained(this)));
   PumpLoop();
 
@@ -1193,8 +1195,8 @@ TEST_F(RequestCoordinatorTest,
   // Set request to allow one more completed attempt.
   int max_tries = coordinator()->policy()->GetMaxCompletedTries();
   request.set_completed_attempt_count(max_tries - 1);
-  coordinator()->queue()->AddRequest(
-      request, base::BindOnce(&RequestCoordinatorTest::AddRequestDone,
+  queue()->AddRequest(request,
+                      base::BindOnce(&RequestCoordinatorTest::AddRequestDone,
                                      base::Unretained(this)));
   PumpLoop();
 
@@ -1266,8 +1268,8 @@ TEST_F(RequestCoordinatorTest, TimeBudgetExceeded) {
   offline_pages::SavePageRequest request2(kRequestId1 + 1, kUrl1, kClientId1,
                                           base::Time::Now(), kUserRequested);
   request2.set_completed_attempt_count(kAttemptCount);
-  coordinator()->queue()->AddRequest(
-      request2, base::BindOnce(&RequestCoordinatorTest::AddRequestDone,
+  queue()->AddRequest(request2,
+                      base::BindOnce(&RequestCoordinatorTest::AddRequestDone,
                                      base::Unretained(this)));
   PumpLoop();
 
@@ -1285,8 +1287,8 @@ TEST_F(RequestCoordinatorTest, TimeBudgetExceeded) {
   // TryNextRequest should decide that there is no more work to be done,
   // and call back to the scheduler, even though there is another request in the
   // queue.  Both requests should be left in the queue.
-  coordinator()->queue()->GetRequests(base::BindOnce(
-      &RequestCoordinatorTest::GetRequestsDone, base::Unretained(this)));
+  queue()->GetRequests(base::BindOnce(&RequestCoordinatorTest::GetRequestsDone,
+                                      base::Unretained(this)));
   PumpLoop();
 
   // We should find two requests in the queue.
@@ -1322,8 +1324,8 @@ TEST_F(RequestCoordinatorTest, TryNextRequestWithNoNetwork) {
   EXPECT_FALSE(state() == RequestCoordinatorState::OFFLINING);
 
   // Get queued requests.
-  coordinator()->queue()->GetRequests(base::BindOnce(
-      &RequestCoordinatorTest::GetRequestsDone, base::Unretained(this)));
+  queue()->GetRequests(base::BindOnce(&RequestCoordinatorTest::GetRequestsDone,
+                                      base::Unretained(this)));
   PumpLoop();
 
   // We should find one request in the queue.
@@ -1541,8 +1543,8 @@ TEST_F(RequestCoordinatorTest, SnapshotOnLastTryForScheduledProcessing) {
   // be the last retry.
   int max_tries = coordinator()->policy()->GetMaxCompletedTries();
   request.set_completed_attempt_count(max_tries - 1);
-  coordinator()->queue()->AddRequest(
-      request, base::BindOnce(&RequestCoordinatorTest::AddRequestDone,
+  queue()->AddRequest(request,
+                      base::BindOnce(&RequestCoordinatorTest::AddRequestDone,
                                      base::Unretained(this)));
   PumpLoop();