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_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"
 #include "third_party/WebKit/public/web/WebView.h"
 #include "url/gurl.h"
 
 using blink::WebDataSource;
 using blink::WebFrame;
 using blink::WebPerformance;
 using blink::WebString;
 using base::Time;
 using base::TimeDelta;
 using content::DocumentState;
 
 const size_t kPLTCount = 100;
 
 namespace {
 
+const char kEnabled[] = "Enabled";
+const char kControl[] = "Control";
+
 // ID indicating that no GWS-Chrome joint experiment is active.
 const int kNoExperiment = 0;
 
 // Max ID of GWS-Chrome joint experiment. If you change this value, please
 // update PLT_HISTOGRAM_WITH_GWS_VARIANT accordingly.
 const int kMaxExperimentID = 20;
 
 TimeDelta kPLTMin() {
   return TimeDelta::FromMilliseconds(10);
 }
@@ skipping 151 matching lines @@
   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 lofi_on,
                     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();
@@ skipping 171 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 =

[bengr, 2015/08/25 00:00:01]

Rename as in_lofi_enabled_group, or lofi_available.

[megjablon, 2015/08/25 20:29:45]

Done.

+          (base::FieldTrialList::FindFullName(
+               data_reduction_proxy::params::GetLoFiFieldTrialName()) ==
+           kEnabled);
+      bool in_lofi_control =

[bengr, 2015/08/25 00:00:01]

Rename as in_lofi_control_group, or lofi_control_group.

[megjablon, 2015/08/25 20:29:45]

Done.

+          (base::FieldTrialList::FindFullName(
+               data_reduction_proxy::params::GetLoFiFieldTrialName()) ==
+           kControl);
       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 (lofi_on && in_lofi_enabled) {
           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);
           }
-        } else if (lofi_status ==
-                   data_reduction_proxy::LOFI_STATUS_ACTIVE_CONTROL) {
+        } else if (lofi_on && in_lofi_control) {
           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 (lofi_on && in_lofi_enabled) {
           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 (lofi_on && in_lofi_control) {
           PLT_HISTOGRAM(
               "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(
@@ skipping 26 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 =
+          (base::FieldTrialList::FindFullName(
+               data_reduction_proxy::params::GetLoFiFieldTrialName()) ==
+           kEnabled);
+      bool in_lofi_control =
+          (base::FieldTrialList::FindFullName(
+               data_reduction_proxy::params::GetLoFiFieldTrialName()) ==

[bengr, 2015/08/25 00:00:01]

It's probably worth the extra few lines of code to have methods in an anonymous
namespace for these.

E.g.:
bool IsInLoFiControlGroup() {
  return base::FieldTrialList::FindFullName(
    data_reduction_proxy::params::GetLoFiFieldTrialName()) == kControl;
}

Alternatively, you could have one function return an enum. I'm fine with either.

[megjablon, 2015/08/25 20:29:45]

Done.

+           kControl);
       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) {
           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) {
           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 (lofi_on && in_lofi_enabled) {
           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) {
           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 299 matching lines @@
     return false;
   }
 
   // Ignore multipart requests.
   if (frame->dataSource()->response().isMultipartPayload())
     return false;
 
   return true;
 }
 
-void PageLoadHistograms::Dump(WebFrame* frame) {
+void PageLoadHistograms::Dump(WebFrame* frame, bool lofi_used) {
   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()) {
     Send(new DataReductionProxyViewHostMsg_DataReductionProxyStatus(
-        document_state->proxy_server(), &data_reduction_proxy_was_used,
-        &lofi_status));
+        document_state->proxy_server(), &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);
+                 data_reduction_proxy_was_used, lofi_used, 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 28 matching lines @@
                   first_layout - navigation_start);
 
   Time response_start = Time::FromDoubleT(performance.responseStart());
   if (!response_start.is_null())
     PLT_HISTOGRAM("PLT.PT.ResponseStartToFirstLayout",
                   first_layout - response_start);
 
   dumped_first_layout_histograms_ = true;
 }
 
-void PageLoadHistograms::FrameWillClose(WebFrame* frame) {
-  Dump(frame);
+void PageLoadHistograms::FrameWillClose(WebFrame* frame, bool lofi_used) {
+  Dump(frame, lofi_used);
 }
 
-void PageLoadHistograms::ClosePage() {
+void PageLoadHistograms::ClosePage(bool lofi_used) {
   // TODO(davemoore) This code should be removed once willClose() gets
   // called when a page is destroyed. page_load_histograms_.Dump() is safe
   // to call multiple times for the same frame, but it will simplify things.
-  Dump(render_view()->GetWebView()->mainFrame());
+  Dump(render_view()->GetWebView()->mainFrame(), lofi_used);
 }
 
 void PageLoadHistograms::DidUpdateLayout() {
   DCHECK(content::RenderThread::Get());
   // Normally, PageLoadHistograms dumps all histograms in the FrameWillClose or
   // ClosePage callbacks, which happen as a page is being torn down. However,
   // renderers that are killed by fast shutdown (for example, renderers closed
   // due to the user closing a tab) don't get a chance to run these callbacks
   // (see crbug.com/382542 for details).
   //
@@ skipping 41 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();
 }