Fix ChromeBrowserMainBrowserTest.VariationsServiceStartsRequestOnNetworkChange

The browsertest changes in the following CL is moved to this CL since it
is not related to the following CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1323658/7/chrome/browser/chrome_browser_main_browsertest.cc

This CL fixes the ChromeBrowserMainBrowserTest.VariationsServiceStartsRequestOnNetworkChange
browser test which failed on the release Builder linux64 machine. For
more details, please refer to the following issue link.

In the release Builder linux64 machine, the intermediate connection state
is CONNECTION_ETHERNET, so in the test it needs to make sure the
NetworkConnectionTracker's underlying network connection state is
CONNECTION_NONE before it starts to run the actual test code.

Bug: 902784
Change-Id: I16866f1fb0e8834dfa621930efa83aba26aa52d0
Reviewed-on: https://chromium-review.googlesource.com/c/1336433
Commit-Queue: Jun Cai <juncai@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608457}

chrome/browser/chrome_browser_main_browsertest.cc

@@ -41,50 +41,93 @@ class ChromeBrowserMainPartsTestApi {
 
 namespace {
 
-// Simulates a network connection change.
-void SimulateNetworkChange(network::mojom::ConnectionType type) {
-  if (base::FeatureList::IsEnabled(network::features::kNetworkService) &&
-      !content::IsNetworkServiceRunningInProcess()) {
-    network::mojom::NetworkServiceTestPtr network_service_test;
-    content::ServiceManagerConnection::GetForProcess()
-        ->GetConnector()
-        ->BindInterface(content::mojom::kNetworkServiceName,
-                        &network_service_test);
-    base::RunLoop run_loop;
-    network_service_test->SimulateNetworkChange(type, run_loop.QuitClosure());
-    run_loop.Run();
-    return;
+class NetworkConnectionChangeSimulator
+    : public network::NetworkConnectionTracker::NetworkConnectionObserver {
+ public:
+  NetworkConnectionChangeSimulator() = default;
+
+  void SetConnectionType(network::mojom::ConnectionType type) {
+    network::NetworkConnectionTracker* network_connection_tracker =
+        content::GetNetworkConnectionTracker();
+    network::mojom::ConnectionType connection_type =
+        network::mojom::ConnectionType::CONNECTION_UNKNOWN;
+    run_loop_ = std::make_unique<base::RunLoop>();
+    network_connection_tracker->AddNetworkConnectionObserver(this);
+    SimulateNetworkChange(type);
+    // Make sure the underlying network connection type becomes |type|.
+    // The while loop is necessary because in some machine such as "Builder
+    // linux64 trunk", the |connection_type| can be CONNECTION_ETHERNET before
+    // it changes to |type|. So here it needs to wait until the
+    // |connection_type| becomes |type|.
+    while (!network_connection_tracker->GetConnectionType(
+               &connection_type,
+               base::BindOnce(
+                   &NetworkConnectionChangeSimulator::OnConnectionChanged,
+                   base::Unretained(this))) ||
+           connection_type != type) {
+      SimulateNetworkChange(type);
+      run_loop_->Run();
+      run_loop_ = std::make_unique<base::RunLoop>();
+    }
+    network_connection_tracker->RemoveNetworkConnectionObserver(this);
   }
-  net::NetworkChangeNotifier::NotifyObserversOfNetworkChangeForTests(
-      net::NetworkChangeNotifier::ConnectionType(type));
-}
+
+ private:
+  // Simulates a network connection change.
+  static void SimulateNetworkChange(network::mojom::ConnectionType type) {
+    if (base::FeatureList::IsEnabled(network::features::kNetworkService) &&
+        !content::IsNetworkServiceRunningInProcess()) {
+      network::mojom::NetworkServiceTestPtr network_service_test;
+      content::ServiceManagerConnection::GetForProcess()
+          ->GetConnector()
+          ->BindInterface(content::mojom::kNetworkServiceName,
+                          &network_service_test);
+      base::RunLoop run_loop;
+      network_service_test->SimulateNetworkChange(type, run_loop.QuitClosure());
+      run_loop.Run();
+      return;
+    }
+    net::NetworkChangeNotifier::NotifyObserversOfNetworkChangeForTests(
+        net::NetworkChangeNotifier::ConnectionType(type));
+  }
+
+  // network::NetworkConnectionTracker::NetworkConnectionObserver:
+  void OnConnectionChanged(network::mojom::ConnectionType type) override {
+    run_loop_->Quit();
+  }
+
+  std::unique_ptr<base::RunLoop> run_loop_;
+
+  DISALLOW_COPY_AND_ASSIGN(NetworkConnectionChangeSimulator);
+};
 
 // ChromeBrowserMainExtraParts is used to initialize the network state.
 class ChromeBrowserMainExtraPartsNetFactoryInstaller
     : public ChromeBrowserMainExtraParts {
  public:
-  ChromeBrowserMainExtraPartsNetFactoryInstaller() = default;
+  explicit ChromeBrowserMainExtraPartsNetFactoryInstaller(
+      NetworkConnectionChangeSimulator* network_change_simulator)
+      : network_change_simulator_(network_change_simulator) {
+    EXPECT_TRUE(network_change_simulator_);
+  }
 
   // ChromeBrowserMainExtraParts:
   void PreEarlyInitialization() override {}
   void ServiceManagerConnectionStarted(
       content::ServiceManagerConnection* connection) override {
-    SimulateNetworkChange(network::mojom::ConnectionType::CONNECTION_NONE);
+    network_change_simulator_->SetConnectionType(
+        network::mojom::ConnectionType::CONNECTION_NONE);
   }
 
  private:
+  NetworkConnectionChangeSimulator* network_change_simulator_ = nullptr;
+
   DISALLOW_COPY_AND_ASSIGN(ChromeBrowserMainExtraPartsNetFactoryInstaller);
 };
 
-class ChromeBrowserMainBrowserTest
-    : public InProcessBrowserTest,
-      network::NetworkConnectionTracker::NetworkConnectionObserver {
+class ChromeBrowserMainBrowserTest : public InProcessBrowserTest {
  public:
   ChromeBrowserMainBrowserTest() = default;
-  ~ChromeBrowserMainBrowserTest() override {
-    content::GetNetworkConnectionTracker()->RemoveNetworkConnectionObserver(
-        this);
-  }
 
  protected:
   // InProcessBrowserTest:
@@ -102,38 +145,17 @@ class ChromeBrowserMainBrowserTest
         static_cast<ChromeBrowserMainParts*>(browser_main_parts);
     ChromeBrowserMainPartsTestApi(chrome_browser_main_parts)
         .EnableVariationsServiceInit();
-    extra_parts_ = new ChromeBrowserMainExtraPartsNetFactoryInstaller();
+    network_change_simulator_ =
+        std::make_unique<NetworkConnectionChangeSimulator>();
+    extra_parts_ = new ChromeBrowserMainExtraPartsNetFactoryInstaller(
+        network_change_simulator_.get());
     chrome_browser_main_parts->AddParts(extra_parts_);
   }
 
-  void SetUpOnMainThread() override {
-    content::GetNetworkConnectionTracker()->AddNetworkConnectionObserver(this);
-  }
-
-  void WaitForConnectionType(network::mojom::ConnectionType type) {
-    if (connection_type_ == type)
-      return;
-
-    expected_connection_type_ = type;
-    run_loop_ = std::make_unique<base::RunLoop>();
-    run_loop_->Run();
-  }
-
+  std::unique_ptr<NetworkConnectionChangeSimulator> network_change_simulator_;
   ChromeBrowserMainExtraPartsNetFactoryInstaller* extra_parts_ = nullptr;
 
  private:
-  // network::NetworkConnectionTracker::NetworkConnectionObserver:
-  void OnConnectionChanged(network::mojom::ConnectionType type) override {
-    connection_type_ = type;
-    if (expected_connection_type_ == connection_type_ && run_loop_)
-      run_loop_->Quit();
-  }
-
-  network::mojom::ConnectionType expected_connection_type_ =
-      network::mojom::ConnectionType::CONNECTION_UNKNOWN;
-  network::mojom::ConnectionType connection_type_ =
-      network::mojom::ConnectionType::CONNECTION_UNKNOWN;
-  std::unique_ptr<base::RunLoop> run_loop_;
 
   DISALLOW_COPY_AND_ASSIGN(ChromeBrowserMainBrowserTest);
 };
@@ -145,8 +167,8 @@ IN_PROC_BROWSER_TEST_F(ChromeBrowserMainBrowserTest,
   const int initial_request_count =
       g_browser_process->variations_service()->request_count();
   ASSERT_TRUE(extra_parts_);
-  SimulateNetworkChange(network::mojom::ConnectionType::CONNECTION_WIFI);
-  WaitForConnectionType(network::mojom::ConnectionType::CONNECTION_WIFI);
+  network_change_simulator_->SetConnectionType(
+      network::mojom::ConnectionType::CONNECTION_WIFI);
   // NotifyObserversOfNetworkChangeForTests uses PostTask, so run the loop until
   // idle to ensure VariationsService processes the network change.
   base::RunLoop().RunUntilIdle();