[Offline Pages] Improve RequestCoordinator state tracking.

components/offline_pages/core/background/request_coordinator_unittest.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/background/request_coordinator.h"
 
 #include <memory>
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 111 matching lines @@
   ~RequestCoordinatorTest() override;
 
   void SetUp() override;
 
   void PumpLoop();
 
   RequestCoordinator* coordinator() const {
     return coordinator_taco_->request_coordinator();
   }
 
-  bool is_busy() { return coordinator()->is_busy(); }
+  bool is_busy() {

[Pete Williamson, 2017/03/28 00:35:13]

Better to get these to return the state, and check the state in the tests. 
Sure, it is a bigger change, but makes the test clearer.

[romax, 2017/03/28 01:09:56]

Done.

+    return coordinator()->state() ==
+           RequestCoordinator::RequestCoordinatorState::OFFLINING;
+  }
 
-  bool is_starting() { return coordinator()->is_starting(); }
+  bool is_starting() {
+    return coordinator()->state() ==
+           RequestCoordinator::RequestCoordinatorState::STARTING;
+  }
 
   // Test processing callback function.
   void ProcessingCallbackFunction(bool result) {
     processing_callback_called_ = true;
     processing_callback_result_ = result;
   }
 
   // Callback function which releases a wait for it.
   void WaitingCallbackFunction(bool result) { waiter_.Signal(); }
 
@@ skipping 1019 matching lines @@
       1));
   PumpLoop();
 
   // Wait for timeout to expire.  Use a TaskRunner with a DelayedTaskRunner
   // which won't time out immediately, so the watchdog thread doesn't kill valid
   // tasks too soon.
   WaitForCallback();
   PumpLoop();
 
   EXPECT_FALSE(is_starting());
-  EXPECT_FALSE(coordinator()->is_busy());
+  EXPECT_FALSE(is_busy());
   EXPECT_TRUE(OfflinerWasCanceled());
 }
 
 TEST_F(RequestCoordinatorTest,
        WatchdogTimeoutForImmediateProcessingNoLastSnapshot) {
   // Ensure that the new request does not finish - we simulate it being
   // in progress by asking it to skip making the completion callback.
   EnableOfflinerCallback(false);
 
   EXPECT_NE(0, SavePageLater());
   PumpLoop();
 
   // Verify that immediate start from adding the request did happen.
-  EXPECT_TRUE(coordinator()->is_busy());
+  EXPECT_TRUE(is_busy());
 
   // Advance the mock clock 1 second before the watchdog timeout.
   AdvanceClockBy(base::TimeDelta::FromSeconds(
       coordinator()
           ->policy()
           ->GetSinglePageTimeLimitForImmediateLoadInSeconds() -
       1));
   PumpLoop();
 
   // Verify still busy.
-  EXPECT_TRUE(coordinator()->is_busy());
+  EXPECT_TRUE(is_busy());
   EXPECT_FALSE(OfflinerWasCanceled());
 
   // Advance the mock clock past the watchdog timeout now.
   AdvanceClockBy(base::TimeDelta::FromSeconds(2));
   PumpLoop();
 
   // Verify the request timed out.
   EXPECT_TRUE(OfflinerWasCanceled());
 }
 
@@ skipping 43 matching lines @@
   AddRequest2();
   PumpLoop();
 
   // Set up for the call to StartScheduledProcessing.
   EnableOfflinerCallback(false);
 
   // Sending the request to the offliner.
   EXPECT_TRUE(coordinator()->StartScheduledProcessing(device_conditions(),
                                                       waiting_callback()));
   PumpLoop();
-  EXPECT_TRUE(coordinator()->is_busy());
+  EXPECT_TRUE(is_busy());
 
   // Now lose the network connection.
   SetNetworkConnected(false);
 
   // Complete first request and then TryNextRequest should decide not
   // to pick another request (because of no network connection).
   SendOfflinerDoneCallback(request1, Offliner::RequestStatus::SAVED);
   PumpLoop();
 
   // Not starting nor busy with next request.
-  EXPECT_FALSE(coordinator()->is_starting());
-  EXPECT_FALSE(coordinator()->is_busy());
+  EXPECT_FALSE(is_starting());
+  EXPECT_FALSE(is_busy());
 
   // Get queued requests.
   coordinator()->queue()->GetRequests(base::Bind(
       &RequestCoordinatorTest::GetRequestsDone, base::Unretained(this)));
   PumpLoop();
 
   // We should find one request in the queue.
   EXPECT_EQ(1UL, last_requests().size());
 }
 
@@ skipping 259 matching lines @@
   // Repeat the timeout for MaxCompleteTries - 1 times in order to increase the
   // completed tries on this request.
   int max_tries = coordinator()->policy()->GetMaxCompletedTries();
   for (int i = 0; i < max_tries - 1; i++) {
     PumpLoop();
     // Reset states.
     ResetOfflinerWasCanceled();
     observer().Clear();
 
     // Verify that the request is being processed.
-    EXPECT_TRUE(coordinator()->is_busy());
+    EXPECT_TRUE(is_busy());
 
     // Advance the mock clock 1 second more than the watchdog timeout.
     AdvanceClockBy(base::TimeDelta::FromSeconds(
         coordinator()
             ->policy()
             ->GetSinglePageTimeLimitForImmediateLoadInSeconds() +
         1));
     PumpLoop();
 
     // Verify the request timed out.
@@ skipping 16 matching lines @@
   PumpLoop();
 
   // The last time would trigger the snapshot on last retry and succeed.
   EXPECT_FALSE(OfflinerWasCanceled());
   EXPECT_EQ(RequestCoordinator::BackgroundSavePageResult::SUCCESS,
             observer().last_status());
   EXPECT_TRUE(observer().completed_called());
 }
 
 }  // namespace offline_pages