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}

9 files changed, 273 insertions, 25 deletions

diff --git a/content/browser/android/background_sync_network_observer_android.cc b/content/browser/android/background_sync_network_observer_android.cc
new file mode 100644
index 0000000..533fc36
--- /dev/null
+++ b/content/browser/android/background_sync_network_observer_android.cc
@@ -0,0 +1,86 @@
+// Copyright 2015 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 "content/browser/android/background_sync_network_observer_android.h"
+
+#include "jni/BackgroundSyncNetworkObserver_jni.h"
+
+namespace content {
+
+// static
+bool BackgroundSyncNetworkObserverAndroid::Observer::RegisterNetworkObserver(
+    JNIEnv* env) {
+  return RegisterNativesImpl(env);
+}
+
+// static
+scoped_refptr<BackgroundSyncNetworkObserverAndroid::Observer>
+BackgroundSyncNetworkObserverAndroid::Observer::Create(
+    base::Callback<void(net::NetworkChangeNotifier::ConnectionType)> callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  scoped_refptr<BackgroundSyncNetworkObserverAndroid::Observer> observer(
+      new BackgroundSyncNetworkObserverAndroid::Observer(callback));
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&BackgroundSyncNetworkObserverAndroid::Observer::Init,
+                 observer));
+  return observer;
+}
+
+void BackgroundSyncNetworkObserverAndroid::Observer::Init() {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  // Attach a Java BackgroundSyncNetworkObserver object. Its lifetime will be
+  // scoped to the lifetime of this object.
+  JNIEnv* env = base::android::AttachCurrentThread();
+  base::android::ScopedJavaGlobalRef<jobject> obj(
+      Java_BackgroundSyncNetworkObserver_createObserver(
+          env, base::android::GetApplicationContext(),
+          reinterpret_cast<jlong>(this)));
+  j_observer_.Reset(obj);
+}
+
+BackgroundSyncNetworkObserverAndroid::Observer::~Observer() {
+  JNIEnv* env = base::android::AttachCurrentThread();
+  Java_BackgroundSyncNetworkObserver_removeObserver(
+      env, j_observer_.obj(), reinterpret_cast<jlong>(this));
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  j_observer_.Release();
+}
+
+void BackgroundSyncNetworkObserverAndroid::Observer::
+    NotifyConnectionTypeChanged(JNIEnv* env,
+                                jobject jcaller,
+                                jint new_connection_type) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(callback_,
+                 static_cast<net::NetworkChangeNotifier::ConnectionType>(
+                     new_connection_type)));
+}
+
+BackgroundSyncNetworkObserverAndroid::Observer::Observer(
+    base::Callback<void(net::NetworkChangeNotifier::ConnectionType)> callback)
+    : callback_(callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+}
+
+BackgroundSyncNetworkObserverAndroid::BackgroundSyncNetworkObserverAndroid(
+    const base::Closure& network_changed_callback)
+    : BackgroundSyncNetworkObserver(network_changed_callback),
+      weak_ptr_factory_(this) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  // Remove the observer attached by the NetworkObserver constructor
+  net::NetworkChangeNotifier::RemoveNetworkChangeObserver(this);
+
+  observer_ = Observer::Create(
+      base::Bind(&BackgroundSyncNetworkObserverAndroid::OnNetworkChanged,
+                 weak_ptr_factory_.GetWeakPtr()));
+}
+
+BackgroundSyncNetworkObserverAndroid::~BackgroundSyncNetworkObserverAndroid() {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+}
+}  // namespace content
diff --git a/content/browser/android/background_sync_network_observer_android.h b/content/browser/android/background_sync_network_observer_android.h
new file mode 100644
index 0000000..49e8b0a
--- /dev/null
+++ b/content/browser/android/background_sync_network_observer_android.h
@@ -0,0 +1,80 @@
+// Copyright 2015 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.
+
+#ifndef CONTENT_BROWSER_ANDROID_BACKGROUND_SYNC_NETWORK_OBSERVER_ANDROID_H_
+#define CONTENT_BROWSER_ANDROID_BACKGROUND_SYNC_NETWORK_OBSERVER_ANDROID_H_
+
+#include "base/android/jni_android.h"
+#include "base/android/scoped_java_ref.h"
+#include "base/memory/weak_ptr.h"
+#include "content/browser/background_sync/background_sync_network_observer.h"
+#include "content/public/browser/browser_thread.h"
+
+namespace content {
+
+// BackgroundSyncNetworkObserverAndroid is a specialized
+// BackgroundSyncNetworkObserver which is backed by a NetworkChangeNotifier
+// that listens for network events even when the browser is paused, unlike the
+// standard NetworkChangeNotifier. This ensures that sync events can be fired
+// even when the browser is backgrounded, and other network observers are
+// disabled.
+class BackgroundSyncNetworkObserverAndroid
+    : public BackgroundSyncNetworkObserver {
+ public:
+  // Creates a BackgroundSyncNetworkObserver. |network_changed_callback| is
+  // called via PostMessage when the network connection changes.
+  BackgroundSyncNetworkObserverAndroid(
+      const base::Closure& network_changed_callback);
+
+  ~BackgroundSyncNetworkObserverAndroid() override;
+
+  // This class lives on the UI thread and mediates all access to the Java
+  // BackgroundSyncNetworkObserver, which it creates and owns. It is in turn
+  // owned by the BackgroundSyncNetworkObserverAndroid.
+  class Observer : public base::RefCountedThreadSafe<
+                       BackgroundSyncNetworkObserverAndroid::Observer,
+                       content::BrowserThread::DeleteOnUIThread> {
+   public:
+    static scoped_refptr<BackgroundSyncNetworkObserverAndroid::Observer> Create(
+        base::Callback<void(net::NetworkChangeNotifier::ConnectionType)>
+            callback);
+
+    static bool RegisterNetworkObserver(JNIEnv* env);
+
+    // Called from BackgroundSyncNetworkObserver.java over JNI whenever the
+    // connection type changes. This updates the current connection type seen by
+    // this class and calls the |network_changed_callback| provided to the
+    // constructor, on the IO thread, with the new connection type.
+    void NotifyConnectionTypeChanged(JNIEnv* env,
+                                     jobject jcaller,
+                                     jint new_connection_type);
+
+   private:
+    friend struct BrowserThread::DeleteOnThread<BrowserThread::UI>;
+    friend class base::DeleteHelper<
+        BackgroundSyncNetworkObserverAndroid::Observer>;
+
+    Observer(base::Callback<void(net::NetworkChangeNotifier::ConnectionType)>
+                 callback);
+    void Init();
+    ~Observer();
+
+    // This callback is to be run on the IO thread whenever the connection type
+    // changes.
+    base::Callback<void(net::NetworkChangeNotifier::ConnectionType)> callback_;
+    base::android::ScopedJavaGlobalRef<jobject> j_observer_;
+
+    DISALLOW_COPY_AND_ASSIGN(Observer);
+  };
+
+ private:
+  // Accessed on UI Thread
+  scoped_refptr<Observer> observer_;
+
+  base::WeakPtrFactory<BackgroundSyncNetworkObserverAndroid> weak_ptr_factory_;
+};
+
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_ANDROID_BACKGROUND_SYNC_NETWORK_OBSERVER_ANDROID_H_
diff --git a/content/browser/android/browser_jni_registrar.cc b/content/browser/android/browser_jni_registrar.cc
index 43adba8..bea2065 100644
--- a/content/browser/android/browser_jni_registrar.cc
+++ b/content/browser/android/browser_jni_registrar.cc
@@ -9,6 +9,7 @@
 #include "content/browser/accessibility/browser_accessibility_android.h"
 #include "content/browser/accessibility/browser_accessibility_manager_android.h"
 #include "content/browser/android/background_sync_launcher_android.h"
+#include "content/browser/android/background_sync_network_observer_android.h"
 #include "content/browser/android/browser_startup_controller.h"
 #include "content/browser/android/child_process_launcher_android.h"
 #include "content/browser/android/composited_touch_handle_drawable.h"
@@ -50,6 +51,9 @@ base::android::RegistrationMethod kContentRegisteredMethods[] = {
      content::AndroidLocationApiAdapter::RegisterGeolocationService},
     {"BackgroundSyncLauncherAndroid",
      content::BackgroundSyncLauncherAndroid::RegisterLauncher},
+    {"BackgroundSyncNetworkObserverAndroid",
+     content::BackgroundSyncNetworkObserverAndroid::Observer::
+         RegisterNetworkObserver},
     {"BrowserAccessibilityManager",
      content::RegisterBrowserAccessibilityManager},
     {"BrowserStartupController", content::RegisterBrowserStartupController},
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);
 };