[Offline pages] Ensuring obtaining GcmNetworkManager in BackgroundScheduler does not throw

[Offline pages] Ensuring obtaining GcmNetworkManager in BackgroundScheduler does not throw

The culprit was devices with no Google Play Services throw when trying to get GcmNetworkManager. This patch prevents it and updates the test.

BUG=692151
R=petewil@chromium.org

Review-Url: https://codereview.chromium.org/2698143005
Cr-Commit-Position: refs/heads/master@{#451428}
Committed: https://chromium.googlesource.com/chromium/src/+/009f63edd2aa777dd51cd48da3b7295636fe0235

[fgorski, 2017-02-16 23:30:36]

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

[fgorski, 2017-02-16 23:30:43]

ptal

[commit-bot: I haz the power, 2017-02-16 23:33:25]

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

[Pete Williamson, 2017-02-16 23:35:41]

lgtm

[commit-bot: I haz the power, 2017-02-17 00:50:43]

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

[commit-bot: I haz the power, 2017-02-17 00:50:44]

Dry run: This issue passed the CQ dry run.

[fgorski, 2017-02-17 18:38:20]

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

[commit-bot: I haz the power, 2017-02-17 18:39:07]

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

[fgorski, 2017-02-17 18:41:10]

Description was changed from

==========
[Offline pages] Ensuring GcmNetworkManager is not null in BackgroundScheduler

This is defensive change to ensure we always have a valid object when
trying to cancel or schedule a task. getInstance() sometimes returns
null.

BUG=692151
R=petewil@chromium.org
==========

to

==========
[Offline pages] Ensuring obtaining GcmNetworkManager in BackgroundScheduler does
not throw

The culprit was devices with no Google Play Services throw when trying to get
GcmNetworkManager. This patch prevents it and updates the test.

BUG=692151
R=petewil@chromium.org
==========

[fgorski, 2017-02-17 18:43:47]

Pete, please take another look.

John, this is FYI. I decided to create a shadow locally to my code so that we
can merge the issue.

[jbudorick, 2017-02-17 18:54:55]

jbudorick@chromium.org changed reviewers:
+ jbudorick@chromium.org

[jbudorick, 2017-02-17 18:54:56]

lgtm, but we should migrate to robolectric's version of
ShadowGoogleApiAvailability once we upgrade beyond 3.1. Filed
https://bugs.chromium.org/p/chromium/issues/detail?id=693660 for that.

[commit-bot: I haz the power, 2017-02-17 20:19:47]

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

[commit-bot: I haz the power, 2017-02-17 20:19:48]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jianli, 2017-02-17 21:44:21]

jianli@chromium.org changed reviewers:
+ jianli@chromium.org

[jianli, 2017-02-17 21:44:22]

Do we fall back to normal download through Chrome if we cannot use
GcmNetworkManager?

[fgorski, 2017-02-17 21:53:53]

On 2017/02/17 21:44:22, jianli wrote:
> Do we fall back to normal download through Chrome if we cannot use
> GcmNetworkManager?

We will not be scheduling in the background as of now, but I am working on
introducing job scheduler.

This patch is to prevent crash when there is no play services.

[jianli, 2017-02-17 21:54:40]

lgtm

[fgorski, 2017-02-17 22:02:21]

The CQ bit was checked by fgorski@chromium.org

[fgorski, 2017-02-17 22:02:22]

The patchset sent to the CQ was uploaded after l-g-t-m from petewil@chromium.org
Link to the patchset: https://codereview.chromium.org/2698143005/#ps40001
(title: "Fixing date in copyright header")

[commit-bot: I haz the power, 2017-02-17 22:04:43]

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

[Pete Williamson, 2017-02-17 22:55:38]

lgtm.  This brings up another issue that we should address separately - if there
are devices without Play services, Async scheduling will never work, we might
want to not offer the button on the dino page for those devices.

[commit-bot: I haz the power, 2017-02-18 00:07:30]

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

[commit-bot: I haz the power, 2017-02-18 00:07:31]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)

[fgorski, 2017-02-18 04:32:05]

The CQ bit was checked by fgorski@chromium.org

[commit-bot: I haz the power, 2017-02-18 04:32:25]

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, 2017-02-18 06:32:59]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1487392325439220,
"parent_rev": "47542d73322fa629f5d02975b7a2694089b28a58", "commit_rev":
"009f63edd2aa777dd51cd48da3b7295636fe0235"}

[commit-bot: I haz the power, 2017-02-18 06:34:00]

Description was changed from

==========
[Offline pages] Ensuring obtaining GcmNetworkManager in BackgroundScheduler does
not throw

The culprit was devices with no Google Play Services throw when trying to get
GcmNetworkManager. This patch prevents it and updates the test.

BUG=692151
R=petewil@chromium.org
==========

to

==========
[Offline pages] Ensuring obtaining GcmNetworkManager in BackgroundScheduler does
not throw

The culprit was devices with no Google Play Services throw when trying to get
GcmNetworkManager. This patch prevents it and updates the test.

BUG=692151
R=petewil@chromium.org

Review-Url: https://codereview.chromium.org/2698143005
Cr-Commit-Position: refs/heads/master@{#451428}
Committed:
https://chromium.googlesource.com/chromium/src/+/009f63edd2aa777dd51cd48da3b7...
==========

[commit-bot: I haz the power, 2017-02-18 06:34:01]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/009f63edd2aa777dd51cd48da3b7...