Prerender enabled in incognito.

Prerender enabled in incognito.

BUG=108982
TEST=Open in incognito window, go to prerender-test.appspot.com and confirm that prerender works.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=127763

[cbentzel, 2012-03-09 16:09:31]

I'm working on browser tests. Is there anything else needed to turn this on for
omnibox?

[dominich, 2012-03-09 17:08:57]

I'll look again when tests appear, but it appears to be covering all that's
necessary for link rel prerendering.

For Omnibox, the predictor needs to be set to return new instances for incognito
as right now they'll be NULL and Omnibox prerendering will not be attempted at
all. We also need to copy the predictor database caches into the new incognito
profile predictor caches and make sure we're not writing back into the database.

But there's already a separate bug for that so don't worry about it for this CL.

[cbentzel, 2012-03-09 19:15:45]

Added a test.

[dominich, 2012-03-09 20:49:38]

LGTM

I'm tempted to suggest that we should run all the tests against an incognito
profile to make sure everything works as expected. From what I know of the code
everything should be fine as long based on the test you've added, but I'm not
sure if it's better to be conservative.

I'm fine with you making that call.

(Looks like you need to rebase before committing)

http://codereview.chromium.org/9663005/diff/7002/chrome/browser/prerender/pre...
File chrome/browser/prerender/prerender_browsertest.cc (right):

http://codereview.chromium.org/9663005/diff/7002/chrome/browser/prerender/pre...
chrome/browser/prerender/prerender_browsertest.cc:717: if
(explicitly_set_browser_)
shorter: return explicitly_set_browser_ ? explicitly_set_browser_ : browser();

[cbentzel, 2012-03-09 20:53:44]

On Fri, Mar 9, 2012 at 3:49 PM, <dominich@chromium.org> wrote:

> LGTM
>
> I'm tempted to suggest that we should run all the tests against an
> incognito
> profile to make sure everything works as expected. From what I know of the
> code
> everything should be fine as long based on the test you've added, but I'm
> not
> sure if it's better to be conservative.
>

Yeah, I considered automatically having all tests run with regular and
incognito profile and was worried about the time to run. May give it a shot.


> I'm fine with you making that call.
>
> (Looks like you need to rebase before committing)


>
>
> http://codereview.chromium.**org/9663005/diff/7002/chrome/**
>
browser/prerender/prerender_**browsertest.cc<http://codereview.chromium.org/9663005/diff/7002/chrome/browser/prerender/prerender_browsertest.cc>
> File chrome/browser/prerender/**prerender_browsertest.cc (right):
>
> http://codereview.chromium.**org/9663005/diff/7002/chrome/**
>
browser/prerender/prerender_**browsertest.cc#newcode717<http://codereview.chromium.org/9663005/diff/7002/chrome/browser/prerender/prerender_browsertest.cc#newcode717>
> chrome/browser/prerender/**prerender_browsertest.cc:717: if
> (explicitly_set_browser_)
> shorter: return explicitly_set_browser_ ? explicitly_set_browser_ :
> browser();
>
>
http://codereview.chromium.**org/9663005/<http://codereview.chromium.org/9663...
>

[cbentzel, 2012-03-12 13:46:27]

http://codereview.chromium.org/9663005/diff/7002/chrome/browser/prerender/pre...
File chrome/browser/prerender/prerender_browsertest.cc (right):

http://codereview.chromium.org/9663005/diff/7002/chrome/browser/prerender/pre...
chrome/browser/prerender/prerender_browsertest.cc:717: if
(explicitly_set_browser_)
On 2012/03/09 20:49:38, dominich wrote:
> shorter: return explicitly_set_browser_ ? explicitly_set_browser_ : browser();

I have a weird dislike of ternary operator because I like to set breakpoints.

[dominich, 2012-03-12 17:25:26]

SLGTM modulo rebase and tests passing.

[cbentzel, 2012-03-20 18:24:36]

I have no idea why codereview doesn't show results from try jobs that I ran
today, but that worked correctly.

[commit-bot: I haz the power, 2012-03-20 18:25:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/cbentzel@chromium.org/9663005/7002

[commit-bot: I haz the power, 2012-03-20 20:22:35]

Change committed as 127763