[DRP] Consistently use LoFi for an entire page

chrome/renderer/page_load_histograms.cc

 // Copyright (c) 2012 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 "chrome/renderer/page_load_histograms.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "chrome/common/chrome_switches.h"
 #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"
 #include "net/base/url_util.h"
 #include "net/http/http_response_headers.h"
 #include "third_party/WebKit/public/platform/WebURLRequest.h"
 #include "third_party/WebKit/public/platform/WebURLResponse.h"
 #include "third_party/WebKit/public/web/WebDocument.h"
 #include "third_party/WebKit/public/web/WebFrame.h"
 #include "third_party/WebKit/public/web/WebPerformance.h"
@@ skipping 173 matching lines @@
     return kNoExperiment;
 
   int experiment_id;
   if (!base::StringToInt(value, &experiment_id))
     return kNoExperiment;
   if (0 < experiment_id && experiment_id <= kMaxExperimentID)
     return experiment_id;
   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,
+    // The current conditions may use the Lo-Fi request header.
+    bool is_using_lofi,

[bengr, 2015/10/12 21:06:40]

How about lofi_active_for_page? Then the comment could be:
// LoFi was used, unless part of the control group.

[megjablon, 2015/10/12 23:08:04]

Done.

+    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
   // check if document_state->request_time() is null. TODO(ppi): find out why
   // and remove DocumentState from the parameter list.
   Time request = document_state->request_time();
 
   Time navigation_start = Time::FromDoubleT(performance.navigationStart());
   Time redirect_start = Time::FromDoubleT(performance.redirectStart());
   Time redirect_end = Time::FromDoubleT(performance.redirectEnd());
@@ skipping 167 matching lines @@
                                    load_event_end - navigation_start,
                                    came_from_websearch,
                                    websearch_chrome_joint_experiment_id,
                                    is_preview);
     PLT_HISTOGRAM_WITH_GWS_VARIANT("PLT.PT_StartToFinish",
                                    load_event_end - request_start,
                                    came_from_websearch,
                                    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);
         PLT_HISTOGRAM("PLT.PT_CommitToFinish_DataReductionProxy",
                       load_event_end - response_start);
         PLT_HISTOGRAM("PLT.PT_RequestToFinish_DataReductionProxy",
                       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 (is_using_lofi && in_lofi_enabled_group) {
           PLT_HISTOGRAM("PLT.PT_BeginToFinish_DataReductionProxy_AutoLoFiOn",
                         load_event_end - begin);
           PLT_HISTOGRAM("PLT.PT_CommitToFinish_DataReductionProxy_AutoLoFiOn",
                         load_event_end - response_start);
           PLT_HISTOGRAM("PLT.PT_RequestToFinish_DataReductionProxy_AutoLoFiOn",
                         load_event_end - navigation_start);
           PLT_HISTOGRAM("PLT.PT_StartToFinish_DataReductionProxy_AutoLoFiOn",
                         load_event_end - request_start);
           if (!first_paint.is_null()) {
             PLT_HISTOGRAM("PLT.BeginToFirstPaint_DataReductionProxy_AutoLoFiOn",
                           first_paint - begin);
           }

[bengr, 2015/10/12 21:06:40]

It looks like the indentation is incorrect here and on the following line.

[megjablon, 2015/10/12 23:08:04]

Indentation is good.

-        } else if (lofi_status ==
-                   data_reduction_proxy::LOFI_STATUS_ACTIVE_CONTROL) {
+        } else if (is_using_lofi && in_lofi_control_group) {
           PLT_HISTOGRAM("PLT.PT_BeginToFinish_DataReductionProxy_AutoLoFiOff",
                         load_event_end - begin);
           PLT_HISTOGRAM("PLT.PT_CommitToFinish_DataReductionProxy_AutoLoFiOff",
                         load_event_end - response_start);
           PLT_HISTOGRAM("PLT.PT_RequestToFinish_DataReductionProxy_AutoLoFiOff",
                         load_event_end - navigation_start);
           PLT_HISTOGRAM("PLT.PT_StartToFinish_DataReductionProxy_AutoLoFiOff",
                         load_event_end - request_start);
           if (!first_paint.is_null()) {
             PLT_HISTOGRAM(
                 "PLT.BeginToFirstPaint_DataReductionProxy_AutoLoFiOff",
                 first_paint - begin);
           }
         }
       } else {
         PLT_HISTOGRAM("PLT.PT_BeginToFinish_HTTPS_DataReductionProxy",
                       load_event_end - begin);
         PLT_HISTOGRAM("PLT.PT_CommitToFinish_HTTPS_DataReductionProxy",
                       load_event_end - response_start);
         PLT_HISTOGRAM("PLT.PT_RequestToFinish_HTTPS_DataReductionProxy",
                       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 (is_using_lofi && in_lofi_enabled_group) {
           PLT_HISTOGRAM(
               "PLT.PT_BeginToFinish_HTTPS_DataReductionProxy_AutoLoFiOn",
               load_event_end - begin);
           PLT_HISTOGRAM(
               "PLT.PT_CommitToFinish_HTTPS_DataReductionProxy_AutoLoFiOn",
               load_event_end - response_start);
           PLT_HISTOGRAM(
               "PLT.PT_RequestToFinish_HTTPS_DataReductionProxy_AutoLoFiOn",
               load_event_end - navigation_start);
           PLT_HISTOGRAM(
               "PLT.PT_StartToFinish_HTTPS_DataReductionProxy_AutoLoFiOn",
               load_event_end - request_start);
           if (!first_paint.is_null()) {
             PLT_HISTOGRAM(
                 "PLT.BeginToFirstPaint_HTTPS_DataReductionProxy_AutoLoFiOn",
                 first_paint - begin);
           }
-        } else if (lofi_status ==
-                   data_reduction_proxy::LOFI_STATUS_ACTIVE_CONTROL) {
+        } else if (is_using_lofi && in_lofi_control_group) {
           PLT_HISTOGRAM(

[bengr, 2015/10/12 21:06:40]

These cases seem very repetitive. Can you put these in one or a small number of
anonymous functions? 

[megjablon, 2015/10/12 23:08:04]

That seems outside of the scope of this cl to fix.

[bengr, 2015/10/14 22:34:10]

OK. Please add a TODO.

[megjablon, 2015/10/15 03:44:05]

Done.

               "PLT.PT_BeginToFinish_HTTPS_DataReductionProxy_AutoLoFiOff",
               load_event_end - begin);
           PLT_HISTOGRAM(
               "PLT.PT_CommitToFinish_HTTPS_DataReductionProxy_AutoLoFiOff",
               load_event_end - response_start);
           PLT_HISTOGRAM(
               "PLT.PT_RequestToFinish_HTTPS_DataReductionProxy_AutoLoFiOff",
               load_event_end - navigation_start);
           PLT_HISTOGRAM(
               "PLT.PT_StartToFinish_HTTPS_DataReductionProxy_AutoLoFiOff",
@@ skipping 25 matching lines @@
       }
     }
   }
   if (!dom_content_loaded_start.is_null()) {
     PLT_HISTOGRAM_WITH_GWS_VARIANT("PLT.PT_RequestToDomContentLoaded",
                                    dom_content_loaded_start - navigation_start,
                                    came_from_websearch,
                                    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 (is_using_lofi && 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 (is_using_lofi && in_lofi_control_group) {
           PLT_HISTOGRAM(
               "PLT.PT_RequestToDomContentLoaded_DataReductionProxy_AutoLoFiOff",
               dom_content_loaded_start - navigation_start);
         }
       } else {
         PLT_HISTOGRAM(
             "PLT.PT_RequestToDomContentLoaded_HTTPS_DataReductionProxy",
             dom_content_loaded_start - navigation_start);
-        if (lofi_status == data_reduction_proxy::LOFI_STATUS_ACTIVE) {
+        if (is_using_lofi && 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 (is_using_lofi && in_lofi_control_group) {
           PLT_HISTOGRAM(
               "PLT.PT_RequestToDomContentLoaded_HTTPS_DataReductionProxy_"
               "AutoLoFiOff",
               dom_content_loaded_start - navigation_start);
         }
       }
     }
   }
   PLT_HISTOGRAM_WITH_GWS_VARIANT("PLT.PT_BeginToCommit",
                                  response_start - begin,
@@ skipping 310 matching lines @@
   if (!ShouldDump(frame))
     return;
 
   URLPattern::SchemeMasks scheme_type =
       GetSupportedSchemeType(frame->document().url());
 
   DocumentState* document_state =
       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);
   }
 
   bool came_from_websearch =
       IsFromGoogleSearchResult(frame->document().url(),
                                GURL(frame->document().referrer()));
   int websearch_chrome_joint_experiment_id = kNoExperiment;
   bool is_preview = false;
   if (came_from_websearch) {
     websearch_chrome_joint_experiment_id =
         GetQueryStringBasedExperiment(GURL(frame->document().referrer()));
     is_preview = ViaHeaderContains(frame, "1.1 Google Instant Proxy Preview");
   }
 
   MaybeDumpFirstLayoutHistograms();
 
   // 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->IsUsingLoFi() */, 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.
   DumpDeprecatedHistograms(frame->performance(), document_state,
                            data_reduction_proxy_was_used,
                            came_from_websearch,
                            websearch_chrome_joint_experiment_id,
                            is_preview,
                            scheme_type);
 
@@ skipping 98 matching lines @@
 
   DCHECK(document_state);
   DCHECK(ds);
   GURL url(ds->request().url());
   Time start = document_state->start_load_time();
   Time finish = document_state->finish_load_time();
   // TODO(mbelshe): should we log more stats?
   VLOG(1) << "PLT: " << (finish - start).InMilliseconds() << "ms "
           << url.spec();
 }