[BackgroundSync] BGSync missing its initial connection type request

After the switch to NetworkConnectionTracker, the GetConnectionType
call wasn't checking for a synchronous return. In the case of a
synchronous return OnConnectionChanged needs to be called. This led to
BackgroundSyncManager not getting the correct connection type on
startup.

Bug: 900943
Change-Id: I21ffcacdea9cca4c0780f40561e55b10fb9866b1
Reviewed-on: https://chromium-review.googlesource.com/c/1312676
Commit-Queue: Josh Karlin <jkarlin@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604689}

content/browser/background_sync/background_sync_network_observer.cc

@@ -52,10 +52,19 @@ void BackgroundSyncNetworkObserver::RegisterWithNetworkConnectionTracker(
   DCHECK(network_connection_tracker);
   network_connection_tracker_ = network_connection_tracker;
   network_connection_tracker_->AddNetworkConnectionObserver(this);
-  network_connection_tracker_->GetConnectionType(
-      &connection_type_,
+
+  UpdateConnectionType();
+}
+
+void BackgroundSyncNetworkObserver::UpdateConnectionType() {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  network::mojom::ConnectionType connection_type;
+  bool synchronous_return = network_connection_tracker_->GetConnectionType(
+      &connection_type,
       base::BindOnce(&BackgroundSyncNetworkObserver::OnConnectionChanged,
                      weak_ptr_factory_.GetWeakPtr()));
+  if (synchronous_return)
+    OnConnectionChanged(connection_type);
 }
 
 bool BackgroundSyncNetworkObserver::NetworkSufficient(
@@ -84,7 +93,6 @@ bool BackgroundSyncNetworkObserver::NetworkSufficient(
 void BackgroundSyncNetworkObserver::OnConnectionChanged(
     network::mojom::ConnectionType connection_type) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
   if (ignore_network_changes_)
     return;
   NotifyManagerIfConnectionChanged(connection_type);

content/browser/background_sync/background_sync_network_observer.h

@@ -45,6 +45,9 @@ class CONTENT_EXPORT BackgroundSyncNetworkObserver
   virtual void RegisterWithNetworkConnectionTracker(
       network::NetworkConnectionTracker* network_connection_tracker);
 
+  // Update the current connection type from the NetworkConnectionTracker.
+  void UpdateConnectionType();
+
   // Calls NotifyConnectionChanged if the connection type has changed.
   void NotifyManagerIfConnectionChanged(
       network::mojom::ConnectionType connection_type);

content/browser/background_sync/background_sync_network_observer_unittest.cc

@@ -12,7 +12,7 @@
 namespace content {
 
 class BackgroundSyncNetworkObserverTest : public testing::Test {
- protected:
+ public:
   BackgroundSyncNetworkObserverTest() : network_changed_count_(0) {
     network_observer_ =
         std::make_unique<BackgroundSyncNetworkObserver>(base::BindRepeating(
@@ -28,6 +28,7 @@ class BackgroundSyncNetworkObserverTest : public testing::Test {
 
   void OnConnectionChanged() { network_changed_count_++; }
 
+ protected:
   TestBrowserThreadBundle browser_thread_bundle_;
 
   std::unique_ptr<BackgroundSyncNetworkObserver> network_observer_;
@@ -104,4 +105,22 @@ TEST_F(BackgroundSyncNetworkObserverTest, ConditionsMetOnline) {
   EXPECT_FALSE(network_observer_->NetworkSufficient(NETWORK_STATE_ONLINE));
 }
 
+TEST_F(BackgroundSyncNetworkObserverTest, GetNetworkOnConstruction) {
+  // We need to emulate being disconnected before creating the network observer.
+  network_observer_.reset();
+  SetNetwork(network::mojom::ConnectionType::CONNECTION_NONE);
+
+  auto observer =
+      std::make_unique<BackgroundSyncNetworkObserver>(base::BindRepeating(
+          &BackgroundSyncNetworkObserverTest::OnConnectionChanged,
+          base::Unretained(this)));
+
+  base::RunLoop().RunUntilIdle();
+
+  // The network observer should have learned that the current network is not
+  // the default (UNKNOWN) but is in fact NONE.
+  EXPECT_EQ(1, network_changed_count_);
+  EXPECT_FALSE(observer->NetworkSufficient(NETWORK_STATE_ONLINE));
+}
+
 }  // namespace content