Reland of [Background Sync] Trigger Background Sync events when Chrome is backgrounded on Android

Original review: https://codereview.chromium.org/1294603003/
  (Committed as https://crrev.com/a5fb97d6fb9c3831994aadebea4664e2acc3c356)
Reverted in https://codereview.chromium.org/1376563003/
  (as https://crrev.com/c532c674b657426cb733c2b735bbf5741bcbea48)

BUG=511129

Review URL: https://codereview.chromium.org/1383663004

Cr-Commit-Position: refs/heads/master@{#352019}

6 files changed, 103 insertions, 25 deletions

diff --git a/content/browser/background_sync/background_sync_browsertest.cc b/content/browser/background_sync/background_sync_browsertest.cc
index eb79df9..ae1ccc9 100644
--- a/content/browser/background_sync/background_sync_browsertest.cc
+++ b/content/browser/background_sync/background_sync_browsertest.cc
@@ -11,6 +11,7 @@
 #include "base/strings/stringprintf.h"
 #include "base/task_runner_util.h"
 #include "content/browser/background_sync/background_sync_manager.h"
+#include "content/browser/background_sync/background_sync_network_observer.h"
 #include "content/browser/background_sync/background_sync_registration_handle.h"
 #include "content/browser/background_sync/background_sync_status.h"
 #include "content/browser/service_worker/service_worker_context_wrapper.h"
@@ -92,20 +93,15 @@ void OneShotPendingOnIOThread(
                       callback));
 }
 
+}  // namespace
+
 class BackgroundSyncBrowserTest : public ContentBrowserTest {
  public:
   BackgroundSyncBrowserTest() {}
   ~BackgroundSyncBrowserTest() override {}
 
   void SetUp() override {
-    NetworkChangeNotifier::SetTestNotificationsOnly(true);
-
-#if defined(OS_CHROMEOS)
-    // ChromeOS's NetworkChangeNotifier doesn't get created in
-    // content_browsertests, so make one now.
-    net::NetworkChangeNotifier::CreateMock();
-#endif
-
+    BackgroundSyncNetworkObserver::SetIgnoreNetworkChangeNotifierForTests(true);
     ContentBrowserTest::SetUp();
   }
 
@@ -113,6 +109,12 @@ class BackgroundSyncBrowserTest : public ContentBrowserTest {
     shell_ = incognito ? CreateOffTheRecordBrowser() : shell();
   }
 
+  BackgroundSyncContext* GetSyncContextFromShell(Shell* shell) {
+    StoragePartition* storage = BrowserContext::GetDefaultStoragePartition(
+        shell_->web_contents()->GetBrowserContext());
+    return storage->GetBackgroundSyncContext();
+  }
+
   void SetUpCommandLine(base::CommandLine* command_line) override {
     // TODO(jkarlin): Remove this once background sync is no longer
     // experimental.
@@ -128,10 +130,10 @@ class BackgroundSyncBrowserTest : public ContentBrowserTest {
         base::FilePath(FILE_PATH_LITERAL("content/test/data/"))));
     ASSERT_TRUE(https_server_->Start());
 
-    SetOnline(true);
-
     SetIncognitoMode(false);
 
+    SetOnline(true);
+
     ASSERT_TRUE(LoadTestPage(kDefaultTestURL));
 
     ContentBrowserTest::SetUpOnMainThread();
@@ -148,7 +150,13 @@ class BackgroundSyncBrowserTest : public ContentBrowserTest {
                                                   script, result);
   }
 
+  // This runs asynchronously on the IO thread, but we don't need to wait for it
+  // to complete before running a background sync operation, since those also
+  // run on the IO thread.
   void SetOnline(bool online);
+  void SetOnlineOnIOThread(
+      const scoped_refptr<BackgroundSyncContext>& sync_context,
+      bool online);
 
   // Returns true if the one-shot sync with tag is currently pending. Fails
   // (assertion failure) if the tag isn't registered.
@@ -175,14 +183,28 @@ class BackgroundSyncBrowserTest : public ContentBrowserTest {
 };
 
 void BackgroundSyncBrowserTest::SetOnline(bool online) {
+  ASSERT_TRUE(shell_);
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&BackgroundSyncBrowserTest::SetOnlineOnIOThread,
+                 base::Unretained(this),
+                 base::Unretained(GetSyncContextFromShell(shell_)), online));
+  base::RunLoop().RunUntilIdle();
+}
+
+void BackgroundSyncBrowserTest::SetOnlineOnIOThread(
+    const scoped_refptr<BackgroundSyncContext>& sync_context,
+    bool online) {
+  BackgroundSyncManager* sync_manager = sync_context->background_sync_manager();
+  BackgroundSyncNetworkObserver* network_observer =
+      sync_manager->GetNetworkObserverForTesting();
   if (online) {
-    NetworkChangeNotifier::NotifyObserversOfNetworkChangeForTests(
+    network_observer->NotifyManagerIfNetworkChanged(
         NetworkChangeNotifier::CONNECTION_WIFI);
   } else {
-    NetworkChangeNotifier::NotifyObserversOfNetworkChangeForTests(
+    network_observer->NotifyManagerIfNetworkChanged(
         NetworkChangeNotifier::CONNECTION_NONE);
   }
-  base::RunLoop().RunUntilIdle();
 }
 
 bool BackgroundSyncBrowserTest::OneShotPending(const std::string& tag) {
@@ -513,6 +535,4 @@ IN_PROC_BROWSER_TEST_F(BackgroundSyncBrowserTest, CallDoneAfterSyncFails) {
   EXPECT_TRUE(NotifyWhenDoneImmediateOneShot("ok - delay result: false"));
 }
 
-}  // namespace
-
 }  // namespace content
diff --git a/content/browser/background_sync/background_sync_manager.cc b/content/browser/background_sync/background_sync_manager.cc
index 07ac7d8..477b31f 100644
--- a/content/browser/background_sync/background_sync_manager.cc
+++ b/content/browser/background_sync/background_sync_manager.cc
@@ -21,6 +21,7 @@
 
 #if defined(OS_ANDROID)
 #include "content/browser/android/background_sync_launcher_android.h"
+#include "content/browser/android/background_sync_network_observer_android.h"
 #endif
 
 namespace content {
@@ -221,9 +222,15 @@ BackgroundSyncManager::BackgroundSyncManager(
 
   service_worker_context_->AddObserver(this);
 
+#if defined(OS_ANDROID)
+  network_observer_.reset(new BackgroundSyncNetworkObserverAndroid(
+      base::Bind(&BackgroundSyncManager::OnNetworkChanged,
+                 weak_ptr_factory_.GetWeakPtr())));
+#else
   network_observer_.reset(new BackgroundSyncNetworkObserver(
       base::Bind(&BackgroundSyncManager::OnNetworkChanged,
                  weak_ptr_factory_.GetWeakPtr())));
+#endif
   power_observer_.reset(new BackgroundSyncPowerObserver(base::Bind(
       &BackgroundSyncManager::OnPowerChanged, weak_ptr_factory_.GetWeakPtr())));
 }
diff --git a/content/browser/background_sync/background_sync_manager.h b/content/browser/background_sync/background_sync_manager.h
index 999b310..a845dd3 100644
--- a/content/browser/background_sync/background_sync_manager.h
+++ b/content/browser/background_sync/background_sync_manager.h
@@ -99,6 +99,10 @@ class CONTENT_EXPORT BackgroundSyncManager
                              const GURL& pattern) override;
   void OnStorageWiped() override;
 
+  BackgroundSyncNetworkObserver* GetNetworkObserverForTesting() {
+    return network_observer_.get();
+  }
+
  protected:
   // A registration might be referenced by the client longer than
   // the BackgroundSyncManager needs to keep track of it (e.g., the event has
diff --git a/content/browser/background_sync/background_sync_manager_unittest.cc b/content/browser/background_sync/background_sync_manager_unittest.cc
index 4817b0c..72db49f 100644
--- a/content/browser/background_sync/background_sync_manager_unittest.cc
+++ b/content/browser/background_sync/background_sync_manager_unittest.cc
@@ -14,6 +14,7 @@
 #include "base/single_thread_task_runner.h"
 #include "base/test/mock_entropy_provider.h"
 #include "base/thread_task_runner_handle.h"
+#include "content/browser/background_sync/background_sync_network_observer.h"
 #include "content/browser/background_sync/background_sync_registration_handle.h"
 #include "content/browser/background_sync/background_sync_status.h"
 #include "content/browser/browser_thread_impl.h"
@@ -233,6 +234,9 @@ class BackgroundSyncManagerTest : public testing::Test {
   }
 
   void SetUp() override {
+    // Don't let the tests be confused by the real-world device connectivity
+    BackgroundSyncNetworkObserver::SetIgnoreNetworkChangeNotifierForTests(true);
+
     helper_.reset(
         new EmbeddedWorkerTestHelper(base::FilePath(), kRenderProcessId));
 
@@ -251,6 +255,12 @@ class BackgroundSyncManagerTest : public testing::Test {
     RegisterServiceWorkers();
   }
 
+  void TearDown() override {
+    // Restore the network observer functionality for subsequent tests
+    BackgroundSyncNetworkObserver::SetIgnoreNetworkChangeNotifierForTests(
+        false);
+  }
+
   void RegisterServiceWorkers() {
     bool called_1 = false;
     bool called_2 = false;
@@ -304,7 +314,12 @@ class BackgroundSyncManagerTest : public testing::Test {
   void SetNetwork(net::NetworkChangeNotifier::ConnectionType connection_type) {
     net::NetworkChangeNotifier::NotifyObserversOfNetworkChangeForTests(
         connection_type);
-    base::RunLoop().RunUntilIdle();
+    if (test_background_sync_manager_) {
+      BackgroundSyncNetworkObserver* network_observer =
+          test_background_sync_manager_->GetNetworkObserverForTesting();
+      network_observer->NotifyManagerIfNetworkChanged(connection_type);
+      base::RunLoop().RunUntilIdle();
+    }
   }
 
   void SetOnBatteryPower(bool on_battery_power) {
diff --git a/content/browser/background_sync/background_sync_network_observer.cc b/content/browser/background_sync/background_sync_network_observer.cc
index 1c6f2bd..a08e7d8 100644
--- a/content/browser/background_sync/background_sync_network_observer.cc
+++ b/content/browser/background_sync/background_sync_network_observer.cc
@@ -11,6 +11,15 @@
 
 namespace content {
 
+// static
+bool BackgroundSyncNetworkObserver::ignore_network_change_notifier_ = false;
+
+// static
+void BackgroundSyncNetworkObserver::SetIgnoreNetworkChangeNotifierForTests(
+    bool ignore) {
+  ignore_network_change_notifier_ = ignore;
+}
+
 BackgroundSyncNetworkObserver::BackgroundSyncNetworkObserver(
     const base::Closure& network_changed_callback)
     : connection_type_(net::NetworkChangeNotifier::GetConnectionType()),
@@ -47,17 +56,18 @@ bool BackgroundSyncNetworkObserver::NetworkSufficient(
   return false;
 }
 
-void BackgroundSyncNetworkObserver::NotifyNetworkChanged() {
+void BackgroundSyncNetworkObserver::OnNetworkChanged(
+    net::NetworkChangeNotifier::ConnectionType connection_type) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-  base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
-                                                network_changed_callback_);
+  if (ignore_network_change_notifier_)
+    return;
+  NotifyManagerIfNetworkChanged(connection_type);
 }
 
-void BackgroundSyncNetworkObserver::OnNetworkChanged(
+void BackgroundSyncNetworkObserver::NotifyManagerIfNetworkChanged(
     net::NetworkChangeNotifier::ConnectionType connection_type) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
   if (connection_type == connection_type_)
     return;
 
@@ -65,4 +75,11 @@ void BackgroundSyncNetworkObserver::OnNetworkChanged(
   NotifyNetworkChanged();
 }
 
+void BackgroundSyncNetworkObserver::NotifyNetworkChanged() {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
+                                                network_changed_callback_);
+}
+
 }  // namespace content
diff --git a/content/browser/background_sync/background_sync_network_observer.h b/content/browser/background_sync/background_sync_network_observer.h
index 7c81f738..4d47cc6 100644
--- a/content/browser/background_sync/background_sync_network_observer.h
+++ b/content/browser/background_sync/background_sync_network_observer.h
@@ -13,7 +13,7 @@
 namespace content {
 
 class CONTENT_EXPORT BackgroundSyncNetworkObserver
-    : net::NetworkChangeNotifier::NetworkChangeObserver {
+    : public net::NetworkChangeNotifier::NetworkChangeObserver {
  public:
   // Creates a BackgroundSyncNetworkObserver. |network_changed_callback| is
   // called when the network connection changes asynchronously via PostMessage.
@@ -21,22 +21,37 @@ class CONTENT_EXPORT BackgroundSyncNetworkObserver
 
   ~BackgroundSyncNetworkObserver() override;
 
+  // Enable or disable notifications coming from the NetworkChangeNotifier. (For
+  // preventing flakes in tests)
+  static void SetIgnoreNetworkChangeNotifierForTests(bool ignore);
+
   // Returns true if the state of the network meets the needs of
   // |network_state|.
   bool NetworkSufficient(SyncNetworkState network_state);
 
- private:
-  void NotifyNetworkChanged();
-
   // NetworkChangeObserver overrides
   void OnNetworkChanged(
       net::NetworkChangeNotifier::ConnectionType connection_type) override;
 
+ private:
+  friend class BackgroundSyncBrowserTest;
+  friend class BackgroundSyncManagerTest;
+
+  // Calls NotifyNetworkChanged if the connection type has changed.
+  void NotifyManagerIfNetworkChanged(
+      net::NetworkChangeNotifier::ConnectionType connection_type);
+
+  void NotifyNetworkChanged();
+
   net::NetworkChangeNotifier::ConnectionType connection_type_;
 
   // The callback to run when the network changes.
   base::Closure network_changed_callback_;
 
+  // Set true to ignore notifications coming from the NetworkChangeNotifier
+  // (to prevent flakes in tests).
+  static bool ignore_network_change_notifier_;
+
   DISALLOW_COPY_AND_ASSIGN(BackgroundSyncNetworkObserver);
 };