[Prerender] Change IPC from bool to enum for more prerender modes

[Prerender] Change IPC from bool to enum for more prerender modes

This change is in preparation for adding a PreFetch mode to
prerendering.

BUG=632361
Committed: https://crrev.com/67b240c6628573aa3437e14fc82edcb7de78e944
Cr-Commit-Position: refs/heads/master@{#409494}

[mattcary, 2016-07-27 11:31:54]

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

[mattcary, 2016-07-27 11:31:54]

https://codereview.chromium.org/2179903005/diff/20001/chrome/renderer/chrome_...
File chrome/renderer/chrome_render_frame_observer.cc (right):

https://codereview.chromium.org/2179903005/diff/20001/chrome/renderer/chrome_...
chrome/renderer/chrome_render_frame_observer.cc:166: if (mode ==
prerender::FULL_PRERENDER) {
Should this be mode != NO_PRERENDER? I thought we would need a prerender helper
for prefetch as well.

https://codereview.chromium.org/2179903005/diff/20001/chrome/renderer/prerend...
File chrome/renderer/prerender/prerender_helper.cc (right):

https://codereview.chromium.org/2179903005/diff/20001/chrome/renderer/prerend...
chrome/renderer/prerender/prerender_helper.cc:64: if (mode ==
prerender::FULL_PRERENDER)
!= NO_PRERENDER depending on previous comment.

[droger, 2016-07-27 13:16:55]

https://codereview.chromium.org/2179903005/diff/20001/chrome/renderer/chrome_...
File chrome/renderer/chrome_render_frame_observer.cc (right):

https://codereview.chromium.org/2179903005/diff/20001/chrome/renderer/chrome_...
chrome/renderer/chrome_render_frame_observer.cc:166: if (mode ==
prerender::FULL_PRERENDER) {
On 2016/07/27 11:31:54, mattcary wrote:
> Should this be mode != NO_PRERENDER? I thought we would need a prerender
helper
> for prefetch as well.

Good point, done.

[mattcary, 2016-07-27 15:23:39]

lgtm

[droger, 2016-07-28 11:58:56]

droger@chromium.org changed reviewers:
+ mmenke@chromium.org, rsesek@chromium.org

[droger, 2016-07-28 11:58:57]

+ mmenke as OWNER of prerender
+ rsesek for IPC messages.

[droger, 2016-07-28 12:00:10]

Description was changed from

==========
[Prerender] Change IPC from bool to enum for more prerender modes
==========

to

==========
[Prerender] Change IPC from bool to enum for more prerender modes

This change is in preparation for adding a PreFetch mode to
prerendering.
==========

[Robert Sesek, 2016-07-28 14:19:24]

_messages LGTM

[mmenke, 2016-07-28 14:37:35]

Could you please add a "bug=" line for this?

Also, is this code going to share any code with prerendering?  Wonder if it
should use the same infrastructure, unless the plan is to replace prerender with
this new prefetch behavior (Which I'd completely support)

[droger, 2016-07-28 15:26:38]

Description was changed from

==========
[Prerender] Change IPC from bool to enum for more prerender modes

This change is in preparation for adding a PreFetch mode to
prerendering.
==========

to

==========
[Prerender] Change IPC from bool to enum for more prerender modes

This change is in preparation for adding a PreFetch mode to
prerendering.

BUG=632361
==========

[droger, 2016-07-28 15:30:11]

On 2016/07/28 14:37:35, mmenke wrote:
> Could you please add a "bug=" line for this?

Sure, sorry about that.
I created a bug and linked the design doc from there.


> 
> Also, is this code going to share any code with prerendering?  Wonder if it
> should use the same infrastructure, unless the plan is to replace prerender
with
> this new prefetch behavior (Which I'd completely support)

The goal is indeed to replace prerender eventually, by something that is simpler
to maintain.
We're still far away from this though, and the first step is to implement
NoState Prefetch so that we can experiment and gather data to decide if it is
worth pursuing.

So at first we want to keep the changes small (and that probably involves
sharing code with prerender), but eventually replace the prerender
infrastructure by a simpler one.

[mmenke, 2016-07-28 15:37:44]

On 2016/07/28 15:30:11, droger wrote:
> On 2016/07/28 14:37:35, mmenke wrote:
> > Could you please add a "bug=" line for this?
> 
> Sure, sorry about that.
> I created a bug and linked the design doc from there.
> 
> 
> > 
> > Also, is this code going to share any code with prerendering?  Wonder if it
> > should use the same infrastructure, unless the plan is to replace prerender
> with
> > this new prefetch behavior (Which I'd completely support)
> 
> The goal is indeed to replace prerender eventually, by something that is
simpler
> to maintain.
> We're still far away from this though, and the first step is to implement
> NoState Prefetch so that we can experiment and gather data to decide if it is
> worth pursuing.
> 
> So at first we want to keep the changes small (and that probably involves
> sharing code with prerender), but eventually replace the prerender
> infrastructure by a simpler one.

Ok, that SGTM.  Just wanted to make sure we don't end up maintaining two
completely different prerender paths.

LGTM

[droger, 2016-08-02 09:42:09]

droger@chromium.org changed reviewers:
+ thakis@chromium.org

[droger, 2016-08-02 09:42:10]

+thakis for:
chrome/renderer/chrome_render_frame_observer
chrome/common/prerender_types.h

[droger, 2016-08-02 09:50:11]

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

[commit-bot: I haz the power, 2016-08-02 09:50:19]

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, 2016-08-02 10:10:46]

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

[commit-bot: I haz the power, 2016-08-02 10:10:47]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[droger, 2016-08-02 11:43:56]

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

[commit-bot: I haz the power, 2016-08-02 11:44:10]

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, 2016-08-02 12:36:06]

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

[commit-bot: I haz the power, 2016-08-02 12:36:07]

Dry run: This issue passed the CQ dry run.

[Nico, 2016-08-02 20:54:40]

lgtm

https://codereview.chromium.org/2179903005/diff/60001/chrome/common/prerender...
File chrome/common/prerender_types.h (right):

https://codereview.chromium.org/2179903005/diff/60001/chrome/common/prerender...
chrome/common/prerender_types.h:21: PRERENDER_MODE_COUNT = 3
nit: include a trailing ,

[droger, 2016-08-03 07:50:45]

Thanks for the reviews.

https://codereview.chromium.org/2179903005/diff/60001/chrome/common/prerender...
File chrome/common/prerender_types.h (right):

https://codereview.chromium.org/2179903005/diff/60001/chrome/common/prerender...
chrome/common/prerender_types.h:21: PRERENDER_MODE_COUNT = 3
On 2016/08/02 20:54:39, Nico wrote:
> nit: include a trailing ,

Done.

[droger, 2016-08-03 07:50:51]

The CQ bit was checked by droger@chromium.org

[droger, 2016-08-03 07:50:51]

The patchset sent to the CQ was uploaded after l-g-t-m from rsesek@chromium.org,
mattcary@chromium.org, mmenke@chromium.org, thakis@chromium.org
Link to the patchset: https://codereview.chromium.org/2179903005/#ps80001
(title: "add trailing comma")

[commit-bot: I haz the power, 2016-08-03 07:51:09]

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-08-03 07:53:21]

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

[commit-bot: I haz the power, 2016-08-03 07:53:22]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[droger, 2016-08-03 11:08:18]

The CQ bit was checked by droger@chromium.org

[droger, 2016-08-03 11:08:18]

The patchset sent to the CQ was uploaded after l-g-t-m from rsesek@chromium.org,
thakis@chromium.org, mattcary@chromium.org, mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/2179903005/#ps100001
(title: "Rebase")

[commit-bot: I haz the power, 2016-08-03 11:08:33]

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-08-03 11:50:41]

Description was changed from

==========
[Prerender] Change IPC from bool to enum for more prerender modes

This change is in preparation for adding a PreFetch mode to
prerendering.

BUG=632361
==========

to

==========
[Prerender] Change IPC from bool to enum for more prerender modes

This change is in preparation for adding a PreFetch mode to
prerendering.

BUG=632361
==========

[commit-bot: I haz the power, 2016-08-03 11:50:42]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-08-03 11:52:58]

Description was changed from

==========
[Prerender] Change IPC from bool to enum for more prerender modes

This change is in preparation for adding a PreFetch mode to
prerendering.

BUG=632361
==========

to

==========
[Prerender] Change IPC from bool to enum for more prerender modes

This change is in preparation for adding a PreFetch mode to
prerendering.

BUG=632361

Committed: https://crrev.com/67b240c6628573aa3437e14fc82edcb7de78e944
Cr-Commit-Position: refs/heads/master@{#409494}
==========

[commit-bot: I haz the power, 2016-08-03 11:53:00]

Patchset 6 (id:??) landed as
https://crrev.com/67b240c6628573aa3437e14fc82edcb7de78e944
Cr-Commit-Position: refs/heads/master@{#409494}