[Offline Prefetch] Create a new JobScheduler task to wake up for net activity.

[Offline Prefetch] Create a new JobScheduler task to wake up for net activity.

This includes:
* Only waking up on unmetered networks
* Responding to JobScheduler events such as stopping job
* Calling back to Android when the job is finished
* An integration with chrome://offline-internals.

BUG=713243
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2820263002
Cr-Commit-Position: refs/heads/master@{#469139}
Committed: https://chromium.googlesource.com/chromium/src/+/4b3550fa52e020ed2aebb55f4a5220f4005435a3

[dewittj, 2017-04-18 01:42:44]

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

[commit-bot: I haz the power, 2017-04-18 01:43:17]

Dry run: 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-04-18 03:07:49]

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

[commit-bot: I haz the power, 2017-04-18 03:07:49]

Dry run: This issue passed the CQ dry run.

[dewittj, 2017-04-18 23:00:02]

Description was changed from

==========
Hook up prefetch service to a new JobScheduler task for use as NWake.

BUG=
==========

to

==========
Hook up prefetch service to a new JobScheduler task for use as NWake.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[dewittj, 2017-04-18 23:00:44]

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

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

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

[dewittj, 2017-04-18 23:02:14]

Description was changed from

==========
Hook up prefetch service to a new JobScheduler task for use as NWake.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Hook up prefetch service to a new JobScheduler task for use as NWake.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[dewittj, 2017-04-18 23:02:14]

dewittj@chromium.org changed reviewers:
+ carlosk@chromium.org

[dewittj, 2017-04-18 23:02:55]

+carlosk for an early look.

I am having trouble unit testing this change, any ideas would be appreciated. 
Also would like to know if this approach seems sane.

[commit-bot: I haz the power, 2017-04-19 00:15:08]

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

[commit-bot: I haz the power, 2017-04-19 00:15:09]

Dry run: This issue passed the CQ dry run.

[dewittj, 2017-04-19 17:18:24]

Description was changed from

==========
Hook up prefetch service to a new JobScheduler task for use as NWake.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Hook up prefetch service to a new JobScheduler task for use as NWake.

BUG=713243
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[dewittj, 2017-04-19 17:19:51]

Description was changed from

==========
Hook up prefetch service to a new JobScheduler task for use as NWake.

BUG=713243
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[Offline Prefetch] Create a new JobScheduler task to wake up for net activity.

This includes:
* Only waking up on unmetered networks
* Responding to JobScheduler events such as stopping job
* Calling back to Android when the job is finished
* An integration with chrome://offline-internals.

BUG=713243
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[dewittj, 2017-04-19 17:50:28]

dewittj@chromium.org changed reviewers:
+ fgorski@chromium.org

[dewittj, 2017-04-19 17:50:43]

+fgorski for background_task_scheduler implementation.

[dewittj, 2017-04-19 21:32:33]

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

[commit-bot: I haz the power, 2017-04-19 21:33:39]

Dry run: 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-04-19 22:42:34]

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

[commit-bot: I haz the power, 2017-04-19 22:42:34]

Dry run: This issue passed the CQ dry run.

[carlosk, 2017-04-19 23:01:38]

https://codereview.chromium.org/2820263002/diff/80001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchBackgroundTask.java
(right):

https://codereview.chromium.org/2820263002/diff/80001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchBackgroundTask.java:47:
* TODO(dewittj): Handle skipping work if the battery percentage is too low.
It is unfortunate that battery level is not an Android supported restriction...

https://codereview.chromium.org/2820263002/diff/80001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchBackgroundTask.java:59:
// TODO(dewittj): Remove the maximum time to wait.
Why this TODO? Do you intend to change the API so that this parameter is not
required? Why not simply set the value to an absurdly long time (like one year)?

https://codereview.chromium.org/2820263002/diff/80001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchBackgroundTask.java:81:
* Loads the antive library and then calls into the PrefetchService, which then
manages the
s/antive/native/

https://codereview.chromium.org/2820263002/diff/80001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchBackgroundTask.java:100:
assert mNativeTask != 0;
Duplicate here the taskId assert from onStartTask.

https://codereview.chromium.org/2820263002/diff/80001/chrome/browser/android/...
File chrome/browser/android/offline_pages/prefetch/prefetch_background_task.h
(right):

https://codereview.chromium.org/2820263002/diff/80001/chrome/browser/android/...
chrome/browser/android/offline_pages/prefetch/prefetch_background_task.h:16:
class PrefetchBackgroundTask : public PrefetchService::ScopedBackgroundTask {
Why is this out of the offline_pages::prefetch namespace?

https://codereview.chromium.org/2820263002/diff/80001/components/offline_page...
File components/offline_pages/core/prefetch/prefetch_service.cc (right):

https://codereview.chromium.org/2820263002/diff/80001/components/offline_page...
components/offline_pages/core/prefetch/prefetch_service.cc:10:
PrefetchService::ScopedBackgroundTask::~ScopedBackgroundTask() = default;
Was it impossible to define these in the .h?

https://codereview.chromium.org/2820263002/diff/80001/components/offline_page...
File components/offline_pages/core/prefetch/prefetch_service.h (right):

https://codereview.chromium.org/2820263002/diff/80001/components/offline_page...
components/offline_pages/core/prefetch/prefetch_service.h:58: virtual void
BeginBackgroundTask(
nit: add descriptive comment.

[dewittj, 2017-04-20 21:40:09]

Patchset #6 (id:100001) has been deleted

[dewittj, 2017-04-20 21:41:45]

+CC Jian

Carlos: I addressed your comments.

Some tests should be forthcoming assuming nobody has structural issues with this
patch.

https://codereview.chromium.org/2820263002/diff/80001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchBackgroundTask.java
(right):

https://codereview.chromium.org/2820263002/diff/80001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchBackgroundTask.java:47:
* TODO(dewittj): Handle skipping work if the battery percentage is too low.
On 2017/04/19 23:01:38, carlosk wrote:
> It is unfortunate that battery level is not an Android supported
restriction...

Acknowledged.

https://codereview.chromium.org/2820263002/diff/80001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchBackgroundTask.java:59:
// TODO(dewittj): Remove the maximum time to wait.
On 2017/04/19 23:01:37, carlosk wrote:
> Why this TODO? Do you intend to change the API so that this parameter is not
> required? Why not simply set the value to an absurdly long time (like one
year)?

I tried a REALLY absurdly long time, Long.MAX_VALUE, and that had the effect of
scheduling the task immediately :/

Filip recommended that maybe we actually want a periodic task.  For now 7 days
should be longer than any expiration time in the DB, so it's probably fine.

https://codereview.chromium.org/2820263002/diff/80001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchBackgroundTask.java:81:
* Loads the antive library and then calls into the PrefetchService, which then
manages the
On 2017/04/19 23:01:38, carlosk wrote:
> s/antive/native/

Done.

https://codereview.chromium.org/2820263002/diff/80001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchBackgroundTask.java:100:
assert mNativeTask != 0;
On 2017/04/19 23:01:38, carlosk wrote:
> Duplicate here the taskId assert from onStartTask.

Done.

https://codereview.chromium.org/2820263002/diff/80001/chrome/browser/android/...
File chrome/browser/android/offline_pages/prefetch/prefetch_background_task.h
(right):

https://codereview.chromium.org/2820263002/diff/80001/chrome/browser/android/...
chrome/browser/android/offline_pages/prefetch/prefetch_background_task.h:16:
class PrefetchBackgroundTask : public PrefetchService::ScopedBackgroundTask {
On 2017/04/19 23:01:38, carlosk wrote:
> Why is this out of the offline_pages::prefetch namespace?

I invented the offline_pages::prefetch namespace only for the JNI generator,
since it makes a static function with a predictable name and the offline_pages::
namespace already had such a function. I don't intend for all this code to go
into the sub namespace.

https://codereview.chromium.org/2820263002/diff/80001/components/offline_page...
File components/offline_pages/core/prefetch/prefetch_service.cc (right):

https://codereview.chromium.org/2820263002/diff/80001/components/offline_page...
components/offline_pages/core/prefetch/prefetch_service.cc:10:
PrefetchService::ScopedBackgroundTask::~ScopedBackgroundTask() = default;
On 2017/04/19 23:01:38, carlosk wrote:
> Was it impossible to define these in the .h?

Yes, lint checks barfed on that

[dewittj, 2017-05-01 16:41:50]

Thanks for waiting! I added a junit test.

-fgorski (out)
+dimich

[dewittj, 2017-05-01 16:42:35]

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

[commit-bot: I haz the power, 2017-05-01 16:42:58]

Dry run: CQ is trying da patch.

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

[dewittj, 2017-05-01 16:43:13]

dewittj@chromium.org changed reviewers:
+ dimich@chromium.org
- fgorski@chromium.org

[dewittj, 2017-05-01 16:51:14]

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

[commit-bot: I haz the power, 2017-05-01 16:51:45]

Dry run: CQ is trying da patch.

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

[carlosk, 2017-05-01 17:26:29]

LGTM with a couple minor comments.

General CR comment: whenever possible upload rebases in isolation from your own
changes. It makes comparing changes between patch sets much easier for
reviewers.

https://codereview.chromium.org/2820263002/diff/80001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchBackgroundTask.java
(right):

https://codereview.chromium.org/2820263002/diff/80001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchBackgroundTask.java:59:
// TODO(dewittj): Remove the maximum time to wait.
On 2017/04/20 21:41:45, dewittj wrote:
> On 2017/04/19 23:01:37, carlosk wrote:
> > Why this TODO? Do you intend to change the API so that this parameter is not
> > required? Why not simply set the value to an absurdly long time (like one
> year)?
> 
> I tried a REALLY absurdly long time, Long.MAX_VALUE, and that had the effect
of
> scheduling the task immediately :/
> 
> Filip recommended that maybe we actually want a periodic task.  For now 7 days
> should be longer than any expiration time in the DB, so it's probably fine.

I think it is still important to not do work -- especially network access --
when not connected to "free" wifi, even if this is triggered after a week. We
don't need to solve this in this patch but we should updateconfirm we are in the
required conditions to do the heavy lifting.

https://codereview.chromium.org/2820263002/diff/140001/components/offline_pag...
File components/offline_pages/core/prefetch/prefetch_service_impl.cc (right):

https://codereview.chromium.org/2820263002/diff/140001/components/offline_pag...
components/offline_pages/core/prefetch/prefetch_service_impl.cc:27:
std::unique_ptr<ScopedBackgroundTask> task) {}
nit: Use NOTIMPLEMENTED as in previous cases here and below.

[commit-bot: I haz the power, 2017-05-01 18:14:43]

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

[commit-bot: I haz the power, 2017-05-01 18:14:44]

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

[Dmitry Titov, 2017-05-01 18:14:56]

https://codereview.chromium.org/2820263002/diff/160001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchBackgroundTask.java
(right):

https://codereview.chromium.org/2820263002/diff/160001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchBackgroundTask.java:69:
// TODO(dewittj): Remove the maximum time to wait.
If this is a reasonable upper limit, then why have a TODO?

https://codereview.chromium.org/2820263002/diff/160001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchBackgroundTask.java:110:
assert mNativeTask != 0;
lets replace assert with 'if' here, above as well.

https://codereview.chromium.org/2820263002/diff/160001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchBackgroundTask.java:145:
// TODO(fgorski): This method is taken from ChromeBackgroundService as a local
fix and will
as a local fix for what exactly? Also, is BackgroundTaskScheduler incomplete?
Could you add a bug number here to track?

https://codereview.chromium.org/2820263002/diff/160001/chrome/browser/android...
File chrome/browser/android/offline_pages/prefetch/prefetch_background_task.h
(right):

https://codereview.chromium.org/2820263002/diff/160001/chrome/browser/android...
chrome/browser/android/offline_pages/prefetch/prefetch_background_task.h:25: //
Schedules the default 'NWake' prefetching task.  Does nothing if
Not clear about JobScheduler not being supported... This seems to work through
scheduler that is always present in some form, right?

[dewittj, 2017-05-01 22:15:33]

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

[commit-bot: I haz the power, 2017-05-01 22:16:18]

Dry run: CQ is trying da patch.

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

[dewittj, 2017-05-01 22:16:35]

thanks, ptal

https://codereview.chromium.org/2820263002/diff/140001/components/offline_pag...
File components/offline_pages/core/prefetch/prefetch_service_impl.cc (right):

https://codereview.chromium.org/2820263002/diff/140001/components/offline_pag...
components/offline_pages/core/prefetch/prefetch_service_impl.cc:27:
std::unique_ptr<ScopedBackgroundTask> task) {}
On 2017/05/01 17:26:29, carlosk wrote:
> nit: Use NOTIMPLEMENTED as in previous cases here and below.

Done.

https://codereview.chromium.org/2820263002/diff/160001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchBackgroundTask.java
(right):

https://codereview.chromium.org/2820263002/diff/160001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchBackgroundTask.java:69:
// TODO(dewittj): Remove the maximum time to wait.
On 2017/05/01 18:14:56, Dmitry Titov wrote:
> If this is a reasonable upper limit, then why have a TODO?

Done.

https://codereview.chromium.org/2820263002/diff/160001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchBackgroundTask.java:110:
assert mNativeTask != 0;
On 2017/05/01 18:14:56, Dmitry Titov wrote:
> lets replace assert with 'if' here, above as well.

What should we return here? the return value is whether the system should
reschedule us.  If we are in a race condition the we should already have told
the system as in |doneProcessing|.  Same question above, not sure what we should
do with the return values.

https://codereview.chromium.org/2820263002/diff/160001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchBackgroundTask.java:145:
// TODO(fgorski): This method is taken from ChromeBackgroundService as a local
fix and will
On 2017/05/01 18:14:56, Dmitry Titov wrote:
> as a local fix for what exactly?
The BackgroundTaskScheduler will eventually be able to load the native library,
but does not yet.

> Also, is BackgroundTaskScheduler incomplete?
Yes, GCMNetworkManager integration is incomplete (along with loading native)

> Could you add a bug number here to track?
Done.

https://codereview.chromium.org/2820263002/diff/160001/chrome/browser/android...
File chrome/browser/android/offline_pages/prefetch/prefetch_background_task.h
(right):

https://codereview.chromium.org/2820263002/diff/160001/chrome/browser/android...
chrome/browser/android/offline_pages/prefetch/prefetch_background_task.h:25: //
Schedules the default 'NWake' prefetching task.  Does nothing if
On 2017/05/01 18:14:56, Dmitry Titov wrote:
> Not clear about JobScheduler not being supported... This seems to work through
> scheduler that is always present in some form, right?

You are right, those comments were outdated.

[Dmitry Titov, 2017-05-01 23:07:43]

https://codereview.chromium.org/2820263002/diff/160001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchBackgroundTask.java
(right):

https://codereview.chromium.org/2820263002/diff/160001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchBackgroundTask.java:110:
assert mNativeTask != 0;
On 2017/05/01 22:16:34, dewittj wrote:
> On 2017/05/01 18:14:56, Dmitry Titov wrote:
> > lets replace assert with 'if' here, above as well.
> 
> What should we return here? the return value is whether the system should
> reschedule us.  If we are in a race condition the we should already have told
> the system as in |doneProcessing|.  Same question above, not sure what we
should
> do with the return values.

onStartTask we should return false which means "remove wakelock, task was not
really started"
onStopTask we should return false because something is going awry apparently.
For this specific bool return, we might always return false and instead
reschedule very early in onStartTask :) if this is possible.

[commit-bot: I haz the power, 2017-05-01 23:50:00]

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

[commit-bot: I haz the power, 2017-05-01 23:50:00]

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

[dewittj, 2017-05-02 22:26:04]

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

[commit-bot: I haz the power, 2017-05-02 22:26:29]

Dry run: CQ is trying da patch.

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

[dewittj, 2017-05-02 22:41:17]

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

[commit-bot: I haz the power, 2017-05-02 22:41:41]

Dry run: CQ is trying da patch.

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

[dewittj, 2017-05-02 22:41:54]

+nyquist for chrome/android/junit/DEPS

[dewittj, 2017-05-02 22:42:22]

https://codereview.chromium.org/2820263002/diff/160001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchBackgroundTask.java
(right):

https://codereview.chromium.org/2820263002/diff/160001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchBackgroundTask.java:110:
assert mNativeTask != 0;
On 2017/05/01 23:07:43, Dmitry Titov wrote:
> On 2017/05/01 22:16:34, dewittj wrote:
> > On 2017/05/01 18:14:56, Dmitry Titov wrote:
> > > lets replace assert with 'if' here, above as well.
> > 
> > What should we return here? the return value is whether the system should
> > reschedule us.  If we are in a race condition the we should already have
told
> > the system as in |doneProcessing|.  Same question above, not sure what we
> should
> > do with the return values.
> 
> onStartTask we should return false which means "remove wakelock, task was not
> really started"
> onStopTask we should return false because something is going awry apparently.
> For this specific bool return, we might always return false and instead
> reschedule very early in onStartTask :) if this is possible.

Done.

[dewittj, 2017-05-02 23:11:15]

dewittj@chromium.org changed reviewers:
+ dtrainor@chromium.org

[dewittj, 2017-05-02 23:12:25]

+dtrainor for chrome/android/junit/DEPS

Thanks!

[Dmitry Titov, 2017-05-02 23:51:42]

lgtm with a nit:

https://codereview.chromium.org/2820263002/diff/220001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchBackgroundTask.java
(right):

https://codereview.chromium.org/2820263002/diff/220001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchBackgroundTask.java:63:
TaskInfo.createOneOffTask(TaskIds.OFFLINE_PAGES_PREFETCH_JOB_ID,
This is where we would change it to be a repeating task, correct? Do you plan to
change it in a later CL?

[commit-bot: I haz the power, 2017-05-03 00:09:03]

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

[commit-bot: I haz the power, 2017-05-03 00:09:04]

Dry run: This issue passed the CQ dry run.

[David Trainor- moved to gerrit, 2017-05-03 05:11:54]

chrome/android/junit/DEPS lgtm!

[dewittj, 2017-05-03 15:45:01]

dewittj@chromium.org changed reviewers:
+ tommycli@chromium.org

[dewittj, 2017-05-03 15:45:40]

+tommycli for webui, offline_internals

https://codereview.chromium.org/2820263002/diff/220001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchBackgroundTask.java
(right):

https://codereview.chromium.org/2820263002/diff/220001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchBackgroundTask.java:63:
TaskInfo.createOneOffTask(TaskIds.OFFLINE_PAGES_PREFETCH_JOB_ID,
On 2017/05/02 23:51:42, Dmitry Titov wrote:
> This is where we would change it to be a repeating task, correct? Do you plan
to
> change it in a later CL?

Yes, in another patch.

[tommycli, 2017-05-03 16:00:13]

lgtm with some nits

https://codereview.chromium.org/2820263002/diff/220001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/offline/offline_internals_ui_message_handler.cc
(right):

https://codereview.chromium.org/2820263002/diff/220001/chrome/browser/ui/webu...
chrome/browser/ui/webui/offline/offline_internals_ui_message_handler.cc:231:
void OfflineInternalsUIMessageHandler::HandleScheduleNwake(
Can these methods be the first method called from the page? If so, they should
have an AllowJavascript() call. Same with HandleSetRecordPageModel and
HandleGetNetworkStatus

https://codereview.chromium.org/2820263002/diff/220001/chrome/browser/ui/webu...
chrome/browser/ui/webui/offline/offline_internals_ui_message_handler.cc:238:
ResolveJavascriptCallback(*callback_id, base::Value("Scheduled."));
I'm assuming this is an internal page and so there's no translations?

https://codereview.chromium.org/2820263002/diff/220001/chrome/browser/ui/webu...
chrome/browser/ui/webui/offline/offline_internals_ui_message_handler.cc:368: 
intentional extra newline?

[dewittj, 2017-05-03 17:57:43]

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

[commit-bot: I haz the power, 2017-05-03 17:58:36]

Dry run: 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-05-03 19:08:01]

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

[commit-bot: I haz the power, 2017-05-03 19:08:02]

Dry run: This issue passed the CQ dry run.

[dewittj, 2017-05-03 21:34:10]

https://codereview.chromium.org/2820263002/diff/220001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/offline/offline_internals_ui_message_handler.cc
(right):

https://codereview.chromium.org/2820263002/diff/220001/chrome/browser/ui/webu...
chrome/browser/ui/webui/offline/offline_internals_ui_message_handler.cc:231:
void OfflineInternalsUIMessageHandler::HandleScheduleNwake(
On 2017/05/03 16:00:12, tommycli wrote:
> Can these methods be the first method called from the page? If so, they should
> have an AllowJavascript() call. Same with HandleSetRecordPageModel and
> HandleGetNetworkStatus

Done.

https://codereview.chromium.org/2820263002/diff/220001/chrome/browser/ui/webu...
chrome/browser/ui/webui/offline/offline_internals_ui_message_handler.cc:238:
ResolveJavascriptCallback(*callback_id, base::Value("Scheduled."));
On 2017/05/03 16:00:12, tommycli wrote:
> I'm assuming this is an internal page and so there's no translations?

Correct.

https://codereview.chromium.org/2820263002/diff/220001/chrome/browser/ui/webu...
chrome/browser/ui/webui/offline/offline_internals_ui_message_handler.cc:368: 
On 2017/05/03 16:00:12, tommycli wrote:
> intentional extra newline?

Done.

[dewittj, 2017-05-03 21:34:15]

The CQ bit was checked by dewittj@chromium.org

[dewittj, 2017-05-03 21:34:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
carlosk@chromium.org, tommycli@chromium.org, dtrainor@chromium.org,
dimich@chromium.org
Link to the patchset: https://codereview.chromium.org/2820263002/#ps240001
(title: "tommycli@ comments.")

[commit-bot: I haz the power, 2017-05-03 21:35:14]

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-05-03 21:44:42]

CQ is committing da patch.

Bot data: {"patchset_id": 240001, "attempt_start_ts": 1493847255481280,
"parent_rev": "847ddc166e042a5e929ac16c9327dc3e02cba21c", "commit_rev":
"4b3550fa52e020ed2aebb55f4a5220f4005435a3"}

[commit-bot: I haz the power, 2017-05-03 21:44:50]

Description was changed from

==========
[Offline Prefetch] Create a new JobScheduler task to wake up for net activity.

This includes:
* Only waking up on unmetered networks
* Responding to JobScheduler events such as stopping job
* Calling back to Android when the job is finished
* An integration with chrome://offline-internals.

BUG=713243
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[Offline Prefetch] Create a new JobScheduler task to wake up for net activity.

This includes:
* Only waking up on unmetered networks
* Responding to JobScheduler events such as stopping job
* Calling back to Android when the job is finished
* An integration with chrome://offline-internals.

BUG=713243
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2820263002
Cr-Commit-Position: refs/heads/master@{#469139}
Committed:
https://chromium.googlesource.com/chromium/src/+/4b3550fa52e020ed2aebb55f4a52...
==========

[commit-bot: I haz the power, 2017-05-03 21:44:51]

Committed patchset #12 (id:240001) as
https://chromium.googlesource.com/chromium/src/+/4b3550fa52e020ed2aebb55f4a52...