Holdback NetInfo Client hints value when holdback experiment is enabled

Set the network quality in the client hints to the holdback
value if network quality web holdback experiment is enabled.

Change-Id: Ifc6bcef11c9f980c340d4286e4472572e871f6f7
Bug: 880545
Reviewed-on: https://chromium-review.googlesource.com/c/1227684Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595924}

chrome/browser/client_hints/client_hints.cc

@@ -9,6 +9,7 @@
 #include "chrome/browser/client_hints/client_hints.h"
 
 #include "base/memory/ptr_util.h"
+#include "base/metrics/field_trial_params.h"
 #include "base/rand_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "build/build_config.h"
@@ -21,10 +22,12 @@
 #include "components/content_settings/core/common/content_settings_utils.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/common/content_features.h"
 #include "content/public/common/origin_util.h"
 #include "net/base/url_util.h"
 #include "net/http/http_request_headers.h"
 #include "net/nqe/effective_connection_type.h"
+#include "net/nqe/network_quality_estimator_params.h"
 #include "net/url_request/url_request.h"
 #include "services/network/public/cpp/network_quality_tracker.h"
 #include "third_party/blink/public/common/client_hints/client_hints.h"
@@ -128,6 +131,30 @@ std::string DoubleToSpecCompliantString(double value) {
   return "0" + result;
 }
 
+// Return the effective connection type value overridden for web APIs.
+// If no override value has been set, a null value is returned.
+base::Optional<net::EffectiveConnectionType>
+GetWebHoldbackEffectiveConnectionType() {
+  if (!base::FeatureList::IsEnabled(
+          features::kNetworkQualityEstimatorWebHoldback)) {
+    return base::nullopt;
+  }
+  std::string effective_connection_type_param =
+      base::GetFieldTrialParamValueByFeature(
+          features::kNetworkQualityEstimatorWebHoldback,
+          "web_effective_connection_type_override");
+
+  base::Optional<net::EffectiveConnectionType> effective_connection_type =
+      net::GetEffectiveConnectionTypeForName(effective_connection_type_param);
+  DCHECK(effective_connection_type_param.empty() || effective_connection_type);
+
+  if (!effective_connection_type)
+    return base::nullopt;
+  DCHECK_NE(net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN,
+            effective_connection_type.value());
+  return effective_connection_type;
+}
+
 }  // namespace
 
 namespace client_hints {
@@ -282,20 +309,42 @@ GetAdditionalNavigationRequestClientHintsHeaders(
       g_browser_process->network_quality_tracker();
 
   if (web_client_hints.IsEnabled(blink::mojom::WebClientHintsType::kRtt)) {
+    base::Optional<net::EffectiveConnectionType> web_holdback_ect =
+        GetWebHoldbackEffectiveConnectionType();
+
+    base::TimeDelta http_rtt;
+    if (web_holdback_ect.has_value()) {
+      http_rtt = net::NetworkQualityEstimatorParams::GetDefaultTypicalHttpRtt(
+          web_holdback_ect.value());
+    } else {
+      http_rtt = network_quality_tracker->GetHttpRTT();
+    }
     additional_headers->SetHeader(
         blink::kClientHintsHeaderMapping[static_cast<int>(
             blink::mojom::WebClientHintsType::kRtt)],
-        base::NumberToString(internal::RoundRtt(
-            url.host(), network_quality_tracker->GetHttpRTT())));
+        base::NumberToString(internal::RoundRtt(url.host(), http_rtt)));
   }
 
   if (web_client_hints.IsEnabled(blink::mojom::WebClientHintsType::kDownlink)) {
+    base::Optional<net::EffectiveConnectionType> web_holdback_ect =
+        GetWebHoldbackEffectiveConnectionType();
+
+    int32_t downlink_throughput_kbps;
+
+    if (web_holdback_ect.has_value()) {
+      downlink_throughput_kbps =
+          net::NetworkQualityEstimatorParams::GetDefaultTypicalDownlinkKbps(
+              web_holdback_ect.value());
+    } else {
+      downlink_throughput_kbps =
+          network_quality_tracker->GetDownstreamThroughputKbps();
+    }
+
     additional_headers->SetHeader(
         blink::kClientHintsHeaderMapping[static_cast<int>(
             blink::mojom::WebClientHintsType::kDownlink)],
-        DoubleToSpecCompliantString(internal::RoundKbpsToMbps(
-            url.host(),
-            network_quality_tracker->GetDownstreamThroughputKbps())));
+        DoubleToSpecCompliantString(
+            internal::RoundKbpsToMbps(url.host(), downlink_throughput_kbps)));
   }
 
   if (web_client_hints.IsEnabled(blink::mojom::WebClientHintsType::kEct)) {
@@ -304,8 +353,14 @@ GetAdditionalNavigationRequestClientHintsHeaders(
     DCHECK_EQ(blink::kWebEffectiveConnectionTypeMappingCount,
               static_cast<size_t>(net::EFFECTIVE_CONNECTION_TYPE_LAST));
 
+    base::Optional<net::EffectiveConnectionType> web_holdback_ect =
+        GetWebHoldbackEffectiveConnectionType();
+
     int effective_connection_type =
-        static_cast<int>(network_quality_tracker->GetEffectiveConnectionType());
+        web_holdback_ect.has_value()
+            ? web_holdback_ect.value()
+            : static_cast<int>(
+                  network_quality_tracker->GetEffectiveConnectionType());
 
     additional_headers->SetHeader(
         blink::kClientHintsHeaderMapping[static_cast<int>(

chrome/browser/client_hints/client_hints_browsertest.cc

@@ -6,6 +6,7 @@
 
 #include "base/bind.h"
 #include "base/command_line.h"
+#include "base/metrics/field_trial_param_associator.h"
 #include "base/run_loop.h"
 #include "base/stl_util.h"
 #include "base/test/metrics/histogram_tester.h"
@@ -23,6 +24,7 @@
 #include "components/content_settings/core/common/pref_names.h"
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/common/content_features.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_utils.h"
@@ -262,6 +264,11 @@ class ClientHintsBrowserTest : public InProcessBrowserTest,
                      ->IsContentBlocked(CONTENT_SETTINGS_TYPE_JAVASCRIPT));
   }
 
+  void SetExpectedEffectiveConnectionType(
+      net::EffectiveConnectionType effective_connection_type) {
+    expected_ect = effective_connection_type;
+  }
+
   const GURL& accept_ch_with_lifetime_http_local_url() const {
     return accept_ch_with_lifetime_http_local_url_;
   }
@@ -523,7 +530,8 @@ class ClientHintsBrowserTest : public InProcessBrowserTest,
     EXPECT_TRUE(IsSimilarToIntABNF(request.headers.find("rtt")->second));
     // Verify that RTT value is a multiple of 50 milliseconds.
     EXPECT_EQ(0, rtt_value % 50);
-    EXPECT_GE(3000, rtt_value);
+    EXPECT_GE(expected_ect == net::EFFECTIVE_CONNECTION_TYPE_2G ? 3000 : 500,
+              rtt_value);
 
     double mbps_value = 0.0;
     EXPECT_TRUE(base::StringToDouble(request.headers.find("downlink")->second,
@@ -549,14 +557,27 @@ class ClientHintsBrowserTest : public InProcessBrowserTest,
       // Effective connection type is forced to 2G using command line in these
       // tests. RTT is expected to be 1800 msec but leave some gap to account
       // for added noise and randomization.
-      EXPECT_NEAR(1800, rtt_value, 360);
+      if (expected_ect == net::EFFECTIVE_CONNECTION_TYPE_2G) {
+        EXPECT_NEAR(1800, rtt_value, 360);
+      } else if (expected_ect == net::EFFECTIVE_CONNECTION_TYPE_3G) {
+        EXPECT_NEAR(450, rtt_value, 90);
+      } else {
+        NOTREACHED();
+      }
 
       // Effective connection type is forced to 2G using command line in these
       // tests. downlink is expected to be 0.075 Mbps but leave some gap to
       // account for added noise and randomization.
-      EXPECT_NEAR(0.075, mbps_value, 0.05);
+      if (expected_ect == net::EFFECTIVE_CONNECTION_TYPE_2G) {
+        EXPECT_NEAR(0.075, mbps_value, 0.05);
+      } else if (expected_ect == net::EFFECTIVE_CONNECTION_TYPE_3G) {
+        EXPECT_NEAR(0.4, mbps_value, 0.1);
+      } else {
+        NOTREACHED();
+      }
 
-      EXPECT_EQ("2g", request.headers.find("ect")->second);
+      EXPECT_EQ(expected_ect == net::EFFECTIVE_CONNECTION_TYPE_2G ? "2g" : "3g",
+                request.headers.find("ect")->second);
     }
   }
 
@@ -595,6 +616,9 @@ class ClientHintsBrowserTest : public InProcessBrowserTest,
 
   std::unique_ptr<ThirdPartyURLLoaderInterceptor> request_interceptor_;
 
+  // Set to 2G in SetUpCommandLine().
+  net::EffectiveConnectionType expected_ect = net::EFFECTIVE_CONNECTION_TYPE_2G;
+
   DISALLOW_COPY_AND_ASSIGN(ClientHintsBrowserTest);
 };
 
@@ -1418,3 +1442,75 @@ IN_PROC_BROWSER_TEST_P(ClientHintsBrowserTest,
   // frame request.
   EXPECT_EQ(12u, count_client_hints_headers_seen());
 }
+
+class ClientHintsWebHoldbackBrowserTest : public ClientHintsBrowserTest {
+ public:
+  ClientHintsWebHoldbackBrowserTest() : ClientHintsBrowserTest() {
+    ConfigureHoldbackExperiment();
+  }
+
+  net::EffectiveConnectionType web_effective_connection_type_override() const {
+    return web_effective_connection_type_override_;
+  }
+
+ private:
+  void ConfigureHoldbackExperiment() {
+    base::FieldTrialParamAssociator::GetInstance()->ClearAllParamsForTesting();
+    const std::string kTrialName = "TrialFoo";
+    const std::string kGroupName = "GroupFoo";  // Value not used
+
+    scoped_refptr<base::FieldTrial> trial =
+        base::FieldTrialList::CreateFieldTrial(kTrialName, kGroupName);
+
+    std::map<std::string, std::string> params;
+
+    params["web_effective_connection_type_override"] =
+        net::GetNameForEffectiveConnectionType(
+            web_effective_connection_type_override_);
+    ASSERT_TRUE(
+        base::FieldTrialParamAssociator::GetInstance()
+            ->AssociateFieldTrialParams(kTrialName, kGroupName, params));
+
+    std::unique_ptr<base::FeatureList> feature_list(new base::FeatureList);
+    feature_list->RegisterFieldTrialOverride(
+        features::kNetworkQualityEstimatorWebHoldback.name,
+        base::FeatureList::OVERRIDE_ENABLE_FEATURE, trial.get());
+    scoped_feature_list_override_.InitWithFeatureList(std::move(feature_list));
+  }
+
+  const net::EffectiveConnectionType web_effective_connection_type_override_ =
+      net::EFFECTIVE_CONNECTION_TYPE_3G;
+
+  base::test::ScopedFeatureList scoped_feature_list_override_;
+};
+
+// Make sure that when NetInfo holdback experiment is enabled, the NetInfo APIs
+// and client hints return the overridden values. Verify that the client hints
+// are overridden on both main frame and subresource requests.
+IN_PROC_BROWSER_TEST_F(ClientHintsWebHoldbackBrowserTest,
+                       EffectiveConnectionTypeChangeNotified) {
+  SetExpectedEffectiveConnectionType(web_effective_connection_type_override());
+
+  SetClientHintExpectationsOnMainFrame(false);
+  SetClientHintExpectationsOnSubresources(true);
+
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_TRUE(embedded_test_server()->Start());
+  ui_test_utils::NavigateToURL(browser(), accept_ch_with_lifetime_url());
+  EXPECT_EQ(0u, count_client_hints_headers_seen());
+  EXPECT_EQ(0u, third_party_request_count_seen());
+  EXPECT_EQ(0u, third_party_client_hints_count_seen());
+
+  SetClientHintExpectationsOnMainFrame(true);
+  SetClientHintExpectationsOnSubresources(true);
+  ui_test_utils::NavigateToURL(
+      browser(), accept_ch_without_lifetime_with_subresource_url());
+  base::RunLoop().RunUntilIdle();
+  content::FetchHistogramsFromChildProcesses();
+  SubprocessMetricsProvider::MergeHistogramDeltasForTesting();
+
+  EXPECT_EQ(12u, count_client_hints_headers_seen());
+  EXPECT_EQ(0u, third_party_request_count_seen());
+  EXPECT_EQ(0u, third_party_client_hints_count_seen());
+}

third_party/blink/renderer/core/loader/frame_fetch_context.cc

@@ -991,8 +991,14 @@ void FrameFetchContext::AddClientHintsIfNecessary(
 
   if (ShouldSendClientHint(mojom::WebClientHintsType::kRtt, hints_preferences,
                            enabled_hints)) {
-    unsigned long rtt = GetNetworkStateNotifier().RoundRtt(
-        request.Url().Host(), GetNetworkStateNotifier().HttpRtt());
+    base::Optional<TimeDelta> http_rtt =
+        GetNetworkStateNotifier().GetWebHoldbackHttpRtt();
+    if (!http_rtt) {
+      http_rtt = GetNetworkStateNotifier().HttpRtt();
+    }
+
+    unsigned long rtt =
+        GetNetworkStateNotifier().RoundRtt(request.Url().Host(), http_rtt);
     request.AddHTTPHeaderField(
         blink::kClientHintsHeaderMapping[static_cast<size_t>(
             mojom::WebClientHintsType::kRtt)],
@@ -1001,9 +1007,14 @@ void FrameFetchContext::AddClientHintsIfNecessary(
 
   if (ShouldSendClientHint(mojom::WebClientHintsType::kDownlink,
                            hints_preferences, enabled_hints)) {
-    double mbps = GetNetworkStateNotifier().RoundMbps(
-        request.Url().Host(),
-        GetNetworkStateNotifier().DownlinkThroughputMbps());
+    base::Optional<double> throughput_mbps =
+        GetNetworkStateNotifier().GetWebHoldbackDownlinkThroughputMbps();
+    if (!throughput_mbps) {
+      throughput_mbps = GetNetworkStateNotifier().DownlinkThroughputMbps();
+    }
+
+    double mbps = GetNetworkStateNotifier().RoundMbps(request.Url().Host(),
+                                                      throughput_mbps);
     request.AddHTTPHeaderField(
         blink::kClientHintsHeaderMapping[static_cast<size_t>(
             mojom::WebClientHintsType::kDownlink)],
@@ -1012,11 +1023,16 @@ void FrameFetchContext::AddClientHintsIfNecessary(
 
   if (ShouldSendClientHint(mojom::WebClientHintsType::kEct, hints_preferences,
                            enabled_hints)) {
+    base::Optional<WebEffectiveConnectionType> holdback_ect =
+        GetNetworkStateNotifier().GetWebHoldbackEffectiveType();
+    if (!holdback_ect)
+      holdback_ect = GetNetworkStateNotifier().EffectiveType();
+
     request.AddHTTPHeaderField(
         blink::kClientHintsHeaderMapping[static_cast<size_t>(
             mojom::WebClientHintsType::kEct)],
         AtomicString(NetworkStateNotifier::EffectiveConnectionTypeToString(
-            GetNetworkStateNotifier().EffectiveType())));
+            holdback_ect.value())));
   }
 }