Change database scheme - add state and start tracking

components/offline_pages/background/request_picker.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/offline_pages/background/request_picker.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "components/offline_pages/background/save_page_request.h"
 
@@ skipping 71 matching lines @@
     empty_callback_.Run();
   }
 }
 
 // Filter out requests that don't meet the current conditions.  For instance, if
 // this is a predictive request, and we are not on WiFi, it should be ignored
 // this round.
 bool RequestPicker::RequestConditionsSatisfied(const SavePageRequest& request) {
   // If the user did not request the page directly, make sure we are connected
   // to power and have WiFi and sufficient battery remaining before we take this
   // reqeust.

[chili, 2016/08/08 18:17:01]

tiny nit - (since this file's touched anyway....) can we fix spelling on
request?

[Pete Williamson, 2016/08/08 18:44:28]

Done.

   // TODO(petewil): We may later want to configure whether to allow 2G for non
   // user_requested items, add that to policy.
   if (!request.user_requested()) {
     if (!current_conditions_->IsPowerConnected())
       return false;
 
     if (current_conditions_->GetNetConnectionType() !=
         net::NetworkChangeNotifier::ConnectionType::CONNECTION_WIFI) {
       return false;
     }
 
     if (current_conditions_->GetBatteryPercentage() <
         policy_->GetMinimumBatteryPercentageForNonUserRequestOfflining()) {
       return false;
     }
   }
 
+  // If we have already started this page the max number of times, it is not
+  // eligible to try again.
+  // TODO(petewil): We should have code to remove the page from the
+  // queue after the last retry.
+  if (request.started_attempt_count() >= policy_->GetMaxStartedTries())
+    return false;
+
   // If we have already tried this page the max number of times, it is not
   // eligible to try again.

[chili, 2016/08/08 18:17:01]

nit: this comment is the same as the one above.  Perhaps update to "the tries
completed max number of times"?

[Pete Williamson, 2016/08/08 18:44:28]

comment updated.

-  // TODO(petewil): Instead, we should have code to remove the page from the
+  // TODO(petewil): We should have code to remove the page from the
   // queue after the last retry.
-  if (request.attempt_count() > policy_->GetMaxTries())
+  if (request.completed_attempt_count() >= policy_->GetMaxCompletedTries())
     return false;
 
   // If the request is expired, do not consider it.
   // TODO(petewil): We need to remove this from the queue.
   base::TimeDelta requestAge = base::Time::Now() - request.creation_time();
   if (requestAge >
       base::TimeDelta::FromSeconds(
           policy_->GetRequestExpirationTimeInSeconds()))
     return false;
 
@@ skipping 55 matching lines @@
   result = CompareRetryCount(left, right);
 
   return (result > 0);
 }
 
 // Compare left and right side, returning 1 if the left side is better
 // (preferred by policy), 0 if the same, and -1 if the right side is better.
 int RequestPicker::CompareRetryCount(
     const SavePageRequest* left, const SavePageRequest* right) {
   // Check the attempt count.
-  int result = signum(left->attempt_count() - right->attempt_count());
+  int result = signum(left->completed_attempt_count() -
+                      right->completed_attempt_count());
 
   // Flip the direction of comparison if policy prefers fewer retries.
   if (fewer_retries_better_)
     result *= -1;
 
   return result;
 }
 
 // Compare left and right side, returning 1 if the left side is better
 // (preferred by policy), 0 if the same, and -1 if the right side is better.
 int RequestPicker::CompareCreationTime(
     const SavePageRequest* left, const SavePageRequest* right) {
   // Check the recency.
   base::TimeDelta difference = left->creation_time() - right->creation_time();
   int result = signum(difference.InMilliseconds());
 
   // Flip the direction of comparison if policy prefers fewer retries.
   if (earlier_requests_better_)
     result *= -1;
 
   return result;
 }
 
 }  // namespace offline_pages