Add calls to the offliner and unit tests.

Add calls to the offliner and unit tests.

This forwards calls that make it to the request coordinator on to the
offliner.

BUG=610521
Committed: https://crrev.com/c2711bdfc2538ba6814466d1fc1165acde286717
Cr-Commit-Position: refs/heads/master@{#393699}

[Pete Williamson, 2016-05-12 18:46:03]

petewil@chromium.org changed reviewers:
+ dougarnett@chromium.org*

[Pete Williamson, 2016-05-12 20:52:42]

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

[commit-bot: I haz the power, 2016-05-12 20:53:14]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1971923004/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1971923004/1

[commit-bot: I haz the power, 2016-05-12 21:48:20]

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

[commit-bot: I haz the power, 2016-05-12 21:48:21]

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

[Pete Williamson, 2016-05-13 16:44:54]

Description was changed from

==========
Add calls to the offliner and unit tests.

This forwards calls that make it to the request coordinator on to the
offliner.

BUG=610521
==========

to

==========
Add calls to the offliner and unit tests.

This forwards calls that make it to the request coordinator on to the
offliner.

BUG=610521
==========

[Pete Williamson, 2016-05-13 16:44:54]

petewil@chromium.org changed reviewers:
+ dougarnett@chromium.org
- dougarnett@chromium.org*

[Pete Williamson, 2016-05-13 16:53:31]

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

[commit-bot: I haz the power, 2016-05-13 16:54:02]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1971923004/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1971923004/40001

[commit-bot: I haz the power, 2016-05-13 18:30:54]

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

[commit-bot: I haz the power, 2016-05-13 18:30:55]

Dry run: This issue passed the CQ dry run.

[dougarnett, 2016-05-13 19:33:00]

I'd like to avoid "prerender" terminology into the Coordinator unless there is a
good reason for it.

https://codereview.chromium.org/1971923004/diff/40001/components/offline_page...
File components/offline_pages/background/offliner.h (right):

https://codereview.chromium.org/1971923004/diff/40001/components/offline_page...
components/offline_pages/background/offliner.h:24: NO_STATUS_YET = 4,
How about "UNKNOWN"? My pending CL adds new enum with that name

[Btw, my CL is currently renaming CompletionStatus to RequestStatus as I want to
use the same enum from Loader to Offliner to report new LOADED value.]

https://codereview.chromium.org/1971923004/diff/40001/components/offline_page...
File components/offline_pages/background/request_coordinator.cc (right):

https://codereview.chromium.org/1971923004/diff/40001/components/offline_page...
components/offline_pages/background/request_coordinator.cc:26:
last_offlining_status_(Offliner::NO_STATUS_YET) {
suggesting Offliner::UNKNOWN

https://codereview.chromium.org/1971923004/diff/40001/components/offline_page...
components/offline_pages/background/request_coordinator.cc:78: Offliner*
prerendererOffliner = factory_->GetOffliner(policy_.get());
just name as "offliner" here?

https://codereview.chromium.org/1971923004/diff/40001/components/offline_page...
components/offline_pages/background/request_coordinator.cc:85: // Start the load
and save process in the prerenderer (Async).
... by the offliner

https://codereview.chromium.org/1971923004/diff/40001/components/offline_page...
File components/offline_pages/background/request_coordinator.h (right):

https://codereview.chromium.org/1971923004/diff/40001/components/offline_page...
components/offline_pages/background/request_coordinator.h:73: void
PrerendererDoneCallback(const SavePageRequest& request,
LoadDoneCallback?  Coordinator maybe doesn't need to know anything about
"prerender" impl.

https://codereview.chromium.org/1971923004/diff/40001/components/offline_page...
File components/offline_pages/background/request_coordinator_unittest.cc
(right):

https://codereview.chromium.org/1971923004/diff/40001/components/offline_page...
components/offline_pages/background/request_coordinator_unittest.cc:56: // Call
the callback, on another thread.
another thread? I don't think we want that and I don't think this does that.

Maybe: // Schedule the callback. or // Post the callback on the run loop.  ?

[Pete Williamson, 2016-05-13 22:28:24]

https://codereview.chromium.org/1971923004/diff/40001/components/offline_page...
File components/offline_pages/background/offliner.h (right):

https://codereview.chromium.org/1971923004/diff/40001/components/offline_page...
components/offline_pages/background/offliner.h:24: NO_STATUS_YET = 4,
On 2016/05/13 19:33:00, dougarnett wrote:
> How about "UNKNOWN"? My pending CL adds new enum with that name
> 
> [Btw, my CL is currently renaming CompletionStatus to RequestStatus as I want
to
> use the same enum from Loader to Offliner to report new LOADED value.]

Done.

https://codereview.chromium.org/1971923004/diff/40001/components/offline_page...
File components/offline_pages/background/request_coordinator.cc (right):

https://codereview.chromium.org/1971923004/diff/40001/components/offline_page...
components/offline_pages/background/request_coordinator.cc:26:
last_offlining_status_(Offliner::NO_STATUS_YET) {
On 2016/05/13 19:33:00, dougarnett wrote:
> suggesting Offliner::UNKNOWN

Done.

https://codereview.chromium.org/1971923004/diff/40001/components/offline_page...
components/offline_pages/background/request_coordinator.cc:78: Offliner*
prerendererOffliner = factory_->GetOffliner(policy_.get());
On 2016/05/13 19:33:00, dougarnett wrote:
> just name as "offliner" here?

Done.  However, eventually we will have two factories, and two different
offliners, I suspect we will want distinctive names for them when we do.

https://codereview.chromium.org/1971923004/diff/40001/components/offline_page...
components/offline_pages/background/request_coordinator.cc:85: // Start the load
and save process in the prerenderer (Async).
On 2016/05/13 19:33:00, dougarnett wrote:
> ... by the offliner

Done.

https://codereview.chromium.org/1971923004/diff/40001/components/offline_page...
File components/offline_pages/background/request_coordinator.h (right):

https://codereview.chromium.org/1971923004/diff/40001/components/offline_page...
components/offline_pages/background/request_coordinator.h:73: void
PrerendererDoneCallback(const SavePageRequest& request,
On 2016/05/13 19:33:00, dougarnett wrote:
> LoadDoneCallback?  Coordinator maybe doesn't need to know anything about
> "prerender" impl.

Done.

https://codereview.chromium.org/1971923004/diff/40001/components/offline_page...
File components/offline_pages/background/request_coordinator_unittest.cc
(right):

https://codereview.chromium.org/1971923004/diff/40001/components/offline_page...
components/offline_pages/background/request_coordinator_unittest.cc:56: // Call
the callback, on another thread.
On 2016/05/13 19:33:00, dougarnett wrote:
> another thread? I don't think we want that and I don't think this does that.
> 
> Maybe: // Schedule the callback. or // Post the callback on the run loop.  ?

Done.

[dougarnett, 2016-05-13 22:56:29]

lgtm

https://codereview.chromium.org/1971923004/diff/40001/components/offline_page...
File components/offline_pages/background/request_coordinator.cc (right):

https://codereview.chromium.org/1971923004/diff/40001/components/offline_page...
components/offline_pages/background/request_coordinator.cc:78: Offliner*
prerendererOffliner = factory_->GetOffliner(policy_.get());
On 2016/05/13 22:28:24, Pete Williamson wrote:
> On 2016/05/13 19:33:00, dougarnett wrote:
> > just name as "offliner" here?
> 
> Done.  However, eventually we will have two factories, and two different
> offliners, I suspect we will want distinctive names for them when we do.

I expect so but I'm not confident at this point what aspect will be interesting
to the Coordinator or a policy piece to identify between them - which might be
Client/Server, Local/Remote, Single/Batch, or Prerender/Headless?

[Pete Williamson, 2016-05-13 23:00:34]

The CQ bit was checked by petewil@chromium.org

[commit-bot: I haz the power, 2016-05-13 23:01:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1971923004/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1971923004/60001

[commit-bot: I haz the power, 2016-05-14 00:16:14]

Description was changed from

==========
Add calls to the offliner and unit tests.

This forwards calls that make it to the request coordinator on to the
offliner.

BUG=610521
==========

to

==========
Add calls to the offliner and unit tests.

This forwards calls that make it to the request coordinator on to the
offliner.

BUG=610521
==========

[commit-bot: I haz the power, 2016-05-14 00:16:15]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-05-14 00:17:04]

Description was changed from

==========
Add calls to the offliner and unit tests.

This forwards calls that make it to the request coordinator on to the
offliner.

BUG=610521
==========

to

==========
Add calls to the offliner and unit tests.

This forwards calls that make it to the request coordinator on to the
offliner.

BUG=610521

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

[commit-bot: I haz the power, 2016-05-14 00:17:06]

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