BASELINE: Update Chromium to 59.0.3071.134

Change-Id: Id02ef6fb2204c5fd21668a1c3e6911c83b17585a
Reviewed-by: Alexandru Croitor <alexandru.croitor@qt.io>

37 files changed, 386 insertions, 253 deletions

diff --git a/chromium/components/offline_pages/OWNERS b/chromium/components/offline_pages/OWNERS
index 394e90ea958..4b3428eacec 100644
--- a/chromium/components/offline_pages/OWNERS
+++ b/chromium/components/offline_pages/OWNERS
@@ -3,3 +3,5 @@ dimich@chromium.org
 fgorski@chromium.org
 jianli@chromium.org
 petewil@chromium.org
+
+# COMPONENT: UI>Browser>Offline
diff --git a/chromium/components/offline_pages/core/background/cleanup_task.cc b/chromium/components/offline_pages/core/background/cleanup_task.cc
index 84e2a7864d4..d66d48b610e 100644
--- a/chromium/components/offline_pages/core/background/cleanup_task.cc
+++ b/chromium/components/offline_pages/core/background/cleanup_task.cc
@@ -56,8 +56,8 @@ void CleanupTask::Prune(
     return;
   }
 
-  // TODO(petewil): Add UMA saying why we remove them
-  // TODO(petewil): Round trip the reason for deleting through the RQ
+  // TODO(petewil): Add UMA saying why we remove them.  Round trip the reason
+  // for deleting through the RQ callbacks.  crbug.com/705115.
   store_->RemoveRequests(expired_request_ids,
                          base::Bind(&CleanupTask::OnRequestsExpired,
                                     weak_ptr_factory_.GetWeakPtr()));
diff --git a/chromium/components/offline_pages/core/background/network_quality_provider_stub.cc b/chromium/components/offline_pages/core/background/network_quality_provider_stub.cc
index aaca69530b1..f4243af9ade 100644
--- a/chromium/components/offline_pages/core/background/network_quality_provider_stub.cc
+++ b/chromium/components/offline_pages/core/background/network_quality_provider_stub.cc
@@ -36,6 +36,14 @@ void NetworkQualityProviderStub::AddEffectiveConnectionTypeObserver(
 void NetworkQualityProviderStub::RemoveEffectiveConnectionTypeObserver(
     net::NetworkQualityEstimator::EffectiveConnectionTypeObserver* observer) {}
 
+void NetworkQualityProviderStub::AddRTTAndThroughputEstimatesObserver(
+    net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver* observer) {
+}
+
+void NetworkQualityProviderStub::RemoveRTTAndThroughputEstimatesObserver(
+    net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver* observer) {
+}
+
 net::EffectiveConnectionType
 NetworkQualityProviderStub::GetEffectiveConnectionType() const {
   return connection_type_;
diff --git a/chromium/components/offline_pages/core/background/network_quality_provider_stub.h b/chromium/components/offline_pages/core/background/network_quality_provider_stub.h
index 1c13a17b3ea..525e1141f39 100644
--- a/chromium/components/offline_pages/core/background/network_quality_provider_stub.h
+++ b/chromium/components/offline_pages/core/background/network_quality_provider_stub.h
@@ -35,6 +35,14 @@ class NetworkQualityProviderStub
       net::NetworkQualityEstimator::EffectiveConnectionTypeObserver* observer)
       override;
 
+  void AddRTTAndThroughputEstimatesObserver(
+      net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver* observer)
+      override;
+
+  void RemoveRTTAndThroughputEstimatesObserver(
+      net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver* observer)
+      override;
+
   void SetEffectiveConnectionTypeForTest(net::EffectiveConnectionType type) {
     connection_type_ = type;
   }
diff --git a/chromium/components/offline_pages/core/background/offliner_policy.h b/chromium/components/offline_pages/core/background/offliner_policy.h
index d828d4c6ea6..f61085b4b51 100644
--- a/chromium/components/offline_pages/core/background/offliner_policy.h
+++ b/chromium/components/offline_pages/core/background/offliner_policy.h
@@ -66,7 +66,7 @@ class OfflinerPolicy {
 
   // TODO(petewil): Numbers here are chosen arbitrarily, do the proper studies
   // to get good policy numbers. Eventually this should get data from a finch
-  // experiment.
+  // experiment. crbug.com/705112.
 
   // Returns true if we should prefer retrying lesser tried requests.
   bool ShouldPreferUntriedRequests() const { return prefer_untried_requests_; }
diff --git a/chromium/components/offline_pages/core/background/pick_request_task.cc b/chromium/components/offline_pages/core/background/pick_request_task.cc
index 006d08b47af..d2faeac9cb1 100644
--- a/chromium/components/offline_pages/core/background/pick_request_task.cc
+++ b/chromium/components/offline_pages/core/background/pick_request_task.cc
@@ -4,6 +4,8 @@
 
 #include "components/offline_pages/core/background/pick_request_task.h"
 
+#include <unordered_set>
+
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/time/time.h"
@@ -35,13 +37,15 @@ PickRequestTask::PickRequestTask(RequestQueueStore* store,
                                  RequestNotPickedCallback not_picked_callback,
                                  RequestCountCallback request_count_callback,
                                  DeviceConditions& device_conditions,
-                                 const std::set<int64_t>& disabled_requests)
+                                 const std::set<int64_t>& disabled_requests,
+                                 std::deque<int64_t>& prioritized_requests)
     : store_(store),
       policy_(policy),
       picked_callback_(picked_callback),
       not_picked_callback_(not_picked_callback),
       request_count_callback_(request_count_callback),
       disabled_requests_(disabled_requests),
+      prioritized_requests_(prioritized_requests),
       weak_ptr_factory_(this) {
   device_conditions_.reset(new DeviceConditions(device_conditions));
 }
@@ -80,24 +84,26 @@ void PickRequestTask::Choose(
   else
     comparator = &PickRequestTask::RecencyFirstCompareFunction;
 
-  // TODO(petewil): Consider replacing this bool with a better named enum.
   bool non_user_requested_tasks_remaining = false;
   bool cleanup_needed = false;
 
   size_t available_request_count = 0;
+  // Request ids which are available for picking.
+  std::unordered_set<int64_t> available_request_ids;
 
-  // Iterate once through the requests, keeping track of best candidate.
-  for (unsigned i = 0; i < requests.size(); ++i) {
+  // Iterate through the requests, filter out unavailable requests and get other
+  // information (if cleanup is needed and number of non-user-requested
+  // requests).
+  for (const auto& request : requests) {
     // If the request is expired or has exceeded the retry count, skip it.
-    if (OfflinerPolicyUtils::CheckRequestExpirationStatus(requests[i].get(),
+    if (OfflinerPolicyUtils::CheckRequestExpirationStatus(request.get(),
                                                           policy_) !=
         OfflinerPolicyUtils::RequestExpirationStatus::VALID) {
       cleanup_needed = true;
       continue;
     }
-
-    // If the  request is on the disabled list, skip it.
-    auto search = disabled_requests_.find(requests[i]->request_id());
+    // If the request is on the disabled list, skip it.
+    auto search = disabled_requests_.find(request->request_id());
     if (search != disabled_requests_.end())
       continue;
 
@@ -106,21 +112,50 @@ void PickRequestTask::Choose(
     // detect if any exist. If we don't find any user-requested tasks, we will
     // inform the "not_picked_callback_" that it needs to schedule a task for
     // non-user-requested items, which have different network and power needs.
-    if (!requests[i]->user_requested())
+    if (!request->user_requested())
       non_user_requested_tasks_remaining = true;
-    if (requests[i]->request_state() ==
-        SavePageRequest::RequestState::AVAILABLE) {
+    if (request->request_state() == SavePageRequest::RequestState::AVAILABLE) {
       available_request_count++;
     }
-    if (!RequestConditionsSatisfied(requests[i].get()))
+    if (!RequestConditionsSatisfied(request.get()))
       continue;
-    if (IsNewRequestBetter(picked_request, requests[i].get(), comparator))
-      picked_request = requests[i].get();
+    available_request_ids.insert(request->request_id());
   }
-
   // Report the request queue counts.
   request_count_callback_.Run(requests.size(), available_request_count);
 
+  // Search for and pick the prioritized request which is available for picking
+  // from |available_request_ids|, the closer to the end means higher priority.
+  // Also if a request in |prioritized_requests_| doesn't exist in |requests|
+  // we're going to remove it.
+  // For every ID in |available_request_ids|, there exists a corresponding
+  // request in |requests|, so this won't be an infinite loop: either we pick a
+  // request, or there's a request being poped from |prioritized_requests_|.
+  while (!picked_request && !prioritized_requests_.empty()) {
+    if (available_request_ids.count(prioritized_requests_.back()) > 0) {
+      for (const auto& request : requests) {
+        if (request->request_id() == prioritized_requests_.back()) {
+          picked_request = request.get();
+          break;
+        }
+      }
+      DCHECK(picked_request);
+    } else {
+      prioritized_requests_.pop_back();
+    }
+  }
+
+  // If no request was found from the priority list, find the best request
+  // according to current policies.
+  if (!picked_request) {
+    for (const auto& request : requests) {
+      if ((available_request_ids.count(request->request_id()) > 0) &&
+          (IsNewRequestBetter(picked_request, request.get(), comparator))) {
+        picked_request = request.get();
+      }
+    }
+  }
+
   // If we have a best request to try next, get the request coodinator to
   // start it.  Otherwise return that we have no candidates.
   if (picked_request != nullptr) {
@@ -161,13 +196,6 @@ bool PickRequestTask::RequestConditionsSatisfied(
   if (request->request_state() == SavePageRequest::RequestState::PAUSED)
     return false;
 
-  // If this request is not active yet, return false.
-  // TODO(petewil): If the only reason we return nothing to do is that we have
-  // inactive requests, we still want to try again later after their activation
-  // time elapses, we shouldn't take ourselves completely off the scheduler.
-  if (request->activation_time() > base::Time::Now())
-    return false;
-
   return true;
 }
 
diff --git a/chromium/components/offline_pages/core/background/pick_request_task.h b/chromium/components/offline_pages/core/background/pick_request_task.h
index ef046124b1e..b6cad2cce18 100644
--- a/chromium/components/offline_pages/core/background/pick_request_task.h
+++ b/chromium/components/offline_pages/core/background/pick_request_task.h
@@ -5,6 +5,7 @@
 #ifndef COMPONENTS_OFFLINE_PAGES_CORE_BACKGROUND_PICK_REQUEST_TASK_H_
 #define COMPONENTS_OFFLINE_PAGES_CORE_BACKGROUND_PICK_REQUEST_TASK_H_
 
+#include <deque>
 #include <set>
 
 #include "base/memory/weak_ptr.h"
@@ -43,7 +44,8 @@ class PickRequestTask : public Task {
                   RequestNotPickedCallback not_picked_callback,
                   RequestCountCallback request_count_callback,
                   DeviceConditions& device_conditions,
-                  const std::set<int64_t>& disabled_requests);
+                  const std::set<int64_t>& disabled_requests,
+                  std::deque<int64_t>& prioritized_requests);
 
   ~PickRequestTask() override;
 
@@ -94,6 +96,7 @@ class PickRequestTask : public Task {
   RequestCountCallback request_count_callback_;
   std::unique_ptr<DeviceConditions> device_conditions_;
   const std::set<int64_t>& disabled_requests_;
+  std::deque<int64_t>& prioritized_requests_;
   // Allows us to pass a weak pointer to callbacks.
   base::WeakPtrFactory<PickRequestTask> weak_ptr_factory_;
 };
diff --git a/chromium/components/offline_pages/core/background/pick_request_task_unittest.cc b/chromium/components/offline_pages/core/background/pick_request_task_unittest.cc
index 1b3ca4642f0..7f686246551 100644
--- a/chromium/components/offline_pages/core/background/pick_request_task_unittest.cc
+++ b/chromium/components/offline_pages/core/background/pick_request_task_unittest.cc
@@ -4,6 +4,7 @@
 
 #include "components/offline_pages/core/background/pick_request_task.h"
 
+#include <deque>
 #include <memory>
 #include <set>
 
@@ -127,6 +128,7 @@ class PickRequestTaskTest : public testing::Test {
   std::unique_ptr<OfflinerPolicy> policy_;
   RequestCoordinatorEventLogger event_logger_;
   std::set<int64_t> disabled_requests_;
+  std::deque<int64_t> prioritized_requests_;
   std::unique_ptr<PickRequestTask> task_;
   bool request_queue_not_picked_called_;
   bool cleanup_needed_;
@@ -215,7 +217,7 @@ void PickRequestTaskTest::MakePickRequestTask() {
                  base::Unretained(this)),
       base::Bind(&PickRequestTaskTest::RequestCountCallback,
                  base::Unretained(this)),
-      conditions, disabled_requests_));
+      conditions, disabled_requests_, prioritized_requests_));
   task_->SetTaskCompletionCallbackForTesting(
       task_runner_.get(),
       base::Bind(&PickRequestTaskTest::TaskCompletionCallback,
@@ -493,4 +495,74 @@ TEST_F(PickRequestTaskTest, ChooseRequestThatIsNotDisabled) {
   EXPECT_TRUE(task_complete_called_);
 }
 
+TEST_F(PickRequestTaskTest, ChoosePrioritizedRequests) {
+  prioritized_requests_.push_back(kRequestId2);
+  MakePickRequestTask();
+
+  base::Time creation_time = base::Time::Now();
+  SavePageRequest request1(kRequestId1, kUrl1, kClientId1, creation_time,
+                           kUserRequested);
+  SavePageRequest request2(kRequestId2, kUrl2, kClientId2, creation_time,
+                           kUserRequested);
+  // Since default policy prefer untried requests, make request1 the favorable
+  // pick if no prioritized requests. But request2 is prioritized so it should
+  // be picked.
+  request2.set_completed_attempt_count(kAttemptCount);
+
+  // Add test requests on the Queue.
+  QueueRequests(request1, request2);
+
+  task()->Run();
+  PumpLoop();
+
+  // Pump the loop again to give the async queue the opportunity to return
+  // results from the Get operation, and for the picker to call the "picked"
+  // callback.
+  PumpLoop();
+
+  EXPECT_EQ(kRequestId2, last_picked_->request_id());
+  EXPECT_FALSE(request_queue_not_picked_called_);
+  EXPECT_EQ(2UL, total_request_count_);
+  EXPECT_EQ(2UL, available_request_count_);
+  EXPECT_TRUE(task_complete_called_);
+  EXPECT_EQ(1UL, prioritized_requests_.size());
+}
+
+TEST_F(PickRequestTaskTest, ChooseFromTwoPrioritizedRequests) {
+  // Make two prioritized requests, the second one should be picked because
+  // higher priority requests are later on the list.
+  prioritized_requests_.push_back(kRequestId1);
+  prioritized_requests_.push_back(kRequestId2);
+  MakePickRequestTask();
+
+  // Making request 1 more attractive to be picked not considering the
+  // prioritizing issues with older creation time, fewer attempt count and it's
+  // earlier in the request queue.
+  base::Time creation_time = base::Time::Now();
+  base::Time older_creation_time =
+      creation_time - base::TimeDelta::FromMinutes(10);
+  SavePageRequest request1(kRequestId1, kUrl1, kClientId1, older_creation_time,
+                           kUserRequested);
+  SavePageRequest request2(kRequestId2, kUrl2, kClientId2, creation_time,
+                           kUserRequested);
+  request2.set_completed_attempt_count(kAttemptCount);
+
+  // Add test requests on the Queue.
+  QueueRequests(request1, request2);
+
+  task()->Run();
+  PumpLoop();
+
+  // Pump the loop again to give the async queue the opportunity to return
+  // results from the Get operation, and for the picker to call the "picked"
+  // callback.
+  PumpLoop();
+
+  EXPECT_EQ(kRequestId2, last_picked_->request_id());
+  EXPECT_FALSE(request_queue_not_picked_called_);
+  EXPECT_EQ(2UL, total_request_count_);
+  EXPECT_EQ(2UL, available_request_count_);
+  EXPECT_TRUE(task_complete_called_);
+  EXPECT_EQ(2UL, prioritized_requests_.size());
+}
 }  // namespace offline_pages
diff --git a/chromium/components/offline_pages/core/background/reconcile_task.cc b/chromium/components/offline_pages/core/background/reconcile_task.cc
index 9b8427530b5..66833e859e4 100644
--- a/chromium/components/offline_pages/core/background/reconcile_task.cc
+++ b/chromium/components/offline_pages/core/background/reconcile_task.cc
@@ -48,8 +48,6 @@ void ReconcileTask::Reconcile(
     if (request->request_state() == SavePageRequest::RequestState::OFFLINING) {
       request->set_request_state(SavePageRequest::RequestState::AVAILABLE);
       items_to_update.push_back(*request.get());
-      // TODO(petewil): Consider adding UMA to see how often chrome gets killed
-      // while processing a request.
     }
   }
 
diff --git a/chromium/components/offline_pages/core/background/request_coordinator.cc b/chromium/components/offline_pages/core/background/request_coordinator.cc
index 1a859fbcf11..167ffa78c7f 100644
--- a/chromium/components/offline_pages/core/background/request_coordinator.cc
+++ b/chromium/components/offline_pages/core/background/request_coordinator.cc
@@ -197,8 +197,7 @@ RequestCoordinator::RequestCoordinator(
     net::NetworkQualityEstimator::NetworkQualityProvider*
         network_quality_estimator)
     : is_low_end_device_(base::SysInfo::IsLowEndDevice()),
-      is_busy_(false),
-      is_starting_(false),
+      state_(RequestCoordinatorState::IDLE),
       processing_state_(ProcessingWindowState::STOPPED),
       use_test_device_conditions_(false),
       offliner_(std::move(offliner)),
@@ -287,7 +286,7 @@ void RequestCoordinator::GetQueuedRequestsCallback(
 void RequestCoordinator::StopPrerendering(
     const Offliner::CancelCallback& final_callback,
     Offliner::RequestStatus stop_status) {
-  if (offliner_ && is_busy_) {
+  if (offliner_ && state_ == RequestCoordinatorState::OFFLINING) {
     DCHECK(active_request_.get());
     offliner_->Cancel(base::Bind(
         &RequestCoordinator::HandleCancelUpdateStatusCallback,
@@ -415,6 +414,15 @@ void RequestCoordinator::RemoveRequests(
 void RequestCoordinator::PauseRequests(
     const std::vector<int64_t>& request_ids) {
   bool canceled = CancelActiveRequestIfItMatches(request_ids);
+
+  // Remove the paused requests from prioritized list.
+  for (int64_t id : request_ids) {
+    auto it = std::find(prioritized_requests_.begin(),
+                        prioritized_requests_.end(), id);
+    if (it != prioritized_requests_.end())
+      prioritized_requests_.erase(it);
+  }
+
   queue_->ChangeRequestsState(
       request_ids, SavePageRequest::RequestState::PAUSED,
       base::Bind(&RequestCoordinator::UpdateMultipleRequestsCallback,
@@ -434,6 +442,8 @@ void RequestCoordinator::PauseRequests(
 
 void RequestCoordinator::ResumeRequests(
     const std::vector<int64_t>& request_ids) {
+  prioritized_requests_.insert(prioritized_requests_.end(), request_ids.begin(),
+                               request_ids.end());
   queue_->ChangeRequestsState(
       request_ids, SavePageRequest::RequestState::AVAILABLE,
       base::Bind(&RequestCoordinator::UpdateMultipleRequestsCallback,
@@ -544,9 +554,9 @@ void RequestCoordinator::UpdateStatusForCancel(
     RecordOfflinerResultUMA(active_request_->client_id(),
                             active_request_->creation_time(),
                             last_offlining_status_);
-    is_busy_ = false;
     active_request_.reset();
   }
+  state_ = RequestCoordinatorState::IDLE;
 }
 
 void RequestCoordinator::ResetActiveRequestCallback(int64_t offline_id) {
@@ -613,7 +623,7 @@ bool RequestCoordinator::StartImmediateProcessing(
 bool RequestCoordinator::StartProcessingInternal(
     const ProcessingWindowState processing_state,
     const base::Callback<void(bool)>& callback) {
-  if (is_starting_ || is_busy_)
+  if (state_ != RequestCoordinatorState::IDLE)
     return false;
   processing_state_ = processing_state;
   scheduler_callback_ = callback;
@@ -636,7 +646,7 @@ RequestCoordinator::TryImmediateStart(
     const base::Callback<void(bool)>& callback) {
   DVLOG(2) << "Immediate " << __func__;
   // Make sure not already busy processing.
-  if (is_busy_)
+  if (state_ == RequestCoordinatorState::OFFLINING)
     return OfflinerImmediateStartStatus::BUSY;
 
   // Make sure we are not on svelte device to start immediately.
@@ -691,13 +701,10 @@ void RequestCoordinator::UpdateCurrentConditionsFromAndroid() {
 }
 
 void RequestCoordinator::TryNextRequest(bool is_start_of_processing) {
-  is_starting_ = true;
+  state_ = RequestCoordinatorState::PICKING;
 
   // If this is the first call, the device conditions are current, no need to
   // update them.
-  // TODO(petewil): Now that we can get conditions any time, consider getting
-  // them now instead of passing them in earlier when we start scheduled
-  // processing.
   if (!is_start_of_processing) {
     // Get current device conditions from the Java side across the bridge.
     // NetworkChangeNotifier will not have the right conditions if chromium is
@@ -726,7 +733,7 @@ void RequestCoordinator::TryNextRequest(bool is_start_of_processing) {
   if (connection_type ==
           net::NetworkChangeNotifier::ConnectionType::CONNECTION_NONE ||
       (base::Time::Now() - operation_start_time_) > processing_time_budget) {
-    is_starting_ = false;
+    state_ = RequestCoordinatorState::IDLE;
 
     // If we were doing immediate processing, try to start it again
     // when we get connected.
@@ -734,37 +741,34 @@ void RequestCoordinator::TryNextRequest(bool is_start_of_processing) {
       RequestConnectedEventForStarting();
 
     // Let the scheduler know we are done processing.
-    // TODO: Make sure the scheduler callback is valid before running it.
     scheduler_callback_.Run(true);
     DVLOG(2) << " out of time, giving up. " << __func__;
 
     return;
   }
 
-  // Ask request queue to make a new PickRequestTask object, then put it on the
-  // task queue.
+  // Ask request queue to make a new PickRequestTask object, then put it on
+  // the task queue.
   queue_->PickNextRequest(
-      policy_.get(), base::Bind(&RequestCoordinator::RequestPicked,
-                                weak_ptr_factory_.GetWeakPtr()),
+      policy_.get(),
+      base::Bind(&RequestCoordinator::RequestPicked,
+                 weak_ptr_factory_.GetWeakPtr()),
       base::Bind(&RequestCoordinator::RequestNotPicked,
                  weak_ptr_factory_.GetWeakPtr()),
       base::Bind(&RequestCoordinator::RequestCounts,
                  weak_ptr_factory_.GetWeakPtr(), is_start_of_processing),
-      *current_conditions_.get(), disabled_requests_);
-  // TODO(petewil): Verify current_conditions has a good value on all calling
-  // paths.  It is really more of a "last known conditions" than "current
-  // conditions".  Consider having a call to Java to check the current
-  // conditions.
+      *current_conditions_.get(), disabled_requests_, prioritized_requests_);
 }
 
 // Called by the request picker when a request has been picked.
 void RequestCoordinator::RequestPicked(const SavePageRequest& request,
                                        bool cleanup_needed) {
   DVLOG(2) << request.url() << " " << __func__;
-  is_starting_ = false;
 
-  // Make sure we were not stopped while picking.
-  if (processing_state_ != ProcessingWindowState::STOPPED) {
+  // Make sure we were not stopped while picking, since any kind of cancel/stop
+  // will reset the state back to IDLE.
+  if (state_ == RequestCoordinatorState::PICKING) {
+    state_ = RequestCoordinatorState::OFFLINING;
     // Send the request on to the offliner.
     SendRequestToOffliner(request);
   }
@@ -778,7 +782,7 @@ void RequestCoordinator::RequestNotPicked(
     bool non_user_requested_tasks_remaining,
     bool cleanup_needed) {
   DVLOG(2) << __func__;
-  is_starting_ = false;
+  state_ = RequestCoordinatorState::IDLE;
 
   // Clear the outstanding "safety" task in the scheduler.
   scheduler_->Unschedule();
@@ -848,21 +852,7 @@ void RequestCoordinator::RequestCounts(bool is_start_of_processing,
 }
 
 void RequestCoordinator::SendRequestToOffliner(const SavePageRequest& request) {
-  // Check that offlining didn't get cancelled while performing some async
-  // steps.
-  if (processing_state_ == ProcessingWindowState::STOPPED)
-    return;
-
-  if (!offliner_) {
-    // TODO(chili,petewil): We should have UMA here to track frequency of this,
-    // if it happens at all.
-    DVLOG(0) << "Offliner crashed. Cannot background offline page.";
-    return;
-  }
-
-  DCHECK(!is_busy_);
-  is_busy_ = true;
-
+  DCHECK(state_ == RequestCoordinatorState::OFFLINING);
   // Record start time if this is first attempt.
   if (request.started_attempt_count() == 0) {
     RecordStartTimeUMA(request);
@@ -884,7 +874,7 @@ void RequestCoordinator::StartOffliner(
       update_result->item_statuses.size() != 1 ||
       update_result->item_statuses.at(0).first != request_id ||
       update_result->item_statuses.at(0).second != ItemActionStatus::SUCCESS) {
-    is_busy_ = false;
+    state_ = RequestCoordinatorState::IDLE;
     StopProcessing(Offliner::QUEUE_UPDATE_FAILED);
     DVLOG(1) << "Failed to mark attempt started: " << request_id;
     UpdateRequestResult request_result =
@@ -924,7 +914,7 @@ void RequestCoordinator::StartOffliner(
     watchdog_timer_.Start(FROM_HERE, timeout, this,
                           &RequestCoordinator::HandleWatchdogTimeout);
   } else {
-    is_busy_ = false;
+    state_ = RequestCoordinatorState::IDLE;
     DVLOG(0) << "Unable to start LoadAndSave";
     StopProcessing(Offliner::LOADING_NOT_ACCEPTED);
 
@@ -947,7 +937,7 @@ void RequestCoordinator::OfflinerDoneCallback(const SavePageRequest& request,
   RecordOfflinerResultUMA(request.client_id(), request.creation_time(),
                           last_offlining_status_);
   watchdog_timer_.Stop();
-  is_busy_ = false;
+  state_ = RequestCoordinatorState::IDLE;
   active_request_.reset(nullptr);
 
   UpdateRequestForCompletedAttempt(request, status);
diff --git a/chromium/components/offline_pages/core/background/request_coordinator.h b/chromium/components/offline_pages/core/background/request_coordinator.h
index d4b91009ebd..412808c9da6 100644
--- a/chromium/components/offline_pages/core/background/request_coordinator.h
+++ b/chromium/components/offline_pages/core/background/request_coordinator.h
@@ -62,6 +62,12 @@ class RequestCoordinator : public KeyedService,
     DISABLED_FOR_OFFLINER,
   };
 
+  enum class RequestCoordinatorState {
+    IDLE,
+    PICKING,
+    OFFLINING,
+  };
+
   // Describes the parameters to control how to save a page when system
   // conditions allow.
   struct SavePageLaterParams {
@@ -204,11 +210,8 @@ class RequestCoordinator : public KeyedService,
     return last_offlining_status_;
   }
 
-  bool is_busy() { return is_busy_; }
-
-  // Returns whether processing is starting (before it is decided to actually
-  // process a request (is_busy()) at this time or not.
-  bool is_starting() { return is_starting_; }
+  // Return the state of the request coordinator.
+  RequestCoordinatorState state() { return state_; }
 
   // Tracks whether the last offlining attempt got canceled.  This is reset by
   // the next call to start processing.
@@ -406,13 +409,8 @@ class RequestCoordinator : public KeyedService,
   // Cached value of whether low end device. Overwritable for testing.
   bool is_low_end_device_;
 
-  // The offliner can only handle one request at a time - if the offliner is
-  // busy, prevent other requests.  This flag marks whether the offliner is in
-  // use.
-  bool is_busy_;
-  // There is more than one path to start processing so this flag is used
-  // to avoid race conditions before is_busy_ is established.
-  bool is_starting_;
+  // Current state of the request coordinator.
+  RequestCoordinatorState state_;
   // Identifies the type of current processing window or if processing stopped.
   ProcessingWindowState processing_state_;
   // True if we should use the test device conditions instead of actual
@@ -461,6 +459,15 @@ class RequestCoordinator : public KeyedService,
   base::OneShotTimer watchdog_timer_;
   // Used for potential immediate processing when we get network connection.
   std::unique_ptr<ConnectionNotifier> connection_notifier_;
+  // Used to track prioritized requests.
+  // The requests can only be added by RC when they are resumed and there are
+  // two places where deletion from the |prioritized_requests_| would happen:
+  //   1. When request is paused RC will remove it.
+  //   2. When a task is not available to be picked by PickRequestTask (because
+  //   it was completed or cancelled), the task will remove it.
+  // Currently it's used as LIFO.
+  // TODO(romax): see if LIFO is a good idea or change to FIFO. crbug.com/705106
+  std::deque<int64_t> prioritized_requests_;
   // Allows us to pass a weak pointer to callbacks.
   base::WeakPtrFactory<RequestCoordinator> weak_ptr_factory_;
 
diff --git a/chromium/components/offline_pages/core/background/request_coordinator_unittest.cc b/chromium/components/offline_pages/core/background/request_coordinator_unittest.cc
index b732165b4e4..b45cf5547f3 100644
--- a/chromium/components/offline_pages/core/background/request_coordinator_unittest.cc
+++ b/chromium/components/offline_pages/core/background/request_coordinator_unittest.cc
@@ -119,6 +119,8 @@ class ObserverStub : public RequestCoordinator::Observer {
 
 class RequestCoordinatorTest : public testing::Test {
  public:
+  using RequestCoordinatorState = RequestCoordinator::RequestCoordinatorState;
+
   RequestCoordinatorTest();
   ~RequestCoordinatorTest() override;
 
@@ -130,9 +132,7 @@ class RequestCoordinatorTest : public testing::Test {
     return coordinator_taco_->request_coordinator();
   }
 
-  bool is_busy() { return coordinator()->is_busy(); }
-
-  bool is_starting() { return coordinator()->is_starting(); }
+  RequestCoordinatorState state() { return coordinator()->state(); }
 
   // Test processing callback function.
   void ProcessingCallbackFunction(bool result) {
@@ -310,6 +310,10 @@ class RequestCoordinatorTest : public testing::Test {
     return coordinator()->disabled_requests_;
   }
 
+  const std::deque<int64_t>& prioritized_requests() {
+    return coordinator()->prioritized_requests_;
+  }
+
  private:
   GetRequestsResult last_get_requests_result_;
   MultipleItemStatuses last_remove_results_;
@@ -489,7 +493,7 @@ TEST_F(RequestCoordinatorTest, StartScheduledProcessingWithRequestInProgress) {
                                                       processing_callback()));
   PumpLoop();
 
-  EXPECT_TRUE(is_busy());
+  EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);
   // Since the offliner is disabled, this callback should not be called.
   EXPECT_FALSE(processing_callback_called());
 
@@ -538,7 +542,7 @@ TEST_F(RequestCoordinatorTest, StartImmediateProcessingWithRequestInProgress) {
   EXPECT_TRUE(coordinator()->StartImmediateProcessing(processing_callback()));
   PumpLoop();
 
-  EXPECT_TRUE(is_busy());
+  EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);
   // Since the offliner is disabled, this callback should not be called.
   EXPECT_FALSE(processing_callback_called());
 
@@ -693,13 +697,13 @@ TEST_F(RequestCoordinatorTest, OfflinerDoneRequestSucceededButLostNetwork) {
   EXPECT_TRUE(processing_callback_called());
 
   // Verify not busy with 2nd request (since no connection).
-  EXPECT_FALSE(is_busy());
+  EXPECT_FALSE(state() == RequestCoordinatorState::OFFLINING);
 
   // Now connect network and verify processing starts.
   SetNetworkConnected(true);
   CallConnectionTypeObserver();
   PumpLoop();
-  EXPECT_TRUE(is_busy());
+  EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);
 }
 
 TEST_F(RequestCoordinatorTest, OfflinerDoneRequestFailed) {
@@ -725,7 +729,7 @@ TEST_F(RequestCoordinatorTest, OfflinerDoneRequestFailed) {
   EXPECT_FALSE(processing_callback_called());
 
   // Busy processing 2nd request.
-  EXPECT_TRUE(is_busy());
+  EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);
 
   coordinator()->queue()->GetRequests(base::Bind(
       &RequestCoordinatorTest::GetRequestsDone, base::Unretained(this)));
@@ -767,7 +771,7 @@ TEST_F(RequestCoordinatorTest, OfflinerDoneRequestFailedNoRetryFailure) {
   EXPECT_FALSE(processing_callback_called());
 
   // Busy processing 2nd request.
-  EXPECT_TRUE(is_busy());
+  EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);
 
   coordinator()->queue()->GetRequests(base::Bind(
       &RequestCoordinatorTest::GetRequestsDone, base::Unretained(this)));
@@ -804,7 +808,7 @@ TEST_F(RequestCoordinatorTest, OfflinerDoneRequestFailedNoNextFailure) {
   EXPECT_TRUE(processing_callback_called());
 
   // Not busy for NO_NEXT failure.
-  EXPECT_FALSE(is_busy());
+  EXPECT_FALSE(state() == RequestCoordinatorState::OFFLINING);
 
   coordinator()->queue()->GetRequests(base::Bind(
       &RequestCoordinatorTest::GetRequestsDone, base::Unretained(this)));
@@ -868,13 +872,13 @@ TEST_F(RequestCoordinatorTest, OfflinerDonePrerenderingCancel) {
 TEST_F(RequestCoordinatorTest, RequestNotPickedDisabledItemsRemain) {
   coordinator()->StartScheduledProcessing(device_conditions(),
                                           processing_callback());
-  EXPECT_TRUE(is_starting());
+  EXPECT_TRUE(state() == RequestCoordinatorState::PICKING);
 
   // Call RequestNotPicked, simulating a request on the disabled list.
   CallRequestNotPicked(false, true);
   PumpLoop();
 
-  EXPECT_FALSE(is_starting());
+  EXPECT_FALSE(state() == RequestCoordinatorState::PICKING);
 
   // The scheduler should have been called to schedule the disabled task for
   // 5 minutes from now.
@@ -889,14 +893,14 @@ TEST_F(RequestCoordinatorTest, RequestNotPickedDisabledItemsRemain) {
 TEST_F(RequestCoordinatorTest, RequestNotPickedNonUserRequestedItemsRemain) {
   coordinator()->StartScheduledProcessing(device_conditions(),
                                           processing_callback());
-  EXPECT_TRUE(is_starting());
+  EXPECT_TRUE(state() == RequestCoordinatorState::PICKING);
 
   // Call RequestNotPicked, and make sure we pick schedule a task for non user
   // requested conditions, with no tasks on the disabled list.
   CallRequestNotPicked(true, false);
   PumpLoop();
 
-  EXPECT_FALSE(is_starting());
+  EXPECT_FALSE(state() == RequestCoordinatorState::PICKING);
   EXPECT_TRUE(processing_callback_called());
 
   // The scheduler should have been called to schedule the non-user requested
@@ -959,7 +963,7 @@ TEST_F(RequestCoordinatorTest, StartScheduledProcessingWithLoadingDisabled) {
   PumpLoop();
   EXPECT_TRUE(processing_callback_called());
 
-  EXPECT_FALSE(is_starting());
+  EXPECT_FALSE(state() == RequestCoordinatorState::PICKING);
   EXPECT_EQ(Offliner::LOADING_NOT_ACCEPTED, last_offlining_status());
 }
 
@@ -975,7 +979,7 @@ TEST_F(RequestCoordinatorTest,
 
   EXPECT_TRUE(coordinator()->StartScheduledProcessing(device_conditions(),
                                                       processing_callback()));
-  EXPECT_TRUE(is_starting());
+  EXPECT_TRUE(state() == RequestCoordinatorState::PICKING);
 
   // Now, quick, before it can do much (we haven't called PumpLoop), cancel it.
   coordinator()->StopProcessing(Offliner::REQUEST_COORDINATOR_CANCELED);
@@ -984,7 +988,7 @@ TEST_F(RequestCoordinatorTest,
   PumpLoop();
   EXPECT_TRUE(processing_callback_called());
 
-  EXPECT_FALSE(is_starting());
+  EXPECT_FALSE(state() == RequestCoordinatorState::PICKING);
 
   // OfflinerDoneCallback will not end up getting called with status SAVED,
   // since we cancelled the event before it called offliner_->LoadAndSave().
@@ -1007,7 +1011,7 @@ TEST_F(RequestCoordinatorTest,
 
   EXPECT_TRUE(coordinator()->StartScheduledProcessing(device_conditions(),
                                                       processing_callback()));
-  EXPECT_TRUE(is_starting());
+  EXPECT_TRUE(state() == RequestCoordinatorState::PICKING);
 
   // Let all the async parts of the start processing pipeline run to completion.
   PumpLoop();
@@ -1021,8 +1025,8 @@ TEST_F(RequestCoordinatorTest,
   EXPECT_FALSE(processing_callback_called());
 
   // Coordinator should now be busy.
-  EXPECT_TRUE(is_busy());
-  EXPECT_FALSE(is_starting());
+  EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);
+  EXPECT_FALSE(state() == RequestCoordinatorState::PICKING);
 
   // Now we cancel it while the prerenderer is busy.
   coordinator()->StopProcessing(Offliner::REQUEST_COORDINATOR_CANCELED);
@@ -1035,7 +1039,7 @@ TEST_F(RequestCoordinatorTest,
   EXPECT_EQ(SavePageRequest::RequestState::AVAILABLE, observer().state());
   observer().Clear();
 
-  EXPECT_FALSE(is_busy());
+  EXPECT_FALSE(state() == RequestCoordinatorState::OFFLINING);
 
   // OfflinerDoneCallback will not end up getting called with status SAVED,
   // since we cancelled the event before the LoadAndSave completed.
@@ -1167,8 +1171,8 @@ TEST_F(RequestCoordinatorTest,
   WaitForCallback();
   PumpLoop();
 
-  EXPECT_FALSE(is_starting());
-  EXPECT_FALSE(coordinator()->is_busy());
+  EXPECT_FALSE(state() == RequestCoordinatorState::PICKING);
+  EXPECT_FALSE(state() == RequestCoordinatorState::OFFLINING);
   EXPECT_TRUE(OfflinerWasCanceled());
 }
 
@@ -1182,7 +1186,7 @@ TEST_F(RequestCoordinatorTest,
   PumpLoop();
 
   // Verify that immediate start from adding the request did happen.
-  EXPECT_TRUE(coordinator()->is_busy());
+  EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);
 
   // Advance the mock clock 1 second before the watchdog timeout.
   AdvanceClockBy(base::TimeDelta::FromSeconds(
@@ -1193,7 +1197,7 @@ TEST_F(RequestCoordinatorTest,
   PumpLoop();
 
   // Verify still busy.
-  EXPECT_TRUE(coordinator()->is_busy());
+  EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);
   EXPECT_FALSE(OfflinerWasCanceled());
 
   // Advance the mock clock past the watchdog timeout now.
@@ -1253,7 +1257,7 @@ TEST_F(RequestCoordinatorTest, TryNextRequestWithNoNetwork) {
   EXPECT_TRUE(coordinator()->StartScheduledProcessing(device_conditions(),
                                                       waiting_callback()));
   PumpLoop();
-  EXPECT_TRUE(coordinator()->is_busy());
+  EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);
 
   // Now lose the network connection.
   SetNetworkConnected(false);
@@ -1264,8 +1268,8 @@ TEST_F(RequestCoordinatorTest, TryNextRequestWithNoNetwork) {
   PumpLoop();
 
   // Not starting nor busy with next request.
-  EXPECT_FALSE(coordinator()->is_starting());
-  EXPECT_FALSE(coordinator()->is_busy());
+  EXPECT_FALSE(state() == RequestCoordinatorState::PICKING);
+  EXPECT_FALSE(state() == RequestCoordinatorState::OFFLINING);
 
   // Get queued requests.
   coordinator()->queue()->GetRequests(base::Bind(
@@ -1369,7 +1373,7 @@ TEST_F(RequestCoordinatorTest,
   EXPECT_NE(0, SavePageLater());
   PumpLoop();
 
-  EXPECT_TRUE(is_busy());
+  EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);
 }
 
 TEST_F(RequestCoordinatorTest,
@@ -1383,7 +1387,7 @@ TEST_F(RequestCoordinatorTest,
   PumpLoop();
 
   // Verify not immediately busy (since low-end device).
-  EXPECT_FALSE(is_busy());
+  EXPECT_FALSE(state() == RequestCoordinatorState::OFFLINING);
 
   // Set feature flag to allow concurrent loads.
   base::test::ScopedFeatureList scoped_feature_list;
@@ -1404,7 +1408,7 @@ TEST_F(RequestCoordinatorTest,
   PumpLoop();
 
   // Verify immediate processing did start this time.
-  EXPECT_TRUE(is_busy());
+  EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);
 }
 
 TEST_F(RequestCoordinatorTest, SavePageDoesntStartProcessingWhenDisconnected) {
@@ -1412,13 +1416,13 @@ TEST_F(RequestCoordinatorTest, SavePageDoesntStartProcessingWhenDisconnected) {
   EnableOfflinerCallback(false);
   EXPECT_NE(0, SavePageLater());
   PumpLoop();
-  EXPECT_FALSE(is_busy());
+  EXPECT_FALSE(state() == RequestCoordinatorState::OFFLINING);
 
   // Now connect network and verify processing starts.
   SetNetworkConnected(true);
   CallConnectionTypeObserver();
   PumpLoop();
-  EXPECT_TRUE(is_busy());
+  EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);
 }
 
 TEST_F(RequestCoordinatorTest,
@@ -1436,7 +1440,7 @@ TEST_F(RequestCoordinatorTest,
 
   EXPECT_NE(0, SavePageLater());
   PumpLoop();
-  EXPECT_TRUE(is_busy());
+  EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);
 }
 
 TEST_F(RequestCoordinatorTest,
@@ -1451,7 +1455,7 @@ TEST_F(RequestCoordinatorTest,
   // Add a request to the queue.
   AddRequest1();
   PumpLoop();
-  EXPECT_FALSE(is_busy());
+  EXPECT_FALSE(state() == RequestCoordinatorState::OFFLINING);
 
   // Pause the request.
   std::vector<int64_t> request_ids;
@@ -1462,21 +1466,24 @@ TEST_F(RequestCoordinatorTest,
   // Resume the request while disconnected.
   coordinator()->ResumeRequests(request_ids);
   PumpLoop();
-  EXPECT_FALSE(is_busy());
+  EXPECT_FALSE(state() == RequestCoordinatorState::OFFLINING);
+  EXPECT_EQ(1UL, prioritized_requests().size());
 
   // Pause the request again.
   coordinator()->PauseRequests(request_ids);
   PumpLoop();
+  EXPECT_EQ(0UL, prioritized_requests().size());
 
   // Now simulate reasonable connection.
   SetNetworkConnected(true);
 
   // Resume the request while connected.
   coordinator()->ResumeRequests(request_ids);
-  EXPECT_FALSE(is_busy());
+  EXPECT_FALSE(state() == RequestCoordinatorState::OFFLINING);
   PumpLoop();
+  EXPECT_EQ(1UL, prioritized_requests().size());
 
-  EXPECT_TRUE(is_busy());
+  EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);
 }
 
 TEST_F(RequestCoordinatorTest, SnapshotOnLastTryForScheduledProcessing) {
@@ -1542,7 +1549,7 @@ TEST_F(RequestCoordinatorTest, SnapshotOnLastTryForImmediateProcessing) {
     observer().Clear();
 
     // Verify that the request is being processed.
-    EXPECT_TRUE(coordinator()->is_busy());
+    EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);
 
     // Advance the mock clock 1 second more than the watchdog timeout.
     AdvanceClockBy(base::TimeDelta::FromSeconds(
diff --git a/chromium/components/offline_pages/core/background/request_queue.cc b/chromium/components/offline_pages/core/background/request_queue.cc
index 406f0ae0ce4..bac52ae5abb 100644
--- a/chromium/components/offline_pages/core/background/request_queue.cc
+++ b/chromium/components/offline_pages/core/background/request_queue.cc
@@ -31,10 +31,6 @@ void GetRequestsDone(const RequestQueue::GetRequestsCallback& callback,
                      std::vector<std::unique_ptr<SavePageRequest>> requests) {
   GetRequestsResult result =
       success ? GetRequestsResult::SUCCESS : GetRequestsResult::STORE_FAILURE;
-  // TODO(fgorski): Filter out expired requests based on policy.
-  // This may trigger the purging if necessary.
-  // Also this may be turned into a method on the request queue or add a policy
-  // parameter in the process.
   callback.Run(result, std::move(requests));
 }
 
@@ -78,8 +74,6 @@ void RequestQueue::GetRequests(const GetRequestsCallback& callback) {
 
 void RequestQueue::AddRequest(const SavePageRequest& request,
                               const AddRequestCallback& callback) {
-  // TODO(fgorski): check that request makes sense.
-  // TODO(fgorski): check that request does not violate policy.
   std::unique_ptr<AddRequestTask> task(new AddRequestTask(
       store_.get(), request, base::Bind(&AddRequestDone, callback, request)));
   task_queue_.AddTask(std::move(task));
@@ -128,11 +122,13 @@ void RequestQueue::PickNextRequest(
     PickRequestTask::RequestNotPickedCallback not_picked_callback,
     PickRequestTask::RequestCountCallback request_count_callback,
     DeviceConditions& conditions,
-    std::set<int64_t>& disabled_requests) {
+    std::set<int64_t>& disabled_requests,
+    std::deque<int64_t>& prioritized_requests) {
   // Using the PickerContext, create a picker task.
-  std::unique_ptr<Task> task(new PickRequestTask(
-      store_.get(), policy, picked_callback, not_picked_callback,
-      request_count_callback, conditions, disabled_requests));
+  std::unique_ptr<Task> task(
+      new PickRequestTask(store_.get(), policy, picked_callback,
+                          not_picked_callback, request_count_callback,
+                          conditions, disabled_requests, prioritized_requests));
 
   // Queue up the picking task, it will call one of the callbacks when it
   // completes.
diff --git a/chromium/components/offline_pages/core/background/request_queue.h b/chromium/components/offline_pages/core/background/request_queue.h
index 8325dd07173..d0c66a7ad48 100644
--- a/chromium/components/offline_pages/core/background/request_queue.h
+++ b/chromium/components/offline_pages/core/background/request_queue.h
@@ -7,6 +7,7 @@
 
 #include <stdint.h>
 
+#include <deque>
 #include <memory>
 #include <set>
 #include <string>
@@ -95,7 +96,8 @@ class RequestQueue {
       PickRequestTask::RequestNotPickedCallback not_picked_callback,
       PickRequestTask::RequestCountCallback request_count_callback,
       DeviceConditions& conditions,
-      std::set<int64_t>& disabled_requests);
+      std::set<int64_t>& disabled_requests,
+      std::deque<int64_t>& prioritized_requests);
 
   // Reconcile any requests that were active the last time chrome exited.
   void ReconcileRequests(const UpdateCallback& callback);
diff --git a/chromium/components/offline_pages/core/background/request_queue_store_sql.cc b/chromium/components/offline_pages/core/background/request_queue_store_sql.cc
index 099f774180f..16a13e49c6b 100644
--- a/chromium/components/offline_pages/core/background/request_queue_store_sql.cc
+++ b/chromium/components/offline_pages/core/background/request_queue_store_sql.cc
@@ -97,7 +97,7 @@ bool CreateSchema(sql::Connection* db) {
       return false;
   }
 
-  // TODO(fgorski): Add indices here.
+  // This would be a great place to add indices when we need them.
   return transaction.Commit();
 }
 
@@ -109,8 +109,6 @@ std::unique_ptr<SavePageRequest> MakeSavePageRequest(
   const int64_t id = statement.ColumnInt64(0);
   const base::Time creation_time =
       base::Time::FromInternalValue(statement.ColumnInt64(1));
-  const base::Time activation_time =
-      base::Time::FromInternalValue(statement.ColumnInt64(2));
   const base::Time last_attempt_time =
       base::Time::FromInternalValue(statement.ColumnInt64(3));
   const int64_t started_attempt_count = statement.ColumnInt64(4);
@@ -127,8 +125,8 @@ std::unique_ptr<SavePageRequest> MakeSavePageRequest(
            << " creation time " << creation_time << " user requested "
            << kUserRequested << " original_url " << original_url;
 
-  std::unique_ptr<SavePageRequest> request(new SavePageRequest(
-      id, url, client_id, creation_time, activation_time, kUserRequested));
+  std::unique_ptr<SavePageRequest> request(
+      new SavePageRequest(id, url, client_id, creation_time, kUserRequested));
   request->set_last_attempt_time(last_attempt_time);
   request->set_started_attempt_count(started_attempt_count);
   request->set_completed_attempt_count(completed_attempt_count);
@@ -178,7 +176,7 @@ ItemActionStatus Insert(sql::Connection* db, const SavePageRequest& request) {
   sql::Statement statement(db->GetCachedStatement(SQL_FROM_HERE, kSql));
   statement.BindInt64(0, request.request_id());
   statement.BindInt64(1, request.creation_time().ToInternalValue());
-  statement.BindInt64(2, request.activation_time().ToInternalValue());
+  statement.BindInt64(2, 0);
   statement.BindInt64(3, request.last_attempt_time().ToInternalValue());
   statement.BindInt64(4, request.started_attempt_count());
   statement.BindInt64(5, request.completed_attempt_count());
@@ -205,7 +203,7 @@ ItemActionStatus Update(sql::Connection* db, const SavePageRequest& request) {
 
   sql::Statement statement(db->GetCachedStatement(SQL_FROM_HERE, kSql));
   statement.BindInt64(0, request.creation_time().ToInternalValue());
-  statement.BindInt64(1, request.activation_time().ToInternalValue());
+  statement.BindInt64(1, 0);
   statement.BindInt64(2, request.last_attempt_time().ToInternalValue());
   statement.BindInt64(3, request.started_attempt_count());
   statement.BindInt64(4, request.completed_attempt_count());
@@ -294,7 +292,6 @@ void GetRequestsByIdsSync(sql::Connection* db,
                           scoped_refptr<base::SingleThreadTaskRunner> runner,
                           const std::vector<int64_t>& request_ids,
                           const RequestQueueStore::UpdateCallback& callback) {
-  // TODO(fgorski): Perhaps add metrics here.
   std::unique_ptr<UpdateRequestsResult> result(
       new UpdateRequestsResult(StoreState::LOADED));
 
@@ -332,7 +329,6 @@ void AddRequestSync(sql::Connection* db,
                     scoped_refptr<base::SingleThreadTaskRunner> runner,
                     const SavePageRequest& request,
                     const RequestQueueStore::AddCallback& callback) {
-  // TODO(fgorski): add UMA metrics here.
   ItemActionStatus status = Insert(db, request);
   runner->PostTask(FROM_HERE, base::Bind(callback, status));
 }
@@ -341,7 +337,6 @@ void UpdateRequestsSync(sql::Connection* db,
                         scoped_refptr<base::SingleThreadTaskRunner> runner,
                         const std::vector<SavePageRequest>& requests,
                         const RequestQueueStore::UpdateCallback& callback) {
-  // TODO(fgorski): add UMA metrics here.
   std::unique_ptr<UpdateRequestsResult> result(
       new UpdateRequestsResult(StoreState::LOADED));
 
@@ -371,7 +366,6 @@ void RemoveRequestsSync(sql::Connection* db,
                         scoped_refptr<base::SingleThreadTaskRunner> runner,
                         const std::vector<int64_t>& request_ids,
                         const RequestQueueStore::UpdateCallback& callback) {
-  // TODO(fgorski): Perhaps add metrics here.
   std::unique_ptr<UpdateRequestsResult> result(
       new UpdateRequestsResult(StoreState::LOADED));
 
diff --git a/chromium/components/offline_pages/core/background/request_queue_store_sql.h b/chromium/components/offline_pages/core/background/request_queue_store_sql.h
index 6bd9b9a4ccd..7c694f39db7 100644
--- a/chromium/components/offline_pages/core/background/request_queue_store_sql.h
+++ b/chromium/components/offline_pages/core/background/request_queue_store_sql.h
@@ -24,6 +24,17 @@ class Connection;
 namespace offline_pages {
 
 // SQLite implementation of RequestQueueStore.
+//
+// This store has a history of schema updates.
+// Original schema was delivered in M57. Since then the following changes
+// happened:
+// * In M58 original_url was added.
+//
+// TODO(romax): remove all activation_time related code the next we change the
+// schema.
+//
+// Looking for procesure to update the schema, please refer to
+// offline_page_metadata_store_sql.h
 class RequestQueueStoreSQL : public RequestQueueStore {
  public:
   RequestQueueStoreSQL(
diff --git a/chromium/components/offline_pages/core/background/request_queue_store_unittest.cc b/chromium/components/offline_pages/core/background/request_queue_store_unittest.cc
index bf78969fc92..9b0009fde61 100644
--- a/chromium/components/offline_pages/core/background/request_queue_store_unittest.cc
+++ b/chromium/components/offline_pages/core/background/request_queue_store_unittest.cc
@@ -428,16 +428,13 @@ TYPED_TEST(RequestQueueStoreTest, UpdateRequest) {
 
   base::Time new_creation_time =
       creation_time + base::TimeDelta::FromMinutes(1);
-  base::Time activation_time = creation_time + base::TimeDelta::FromHours(6);
   // Try updating an existing request.
   SavePageRequest updated_request(kRequestId, kUrl, kClientId,
-                                  new_creation_time, activation_time,
-                                  kUserRequested);
+                                  new_creation_time, kUserRequested);
   updated_request.set_original_url(kUrl2);
   // Try to update a non-existing request.
   SavePageRequest updated_request2(kRequestId2, kUrl, kClientId,
-                                   new_creation_time, activation_time,
-                                   kUserRequested);
+                                   new_creation_time, kUserRequested);
   std::vector<SavePageRequest> requests_to_update{updated_request,
                                                   updated_request2};
   store->UpdateRequests(
diff --git a/chromium/components/offline_pages/core/background/request_queue_unittest.cc b/chromium/components/offline_pages/core/background/request_queue_unittest.cc
index ed9054bc6c0..24903504eaf 100644
--- a/chromium/components/offline_pages/core/background/request_queue_unittest.cc
+++ b/chromium/components/offline_pages/core/background/request_queue_unittest.cc
@@ -85,7 +85,6 @@ class RequestNotifierStub : public RequestNotifier {
   int32_t total_expired_requests_;
 };
 
-// TODO(fgorski): Add tests for store failures in add/remove/get.
 class RequestQueueTest : public testing::Test {
  public:
   RequestQueueTest();
diff --git a/chromium/components/offline_pages/core/background/save_page_request.cc b/chromium/components/offline_pages/core/background/save_page_request.cc
index 9b19dc4d429..d95df18069e 100644
--- a/chromium/components/offline_pages/core/background/save_page_request.cc
+++ b/chromium/components/offline_pages/core/background/save_page_request.cc
@@ -15,23 +15,6 @@ SavePageRequest::SavePageRequest(int64_t request_id,
       url_(url),
       client_id_(client_id),
       creation_time_(creation_time),
-      activation_time_(creation_time),
-      started_attempt_count_(0),
-      completed_attempt_count_(0),
-      user_requested_(user_requested),
-      state_(RequestState::AVAILABLE) {}
-
-SavePageRequest::SavePageRequest(int64_t request_id,
-                                 const GURL& url,
-                                 const ClientId& client_id,
-                                 const base::Time& creation_time,
-                                 const base::Time& activation_time,
-                                 const bool user_requested)
-    : request_id_(request_id),
-      url_(url),
-      client_id_(client_id),
-      creation_time_(creation_time),
-      activation_time_(activation_time),
       started_attempt_count_(0),
       completed_attempt_count_(0),
       user_requested_(user_requested),
@@ -42,7 +25,6 @@ SavePageRequest::SavePageRequest(const SavePageRequest& other)
       url_(other.url_),
       client_id_(other.client_id_),
       creation_time_(other.creation_time_),
-      activation_time_(other.activation_time_),
       started_attempt_count_(other.started_attempt_count_),
       completed_attempt_count_(other.completed_attempt_count_),
       last_attempt_time_(other.last_attempt_time_),
@@ -56,7 +38,6 @@ bool SavePageRequest::operator==(const SavePageRequest& other) const {
   return request_id_ == other.request_id_ && url_ == other.url_ &&
          client_id_ == other.client_id_ &&
          creation_time_ == other.creation_time_ &&
-         activation_time_ == other.activation_time_ &&
          started_attempt_count_ == other.started_attempt_count_ &&
          completed_attempt_count_ == other.completed_attempt_count_ &&
          last_attempt_time_ == other.last_attempt_time_ &&
@@ -64,10 +45,6 @@ bool SavePageRequest::operator==(const SavePageRequest& other) const {
 }
 
 void SavePageRequest::MarkAttemptStarted(const base::Time& start_time) {
-  DCHECK_LE(activation_time_, start_time);
-  // TODO(fgorski): As part of introducing policy in GetStatus, we can make a
-  // check here to ensure we only start tasks in status pending, and bail out in
-  // other cases.
   last_attempt_time_ = start_time;
   ++started_attempt_count_;
   state_ = RequestState::OFFLINING;
diff --git a/chromium/components/offline_pages/core/background/save_page_request.h b/chromium/components/offline_pages/core/background/save_page_request.h
index b1bd54e806b..52dee9556a1 100644
--- a/chromium/components/offline_pages/core/background/save_page_request.h
+++ b/chromium/components/offline_pages/core/background/save_page_request.h
@@ -28,12 +28,6 @@ class SavePageRequest {
                   const ClientId& client_id,
                   const base::Time& creation_time,
                   const bool user_requested);
-  SavePageRequest(int64_t request_id,
-                  const GURL& url,
-                  const ClientId& client_id,
-                  const base::Time& creation_time,
-                  const base::Time& activation_time,
-                  const bool user_requested);
   SavePageRequest(const SavePageRequest& other);
   ~SavePageRequest();
 
@@ -64,8 +58,6 @@ class SavePageRequest {
 
   const base::Time& creation_time() const { return creation_time_; }
 
-  const base::Time& activation_time() const { return activation_time_; }
-
   int64_t started_attempt_count() const { return started_attempt_count_; }
   void set_started_attempt_count(int64_t started_attempt_count) {
     started_attempt_count_ = started_attempt_count;
@@ -106,9 +98,6 @@ class SavePageRequest {
   // Time when this request was created. (Alternative 2).
   base::Time creation_time_;
 
-  // Time when this request will become active.
-  base::Time activation_time_;
-
   // Number of attempts started to get the page.  This may be different than the
   // number of attempts completed because we could crash.
   int started_attempt_count_;
diff --git a/chromium/components/offline_pages/core/background/save_page_request_unittest.cc b/chromium/components/offline_pages/core/background/save_page_request_unittest.cc
index ddfc2b990b7..df073bd61e3 100644
--- a/chromium/components/offline_pages/core/background/save_page_request_unittest.cc
+++ b/chromium/components/offline_pages/core/background/save_page_request_unittest.cc
@@ -32,7 +32,6 @@ TEST_F(SavePageRequestTest, CreatePendingReqeust) {
   EXPECT_EQ(kUrl, request.url());
   EXPECT_EQ(kClientId, request.client_id());
   EXPECT_EQ(creation_time, request.creation_time());
-  EXPECT_EQ(creation_time, request.activation_time());
   EXPECT_EQ(base::Time(), request.last_attempt_time());
   EXPECT_EQ(0, request.completed_attempt_count());
   EXPECT_EQ(SavePageRequest::RequestState::AVAILABLE, request.request_state());
@@ -43,11 +42,10 @@ TEST_F(SavePageRequestTest, CreatePendingReqeust) {
 
 TEST_F(SavePageRequestTest, StartAndCompleteRequest) {
   base::Time creation_time = base::Time::Now();
-  base::Time activation_time = creation_time + base::TimeDelta::FromHours(6);
   SavePageRequest request(kRequestId, kUrl, kClientId, creation_time,
-                          activation_time, kUserRequested);
+                          kUserRequested);
 
-  base::Time start_time = activation_time + base::TimeDelta::FromHours(3);
+  base::Time start_time = creation_time + base::TimeDelta::FromHours(3);
   request.MarkAttemptStarted(start_time);
 
   // Most things don't change about the request.
@@ -55,7 +53,6 @@ TEST_F(SavePageRequestTest, StartAndCompleteRequest) {
   EXPECT_EQ(kUrl, request.url());
   EXPECT_EQ(kClientId, request.client_id());
   EXPECT_EQ(creation_time, request.creation_time());
-  EXPECT_EQ(activation_time, request.activation_time());
 
   // Attempt time, attempt count and status will though.
   EXPECT_EQ(start_time, request.last_attempt_time());
@@ -69,7 +66,6 @@ TEST_F(SavePageRequestTest, StartAndCompleteRequest) {
   EXPECT_EQ(kUrl, request.url());
   EXPECT_EQ(kClientId, request.client_id());
   EXPECT_EQ(creation_time, request.creation_time());
-  EXPECT_EQ(activation_time, request.activation_time());
 
   // Last attempt time and status are updated.
   EXPECT_EQ(1, request.completed_attempt_count());
diff --git a/chromium/components/offline_pages/core/offline_page_archiver.h b/chromium/components/offline_pages/core/offline_page_archiver.h
index 17631406db3..cfada7b4dea 100644
--- a/chromium/components/offline_pages/core/offline_page_archiver.h
+++ b/chromium/components/offline_pages/core/offline_page_archiver.h
@@ -39,12 +39,6 @@ namespace offline_pages {
 // archiver whether to respond with ERROR_CONTENT_UNAVAILBLE, wait longer to
 // actually snapshot a complete page, or snapshot whatever is available at that
 // point in time (what the user sees).
-//
-// TODO(fgorski): Add ability to delete archive.
-// TODO(fgorski): Add ability to check that archive exists.
-// TODO(fgorski): Add ability to refresh an existing archive in one step.
-// TODO(fgorski): Add ability to identify all of the archives in the directory,
-// to enable to model to reconcile the archives.
 class OfflinePageArchiver {
  public:
   // Results of the archive creation.
@@ -56,6 +50,8 @@ class OfflinePageArchiver {
     ERROR_ARCHIVE_CREATION_FAILED,  // Creation of archive failed.
     ERROR_SECURITY_CERTIFICATE,     // Page was loaded on secure connection, but
                                     // there was a security error.
+    ERROR_ERROR_PAGE,               // We detected an error page.
+    ERROR_INTERSTITIAL_PAGE,        // We detected an interstitial page.
   };
 
   // Describes the parameters to control how to create an archive.
diff --git a/chromium/components/offline_pages/core/offline_page_feature.cc b/chromium/components/offline_pages/core/offline_page_feature.cc
index 9f283129f19..9dfab58bfca 100644
--- a/chromium/components/offline_pages/core/offline_page_feature.cc
+++ b/chromium/components/offline_pages/core/offline_page_feature.cc
@@ -36,12 +36,18 @@ const base::Feature kOfflinePagesSharingFeature{
 const base::Feature kOfflinePagesSvelteConcurrentLoadingFeature{
     "OfflinePagesSvelteConcurrentLoading", base::FEATURE_DISABLED_BY_DEFAULT};
 
+const base::Feature kOfflinePagesLoadSignalCollectingFeature{
+    "OfflinePagesLoadSignalCollecting", base::FEATURE_DISABLED_BY_DEFAULT};
+
 const base::Feature kBackgroundLoaderForDownloadsFeature{
     "BackgroundLoadingForDownloads", base::FEATURE_ENABLED_BY_DEFAULT};
 
 const base::Feature kOfflinePagesAsyncDownloadFeature{
     "OfflinePagesAsyncDownload", base::FEATURE_DISABLED_BY_DEFAULT};
 
+const base::Feature kPrefetchingOfflinePagesFeature{
+    "OfflinePagesPrefetching", base::FEATURE_DISABLED_BY_DEFAULT};
+
 const base::Feature kNewBackgroundLoaderFeature {
     "BackgroundLoader", base::FEATURE_DISABLED_BY_DEFAULT
 };
@@ -75,6 +81,14 @@ bool IsOfflinePagesAsyncDownloadEnabled() {
   return base::FeatureList::IsEnabled(kOfflinePagesAsyncDownloadFeature);
 }
 
+bool IsPrefetchingOfflinePagesEnabled() {
+  return base::FeatureList::IsEnabled(kPrefetchingOfflinePagesFeature);
+}
+
+bool IsOfflinePagesLoadSignalCollectingEnabled() {
+  return base::FeatureList::IsEnabled(kOfflinePagesLoadSignalCollectingFeature);
+}
+
 bool ShouldUseNewBackgroundLoader() {
   return base::FeatureList::IsEnabled(kNewBackgroundLoaderFeature);
 }
diff --git a/chromium/components/offline_pages/core/offline_page_feature.h b/chromium/components/offline_pages/core/offline_page_feature.h
index fdd42e6ed8c..11a5b68cf83 100644
--- a/chromium/components/offline_pages/core/offline_page_feature.h
+++ b/chromium/components/offline_pages/core/offline_page_feature.h
@@ -17,7 +17,9 @@ extern const base::Feature kOfflinePagesCTFeature;
 extern const base::Feature kOfflinePagesSharingFeature;
 extern const base::Feature kBackgroundLoaderForDownloadsFeature;
 extern const base::Feature kOfflinePagesAsyncDownloadFeature;
+extern const base::Feature kPrefetchingOfflinePagesFeature;
 extern const base::Feature kNewBackgroundLoaderFeature;
+extern const base::Feature kOfflinePagesLoadSignalCollectingFeature;
 
 // Returns true if saving bookmarked pages for offline viewing is enabled.
 bool IsOfflineBookmarksEnabled();
@@ -41,6 +43,12 @@ bool IsOfflinePagesSvelteConcurrentLoadingEnabled();
 // Returns true if downloading a page asynchonously is enabled.
 bool IsOfflinePagesAsyncDownloadEnabled();
 
+// Returns true if prefetching offline pages is enabled.
+bool IsPrefetchingOfflinePagesEnabled();
+
+// Returns true if we enable load timing signals to be collected.
+bool IsOfflinePagesLoadSignalCollectingEnabled();
+
 // Returns true if we should use background loader rather than prerenderer
 // to offline pages.
 bool ShouldUseNewBackgroundLoader();
diff --git a/chromium/components/offline_pages/core/offline_page_metadata_store_impl_unittest.cc b/chromium/components/offline_pages/core/offline_page_metadata_store_impl_unittest.cc
index 36abbcc98f9..7dcb7898685 100644
--- a/chromium/components/offline_pages/core/offline_page_metadata_store_impl_unittest.cc
+++ b/chromium/components/offline_pages/core/offline_page_metadata_store_impl_unittest.cc
@@ -39,7 +39,6 @@ int64_t kFileSize = 234567LL;
 int64_t kOfflineId = 12345LL;
 
 // Build a store with outdated schema to simulate the upgrading process.
-// TODO(romax): move it to sql_unittests.
 void BuildTestStoreWithSchemaFromM52(const base::FilePath& file) {
   sql::Connection connection;
   ASSERT_TRUE(
@@ -392,8 +391,6 @@ void OfflinePageMetadataStoreTest::GetOfflinePagesCallback(
 
 void OfflinePageMetadataStoreTest::AddCallback(ItemActionStatus status) {
   last_called_callback_ = ADD;
-  // TODO(fgorski): Add specific add status.
-  // last_item_status_ = status;
   last_status_ =
       status == ItemActionStatus::SUCCESS ? STATUS_TRUE : STATUS_FALSE;
 }
@@ -636,7 +633,6 @@ TEST_F(OfflinePageMetadataStoreTest, GetOfflinePagesFromInvalidStore) {
 // Loads a store which has an outdated schema.
 // This test case would crash if it's not handling correctly when we're loading
 // old version stores.
-// TODO(romax): Move this to sql_unittest.
 TEST_F(OfflinePageMetadataStoreTest, LoadVersion52Store) {
   std::unique_ptr<OfflinePageMetadataStore> store(
       BuildStoreWithSchemaFromM52());
@@ -648,7 +644,6 @@ TEST_F(OfflinePageMetadataStoreTest, LoadVersion52Store) {
 // Loads a store which has an outdated schema.
 // This test case would crash if it's not handling correctly when we're loading
 // old version stores.
-// TODO(romax): Move this to sql_unittest.
 TEST_F(OfflinePageMetadataStoreTest, LoadVersion53Store) {
   std::unique_ptr<OfflinePageMetadataStore> store(
       BuildStoreWithSchemaFromM53());
@@ -660,7 +655,6 @@ TEST_F(OfflinePageMetadataStoreTest, LoadVersion53Store) {
 // Loads a string with schema from M54.
 // This test case would crash if it's not handling correctly when we're loading
 // old version stores.
-// TODO(romax): Move this to sql_unittest.
 TEST_F(OfflinePageMetadataStoreTest, LoadVersion54Store) {
   std::unique_ptr<OfflinePageMetadataStore> store(
       BuildStoreWithSchemaFromM54());
@@ -672,7 +666,6 @@ TEST_F(OfflinePageMetadataStoreTest, LoadVersion54Store) {
 // Loads a string with schema from M55.
 // This test case would crash if it's not handling correctly when we're loading
 // old version stores.
-// TODO(romax): Move this to sql_unittest.
 TEST_F(OfflinePageMetadataStoreTest, LoadVersion55Store) {
   std::unique_ptr<OfflinePageMetadataStore> store(
       BuildStoreWithSchemaFromM55());
@@ -684,7 +677,6 @@ TEST_F(OfflinePageMetadataStoreTest, LoadVersion55Store) {
 // Loads a string with schema from M56.
 // This test case would crash if it's not handling correctly when we're loading
 // old version stores.
-// TODO(romax): Move this to sql_unittest.
 TEST_F(OfflinePageMetadataStoreTest, LoadVersion56Store) {
   std::unique_ptr<OfflinePageMetadataStore> store(
       BuildStoreWithSchemaFromM56());
diff --git a/chromium/components/offline_pages/core/offline_page_metadata_store_sql.cc b/chromium/components/offline_pages/core/offline_page_metadata_store_sql.cc
index c8e3ee0e953..a4a8ea080b2 100644
--- a/chromium/components/offline_pages/core/offline_page_metadata_store_sql.cc
+++ b/chromium/components/offline_pages/core/offline_page_metadata_store_sql.cc
@@ -158,7 +158,7 @@ bool CreateSchema(sql::Connection* db) {
       return false;
   }
 
-  // TODO(fgorski): Add indices here.
+  // This would be a great place to add indices when we need them.
   return transaction.Commit();
 }
 
@@ -292,7 +292,6 @@ void NotifyLoadResult(scoped_refptr<base::SingleThreadTaskRunner> runner,
                       const OfflinePageMetadataStore::LoadCallback& callback,
                       OfflinePageMetadataStore::LoadStatus status,
                       const std::vector<OfflinePageItem>& result) {
-  // TODO(fgorski): Switch to SQL specific UMA metrics.
   UMA_HISTOGRAM_ENUMERATION("OfflinePages.LoadStatus", status,
                             OfflinePageMetadataStore::LOAD_STATUS_COUNT);
   if (status == OfflinePageMetadataStore::LOAD_SUCCEEDED) {
@@ -427,7 +426,6 @@ void RemoveOfflinePagesSync(
     sql::Connection* db,
     scoped_refptr<base::SingleThreadTaskRunner> runner,
     const OfflinePageMetadataStore::UpdateCallback& callback) {
-  // TODO(fgorski): Perhaps add metrics here.
   std::unique_ptr<OfflinePagesUpdateResult> result(
       new OfflinePagesUpdateResult(StoreState::LOADED));
 
diff --git a/chromium/components/offline_pages/core/offline_page_model_impl.cc b/chromium/components/offline_pages/core/offline_page_model_impl.cc
index 2e87035d1f6..596892d6171 100644
--- a/chromium/components/offline_pages/core/offline_page_model_impl.cc
+++ b/chromium/components/offline_pages/core/offline_page_model_impl.cc
@@ -72,6 +72,11 @@ SavePageResult ToSavePageResult(ArchiverResult archiver_result) {
     case ArchiverResult::ERROR_SECURITY_CERTIFICATE:
       result = SavePageResult::SECURITY_CERTIFICATE_ERROR;
       break;
+    case ArchiverResult::ERROR_ERROR_PAGE:
+      result = SavePageResult::ERROR_PAGE;
+      break;
+    case ArchiverResult::ERROR_INTERSTITIAL_PAGE:
+      result = SavePageResult::INTERSTITIAL_PAGE;
     default:
       NOTREACHED();
       result = SavePageResult::CONTENT_UNAVAILABLE;
@@ -572,7 +577,6 @@ const std::vector<int64_t> OfflinePageModelImpl::MaybeGetOfflineIdsForClientId(
   std::vector<int64_t> results;
 
   // We want only all pages, including those marked for deletion.
-  // TODO(fgorski): actually use an index rather than linear scan.
   for (const auto& id_page_pair : offline_pages_) {
     if (id_page_pair.second.client_id == client_id)
       results.push_back(id_page_pair.second.offline_id);
@@ -684,20 +688,10 @@ void OfflinePageModelImpl::OnCreateArchiveDone(
     const SavePageCallback& callback,
     OfflinePageArchiver* archiver,
     ArchiverResult archiver_result,
-    const GURL& url,
+    const GURL& saved_url,
     const base::FilePath& file_path,
     const base::string16& title,
     int64_t file_size) {
-  if (save_page_params.url != url) {
-    DVLOG(1) << "Saved URL does not match requested URL.";
-    // TODO(fgorski): We have created an archive for a wrong URL. It should be
-    // deleted from here, once archiver has the right functionality.
-    InformSavePageDone(callback, SavePageResult::ARCHIVE_CREATION_FAILED,
-                       save_page_params.client_id, offline_id);
-    DeletePendingArchiver(archiver);
-    return;
-  }
-
   if (archiver_result != ArchiverResult::SUCCESSFULLY_CREATED) {
     SavePageResult result = ToSavePageResult(archiver_result);
     InformSavePageDone(
@@ -705,8 +699,18 @@ void OfflinePageModelImpl::OnCreateArchiveDone(
     DeletePendingArchiver(archiver);
     return;
   }
-  OfflinePageItem offline_page_item(url, offline_id, save_page_params.client_id,
-                                    file_path, file_size, start_time);
+
+  if (save_page_params.url != saved_url) {
+    DVLOG(1) << "Saved URL does not match requested URL.";
+    InformSavePageDone(callback, SavePageResult::ARCHIVE_CREATION_FAILED,
+                       save_page_params.client_id, offline_id);
+    DeletePendingArchiver(archiver);
+    return;
+  }
+
+  OfflinePageItem offline_page_item(saved_url, offline_id,
+                                    save_page_params.client_id, file_path,
+                                    file_size, start_time);
   offline_page_item.title = title;
   offline_page_item.original_url = save_page_params.original_url;
   store_->AddOfflinePage(offline_page_item,
@@ -907,7 +911,6 @@ void OfflinePageModelImpl::OnPagesFoundWithSameURL(
 
 void OfflinePageModelImpl::OnDeleteOldPagesWithSameURL(
     DeletePageResult result) {
-  // TODO(romax) Add UMAs for failure cases.
   PostClearStorageIfNeededTask(false /* delayed */);
 }
 
@@ -956,10 +959,6 @@ void OfflinePageModelImpl::OnRemoveOfflinePagesDone(
       observer.OfflinePageDeleted(page.offline_id, page.client_id);
   }
 
-  // TODO(fgorski): React the FAILED_INITIALIZATION, FAILED_RESET here.
-  // TODO(fgorski): We need a better callback interface for the Remove action on
-  // the this class. Currently removing an item that does not exist is
-  // considered a success, but not called out as such to the caller.
   DeletePageResult delete_result;
   if (result->store_state == StoreState::LOADED)
     delete_result = DeletePageResult::SUCCESS;
diff --git a/chromium/components/offline_pages/core/offline_page_model_impl.h b/chromium/components/offline_pages/core/offline_page_model_impl.h
index 5221644d7f3..64e2e3cfdfd 100644
--- a/chromium/components/offline_pages/core/offline_page_model_impl.h
+++ b/chromium/components/offline_pages/core/offline_page_model_impl.h
@@ -163,7 +163,7 @@ class OfflinePageModelImpl : public OfflinePageModel, public KeyedService {
                            const SavePageCallback& callback,
                            OfflinePageArchiver* archiver,
                            OfflinePageArchiver::ArchiverResult result,
-                           const GURL& url,
+                           const GURL& saved_url,
                            const base::FilePath& file_path,
                            const base::string16& title,
                            int64_t file_size);
diff --git a/chromium/components/offline_pages/core/offline_page_model_impl_unittest.cc b/chromium/components/offline_pages/core/offline_page_model_impl_unittest.cc
index b5806b222b7..8d20e475263 100644
--- a/chromium/components/offline_pages/core/offline_page_model_impl_unittest.cc
+++ b/chromium/components/offline_pages/core/offline_page_model_impl_unittest.cc
@@ -1347,4 +1347,15 @@ TEST(CommandLineFlagsTest, OfflinePagesSvelteConcurrentLoading) {
   EXPECT_TRUE(offline_pages::IsOfflinePagesSvelteConcurrentLoadingEnabled());
 }
 
+TEST(CommandLineFlagsTest, OfflinePagesLoadSignalCollecting) {
+  // This feature is disabled by default.
+  EXPECT_FALSE(offline_pages::IsOfflinePagesLoadSignalCollectingEnabled());
+
+  // Check if feature is correctly enabled by command-line flag.
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(
+      kOfflinePagesLoadSignalCollectingFeature);
+  EXPECT_TRUE(offline_pages::IsOfflinePagesLoadSignalCollectingEnabled());
+}
+
 }  // namespace offline_pages
diff --git a/chromium/components/offline_pages/core/offline_page_storage_manager.cc b/chromium/components/offline_pages/core/offline_page_storage_manager.cc
index efe9ec2f856..1c17729f768 100644
--- a/chromium/components/offline_pages/core/offline_page_storage_manager.cc
+++ b/chromium/components/offline_pages/core/offline_page_storage_manager.cc
@@ -93,7 +93,6 @@ void OfflinePageStorageManager::GetPageIdsToClear(
     const MultipleOfflinePageItemResult& pages,
     const ArchiveManager::StorageStats& stats,
     std::vector<int64_t>* page_ids_to_clear) {
-  // TODO(romax): See how persistent should be considered here.
   // Creating a map from namespace to a vector of page items.
   // Sort each vector based on last accessed time and all pages after index
   // min{size(), page_limit} should be deleted.
diff --git a/chromium/components/offline_pages/core/offline_page_test_archiver.h b/chromium/components/offline_pages/core/offline_page_test_archiver.h
index 6d7c04e8753..1ece7c811eb 100644
--- a/chromium/components/offline_pages/core/offline_page_test_archiver.h
+++ b/chromium/components/offline_pages/core/offline_page_test_archiver.h
@@ -26,8 +26,6 @@ namespace offline_pages {
 // for an actual web contents.
 class OfflinePageTestArchiver : public OfflinePageArchiver {
  public:
-  // TODO(fgorski): Try refactoring the observer out and replace it with a
-  // callback, or completely remove the call to |SetLastPathCreatedByArchiver|.
   class Observer {
    public:
     virtual ~Observer() {}
diff --git a/chromium/components/offline_pages/core/offline_page_test_store.cc b/chromium/components/offline_pages/core/offline_page_test_store.cc
index 556c12a2356..225d777694d 100644
--- a/chromium/components/offline_pages/core/offline_page_test_store.cc
+++ b/chromium/components/offline_pages/core/offline_page_test_store.cc
@@ -44,7 +44,6 @@ void OfflinePageTestStore::GetOfflinePages(const LoadCallback& callback) {
 
 void OfflinePageTestStore::AddOfflinePage(const OfflinePageItem& offline_page,
                                           const AddCallback& callback) {
-  // TODO(fgorski): Add and cover scenario with existing item.
   ItemActionStatus result;
   if (store_state_ == StoreState::LOADED &&
       scenario_ != TestScenario::WRITE_FAILED) {
@@ -61,8 +60,6 @@ void OfflinePageTestStore::AddOfflinePage(const OfflinePageItem& offline_page,
 void OfflinePageTestStore::UpdateOfflinePages(
     const std::vector<OfflinePageItem>& pages,
     const UpdateCallback& callback) {
-  // TODO(fgorski): Cover scenario where new items are being created while they
-  // shouldn't.
   std::unique_ptr<OfflinePagesUpdateResult> result(
       new OfflinePagesUpdateResult(StoreState::LOADED));
   if (scenario_ == TestScenario::WRITE_FAILED) {
diff --git a/chromium/components/offline_pages/core/offline_page_types.h b/chromium/components/offline_pages/core/offline_page_types.h
index ae9e298af8e..4bb3342e1d1 100644
--- a/chromium/components/offline_pages/core/offline_page_types.h
+++ b/chromium/components/offline_pages/core/offline_page_types.h
@@ -34,6 +34,10 @@ enum class SavePageResult {
   // are already locally accessible.
   SKIPPED,
   SECURITY_CERTIFICATE_ERROR,
+  // Returned when we detect trying to save a chrome error page.
+  ERROR_PAGE,
+  // Returned when we detect trying to save a chrome interstitial page.
+  INTERSTITIAL_PAGE,
   // NOTE: always keep this entry at the end. Add new result types only
   // immediately above this line. Make sure to update the corresponding
   // histogram enum accordingly.
diff --git a/chromium/components/offline_pages/core/offline_store_types.h b/chromium/components/offline_pages/core/offline_store_types.h
index 77d6a6a9bdf..aa95914e8ea 100644
--- a/chromium/components/offline_pages/core/offline_store_types.h
+++ b/chromium/components/offline_pages/core/offline_store_types.h
@@ -14,7 +14,6 @@
 // offline page related components.
 namespace offline_pages {
 
-// TODO(fgorski): This enum is meant to replace |LoadStatus|.
 // Current store state. When LOADED, the store is operational. When
 // loading or reset fails, it is reflected appropriately.
 enum class StoreState {
diff --git a/chromium/components/offline_pages/core/snapshot_controller.cc b/chromium/components/offline_pages/core/snapshot_controller.cc
index c683a1a9b10..9d62297fb7b 100644
--- a/chromium/components/offline_pages/core/snapshot_controller.cc
+++ b/chromium/components/offline_pages/core/snapshot_controller.cc
@@ -11,6 +11,8 @@
 #include "components/offline_pages/core/offline_page_feature.h"
 
 namespace {
+const bool kDocumentAvailableTriggersSnapshot = true;
+
 // Default delay, in milliseconds, between the main document parsed event and
 // snapshot. Note: this snapshot might not occur if the OnLoad event and
 // OnLoad delay elapses first to trigger a final snapshot.
@@ -18,7 +20,8 @@ const int64_t kDefaultDelayAfterDocumentAvailableMs = 7000;
 
 // Default delay, in milliseconds, between the main document OnLoad event and
 // snapshot.
-const int64_t kDelayAfterDocumentOnLoadCompletedMs = 1000;
+const int64_t kDelayAfterDocumentOnLoadCompletedMsForeground = 1000;
+const int64_t kDelayAfterDocumentOnLoadCompletedMsBackground = 2000;
 
 // Delay for testing to keep polling times reasonable.
 const int64_t kDelayForTests = 0;
@@ -27,25 +30,42 @@ const int64_t kDelayForTests = 0;
 
 namespace offline_pages {
 
-SnapshotController::SnapshotController(
+// static
+std::unique_ptr<SnapshotController>
+SnapshotController::CreateForForegroundOfflining(
+    const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
+    SnapshotController::Client* client) {
+  return std::unique_ptr<SnapshotController>(new SnapshotController(
+      task_runner, client, kDefaultDelayAfterDocumentAvailableMs,
+      kDelayAfterDocumentOnLoadCompletedMsForeground,
+      kDocumentAvailableTriggersSnapshot));
+}
+
+// static
+std::unique_ptr<SnapshotController>
+SnapshotController::CreateForBackgroundOfflining(
     const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
-    SnapshotController::Client* client)
-    : SnapshotController(task_runner,
-                         client,
-                         kDefaultDelayAfterDocumentAvailableMs,
-                         kDelayAfterDocumentOnLoadCompletedMs) {}
+    SnapshotController::Client* client) {
+  return std::unique_ptr<SnapshotController>(new SnapshotController(
+      task_runner, client, kDefaultDelayAfterDocumentAvailableMs,
+      kDelayAfterDocumentOnLoadCompletedMsBackground,
+      !kDocumentAvailableTriggersSnapshot));
+}
 
 SnapshotController::SnapshotController(
     const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
     SnapshotController::Client* client,
     int64_t delay_after_document_available_ms,
-    int64_t delay_after_document_on_load_completed_ms)
+    int64_t delay_after_document_on_load_completed_ms,
+    bool document_available_triggers_snapshot)
     : task_runner_(task_runner),
       client_(client),
       state_(State::READY),
       delay_after_document_available_ms_(delay_after_document_available_ms),
       delay_after_document_on_load_completed_ms_(
           delay_after_document_on_load_completed_ms),
+      document_available_triggers_snapshot_(
+          document_available_triggers_snapshot),
       weak_ptr_factory_(this) {
   if (offline_pages::ShouldUseTestingSnapshotDelay()) {
     delay_after_document_available_ms_ = kDelayForTests;
@@ -75,13 +95,16 @@ void SnapshotController::PendingSnapshotCompleted() {
 }
 
 void SnapshotController::DocumentAvailableInMainFrame() {
-  DCHECK_EQ(PageQuality::POOR, current_page_quality_);
-  // Post a delayed task to snapshot.
-  task_runner_->PostDelayedTask(
-      FROM_HERE, base::Bind(&SnapshotController::MaybeStartSnapshot,
-                            weak_ptr_factory_.GetWeakPtr(),
-                            PageQuality::FAIR_AND_IMPROVING),
-      base::TimeDelta::FromMilliseconds(delay_after_document_available_ms_));
+  if (document_available_triggers_snapshot_) {
+    DCHECK_EQ(PageQuality::POOR, current_page_quality_);
+    // Post a delayed task to snapshot.
+    task_runner_->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&SnapshotController::MaybeStartSnapshot,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   PageQuality::FAIR_AND_IMPROVING),
+        base::TimeDelta::FromMilliseconds(delay_after_document_available_ms_));
+  }
 }
 
 void SnapshotController::DocumentOnLoadCompletedInMainFrame() {
diff --git a/chromium/components/offline_pages/core/snapshot_controller.h b/chromium/components/offline_pages/core/snapshot_controller.h
index 36e1680a6f0..bcd5ee00b1b 100644
--- a/chromium/components/offline_pages/core/snapshot_controller.h
+++ b/chromium/components/offline_pages/core/snapshot_controller.h
@@ -60,14 +60,23 @@ class SnapshotController {
     virtual ~Client() {}
   };
 
-  SnapshotController(
+  // Creates a SnapshotController with document available delay = 7s,
+  // document on load delay = 1s and triggers snapshot on document available.
+  static std::unique_ptr<SnapshotController> CreateForForegroundOfflining(
+      const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
+      SnapshotController::Client* client);
+  // Creates a SnapshotController with document on load delay = 2s
+  // and ignores document available signal.
+  static std::unique_ptr<SnapshotController> CreateForBackgroundOfflining(
       const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
       SnapshotController::Client* client);
+
   SnapshotController(
       const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
       SnapshotController::Client* client,
       int64_t delay_after_document_available_ms,
-      int64_t delay_after_document_on_load_completed_ms);
+      int64_t delay_after_document_on_load_completed_ms,
+      bool document_available_triggers_snapshot);
   virtual ~SnapshotController();
 
   // Resets the 'session', returning controller to initial state.
@@ -103,6 +112,7 @@ class SnapshotController {
   SnapshotController::State state_;
   int64_t delay_after_document_available_ms_;
   int64_t delay_after_document_on_load_completed_ms_;
+  bool document_available_triggers_snapshot_;
 
   // The expected quality of a snapshot taken at the moment this value is
   // queried.
diff --git a/chromium/components/offline_pages/core/snapshot_controller_unittest.cc b/chromium/components/offline_pages/core/snapshot_controller_unittest.cc
index a57c2da897b..123437b4e48 100644
--- a/chromium/components/offline_pages/core/snapshot_controller_unittest.cc
+++ b/chromium/components/offline_pages/core/snapshot_controller_unittest.cc
@@ -53,7 +53,8 @@ SnapshotControllerTest::SnapshotControllerTest()
 SnapshotControllerTest::~SnapshotControllerTest() {}
 
 void SnapshotControllerTest::SetUp() {
-  controller_.reset(new SnapshotController(task_runner_, this));
+  controller_ =
+      SnapshotController::CreateForForegroundOfflining(task_runner_, this);
   snapshot_started_ = true;
 }