Don't load plugins on prerendered pages until the pages are displayed....

Don't load plugins on prerendered pages until the pages are displayed.

BUG=71216
TEST=in progress


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

[mmenke, 2011-01-18 21:33:36]

I went ahead and merged the plugin prerender CL (Updated according to the
changes to BlockedPlugins) with the prerendering notification CL.

[jam, 2011-01-20 18:49:56]

lgtm

[cbentzel, 2011-01-20 19:46:59]

LGTM

I thought you had a browser test for this already - or was that contingent on
the revalidation change in PrerenderResourceHandler?

http://codereview.chromium.org/6247013/diff/1/chrome/browser/prerender/preren...
File chrome/browser/prerender/prerender_manager.cc (right):

http://codereview.chromium.org/6247013/diff/1/chrome/browser/prerender/preren...
chrome/browser/prerender/prerender_manager.cc:98: rvh->Send(new
ViewMsg_DisplayPrerenderedPage(rvh->routing_id()));
Long term it _feels_ like this should go in the PrerenderContents rather than
the PrerenderManager, but I certainly don't think that change needs to happen
now.

http://codereview.chromium.org/6247013/diff/1/chrome/renderer/blocked_plugin.h
File chrome/renderer/blocked_plugin.h (right):

http://codereview.chromium.org/6247013/diff/1/chrome/renderer/blocked_plugin....
chrome/renderer/blocked_plugin.h:33: bool is_blocked_for_prerendering);
Would it make sense to use an enum like BLOCKED_REASON_PRERENDER and
BLOCKED_REASON_CLICKTOPLAY rather than have a naked bool?

[mmenke, 2011-01-20 20:04:21]

On 2011/01/20 19:46:59, cbentzel wrote:
> LGTM
> 
> I thought you had a browser test for this already - or was that contingent on
> the revalidation change in PrerenderResourceHandler?

I do, but it depends on the revalidation change, which is why it's not included
here.  Once everything is landed and I have it passing the trybots, I'll send it
to you for review.

http://codereview.chromium.org/6247013/diff/1/chrome/browser/prerender/preren...
File chrome/browser/prerender/prerender_manager.cc (right):

http://codereview.chromium.org/6247013/diff/1/chrome/browser/prerender/preren...
chrome/browser/prerender/prerender_manager.cc:98: rvh->Send(new
ViewMsg_DisplayPrerenderedPage(rvh->routing_id()));
On 2011/01/20 19:46:59, cbentzel wrote:
> Long term it _feels_ like this should go in the PrerenderContents rather than
> the PrerenderManager, but I certainly don't think that change needs to happen
> now. 

Hmm...Might make sense to have the PrerenderContents manage the entire swap
itself instead of the PrerenderManager.

Or might make sense to move it over to TabContents::SwapInRenderViewHost(),
since the TabContents really dictates what's visible.

http://codereview.chromium.org/6247013/diff/1/chrome/renderer/blocked_plugin.h
File chrome/renderer/blocked_plugin.h (right):

http://codereview.chromium.org/6247013/diff/1/chrome/renderer/blocked_plugin....
chrome/renderer/blocked_plugin.h:33: bool is_blocked_for_prerendering);
On 2011/01/20 19:46:59, cbentzel wrote:
> Would it make sense to use an enum like BLOCKED_REASON_PRERENDER and
> BLOCKED_REASON_CLICKTOPLAY rather than have a naked bool? 

It might.  Seems we have 4 reasons, I believe - outdated plugin, click to play,
"blocked" (No idea about that one), and prerendering.  The only difference in
behavior for the other 3 are the values of |template_id| and |message|, which
are currently parameters passed to the BlockedPlugin.

It might make sense to assign those strings in BlockedPlugin based on an enum,
rather than in RenderView.  I think it's tangential enough a change to not
really belong in this CL, and I have no idea what the 3rd reason actually means.