Track original URL when downloading a redirected page

components/offline_pages/core/background/request_coordinator_unittest.cc

Index: components/offline_pages/core/background/request_coordinator_unittest.cc
diff --git a/components/offline_pages/core/background/request_coordinator_unittest.cc b/components/offline_pages/core/background/request_coordinator_unittest.cc
index f5dc20d53511a6962c417c6b612c8f4cfecbb619..e77ff50fbfcae06db53d8e65e045d9e9fee487f0 100644
--- a/components/offline_pages/core/background/request_coordinator_unittest.cc
+++ b/components/offline_pages/core/background/request_coordinator_unittest.cc
@@ -243,6 +243,24 @@ class RequestCoordinatorTest : public testing::Test {
 
   SavePageRequest AddRequest2();
 
+  int64_t SavePageLater() {
+    RequestCoordinator::SavePageLaterParams params;

[fgorski, 2017/02/27 17:34:36]

Please add test for default parameters of save_page_later_params, if you wish to
use that in our code (user_requested and availability is what I would like to
have covered)

[jianli, 2017/02/27 23:20:07]

Done. Added in SavePageLater.

+    params.url = kUrl1;
+    params.client_id = kClientId1;
+    params.user_requested = kUserRequested;
+    return coordinator()->SavePageLater(params);
+  }
+
+  int64_t SavePageLaterWithAvailability(
+      RequestCoordinator::RequestAvailability availability) {
+    RequestCoordinator::SavePageLaterParams params;
+    params.url = kUrl1;
+    params.client_id = kClientId1;
+    params.user_requested = kUserRequested;
+    params.availability = availability;
+    return coordinator()->SavePageLater(params);
+  }
+
   Offliner::RequestStatus last_offlining_status() const {
     return coordinator()->last_offlining_status_;
   }
@@ -429,10 +447,7 @@ TEST_F(RequestCoordinatorTest, StartScheduledProcessingWithNoRequests) {
 
 TEST_F(RequestCoordinatorTest, StartScheduledProcessingWithRequestInProgress) {
   // Start processing for this request.
-  EXPECT_NE(coordinator()->SavePageLater(
-                kUrl1, kClientId1, kUserRequested,
-                RequestCoordinator::RequestAvailability::ENABLED_FOR_OFFLINER),
-            0);
+  EXPECT_NE(0, SavePageLater());
 
   // Ensure that the forthcoming request does not finish - we simulate it being
   // in progress by asking it to skip making the completion callback.
@@ -483,10 +498,7 @@ TEST_F(RequestCoordinatorTest, StartImmediateProcessingWhenDisconnected) {
 
 TEST_F(RequestCoordinatorTest, StartImmediateProcessingWithRequestInProgress) {
   // Start processing for this request.
-  EXPECT_NE(coordinator()->SavePageLater(
-                kUrl1, kClientId1, kUserRequested,
-                RequestCoordinator::RequestAvailability::ENABLED_FOR_OFFLINER),
-            0);
+  EXPECT_NE(0, SavePageLater());
 
   // Disable the automatic offliner callback.
   EnableOfflinerCallback(false);
@@ -512,10 +524,12 @@ TEST_F(RequestCoordinatorTest, SavePageLater) {
   coordinator()->SetInternalStartProcessingCallbackForTest(
       processing_callback());
 
-  EXPECT_NE(coordinator()->SavePageLater(
-                kUrl1, kClientId1, kUserRequested,
-                RequestCoordinator::RequestAvailability::ENABLED_FOR_OFFLINER),
-            0);
+  RequestCoordinator::SavePageLaterParams params;
+  params.url = kUrl1;
+  params.client_id = kClientId1;
+  params.user_requested = kUserRequested;
+  params.original_url = kUrl2;
+  EXPECT_NE(0, coordinator()->SavePageLater(params));
 
   // Expect that a request got placed on the queue.
   coordinator()->queue()->GetRequests(base::Bind(
@@ -529,6 +543,7 @@ TEST_F(RequestCoordinatorTest, SavePageLater) {
   EXPECT_EQ(1UL, last_requests().size());
   EXPECT_EQ(kUrl1, last_requests().at(0)->url());
   EXPECT_EQ(kClientId1, last_requests().at(0)->client_id());
+  EXPECT_EQ(kUrl2, last_requests().at(0)->original_url());
 
   // Expect that the scheduler got notified.
   SchedulerStub* scheduler_stub =
@@ -556,10 +571,7 @@ TEST_F(RequestCoordinatorTest, SavePageLaterFailed) {
   coordinator()->SetInternalStartProcessingCallbackForTest(
       processing_callback());
 
-  EXPECT_TRUE(
-      coordinator()->SavePageLater(
-          kUrl1, kClientId1, kUserRequested,
-          RequestCoordinator::RequestAvailability::ENABLED_FOR_OFFLINER) != 0);
+  EXPECT_NE(0, SavePageLater());
 
   // Expect that a request got placed on the queue.
   coordinator()->queue()->GetRequests(base::Bind(
@@ -1025,8 +1037,7 @@ TEST_F(RequestCoordinatorTest, RemoveInflightRequest) {
 }
 
 TEST_F(RequestCoordinatorTest, MarkRequestCompleted) {
-  int64_t request_id = coordinator()->SavePageLater(
-      kUrl1, kClientId1, kUserRequested,
+  int64_t request_id = SavePageLaterWithAvailability(
       RequestCoordinator::RequestAvailability::DISABLED_FOR_OFFLINER);
   PumpLoop();
   EXPECT_NE(request_id, 0l);
@@ -1049,8 +1060,7 @@ TEST_F(RequestCoordinatorTest, EnableForOffliner) {
   // Pretend we are on low-end device so immediate start won't happen.
   SetIsLowEndDeviceForTest(true);
 
-  int64_t request_id = coordinator()->SavePageLater(
-      kUrl1, kClientId1, kUserRequested,
+  int64_t request_id = SavePageLaterWithAvailability(
       RequestCoordinator::RequestAvailability::DISABLED_FOR_OFFLINER);
   PumpLoop();
   EXPECT_NE(request_id, 0l);
@@ -1117,10 +1127,7 @@ TEST_F(RequestCoordinatorTest, WatchdogTimeoutForImmediateProcessing) {
   // in progress by asking it to skip making the completion callback.
   EnableOfflinerCallback(false);
 
-  EXPECT_NE(coordinator()->SavePageLater(
-                kUrl1, kClientId1, kUserRequested,
-                RequestCoordinator::RequestAvailability::ENABLED_FOR_OFFLINER),
-            0);
+  EXPECT_NE(0, SavePageLater());
   PumpLoop();
 
   // Verify that immediate start from adding the request did happen.
@@ -1308,10 +1315,7 @@ TEST_F(RequestCoordinatorTest,
   // Turn off the callback so that the request stops before processing in
   // PumpLoop.
   EnableOfflinerCallback(false);
-  EXPECT_NE(coordinator()->SavePageLater(
-                kUrl1, kClientId1, kUserRequested,
-                RequestCoordinator::RequestAvailability::ENABLED_FOR_OFFLINER),
-            0);
+  EXPECT_NE(0, SavePageLater());
   PumpLoop();
 
   EXPECT_TRUE(is_busy());
@@ -1324,10 +1328,7 @@ TEST_F(RequestCoordinatorTest,
   EXPECT_FALSE(offline_pages::IsOfflinePagesSvelteConcurrentLoadingEnabled());
 
   // Make a request.
-  EXPECT_NE(coordinator()->SavePageLater(
-                kUrl1, kClientId1, kUserRequested,
-                RequestCoordinator::RequestAvailability::ENABLED_FOR_OFFLINER),
-            0);
+  EXPECT_NE(0, SavePageLater());
   PumpLoop();
 
   // Verify not immediately busy (since low-end device).
@@ -1344,10 +1345,11 @@ TEST_F(RequestCoordinatorTest,
   EnableOfflinerCallback(false);
 
   // Make another request.
-  EXPECT_NE(coordinator()->SavePageLater(
-                kUrl2, kClientId2, kUserRequested,
-                RequestCoordinator::RequestAvailability::ENABLED_FOR_OFFLINER),
-            0);
+  RequestCoordinator::SavePageLaterParams params;
+  params.url = kUrl2;
+  params.client_id = kClientId2;
+  params.user_requested = kUserRequested;
+  EXPECT_NE(0, coordinator()->SavePageLater(params));
   PumpLoop();
 
   // Verify immediate processing did start this time.
@@ -1357,10 +1359,7 @@ TEST_F(RequestCoordinatorTest,
 TEST_F(RequestCoordinatorTest, SavePageDoesntStartProcessingWhenDisconnected) {
   SetNetworkConnected(false);
   EnableOfflinerCallback(false);
-  EXPECT_NE(
-      coordinator()->SavePageLater(
-          kUrl1, kClientId1, kUserRequested,
-          RequestCoordinator::RequestAvailability::ENABLED_FOR_OFFLINER), 0);
+  EXPECT_NE(0, SavePageLater());
   PumpLoop();
   EXPECT_FALSE(is_busy());
 
@@ -1384,10 +1383,7 @@ TEST_F(RequestCoordinatorTest,
   // PumpLoop.
   EnableOfflinerCallback(false);
 
-  EXPECT_NE(coordinator()->SavePageLater(
-                kUrl1, kClientId1, kUserRequested,
-                RequestCoordinator::RequestAvailability::ENABLED_FOR_OFFLINER),
-            0);
+  EXPECT_NE(0, SavePageLater());
   PumpLoop();
   EXPECT_TRUE(is_busy());
 }