[NTP Snippets] Remove scheduled re-scheduling

[NTP Snippets] Remove scheduled re-scheduling

We don't need this anymore.
Keep SnippetsBridge.rescheduleFetching() though - we'll need this to re-schedule tasks from the Java side after an application upgrade (which clears them all).

BUG=646842
Committed: https://crrev.com/f8d45bd01dea555d4b124caade5210841dab8f4a
Cr-Commit-Position: refs/heads/master@{#421156}

[Marc Treib, 2016-09-22 12:53:26]

treib@chromium.org changed reviewers:
+ jkrcal@chromium.org

[Marc Treib, 2016-09-22 12:53:27]

One more intermediate cleanup. PTAL!

[jkrcal, 2016-09-22 13:48:51]

lgtm with the comments / questions below.

https://codereview.chromium.org/2357163004/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/ChromeBackgroundService.java
(left):

https://codereview.chromium.org/2357163004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ChromeBackgroundService.java:78:
break;
Is this task scheduled on M54? Does this end up in the default: case after
upgrade to M55?
Not a big deal, just trying to understand.

https://codereview.chromium.org/2357163004/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsBridge.java
(right):

https://codereview.chromium.org/2357163004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsBridge.java:78:
public static void rescheduleFetching() {
Who calls this function after an upgrade to M55 when the case in
ChromeBackgroundService is removed?

https://codereview.chromium.org/2357163004/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsLauncher.java
(right):

https://codereview.chromium.org/2357163004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsLauncher.java:99:
String tag, long periodSeconds, int requiredNetwork, boolean requiresCharging) {
|requiresCharging| is in fact not needed any more. Remove?

[Marc Treib, 2016-09-22 14:27:03]

https://codereview.chromium.org/2357163004/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/ChromeBackgroundService.java
(left):

https://codereview.chromium.org/2357163004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ChromeBackgroundService.java:78:
break;
On 2016/09/22 13:48:50, jkrcal wrote:
> Is this task scheduled on M54? Does this end up in the default: case after
> upgrade to M55?
> Not a big deal, just trying to understand.

Yes, it is scheduled in M54, but all tasks get cleared on app updates, so it
shouldn't be scheduled anymore in M55 (unless you had an early Dev build that
still had the code :) ).
Additionally, there's cleanup code when we actually do the scheduling that
removes any old tasks that might still be around. Per the above, that probably
isn't even strictly necessary.

https://codereview.chromium.org/2357163004/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsBridge.java
(right):

https://codereview.chromium.org/2357163004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsBridge.java:78:
public static void rescheduleFetching() {
On 2016/09/22 13:48:50, jkrcal wrote:
> Who calls this function after an upgrade to M55 when the case in
> ChromeBackgroundService is removed?

Right now, nobody. But it will be called again from
ChromeBackgroundService.onInitializeTasks (next CL :) ).

https://codereview.chromium.org/2357163004/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsLauncher.java
(right):

https://codereview.chromium.org/2357163004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsLauncher.java:99:
String tag, long periodSeconds, int requiredNetwork, boolean requiresCharging) {
On 2016/09/22 13:48:50, jkrcal wrote:
> |requiresCharging| is in fact not needed any more. Remove?

Good catch, thanks! Done.

[Marc Treib, 2016-09-22 14:27:17]

The CQ bit was checked by treib@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-22 14:27:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Marc Treib, 2016-09-22 15:01:49]

treib@chromium.org changed reviewers:
+ nyquist@chromium.org

[Marc Treib, 2016-09-22 15:01:50]

+nyquist for chrome/android/java[tests] - could you please take a look, since
bauerb@ is out until next week? Thanks!

[commit-bot: I haz the power, 2016-09-22 15:47:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-22 15:47:20]

Dry run: This issue passed the CQ dry run.

[Marc Treib, 2016-09-23 09:25:40]

treib@chromium.org changed reviewers:
+ bauerb@chromium.org

[Marc Treib, 2016-09-23 09:25:40]

+bauerb, who's apparently semi-in after all :)
Bernhard, could you take a look at the java files? Thanks!

[Bernhard Bauer, 2016-09-26 14:04:56]

lgtm

https://codereview.chromium.org/2357163004/diff/20001/components/ntp_snippets...
File components/ntp_snippets/ntp_snippets_service.h (right):

https://codereview.chromium.org/2357163004/diff/20001/components/ntp_snippets...
components/ntp_snippets/ntp_snippets_service.h:103: // Ignored while |ready()|
is false.
Super-nit: The pipe symbols are only used for variables (methods are marked as
such via the parentheses).

[nyquist, 2016-09-26 20:34:19]

I'll trust bauerb@ on this.

[Marc Treib, 2016-09-27 08:19:53]

The CQ bit was checked by treib@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-27 08:20:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Marc Treib, 2016-09-27 08:23:53]

https://codereview.chromium.org/2357163004/diff/20001/components/ntp_snippets...
File components/ntp_snippets/ntp_snippets_service.h (right):

https://codereview.chromium.org/2357163004/diff/20001/components/ntp_snippets...
components/ntp_snippets/ntp_snippets_service.h:103: // Ignored while |ready()|
is false.
On 2016/09/26 14:04:55, Bernhard (slow until Sep 27) wrote:
> Super-nit: The pipe symbols are only used for variables (methods are marked as
> such via the parentheses).

Done.

[commit-bot: I haz the power, 2016-09-27 09:04:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-27 09:04:24]

Dry run: This issue passed the CQ dry run.

[Marc Treib, 2016-09-27 09:11:52]

The CQ bit was checked by treib@chromium.org

[Marc Treib, 2016-09-27 09:11:52]

The patchset sent to the CQ was uploaded after l-g-t-m from jkrcal@chromium.org,
bauerb@chromium.org
Link to the patchset: https://codereview.chromium.org/2357163004/#ps40001
(title: "super-nit, rebase")

[commit-bot: I haz the power, 2016-09-27 09:12:04]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-27 09:17:20]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-09-27 09:19:13]

Description was changed from

==========
[NTP Snippets] Remove scheduled re-scheduling

We don't need this anymore.
Keep SnippetsBridge.rescheduleFetching() though - we'll need this to re-schedule
tasks from the Java side after an application upgrade (which clears them all).

BUG=646842
==========

to

==========
[NTP Snippets] Remove scheduled re-scheduling

We don't need this anymore.
Keep SnippetsBridge.rescheduleFetching() though - we'll need this to re-schedule
tasks from the Java side after an application upgrade (which clears them all).

BUG=646842

Committed: https://crrev.com/f8d45bd01dea555d4b124caade5210841dab8f4a
Cr-Commit-Position: refs/heads/master@{#421156}
==========

[commit-bot: I haz the power, 2016-09-27 09:19:14]

Patchset 3 (id:??) landed as
https://crrev.com/f8d45bd01dea555d4b124caade5210841dab8f4a
Cr-Commit-Position: refs/heads/master@{#421156}