[DRP] Consistently use LoFi for an entire page

components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h

Index: components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h
index 9fb003cdb0aa6f63a8521948b8ae80e92325ad9b..de7932d4956fe9954088fcd3dad2f14ad71e569b 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h
@@ -78,38 +78,6 @@ enum SecureProxyCheckFetchResult {
   SECURE_PROXY_CHECK_FETCH_RESULT_COUNT
 };
 
-// Values of the |lofi_status_|.
-// Default state is |LOFI_STATUS_TEMPORARILY_OFF|.
-enum LoFiStatus {
-  // Used if Lo-Fi is permanently off.
-  LOFI_STATUS_OFF = 0,
-
-  // Used if Lo-Fi is disabled temporarily through direct or indirect user
-  // action. The state would be reset on next main frame request.
-  LOFI_STATUS_TEMPORARILY_OFF,
-
-  // Used if Lo-Fi is enabled through flags.
-  LOFI_STATUS_ACTIVE_FROM_FLAGS,
-
-  // Session is in Auto Lo-Fi Control group and the current conditions are
-  // suitable to use Lo-Fi "q=low" header.
-  LOFI_STATUS_ACTIVE_CONTROL,
-
-  // Session is in Auto Lo-Fi Control group and the current conditions are
-  // not suitable to use Lo-Fi "q=low" header.
-  LOFI_STATUS_INACTIVE_CONTROL,
-
-  // Session is in Auto Lo-Fi enabled group and the current conditions are
-  // suitable to use Lo-Fi "q=low" header.
-  LOFI_STATUS_ACTIVE,
-
-  // Session is in Auto Lo-Fi enabled group and the current conditions are
-  // not suitable to use Lo-Fi "q=low" header.
-  LOFI_STATUS_INACTIVE,
-
-  LOFI_STATUS_LAST = LOFI_STATUS_INACTIVE
-};
-
 // Central point for holding the Data Reduction Proxy configuration.
 // This object lives on the IO thread and all of its methods are expected to be
 // called from there.
@@ -213,23 +181,24 @@ class DataReductionProxyConfig
   // tied to whether the Data Reduction Proxy is enabled.
   bool promo_allowed() const;
 
-  // Returns the Lo-Fi status.
-  LoFiStatus GetLoFiStatus() const;
+  // Sets |lofi_off_| to true.
+  void SetLoFiModeOff();
 
-  // Returns true only if Lo-Fi "q=low" header should be added to the Chrome
-  // Proxy header.
-  // Should be called on all URL requests (main frame and non main frame).
-  bool ShouldUseLoFiHeaderForRequests() const;
+  // Returns |lofi_off_|.
+  bool IsLoFiOff() const { return lofi_off_; }

[tbansal1, 2015/09/28 17:49:02]

May be rename the method to lofi_off() if it is simply returning the value.

[megjablon, 2015/09/28 22:43:37]

Done.

 
-  // Sets |lofi_status_| to LOFI_STATUS_OFF.
-  void SetLoFiModeOff();
+  // Returns true if this client is part of Lo-Fi enabled field trial.
+  // Virtualized for unit testing.
+  virtual bool IsIncludedInLoFiEnabledFieldTrial() const;
 
-  // Updates |lofi_status_| based on the arguments provided and the current
-  // value of |lofi_status_|.
-  // |network_quality_estimator| may be NULL.
-  // Should be called only on main frame loads.
-  void UpdateLoFiStatusOnMainFrameRequest(
-      bool user_temporarily_disabled_lofi,
+  // Returns true only when Lo-Fi "q=low" header should be added to the Chrome
+  // Proxy header. Should be called only on main frame loads.
+  bool ShouldEnableLoFiMode(net::URLRequest* request);
+
+  // Returns true only when Lo-Fi "q=low" header should be added to the Chrome
+  // Proxy header. |network_quality_estimator| may be
+  // NULL. Should be called only on main frame loads.
+  bool ShouldTurnOnLoFiOnMainFrameRequest(

[tbansal1, 2015/09/28 17:49:02]

The comment is not correct anymore. The method returns true even if user is in
Control
group. In that case, we should not add q=low to the CP header.

Please update the comments for this method and the above.

[megjablon, 2015/09/28 22:43:37]

Done.

       const net::NetworkQualityEstimator* network_quality_estimator);
 
  protected:
@@ -319,10 +288,6 @@ class DataReductionProxyConfig
       bool is_https,
       base::TimeDelta* min_retry_delay) const;
 
-  // Returns true if this client is part of Lo-Fi enabled field trial.
-  // Virtualized for unit testing.
-  virtual bool IsIncludedInLoFiEnabledFieldTrial() const;
-
   // Returns true if this client is part of Lo-Fi control field trial.
   // Virtualized for unit testing.
   virtual bool IsIncludedInLoFiControlFieldTrial() const;
@@ -335,10 +300,6 @@ class DataReductionProxyConfig
   virtual bool IsNetworkQualityProhibitivelySlow(
       const net::NetworkQualityEstimator* network_quality_estimator);
 
-  // Returns true only if Lo-Fi "q=low" header should be added to the Chrome
-  // Proxy header based on the value of |lofi_status|.
-  static bool ShouldUseLoFiHeaderForRequests(LoFiStatus lofi_status);
-
   // Records Lo-Fi accuracy metric. Should be called only on main frame loads.
   void RecordAutoLoFiAccuracyRate(
       const net::NetworkQualityEstimator* network_quality_estimator) const;
@@ -398,9 +359,8 @@ class DataReductionProxyConfig
   // network quality was last updated (most recent main frame request).
   net::NetworkChangeNotifier::ConnectionType connection_type_;
 
-  // Current Lo-Fi status.
-  // The value changes only on main frame load.
-  LoFiStatus lofi_status_;
+  // If true, Lo-Fi is turned off for the rest of the session.

[tbansal1, 2015/09/28 17:49:02]

Is Lo-Fi turned off if the user is in control group? Alternatively, 
what does it mean by Lo-Fi not turned off?

May be add a comment to explain what can not happen once |lofi_off_| is true.

[megjablon, 2015/09/28 22:43:37]

Done.

+  bool lofi_off_;
 
   // Timestamp when the most recent main frame request started.
   base::TimeTicks last_main_frame_request_;
@@ -411,6 +371,12 @@ class DataReductionProxyConfig
   NetworkQualityAtLastMainFrameRequest
       network_quality_at_last_main_frame_request_;
 
+  // Store if the previous state of Lo-Fi was on, so that change in Lo-Fi status
+  // can be recorded properly. This is not recorded for the control group,
+  // because it is only used to report changes in request headers, and the
+  // request headers are never modified in the control group.
+  bool previous_state_lofi_on_;
+
   DISALLOW_COPY_AND_ASSIGN(DataReductionProxyConfig);
 };