Hook up FieldTrialRecorder to all BrowserChildProcessHostBindings.

Previously, it was only hooked up to the GPU process (Also the renderer
process, though that doesn't use the same bindings). This was causing
Features only read in the NetworkService process to not update
corresponding FieldTrial state.


Bug: 1018329
Change-Id: I94d2c15db86e0fb004c824f9cb66e0e6b6213942
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1884823
Commit-Queue: Matt Menke <mmenke@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#711945}

content/browser/browser_child_process_host_impl_receiver_bindings.cc

@@ -12,6 +12,8 @@
 #include "build/build_config.h"
 #include "components/discardable_memory/public/mojom/discardable_shared_memory_manager.mojom.h"
 #include "components/discardable_memory/service/discardable_shared_memory_manager.h"
+#include "content/browser/field_trial_recorder.h"
+#include "content/common/field_trial_recorder.mojom.h"
 #include "content/public/browser/browser_child_process_host_delegate.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/system_connector.h"
@@ -83,6 +85,12 @@ void BrowserChildProcessHostImpl::BindHostReceiver(
   }
 #endif
 
+  if (auto r = receiver.As<mojom::FieldTrialRecorder>()) {
+    base::PostTask(FROM_HERE, {BrowserThread::UI},
+                   base::BindOnce(&FieldTrialRecorder::Create, std::move(r)));
+    return;
+  }
+
   if (auto r = receiver.As<
                discardable_memory::mojom::DiscardableSharedMemoryManager>()) {
     discardable_memory::DiscardableSharedMemoryManager::Get()->Bind(

content/browser/gpu/gpu_process_host_receiver_bindings.cc

@@ -8,8 +8,6 @@
 
 #include "base/task/post_task.h"
 #include "build/build_config.h"
-#include "content/browser/field_trial_recorder.h"
-#include "content/common/field_trial_recorder.mojom.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
@@ -38,16 +36,6 @@ void BindAndroidOverlayProvider(
 
 void GpuProcessHost::BindHostReceiver(
     mojo::GenericPendingReceiver generic_receiver) {
-  if (auto field_trial_receiver =
-          generic_receiver.As<mojom::FieldTrialRecorder>()) {
-    mojo::PendingReceiver<mojom::FieldTrialRecorder> receiver(
-        std::move(field_trial_receiver));
-    base::PostTask(
-        FROM_HERE, {BrowserThread::UI},
-        base::BindOnce(&FieldTrialRecorder::Create, std::move(receiver)));
-    return;
-  }
-
 #if defined(OS_ANDROID)
   if (auto r = generic_receiver.As<media::mojom::AndroidOverlayProvider>()) {
     base::PostTask(FROM_HERE, {BrowserThread::UI},

content/browser/net/network_field_trial_browsertest.cc

+// Copyright 2019 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 "base/macros.h"
+#include "base/metrics/field_trial.h"
+#include "base/run_loop.h"
+#include "base/test/mock_entropy_provider.h"
+#include "content/browser/startup_helper.h"
+#include "content/public/browser/network_service_instance.h"
+#include "content/public/common/network_service_util.h"
+#include "content/public/test/content_browser_test.h"
+#include "mojo/public/cpp/bindings/remote.h"
+#include "services/network/public/mojom/network_service_test.mojom.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace content {
+
+namespace {
+
+const char kFieldTrialName[] = "UniqueFieldTrialName";
+const char kFieldTrialGroup[] = "UniqueFieldTrialGroupName";
+
+class TestFieldTrialListObserver : public base::FieldTrialList::Observer {
+ public:
+  TestFieldTrialListObserver() { base::FieldTrialList::AddObserver(this); }
+
+  ~TestFieldTrialListObserver() override {
+    base::FieldTrialList::RemoveObserver(this);
+  }
+
+  void WaitForTrialGroupToBeFinalized() { run_loop_.Run(); }
+
+  // base::FieldTrialList::Observer:
+  void OnFieldTrialGroupFinalized(const std::string& trial_name,
+                                  const std::string& group_name) override {
+    if (trial_name == kFieldTrialName) {
+      EXPECT_EQ(kFieldTrialGroup, group_name);
+      run_loop_.Quit();
+    }
+  }
+
+ private:
+  base::RunLoop run_loop_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestFieldTrialListObserver);
+};
+
+}  // namespace
+
+class NetworkFieldTrialBrowserTest : public ContentBrowserTest {
+ public:
+  NetworkFieldTrialBrowserTest()
+      : field_trial_list_(SetUpFieldTrialsAndFeatureList()) {
+    // The field trial needs to be created before the NetworkService is created
+    // so that the NetworkService will automatically be informed of its
+    // existence.
+    base::MockEntropyProvider entropy_provider;
+    base::FieldTrial* trial =
+        base::FieldTrialList::FactoryGetFieldTrialWithRandomizationSeed(
+            kFieldTrialName, 1 /* total_probability */, kFieldTrialGroup,
+            base::FieldTrial::ONE_TIME_RANDOMIZED, 0 /* randomization_seed */,
+            nullptr /* default_group_number */, &entropy_provider);
+    EXPECT_TRUE(trial);
+  }
+
+  ~NetworkFieldTrialBrowserTest() override = default;
+
+ private:
+  std::unique_ptr<base::FieldTrialList> field_trial_list_;
+
+  DISALLOW_COPY_AND_ASSIGN(NetworkFieldTrialBrowserTest);
+};
+
+// Test that when the network process activates a field trial, the browser
+// process is informed. See https://crbug.com/1018329
+IN_PROC_BROWSER_TEST_F(NetworkFieldTrialBrowserTest, FieldTrialRegistered) {
+  // The network::mojom::NetworkServiceTest interface is only available when the
+  // network service is out of process.
+  if (!IsOutOfProcessNetworkService())
+    return;
+
+  EXPECT_FALSE(base::FieldTrialList::IsTrialActive(kFieldTrialName));
+
+  // Tell the network service to activate the field trial and wait for it to
+  // inform the browser process about it.
+  TestFieldTrialListObserver observer;
+  mojo::Remote<network::mojom::NetworkServiceTest> network_service_test;
+  GetNetworkService()->BindTestInterface(
+      network_service_test.BindNewPipeAndPassReceiver());
+  network_service_test->ActivateFieldTrial(kFieldTrialName);
+  observer.WaitForTrialGroupToBeFinalized();
+
+  EXPECT_TRUE(base::FieldTrialList::IsTrialActive(kFieldTrialName));
+}
+
+}  // namespace content

content/public/test/network_service_test_helper.cc

@@ -14,6 +14,7 @@
 #include "base/feature_list.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop_current.h"
+#include "base/metrics/field_trial.h"
 #include "base/process/process.h"
 #include "build/build_config.h"
 #include "content/public/common/content_features.h"
@@ -230,6 +231,10 @@ class NetworkServiceTestHelper::NetworkServiceTestImpl
     std::move(callback).Run();
   }
 
+  void ActivateFieldTrial(const std::string& field_trial_name) override {
+    base::FieldTrialList::FindFullName(field_trial_name);
+  }
+
   void BindReceiver(
       mojo::PendingReceiver<network::mojom::NetworkServiceTest> receiver) {
     receivers_.Add(this, std::move(receiver));

content/test/BUILD.gn

@@ -940,6 +940,7 @@ test("content_browsertests") {
     "../browser/navigation_mhtml_browsertest.cc",
     "../browser/net/accept_header_browsertest.cc",
     "../browser/net/net_command_line_flags_browsertest.cc",
+    "../browser/net/network_field_trial_browsertest.cc",
     "../browser/net_info_browsertest.cc",
     "../browser/network_service_browsertest.cc",
     "../browser/network_service_restart_browsertest.cc",

services/network/public/mojom/network_service_test.mojom

@@ -100,4 +100,8 @@ interface NetworkServiceTest {
   // redirection.
   [Sync]
   Log(string message) => ();
+
+  // Activates the specified field trial. Intended for use verifying that the
+  // network service informs the main process when a field trial is activated.
+  ActivateFieldTrial(string field_trial_name);
 };