[Fuchsia] Initialize NetworkChangeNotifierFuchsia synchronously.

Previously NetworkChangeNotifierFuchsia would start in the UNKNOWN
state and dispatch an asynchronous request for interface and route
information.

This could result in the network state not becoming available until
after requests had been started, causing them to be cancelled with
ERR_NETWORK_CHANGED.

NetworkChangeNotifierFuchsiaTests now run the FakeNetstack on a
separate thread, requiring explicit synchronization to ensure that
requests are processed before expectations are checked. The tests are
also migrated to verify OnIPAddressChanged() and
OnConnectionTypeChanged() rather than the derived OnNetworkChanged()
signal.

Bug: 964161, b/130731732
Change-Id: Ibb59f676468e66190ccc5b88af8e492a5102f8db
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1616324
Commit-Queue: Wez <wez@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Auto-Submit: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#661082}

net/base/network_change_notifier_fuchsia.cc

@@ -29,9 +29,25 @@ NetworkChangeNotifierFuchsia::NetworkChangeNotifierFuchsia(
 NetworkChangeNotifierFuchsia::NetworkChangeNotifierFuchsia(
     fuchsia::netstack::NetstackPtr netstack,
     uint32_t required_features)
-    : required_features_(required_features), netstack_(std::move(netstack)) {
-  DCHECK(netstack_);
-
+    : required_features_(required_features) {
+  DCHECK(netstack);
+
+  // Temporarily re-wrap our Netstack channel so we can query the interfaces
+  // and routing table synchronously to populate the initial state.
+  fuchsia::netstack::NetstackSyncPtr sync_netstack;
+  sync_netstack.Bind(netstack.Unbind());
+
+  // Manually fetch the interfaces and routes.
+  std::vector<fuchsia::netstack::NetInterface> interfaces;
+  zx_status_t status = sync_netstack->GetInterfaces(&interfaces);
+  ZX_CHECK(status == ZX_OK, status) << "synchronous GetInterfaces()";
+  std::vector<fuchsia::netstack::RouteTableEntry> routes;
+  status = sync_netstack->GetRouteTable(&routes);
+  ZX_CHECK(status == ZX_OK, status) << "synchronous GetInterfaces()";
+  OnRouteTableReceived(std::move(interfaces), std::move(routes), false);
+
+  // Re-wrap Netstack back into an asynchronous pointer.
+  netstack_.Bind(sync_netstack.Unbind());
   netstack_.set_error_handler([](zx_status_t status) {
     // TODO(https://crbug.com/901092): Unit tests that use NetworkService are
     // crashing because NetworkService does not clean up properly, and the
@@ -41,11 +57,6 @@ NetworkChangeNotifierFuchsia::NetworkChangeNotifierFuchsia(
   });
   netstack_.events().OnInterfacesChanged = fit::bind_member(
       this, &NetworkChangeNotifierFuchsia::ProcessInterfaceList);
-
-  // Manually fetch the interface list, on which to base an initial
-  // ConnectionType.
-  netstack_->GetInterfaces(fit::bind_member(
-      this, &NetworkChangeNotifierFuchsia::ProcessInterfaceList));
 }
 
 NetworkChangeNotifierFuchsia::~NetworkChangeNotifierFuchsia() {
@@ -64,13 +75,15 @@ void NetworkChangeNotifierFuchsia::ProcessInterfaceList(
   netstack_->GetRouteTable(
       [this, interfaces = std::move(interfaces)](
           std::vector<fuchsia::netstack::RouteTableEntry> route_table) mutable {
-        OnRouteTableReceived(std::move(interfaces), std::move(route_table));
+        OnRouteTableReceived(std::move(interfaces), std::move(route_table),
+                             true);
       });
 }
 
 void NetworkChangeNotifierFuchsia::OnRouteTableReceived(
     std::vector<fuchsia::netstack::NetInterface> interfaces,
-    std::vector<fuchsia::netstack::RouteTableEntry> route_table) {
+    std::vector<fuchsia::netstack::RouteTableEntry> route_table,
+    bool notify_observers) {
   // Create a set of NICs that have default routes (ie 0.0.0.0).
   base::flat_set<uint32_t> default_route_ids;
   for (const auto& route : route_table) {
@@ -119,20 +132,16 @@ void NetworkChangeNotifierFuchsia::OnRouteTableReceived(
     }
   }
 
-  bool connection_type_changed = false;
-  if (connection_type != cached_connection_type_) {
-    base::subtle::Release_Store(&cached_connection_type_, connection_type);
-    connection_type_changed = true;
-  }
-
   if (addresses != cached_addresses_) {
     std::swap(cached_addresses_, addresses);
-    NotifyObserversOfIPAddressChange();
-    connection_type_changed = true;
+    if (notify_observers)
+      NotifyObserversOfIPAddressChange();
   }
 
-  if (connection_type_changed) {
-    NotifyObserversOfConnectionTypeChange();
+  if (connection_type != cached_connection_type_) {
+    base::subtle::Release_Store(&cached_connection_type_, connection_type);
+    if (notify_observers)
+      NotifyObserversOfConnectionTypeChange();
   }
 }
 

net/base/network_change_notifier_fuchsia.h

@@ -27,15 +27,11 @@ class NET_EXPORT_PRIVATE NetworkChangeNotifierFuchsia
   explicit NetworkChangeNotifierFuchsia(uint32_t required_features);
   ~NetworkChangeNotifierFuchsia() override;
 
+  // NetworkChangeNotifier implementation.
+  ConnectionType GetCurrentConnectionType() const override;
+
  private:
   friend class NetworkChangeNotifierFuchsiaTest;
-  FRIEND_TEST_ALL_PREFIXES(NetworkChangeNotifierFuchsiaTest,
-                           FindsInterfaceWithRequiredFeature);
-  FRIEND_TEST_ALL_PREFIXES(NetworkChangeNotifierFuchsiaTest, FoundWiFi);
-  FRIEND_TEST_ALL_PREFIXES(NetworkChangeNotifierFuchsiaTest,
-                           FoundWiFiNonDefault);
-  FRIEND_TEST_ALL_PREFIXES(NetworkChangeNotifierFuchsiaTest,
-                           IgnoresInterfaceWithMissingFeature);
 
   // For testing purposes. Receives a |netstack| pointer for easy mocking.
   // Interfaces can be filtered out by passing in |required_features|, which is
@@ -53,10 +49,8 @@ class NET_EXPORT_PRIVATE NetworkChangeNotifierFuchsia
   // connection type changes are detected.
   void OnRouteTableReceived(
       std::vector<fuchsia::netstack::NetInterface> interfaces,
-      std::vector<fuchsia::netstack::RouteTableEntry> table);
-
-  // NetworkChangeNotifier implementation.
-  ConnectionType GetCurrentConnectionType() const override;
+      std::vector<fuchsia::netstack::RouteTableEntry> table,
+      bool notify_observers);
 
   // Bitmap of required features for an interface to be taken into account. The
   // features are defined in fuchsia::hardware::ethernet.