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

components/offline_pages/background/offliner_policy.h

 // 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.
 
 #ifndef COMPONENTS_OFFLINE_PAGES_BACKGROUND_OFFLINER_POLICY_H_
 #define COMPONENTS_OFFLINE_PAGES_BACKGROUND_OFFLINER_POLICY_H_
 
 namespace {
 const int kMaxStartedTries = 4;
 const int kMaxCompletedTries = 1;
-const int kDefaultBackgroundProcessingTimeBudgetSeconds = 170;
-const int kSinglePageTimeLimitWhenBackgroundScheduledSeconds = 120;
-const int kSinglePageTimeLimitForImmediateLoadSeconds = 300;
+const int kDefaultBackgroundProcessingTimeBudgetSeconds = 60 * 3 - 10;

[Dmitry Titov, 2016/10/19 21:41:26]

Would be good to have some comments reflecting why these specific values are
selected. Are they arbitrary? Or not? "-10" looks like it tries to be 10 seconds
less than some other constant. Which one?

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

This is based on the Marshmallow Doze mode 3 minute limit.  We want to avoid
being killed by doze mode by finishing up before it is over.  In the future, we
might be smarter about what OS we are running on, and use longer timeouts on
pre-marshmallow OSs.

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

Did some doc and naming here to try to communicate a bit about these
values/guesses

+const int kImmediateLoadProcessingTimeBudgetSeconds = 60 * 10;
+const int kSinglePageTimeLimitWhenBackgroundScheduledSeconds = 60 * 2;

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

Based on our recent Gin2G-poor testing, we might also put this at 170 seconds.

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

Done.

+const int kSinglePageTimeLimitForImmediateLoadSeconds = 60 * 5;
 const int kRequestExpirationTimeInSeconds = 60 * 60 * 24 * 7;
 }  // namespace
 
 namespace offline_pages {
 
 // Policy for the Background Offlining system.  Some policy will belong to the
 // RequestCoordinator, some to the RequestQueue, and some to the Offliner.
 class OfflinerPolicy {
  public:
   OfflinerPolicy()
       : prefer_untried_requests_(false),
         prefer_earlier_requests_(true),
         retry_count_is_more_important_than_recency_(false),
         max_started_tries_(kMaxStartedTries),
         max_completed_tries_(kMaxCompletedTries),
-        background_processing_time_budget_(
+        background_scheduled_processing_time_budget_(
             kDefaultBackgroundProcessingTimeBudgetSeconds) {}
 
   // Constructor for unit tests.
   OfflinerPolicy(bool prefer_untried,
                  bool prefer_earlier,
                  bool prefer_retry_count,
                  int max_started_tries,
                  int max_completed_tries,
                  int background_processing_time_budget)
       : prefer_untried_requests_(prefer_untried),
         prefer_earlier_requests_(prefer_earlier),
         retry_count_is_more_important_than_recency_(prefer_retry_count),
         max_started_tries_(max_started_tries),
         max_completed_tries_(max_completed_tries),
-        background_processing_time_budget_(background_processing_time_budget) {}
+        background_scheduled_processing_time_budget_(
+            background_processing_time_budget) {}
 
   // 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.
 
   // Returns true if we should prefer retrying lesser tried requests.
   bool ShouldPreferUntriedRequests() const { return prefer_untried_requests_; }
 
   // Returns true if we should prefer older requests of equal number of tries.
   bool ShouldPreferEarlierRequests() const { return prefer_earlier_requests_; }
@@ skipping 23 matching lines @@
 
   int BatteryPercentageRequired(bool user_requested) const {
     if (user_requested)
       return 0;
     // This is so low because we require the device to be plugged in and
     // charging.  If we decide to allow non-user requested pages when not
     // plugged in, we should raise this somewhat higher.
     return 25;
   }
 
-  // How many seconds to keep trying new pages for, before we give up,  and
+  // How many seconds to keep trying new pages for, before we give up, and
   // return to the scheduler.
-  int GetBackgroundProcessingTimeBudgetSeconds() const {
-    return background_processing_time_budget_;
+  int GetProcessingTimeBudgetWhenBackgroundScheduledInSeconds() const {

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

Maybe better to have one method that takes an enum instead of two.
SCHEDULED_WINDOW or IMMEDIATE_WINDOW might work here.

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

Adding TODO for now. Currently this enum is private so let's talk more about
exposing and possible other policy considerations between these modes - eg, I'm
wondering about different TryNextRequest behaviour on failures/timeouts with
immediate mode (but will need design discussion).

+    return background_scheduled_processing_time_budget_;
+  }
+
+  // How many seconds to keep trying new pages for, before we give up, when
+  // processing started immediately (without scheduler).
+  int GetProcessingTimeBudgetForImmediateLoadInSeconds() const {
+    return kImmediateLoadProcessingTimeBudgetSeconds;
   }
 
   // How long do we allow a page to load before giving up on it when
   // background loading was scheduled.
   int GetSinglePageTimeLimitWhenBackgroundScheduledInSeconds() const {
     return kSinglePageTimeLimitWhenBackgroundScheduledSeconds;
   }
 
   // How long do we allow a page to load before giving up on it when
   // immediately background loading.
   int GetSinglePageTimeLimitForImmediateLoadInSeconds() const {
     return kSinglePageTimeLimitForImmediateLoadSeconds;
   }
 
   // How long we allow requests to remain in the system before giving up.
   int GetRequestExpirationTimeInSeconds() const {
     return kRequestExpirationTimeInSeconds;
   }
 
  private:
   bool prefer_untried_requests_;
   bool prefer_earlier_requests_;
   bool retry_count_is_more_important_than_recency_;
   int max_started_tries_;
   int max_completed_tries_;
-  int background_processing_time_budget_;
+  int background_scheduled_processing_time_budget_;
 };
 }  // namespace offline_pages
 
 
 #endif  // COMPONENTS_OFFLINE_PAGES_BACKGROUND_OFFLINER_POLICY_H_