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

content/browser/background_sync/background_sync_browsertest.cc

 // 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 <set>
 #include <string>
 #include <vector>
 
 #include "base/command_line.h"
 #include "base/strings/string_split.h"
 #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"
 #include "content/browser/service_worker/service_worker_registration.h"
 #include "content/public/browser/background_sync_context.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/content_browser_test.h"
 #include "content/public/test/content_browser_test_utils.h"
 #include "content/public/test/test_utils.h"
 #include "content/shell/browser/shell.h"
 #include "net/base/network_change_notifier.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+#if defined(OS_ANDROID)
+#include "content/browser/android/background_sync_network_observer_android.h"
+#endif
+
 using net::NetworkChangeNotifier;
 
 namespace content {
 
 namespace {
 
 const char kDefaultTestURL[] = "files/background_sync/test.html";
 
 const char kSuccessfulOperationPrefix[] = "ok - ";
 
@@ skipping 58 matching lines @@
 
   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
 
+#if defined(OS_ANDROID)
+    // On Android, the actual device connectivity status can interfere with some
+    // of the browser tests
+    BackgroundSyncNetworkObserverAndroid::DisableNotificationForTesting();

[jkarlin, 2015/09/19 00:16:07]

What if you added IgnoreNetworkChangeNotifierForTesting() to
BackgroundSyncNetworkObserver? Then you could get rid of the ifdefs in this file
and the SetTestNtoificationsOnly(true) line above. You could implement it by
simply not calling the callback from BSNO when notifications come in.

Alternatively, and I like this a bit more, you could make a
MockBackgroundSyncNetworkObserver and call
BackgroundSyncManager->SetNetworkObserver(mock) and just change the network as
necessary via the mock observer. Then the BackgroundSyncNetworkObservers
wouldn't need any ForTesting functions at all. The observers would need a
SetCallback function though so that the BSM could change the mock's callback in
SetNetworkObserver. 

[iclelland, 2015/09/21 18:49:30]

I'll try that approach; it might be cleaner than passing Disable..() up over JNI
to the Java object. It'll have to be a static method, so that all observers
(including those not yet created) are affected, though.
This is actually what I wanted to do, but it seemed more complicated than it
really should be, just for testing. I'd have to set the BSM behaviour before it
gets constructed.
The BSM creates the observer in its constructor, and on Android, on creation,
the Java BSNO needs to send the *actual* connectivity state down to the BSM.
This is fine in the real world, but in testing, if we've set the status to
offline in a regular tab, and then create a new incognito tab, the actual online
state will trigger the BSM code, and fire events that we don't want to happen
yet. We'd need to interfere with the construction process, with something like
an ObserverFactory injected into the BackgroundSyncContext, and it started to
look like a lot more complexity than was warranted for a test method.

[iclelland, 2015/09/22 13:18:15]

This is done now, in the latest patch set.

+#endif
+
     ContentBrowserTest::SetUp();
   }
 
   void SetIncognitoMode(bool incognito) {
     shell_ = incognito ? CreateOffTheRecordBrowser() : shell();
   }
 
+  BackgroundSyncContext* GetSyncContextFromShell(Shell* shell) {

[jkarlin, 2015/09/19 00:16:07]

Can you use this new function in OneShotPending?

[iclelland, 2015/09/21 18:49:30]

I could, but we need the storage partition separately in OneShotPending as well
(to get the ServiceWorkerContext), so it doesn't really seem to make anything
clearer.

+    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.
     command_line->AppendSwitch(
         switches::kEnableExperimentalWebPlatformFeatures);
   }
 
   void SetUpOnMainThread() override {
     https_server_.reset(new net::SpawnedTestServer(
         net::SpawnedTestServer::TYPE_HTTPS,
         net::BaseTestServer::SSLOptions(
             net::BaseTestServer::SSLOptions::CERT_OK),
         base::FilePath(FILE_PATH_LITERAL("content/test/data/"))));
     ASSERT_TRUE(https_server_->Start());
 
+    SetIncognitoMode(false);
+
     SetOnline(true);
 
-    SetIncognitoMode(false);
-
     ASSERT_TRUE(LoadTestPage(kDefaultTestURL));
 
     ContentBrowserTest::SetUpOnMainThread();
   }
 
   void TearDownOnMainThread() override { https_server_.reset(); }
 
   bool LoadTestPage(const std::string& path) {
     return NavigateToURL(shell_, https_server_->GetURL(path));
   }
 
   bool RunScript(const std::string& script, std::string* result) {
     return content::ExecuteScriptAndExtractString(shell_->web_contents(),
                                                   script, result);
   }
 
   void SetOnline(bool online);

[jkarlin, 2015/09/19 00:16:07]

Please add comment noting that 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 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.
   bool OneShotPending(const std::string& tag);
 
   bool PopConsole(const std::string& expected_msg);
   bool RegisterServiceWorker();
   bool RegisterOneShot(const std::string& tag);
   bool UnregisterOneShot(const std::string& tag);
   bool UnregisterOneShotTwice(const std::string& tag);
   bool GetRegistrationOneShot(const std::string& tag);
   bool GetRegistrationsOneShot(const std::vector<std::string>& expected_tags);
   bool CompleteDelayedOneShot();
   bool RejectDelayedOneShot();
 
  private:
   scoped_ptr<net::SpawnedTestServer> https_server_;
   Shell* shell_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(BackgroundSyncBrowserTest);
 };
 
 void BackgroundSyncBrowserTest::SetOnline(bool online) {
-  if (online) {
-    NetworkChangeNotifier::NotifyObserversOfNetworkChangeForTests(
-        NetworkChangeNotifier::CONNECTION_WIFI);
-  } else {
-    NetworkChangeNotifier::NotifyObserversOfNetworkChangeForTests(
-        NetworkChangeNotifier::CONNECTION_NONE);
-  }
+  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) {
+    network_observer->OnNetworkChanged(NetworkChangeNotifier::CONNECTION_WIFI);
+  } else {
+    network_observer->OnNetworkChanged(NetworkChangeNotifier::CONNECTION_NONE);
+  }
+}
+
 bool BackgroundSyncBrowserTest::OneShotPending(const std::string& tag) {
   bool is_pending;
   base::RunLoop run_loop;
 
   StoragePartition* storage = BrowserContext::GetDefaultStoragePartition(
       shell_->web_contents()->GetBrowserContext());
   BackgroundSyncContext* sync_context = storage->GetBackgroundSyncContext();
   ServiceWorkerContextWrapper* service_worker_context =
       static_cast<ServiceWorkerContextWrapper*>(
           storage->GetServiceWorkerContext());
@@ skipping 222 matching lines @@
   EXPECT_TRUE(RegisterServiceWorker());
   EXPECT_TRUE(LoadTestPage(kDefaultTestURL));  // Control the page.
 
   EXPECT_TRUE(RegisterOneShot("unregister"));
   EXPECT_TRUE(PopConsole("ok - unregister completed"));
 }
 
 }  // namespace
 
 }  // namespace content