[Offline Pages] Defines longer processing budget for immediate bg loads.

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 52d442c5c520164747e36414f44336f820127ca9..583cc62eb7188345520054481fe1eeae5b7a91f4 100644
--- a/components/offline_pages/background/request_coordinator_unittest.cc
+++ b/components/offline_pages/background/request_coordinator_unittest.cc
@@ -262,6 +262,14 @@ class RequestCoordinatorTest
   void GetQueuedRequestsDone(
       std::vector<std::unique_ptr<SavePageRequest>> requests);
 
+  void StartProcessingImmediately() {
+    // Be sure conditions will allow the immediate start.
+    SetIsLowEndDeviceForTest(false);

[Pete Williamson, 2016/10/19 23:18:18]

I think I'd prefer to see this set-up in the test, these methods are supposed to
be simple pass-throughs to the same named method.

I'd even be happy with putting these values into the SetUp routine (and
overriding them as needed).

[dougarnett, 2016/10/19 23:29:12]

Had this all repeated in many tests. I can go back to that. Since they are
related to making the immediate trigger work, I was trying to collect together.

[dougarnett, 2016/10/21 20:01:19]

Dropped the immediate start and reworked common setup for OfflinerDoneCallback
tests into helper method that includes setting some necessary processing state.

+    SetEffectiveConnectionTypeForTest(
+        net::EffectiveConnectionType::EFFECTIVE_CONNECTION_TYPE_3G);
+    coordinator()->StartImmediatelyForTest();
+  }
+
   void SendOfflinerDoneCallback(const SavePageRequest& request,
                                 Offliner::RequestStatus status);
 
@@ -290,6 +298,10 @@ class RequestCoordinatorTest
     coordinator()->SetNetworkConditionsForTest(connection);
   }
 
+  void SetIsLowEndDeviceForTest(bool is_low_end_device) {
+    coordinator()->is_low_end_device_ = is_low_end_device;
+  }
+
   void SetEffectiveConnectionTypeForTest(net::EffectiveConnectionType type) {
     network_quality_estimator_->SetEffectiveConnectionTypeForTest(type);
   }
@@ -554,17 +566,17 @@ TEST_F(RequestCoordinatorTest, OfflinerDoneRequestSucceeded) {
                  base::Unretained(this)));
   PumpLoop();
 
-  // We need to give a callback to the request.
+  // Trigger immediate processing of queued request without automatic callback.
+  EnableOfflinerCallback(false);
+  StartProcessingImmediately();

[Pete Williamson, 2016/10/19 23:18:18]

Why add this?  The more of the chain that we bring into the test, the more
complicated to debug the test becomes. I'm trying to isolate the method under
test as much as possible.  If we need it, that's fine, but I wouldn't want to
add it just to make the tests look similar.

[dougarnett, 2016/10/19 23:29:12]

The motivation was that we now have a processing_state that was not being set up
by this test approach where it is never actually started. This was hitting a
DCHECK that I would like to keep. An alternative might be to provide test access
to directly set/mock the processing_state but seemed fragile path.

+  PumpLoop();
+
+  // Override the processing callback for test visiblity.
   base::Callback<void(bool)> callback =
       base::Bind(&RequestCoordinatorTest::ImmediateScheduleCallbackFunction,
                  base::Unretained(this));
   coordinator()->SetProcessingCallbackForTest(callback);
 
-  // Set up device conditions for the test.
-  DeviceConditions device_conditions(
-      false, 75, net::NetworkChangeNotifier::CONNECTION_3G);
-  SetDeviceConditionsForTest(device_conditions);
-
   // Call the OfflinerDoneCallback to simulate the page being completed, wait
   // for callbacks.
   SendOfflinerDoneCallback(request, Offliner::RequestStatus::SAVED);
@@ -608,17 +620,17 @@ TEST_F(RequestCoordinatorTest, OfflinerDoneRequestFailed) {
                  base::Unretained(this)));
   PumpLoop();
 
-  // We need to give a callback to the request.
+  // Trigger immediate processing of queued request without automatic callback.
+  EnableOfflinerCallback(false);
+  StartProcessingImmediately();

[Pete Williamson, 2016/10/19 23:18:18]

Same question here, and the other instances below.

[dougarnett, 2016/10/21 20:01:19]

Done.

+  PumpLoop();
+
+  // Override the processing callback for test visiblity.
   base::Callback<void(bool)> callback =
       base::Bind(&RequestCoordinatorTest::ImmediateScheduleCallbackFunction,
                  base::Unretained(this));
   coordinator()->SetProcessingCallbackForTest(callback);
 
-  // Set up device conditions for the test.
-  DeviceConditions device_conditions(
-      false, 75, net::NetworkChangeNotifier::CONNECTION_3G);
-  SetDeviceConditionsForTest(device_conditions);
-
   // Call the OfflinerDoneCallback to simulate the request failed, wait
   // for callbacks.
   SendOfflinerDoneCallback(request,
@@ -663,23 +675,23 @@ TEST_F(RequestCoordinatorTest, OfflinerDoneRequestFailedNoRetryFailure) {
                            base::Unretained(this)));
   PumpLoop();
 
-  // We need to give a callback to the request.
+  // Trigger immediate processing of queued request without automatic callback.
+  EnableOfflinerCallback(false);
+  StartProcessingImmediately();
+  PumpLoop();
+
+  // Override the processing callback for test visiblity.
   base::Callback<void(bool)> callback =
       base::Bind(&RequestCoordinatorTest::ImmediateScheduleCallbackFunction,
                  base::Unretained(this));
   coordinator()->SetProcessingCallbackForTest(callback);
 
-  // Set up device conditions for the test.
-  DeviceConditions device_conditions(false, 75,
-                                     net::NetworkChangeNotifier::CONNECTION_3G);
-  SetDeviceConditionsForTest(device_conditions);
-
   // Call the OfflinerDoneCallback to simulate the request failed, wait
   // for callbacks.
   SendOfflinerDoneCallback(
       request, Offliner::RequestStatus::PRERENDERING_FAILED_NO_RETRY);
   PumpLoop();
-  EXPECT_TRUE(immediate_schedule_callback_called());
+  EXPECT_FALSE(immediate_schedule_callback_called());
 
   // TODO(dougarnett): Consider injecting mock RequestPicker for this test
   // and verifying that there is as attempt to pick another request following
@@ -708,17 +720,17 @@ TEST_F(RequestCoordinatorTest, OfflinerDoneForegroundCancel) {
                           base::Unretained(this)));
   PumpLoop();
 
-  // We need to give a callback to the request.
+  // Trigger immediate processing of queued request without automatic callback.
+  EnableOfflinerCallback(false);
+  StartProcessingImmediately();
+  PumpLoop();
+
+  // Override the processing callback for test visiblity.
   base::Callback<void(bool)> callback =
       base::Bind(&RequestCoordinatorTest::ImmediateScheduleCallbackFunction,
                  base::Unretained(this));
   coordinator()->SetProcessingCallbackForTest(callback);
 
-  // Set up device conditions for the test.
-  DeviceConditions device_conditions(false, 75,
-                                     net::NetworkChangeNotifier::CONNECTION_3G);
-  SetDeviceConditionsForTest(device_conditions);
-
   // Call the OfflinerDoneCallback to simulate the request failed, wait
   // for callbacks.
   SendOfflinerDoneCallback(request,
@@ -747,17 +759,17 @@ TEST_F(RequestCoordinatorTest, OfflinerDonePrerenderingCancel) {
                           base::Unretained(this)));
   PumpLoop();
 
-  // We need to give a callback to the request.
+  // Trigger immediate processing of queued request without automatic callback.
+  EnableOfflinerCallback(false);
+  StartProcessingImmediately();
+  PumpLoop();
+
+  // Override the processing callback for test visiblity.
   base::Callback<void(bool)> callback =
       base::Bind(&RequestCoordinatorTest::ImmediateScheduleCallbackFunction,
                  base::Unretained(this));
   coordinator()->SetProcessingCallbackForTest(callback);
 
-  // Set up device conditions for the test.
-  DeviceConditions device_conditions(false, 75,
-                                     net::NetworkChangeNotifier::CONNECTION_3G);
-  SetDeviceConditionsForTest(device_conditions);
-
   // Call the OfflinerDoneCallback to simulate the request failed, wait
   // for callbacks.
   SendOfflinerDoneCallback(request,
@@ -1101,9 +1113,8 @@ TEST_F(RequestCoordinatorTest, WatchdogTimeoutForScheduledProcessing) {
 }
 
 TEST_F(RequestCoordinatorTest, WatchdogTimeoutForImmediateProcessing) {
-  // Test only applies on non-svelte device.
-  if (base::SysInfo::IsLowEndDevice())
-    return;
+  // If low end device, pretend it is not so that immediate start happens.
+  SetIsLowEndDeviceForTest(false);
 
   // Set good network connection so that adding request will trigger
   // immediate processing.