[NTP Snippets] Schedule periodic fetching

chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsLauncherService.java

Index: chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsLauncherService.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/BackgroundSyncLauncherService.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsLauncherService.java
similarity index 58%
copy from chrome/android/java/src/org/chromium/chrome/browser/BackgroundSyncLauncherService.java
copy to chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsLauncherService.java
index a6219fd1868d9300af1d642f605d95b7f93b4152..1643a178237176b537b446fffc8cedf24945e972 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/BackgroundSyncLauncherService.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsLauncherService.java
@@ -1,8 +1,8 @@
-// Copyright 2015 The Chromium Authors. All rights reserved.
+// Copyright 2016 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.
 
-package org.chromium.chrome.browser;
+package org.chromium.chrome.browser.ntp.snippets;
 
 import android.content.Context;
 
@@ -12,7 +12,6 @@ import com.google.android.gms.gcm.TaskParams;
 
 import org.chromium.base.Log;
 import org.chromium.base.ThreadUtils;
-import org.chromium.base.VisibleForTesting;
 import org.chromium.base.annotations.SuppressFBWarnings;
 import org.chromium.base.library_loader.LibraryProcessType;
 import org.chromium.base.library_loader.ProcessInitException;
@@ -20,33 +19,29 @@ import org.chromium.content.app.ContentApplication;
 import org.chromium.content.browser.BrowserStartupController;
 
 /**
- * {@link BackgroundSyncLauncherService} is scheduled through the {@link GcmNetworkManager}
- * when the browser needs to be launched in response to changing network or power conditions.
+ * {@link SnippetsLauncherService} is scheduled through the {@link GcmNetworkManager} when the
+ * browser needs to be launched to fetch new snippets.
  */
-public class BackgroundSyncLauncherService extends GcmTaskService {
-    private static final String TAG = "BgSyncLauncher";
+public class SnippetsLauncherService extends GcmTaskService {

[Marc Treib, 2016/02/16 15:03:25]

So, as you can see, this class is currently almost identical to
BackgroundSyncLauncherService. It would be possible to merge the two into a
generic "ChromeGcmTaskService" which would have some sort of dispatch mechanism.
But given how little code this actually is, I find it simpler to just have
separate services. WDYT?

[Bernhard Bauer, 2016/02/16 16:19:08]

Hm... I think there are a couple of things in here that are non-trivial enough
that I wouldn't really want to just copy and paste them... Also, it might be
good to look into bundling these, i.e. if we are starting the browser anyway for
Sync, we could just fetch snippets as well.

[Marc Treib, 2016/02/16 16:54:27]

Hm, I guess that might make sense longer-term. It does seem like a recipe for
confusion though, due to different requirements of jobs. E.g. we want an
unmetered connection and (maybe) charging; background sync is fine with any
connection.
I'll look into combining the two.

[Bernhard Bauer, 2016/02/17 16:53:00]

OK, thanks! Even if we don't want to do coalescing of these jobs, it would at
least be good to share some code.

[Marc Treib, 2016/02/18 10:21:47]

Done. Also added some tests.

+    private static final String TAG = "SnippetsLauncherSvc";

[Marc Treib, 2016/02/16 15:03:25]

Weirdly abbreviated because tags apparently must be <= 20 chars.

[Bernhard Bauer, 2016/02/16 16:19:08]

Eh, I'd be fine with just "SnippetsLauncher".

[Marc Treib, 2016/02/16 16:54:27]

There is another class called SnippetsLauncher, I'd like to have different tags.

[Bernhard Bauer, 2016/02/17 16:53:00]

So, in general that is not a requirement; it's totally fine to use tags just to
group related areas together, not to identify exact classes.

[Marc Treib, 2016/02/18 10:21:47]

Acknowledged (but obsolete now that this has been merged with the other
GcmTaskService implementation).

 
     @Override
-    @VisibleForTesting
     public int onRunTask(TaskParams params) {
-        // Start the browser. The browser's BackgroundSyncManager (for the active profile) will
-        // start, check the network, and run any necessary sync events. This task runs with a wake
-        // lock, but has a three minute timeout, so we need to start the browser in its own task.
-        // TODO(jkarlin): Protect the browser sync event with a wake lock. See crbug.com/486020.
-        Log.v(TAG, "Starting Browser after coming online");
+        Log.i(TAG, "Woken up at " + new java.util.Date().toString());
         final Context context = this;
         ThreadUtils.runOnUiThread(new Runnable() {
             @Override
             public void run() {
-                if (!BackgroundSyncLauncher.hasInstance()) {
+                if (!SnippetsLauncher.hasInstance()) {
+                    Log.i(TAG, "Launching browser");
                     launchBrowser(context);
                 }
+                SnippetsController.get(context).fetchSnippets(true);
             }
         });
         return GcmNetworkManager.RESULT_SUCCESS;
     }
 
-    @VisibleForTesting
     @SuppressFBWarnings("DM_EXIT")
     protected void launchBrowser(Context context) {
         ContentApplication.initCommandLine(context);
@@ -60,10 +55,5 @@ public class BackgroundSyncLauncherService extends GcmTaskService {
             System.exit(-1);
         }
     }
-
-    @Override
-    @VisibleForTesting
-    public void onInitializeTasks() {
-        BackgroundSyncLauncher.rescheduleTasksOnUpgrade(this);
-    }
 }
+