[DRP] Consistently use LoFi for an entire page

chrome/renderer/page_load_histograms.cc

Index: chrome/renderer/page_load_histograms.cc
diff --git a/chrome/renderer/page_load_histograms.cc b/chrome/renderer/page_load_histograms.cc
index b7fd03ec3cc5e3d03c096bf516a86d3c7a3e48d4..102d20538480308e5084d877361f778df53ca575 100644
--- a/chrome/renderer/page_load_histograms.cc
+++ b/chrome/renderer/page_load_histograms.cc
@@ -21,8 +21,10 @@
 #include "chrome/renderer/chrome_content_renderer_client.h"
 #include "chrome/renderer/searchbox/search_bouncer.h"
 #include "components/data_reduction_proxy/content/common/data_reduction_proxy_messages.h"
+#include "components/data_reduction_proxy/core/common/data_reduction_proxy_params.h"
 #include "content/public/common/content_constants.h"
 #include "content/public/renderer/document_state.h"
+#include "content/public/renderer/render_frame.h"
 #include "content/public/renderer/render_thread.h"
 #include "content/public/renderer/render_view.h"
 #include "extensions/common/url_pattern.h"
@@ -216,14 +218,15 @@ int GetQueryStringBasedExperiment(const GURL& referrer) {
   return kNoExperiment;
 }
 
-void DumpHistograms(const WebPerformance& performance,
-                    DocumentState* document_state,
-                    bool data_reduction_proxy_was_used,
-                    data_reduction_proxy::LoFiStatus lofi_status,
-                    bool came_from_websearch,
-                    int websearch_chrome_joint_experiment_id,
-                    bool is_preview,
-                    URLPattern::SchemeMasks scheme_type) {
+void DumpHistograms(
+    const WebPerformance& performance,
+    DocumentState* document_state,
+    bool data_reduction_proxy_was_used,
+    bool lofi_on /* Whether the frame used the Lo-Fi request header. */,

[tbansal1, 2015/09/29 00:47:48]

Is this correct? Are the users in control group using LoFi request header? I am
probably misunderstanding this. It seems like this should be was_network_slow?

[megjablon, 2015/09/29 02:38:38]

How about a different description? I don't like the idea of using
was_network_slow because the metrics we're reporting are based on it being
suitable to use Lo-Fi "q=low" header.

+    bool came_from_websearch,
+    int websearch_chrome_joint_experiment_id,
+    bool is_preview,
+    URLPattern::SchemeMasks scheme_type) {
   // This function records new histograms based on the Navigation Timing
   // records. As such, the histograms should not depend on the deprecated timing
   // information collected in DocumentState. However, here for some reason we
@@ -411,6 +414,10 @@ void DumpHistograms(const WebPerformance& performance,
                                    websearch_chrome_joint_experiment_id,
                                    is_preview);
     if (data_reduction_proxy_was_used) {
+      bool in_lofi_enabled_group =
+          data_reduction_proxy::params::IsIncludedInLoFiEnabledFieldTrial();
+      bool in_lofi_control_group =
+          data_reduction_proxy::params::IsIncludedInLoFiControlFieldTrial();
       if ((scheme_type & URLPattern::SCHEME_HTTPS) == 0) {
         PLT_HISTOGRAM("PLT.PT_BeginToFinish_DataReductionProxy",
                       load_event_end - begin);
@@ -420,7 +427,7 @@ void DumpHistograms(const WebPerformance& performance,
                       load_event_end - navigation_start);
         PLT_HISTOGRAM("PLT.PT_StartToFinish_DataReductionProxy",
                       load_event_end - request_start);
-        if (lofi_status == data_reduction_proxy::LOFI_STATUS_ACTIVE) {
+        if (lofi_on && in_lofi_enabled_group) {
           PLT_HISTOGRAM("PLT.PT_BeginToFinish_DataReductionProxy_AutoLoFiOn",
                         load_event_end - begin);
           PLT_HISTOGRAM("PLT.PT_CommitToFinish_DataReductionProxy_AutoLoFiOn",
@@ -433,8 +440,7 @@ void DumpHistograms(const WebPerformance& performance,
             PLT_HISTOGRAM("PLT.BeginToFirstPaint_DataReductionProxy_AutoLoFiOn",
                           first_paint - begin);
           }
-        } else if (lofi_status ==
-                   data_reduction_proxy::LOFI_STATUS_ACTIVE_CONTROL) {
+        } else if (lofi_on && in_lofi_control_group) {
           PLT_HISTOGRAM("PLT.PT_BeginToFinish_DataReductionProxy_AutoLoFiOff",
                         load_event_end - begin);
           PLT_HISTOGRAM("PLT.PT_CommitToFinish_DataReductionProxy_AutoLoFiOff",
@@ -458,7 +464,7 @@ void DumpHistograms(const WebPerformance& performance,
                       load_event_end - navigation_start);
         PLT_HISTOGRAM("PLT.PT_StartToFinish_HTTPS_DataReductionProxy",
                       load_event_end - request_start);
-        if (lofi_status == data_reduction_proxy::LOFI_STATUS_ACTIVE) {
+        if (lofi_on && in_lofi_enabled_group) {
           PLT_HISTOGRAM(
               "PLT.PT_BeginToFinish_HTTPS_DataReductionProxy_AutoLoFiOn",
               load_event_end - begin);
@@ -476,8 +482,7 @@ void DumpHistograms(const WebPerformance& performance,
                 "PLT.BeginToFirstPaint_HTTPS_DataReductionProxy_AutoLoFiOn",
                 first_paint - begin);
           }
-        } else if (lofi_status ==
-                   data_reduction_proxy::LOFI_STATUS_ACTIVE_CONTROL) {
+        } else if (lofi_on && in_lofi_control_group) {
           PLT_HISTOGRAM(
               "PLT.PT_BeginToFinish_HTTPS_DataReductionProxy_AutoLoFiOff",
               load_event_end - begin);
@@ -524,15 +529,18 @@ void DumpHistograms(const WebPerformance& performance,
                                    websearch_chrome_joint_experiment_id,
                                    is_preview);
     if (data_reduction_proxy_was_used) {
+      bool in_lofi_enabled_group =
+          data_reduction_proxy::params::IsIncludedInLoFiEnabledFieldTrial();
+      bool in_lofi_control_group =
+          data_reduction_proxy::params::IsIncludedInLoFiControlFieldTrial();
       if ((scheme_type & URLPattern::SCHEME_HTTPS) == 0) {
         PLT_HISTOGRAM("PLT.PT_RequestToDomContentLoaded_DataReductionProxy",
                       dom_content_loaded_start - navigation_start);
-        if (lofi_status == data_reduction_proxy::LOFI_STATUS_ACTIVE) {
+        if (lofi_on && in_lofi_enabled_group) {
           PLT_HISTOGRAM(
               "PLT.PT_RequestToDomContentLoaded_DataReductionProxy_AutoLoFiOn",
               dom_content_loaded_start - navigation_start);
-        } else if (lofi_status ==
-                   data_reduction_proxy::LOFI_STATUS_ACTIVE_CONTROL) {
+        } else if (lofi_on && in_lofi_control_group) {
           PLT_HISTOGRAM(
               "PLT.PT_RequestToDomContentLoaded_DataReductionProxy_AutoLoFiOff",
               dom_content_loaded_start - navigation_start);
@@ -541,13 +549,12 @@ void DumpHistograms(const WebPerformance& performance,
         PLT_HISTOGRAM(
             "PLT.PT_RequestToDomContentLoaded_HTTPS_DataReductionProxy",
             dom_content_loaded_start - navigation_start);
-        if (lofi_status == data_reduction_proxy::LOFI_STATUS_ACTIVE) {
+        if (lofi_on && in_lofi_enabled_group) {
           PLT_HISTOGRAM(
               "PLT.PT_RequestToDomContentLoaded_HTTPS_DataReductionProxy_"
               "AutoLoFiOn",
               dom_content_loaded_start - navigation_start);
-        } else if (lofi_status ==
-                   data_reduction_proxy::LOFI_STATUS_ACTIVE_CONTROL) {
+        } else if (lofi_on && in_lofi_control_group) {
           PLT_HISTOGRAM(
               "PLT.PT_RequestToDomContentLoaded_HTTPS_DataReductionProxy_"
               "AutoLoFiOff",
@@ -878,13 +885,10 @@ void PageLoadHistograms::Dump(WebFrame* frame) {
       DocumentState::FromDataSource(frame->dataSource());
 
   bool data_reduction_proxy_was_used = false;
-  data_reduction_proxy::LoFiStatus lofi_status =
-      data_reduction_proxy::LOFI_STATUS_TEMPORARILY_OFF;
   if (!document_state->proxy_server().IsEmpty()) {
     bool handled =
-        Send(new DataReductionProxyViewHostMsg_DataReductionProxyStatus(
-            document_state->proxy_server(), &data_reduction_proxy_was_used,
-            &lofi_status));
+        Send(new DataReductionProxyViewHostMsg_IsDataReductionProxy(
+            document_state->proxy_server(), &data_reduction_proxy_was_used));
     // If the IPC call is not handled, then |data_reduction_proxy_was_used|
     // should remain |false|.
     DCHECK(handled || !data_reduction_proxy_was_used);
@@ -905,10 +909,10 @@ void PageLoadHistograms::Dump(WebFrame* frame) {
 
   // Metrics based on the timing information recorded for the Navigation Timing
   // API - http://www.w3.org/TR/navigation-timing/.
-  DumpHistograms(frame->performance(), document_state,
-                 data_reduction_proxy_was_used, lofi_status,
-                 came_from_websearch, websearch_chrome_joint_experiment_id,
-                 is_preview, scheme_type);
+  DumpHistograms(
+      frame->performance(), document_state, data_reduction_proxy_was_used,
+      false /* TODO: render_frame->IsLoFiOn() */, came_from_websearch,
+      websearch_chrome_joint_experiment_id, is_preview, scheme_type);
 
   // Old metrics based on the timing information stored in DocumentState. These
   // are deprecated and should go away.