[Offline Pages] Add evaluation test support in RequestCoordinator.

components/offline_pages/background/offliner_policy.h

Index: components/offline_pages/background/offliner_policy.h
diff --git a/components/offline_pages/background/offliner_policy.h b/components/offline_pages/background/offliner_policy.h
index a82f5e68715a150a86a1dd4ec4d1f1b7755bd974..234bb6243579916f61826709c877c7c86ad736e3 100644
--- a/components/offline_pages/background/offliner_policy.h
+++ b/components/offline_pages/background/offliner_policy.h
@@ -21,11 +21,12 @@ namespace offline_pages {
 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) {}
+      : OfflinerPolicy(false,
+                       true,

[dougarnett, 2016/10/18 17:26:32]

Hard to read what these bools apply to. I would typically add comments on them
but Pete may suggest another way to document.

[Pete Williamson, 2016/10/18 19:24:32]

boolean arguments are always hard to understand at the point of call, like here.
 Normally I suggest using a const kPreferUntried = true;
and at the point of call, setting either kPreferUntried or !kPreferUntried as
the argument supplied.
Another solution that works is to use an enum with values such as
PREFER_UNTRIED_REQUESTS and PREFER_TRIED_REQUESTS, but that change will touch a
lot more places than here.

Why not just add background_processing_time_budget to the existing second
constructor?  It is already for tests, and your code uses the test hooks.

Alternatively, you could add a method to set the background processing time
budget.

[romax, 2016/10/18 20:31:51]

I forgot why I used this but seems like simply adding the time_budget to the
second c-tor is better. I'll do this.

+                       false,
+                       kMaxStartedTries,
+                       kMaxCompletedTries,
+                       kBackgroundProcessingTimeBudgetSeconds) {}

[dougarnett, 2016/10/18 17:26:32]

We might consider renaming to identify this constant now as the default

[romax, 2016/10/18 20:31:51]

Done.

 
   // Constructor for unit tests.
   OfflinerPolicy(bool prefer_untried,
@@ -33,11 +34,25 @@ class OfflinerPolicy {
                  bool prefer_retry_count,
                  int max_started_tries,
                  int max_completed_tries)
+      : OfflinerPolicy(prefer_untried,
+                       prefer_earlier,
+                       prefer_retry_count,
+                       max_started_tries,
+                       max_completed_tries,
+                       kBackgroundProcessingTimeBudgetSeconds) {}
+
+  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) {}
+        max_completed_tries_(max_completed_tries),
+        background_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
@@ -84,7 +99,7 @@ class OfflinerPolicy {
   // How many seconds to keep trying new pages for, before we give up,  and
   // return to the scheduler.
   int GetBackgroundProcessingTimeBudgetSeconds() const {
-    return kBackgroundProcessingTimeBudgetSeconds;
+    return background_processing_time_budget_;
   }
 
   // How long do we allow a page to load before giving up on it when
@@ -110,6 +125,7 @@ class OfflinerPolicy {
   bool retry_count_is_more_important_than_recency_;
   int max_started_tries_;
   int max_completed_tries_;
+  int background_processing_time_budget_;
 };
 }  // namespace offline_pages