Adjust scheduling for non-user requested items

components/offline_pages/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/background/request_coordinator.h"
 
 #include <memory>
 #include <utility>
 #include <vector>
 
@@ skipping 223 matching lines @@
 
   const RequestQueue::UpdateMultipleRequestResults& last_remove_results()
       const {
     return last_remove_results_;
   }
 
   void EnableOfflinerCallback(bool enable) {
     offliner_->enable_callback(enable);
   }
 
+  void ScheduleForTest() {
+    coordinator_->ScheduleAsNeeded();
+  }
+
+  void CallRequestNotPicked(bool non_user_requested_tasks_remaining) {
+    coordinator_->RequestNotPicked(non_user_requested_tasks_remaining);
+  }
+
   void SetOfflinerTimeoutForTest(const base::TimeDelta& timeout) {
     coordinator_->SetOfflinerTimeoutForTest(timeout);
   }
 
   void SetDeviceConditionsForTest(DeviceConditions device_conditions) {
     coordinator_->SetDeviceConditionsForTest(device_conditions);
   }
 
   void WaitForCallback() {
     waiter_.Wait();
@@ skipping 130 matching lines @@
   // Check the request queue is as expected.
   EXPECT_EQ(1UL, last_requests().size());
   EXPECT_EQ(kUrl1, last_requests()[0].url());
   EXPECT_EQ(kClientId1, last_requests()[0].client_id());
 
   // Expect that the scheduler got notified.
   SchedulerStub* scheduler_stub = reinterpret_cast<SchedulerStub*>(
       coordinator()->scheduler());
   EXPECT_TRUE(scheduler_stub->schedule_called());
   EXPECT_EQ(coordinator()
-                ->GetTriggerConditionsForUserRequest()
+                ->GetTriggerConditions(last_requests()[0].user_requested())
                 .minimum_battery_percentage,
             scheduler_stub->conditions()->minimum_battery_percentage);
 
   // Check that the observer got the notification that a page is available
   EXPECT_TRUE(observer().added_called());
 }
 
 TEST_F(RequestCoordinatorTest, OfflinerDoneRequestSucceeded) {
   // Add a request to the queue, wait for callbacks to finish.
   offline_pages::SavePageRequest request(
@@ skipping 135 matching lines @@
       &RequestCoordinatorTest::GetRequestsDone, base::Unretained(this)));
   PumpLoop();
 
   // Request no longer in the queue (for single attempt policy).
   EXPECT_EQ(1UL, last_requests().size());
   // Verify foreground cancel not counted as an attempt after all.
   const SavePageRequest& found_request = last_requests().front();
   EXPECT_EQ(0L, found_request.completed_attempt_count());
 }
 
+// If one item completes, and there are no more user requeted items left,
+// we should make a scheduler entry for a non-user requested item.
+TEST_F(RequestCoordinatorTest, RequestNotPickedNonUserRequestedItemsRemain) {
+  // Call start processing just to set up a scheduler callback.
+  DeviceConditions device_conditions(false, 75,
+                                     net::NetworkChangeNotifier::CONNECTION_3G);
+  base::Callback<void(bool)> callback =
+      base::Bind(
+          &RequestCoordinatorTest::EmptyCallbackFunction,
+          base::Unretained(this));
+  coordinator()->StartProcessing(device_conditions, callback);
+
+  // Call RequestNotPicked, and make sure we pick schedule a task for non user
+  // requested conditions.
+  CallRequestNotPicked(true);
+  PumpLoop();
+
+  // The scheduler should have been called to schedule the non-user requested
+  // task.
+  SchedulerStub* scheduler_stub =
+      reinterpret_cast<SchedulerStub*>(coordinator()->scheduler());
+  EXPECT_TRUE(scheduler_stub->schedule_called());

[dougarnett, 2016/08/29 15:47:22]

Seems like we also should check that Unschedule() was called

[Pete Williamson, 2016/09/01 00:10:31]

Done.

+  const Scheduler::TriggerConditions* conditions = scheduler_stub->conditions();
+  EXPECT_EQ(conditions->require_power_connected,
+            coordinator()->policy()->PowerRequired(!kUserRequested));
+  EXPECT_EQ(
+      conditions->minimum_battery_percentage,
+      coordinator()->policy()->BatteryPercentageRequired(!kUserRequested));
+  EXPECT_EQ(conditions->require_unmetered_network,
+            coordinator()->policy()->UnmeteredNetworkRequired(!kUserRequested));
+}
+
+TEST_F(RequestCoordinatorTest, SchedulerGetsLeastRestrictiveConditions) {
+  // Put two requests on the queue - The first is user requested, and
+  // the second is not user requested.
+  offline_pages::SavePageRequest request1(kRequestId1, kUrl1, kClientId1,
+                                          base::Time::Now(), kUserRequested);
+  coordinator()->queue()->AddRequest(
+      request1, base::Bind(&RequestCoordinatorTest::AddRequestDone,
+                           base::Unretained(this)));
+  offline_pages::SavePageRequest request2(kRequestId2, kUrl2, kClientId2,
+                                          base::Time::Now(), !kUserRequested);
+  coordinator()->queue()->AddRequest(
+      request2, base::Bind(&RequestCoordinatorTest::AddRequestDone,
+                           base::Unretained(this)));
+  PumpLoop();
+
+  // Trigger the scheduler to schedule for the least restrictive condition.
+  ScheduleForTest();
+  PumpLoop();
+
+  // Expect that the scheduler got notified, and it is at user_requested
+  // priority.
+  SchedulerStub* scheduler_stub =
+      reinterpret_cast<SchedulerStub*>(coordinator()->scheduler());
+  const Scheduler::TriggerConditions* conditions = scheduler_stub->conditions();
+  EXPECT_TRUE(scheduler_stub->schedule_called());
+  EXPECT_EQ(conditions->require_power_connected,
+            coordinator()->policy()->PowerRequired(kUserRequested));
+  EXPECT_EQ(
+      conditions->minimum_battery_percentage,
+      coordinator()->policy()->BatteryPercentageRequired(kUserRequested));
+  EXPECT_EQ(conditions->require_unmetered_network,
+            coordinator()->policy()->UnmeteredNetworkRequired(kUserRequested));
+}
+
 // This tests a StopProcessing call before we have actually started the
 // prerenderer.
 TEST_F(RequestCoordinatorTest, StartProcessingThenStopProcessingImmediately) {
   // Add a request to the queue, wait for callbacks to finish.
   offline_pages::SavePageRequest request(
       kRequestId1, kUrl1, kClientId1, base::Time::Now(), kUserRequested);
   coordinator()->queue()->AddRequest(
       request,
       base::Bind(&RequestCoordinatorTest::AddRequestDone,
                  base::Unretained(this)));
@@ skipping 268 matching lines @@
   PumpLoop();
 
   EXPECT_TRUE(observer().completed_called());
   EXPECT_EQ(RequestCoordinator::SavePageStatus::REMOVED,
             observer().last_status());
   EXPECT_EQ(1UL, last_remove_results().size());
   EXPECT_EQ(kRequestId1, std::get<0>(last_remove_results().at(0)));
 }
 
 }  // namespace offline_pages