Propagate storage partition id and persistence to the browser.

Propagate storage partition id and persistence to the browser.

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

[Fady Samuel, 2012-09-25 19:22:40]

http://codereview.chromium.org/10978028/diff/2001/content/browser/browser_plu...
File content/browser/browser_plugin/browser_plugin_embedder.cc (right):

http://codereview.chromium.org/10978028/diff/2001/content/browser/browser_plu...
content/browser/browser_plugin/browser_plugin_embedder.cc:84: bool
persist_storage) {
Hmm, this isn't actually used yet?

http://codereview.chromium.org/10978028/diff/2001/content/renderer/browser_pl...
File content/renderer/browser_plugin/browser_plugin.cc (right):

http://codereview.chromium.org/10978028/diff/2001/content/renderer/browser_pl...
content/renderer/browser_plugin/browser_plugin.cc:105: if (!navigate_src_sent_
&& !src.empty()) {
You might need to update:

content/renderer/browser_plugin/browser_plugin_browsertest.cc
content/browser/browser_plugin/browser_plugin_host_browsertest.cc

[nasko, 2012-09-25 22:57:22]

Please review this CL, which creates a new message for creating the guests,
which is separate from navigation. We will use this new message to send storage
partition information, as well as anything else used only in the initial
navigation/setup of the guest.

https://codereview.chromium.org/10978028/diff/2001/content/browser/browser_pl...
File content/browser/browser_plugin/browser_plugin_embedder.cc (right):

https://codereview.chromium.org/10978028/diff/2001/content/browser/browser_pl...
content/browser/browser_plugin/browser_plugin_embedder.cc:84: bool
persist_storage) {
On 2012/09/25 19:22:40, Fady Samuel wrote:
> Hmm, this isn't actually used yet?

Yes, as per our discussion, this will be a smaller CL to just add the
CreateGuest message and I will wire it to the storage partition in a separate
CL.

https://codereview.chromium.org/10978028/diff/2001/content/renderer/browser_p...
File content/renderer/browser_plugin/browser_plugin.cc (right):

https://codereview.chromium.org/10978028/diff/2001/content/renderer/browser_p...
content/renderer/browser_plugin/browser_plugin.cc:105: if (!navigate_src_sent_
&& !src.empty()) {
On 2012/09/25 19:22:40, Fady Samuel wrote:
> You might need to update:
> 
> content/renderer/browser_plugin/browser_plugin_browsertest.cc
> content/browser/browser_plugin/browser_plugin_host_browsertest.cc

Done.

[Fady Samuel, 2012-09-25 23:04:38]

LGTM with a nit.

https://codereview.chromium.org/10978028/diff/2001/content/browser/browser_pl...
File content/browser/browser_plugin/browser_plugin_embedder.cc (right):

https://codereview.chromium.org/10978028/diff/2001/content/browser/browser_pl...
content/browser/browser_plugin/browser_plugin_embedder.cc:84: bool
persist_storage) {
On 2012/09/25 22:57:22, nasko wrote:
> On 2012/09/25 19:22:40, Fady Samuel wrote:
> > Hmm, this isn't actually used yet?
> 
> Yes, as per our discussion, this will be a smaller CL to just add the
> CreateGuest message and I will wire it to the storage partition in a separate
> CL.

sgtm.

https://codereview.chromium.org/10978028/diff/6003/content/common/browser_plu...
File content/common/browser_plugin_messages.h (right):

https://codereview.chromium.org/10978028/diff/6003/content/common/browser_plu...
content/common/browser_plugin_messages.h:30:
IPC_MESSAGE_ROUTED4(BrowserPluginHostMsg_CreateGuest,
Short description of what this message does please.

[Charlie Reis, 2012-09-28 18:04:59]

Looks like you have a compile error in browser_plugin_browsertest.cc.

Also, can you add something to the CL description about making guest creation a
separate message from navigation?

https://codereview.chromium.org/10978028/diff/6003/content/browser/browser_pl...
File content/browser/browser_plugin/browser_plugin_embedder.cc (right):

https://codereview.chromium.org/10978028/diff/6003/content/browser/browser_pl...
content/browser/browser_plugin/browser_plugin_embedder.cc:99:
guest_web_contents->GetMutableRendererPrefs()->throttle_input_events = false;
Heads up: I think you'll have a minor merge conflict here with
https://chromiumcodereview.appspot.com/10908099/.

https://codereview.chromium.org/10978028/diff/6003/content/browser/browser_pl...
File content/browser/browser_plugin/browser_plugin_embedder.h (right):

https://codereview.chromium.org/10978028/diff/6003/content/browser/browser_pl...
content/browser/browser_plugin/browser_plugin_embedder.h:71: // Navigates in a
guest (new or existing).
We should probably change this to "Navigates an existing guest."

https://codereview.chromium.org/10978028/diff/6003/content/browser/web_conten...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/10978028/diff/6003/content/browser/web_conten...
content/browser/web_contents/web_contents_impl.cc:2386: false);
Why not pass storage_partition_id and persist_storage here?  It's ok if they're
unused in CreateGuest, but I don't see a need to pass fake values here.

[nasko, 2012-09-28 22:27:22]

Merged with multiple incoming CLs and fixes based on comments.

https://codereview.chromium.org/10978028/diff/6003/content/browser/browser_pl...
File content/browser/browser_plugin/browser_plugin_embedder.h (right):

https://codereview.chromium.org/10978028/diff/6003/content/browser/browser_pl...
content/browser/browser_plugin/browser_plugin_embedder.h:71: // Navigates in a
guest (new or existing).
On 2012/09/28 18:04:59, creis wrote:
> We should probably change this to "Navigates an existing guest."

Done.

https://codereview.chromium.org/10978028/diff/6003/content/browser/web_conten...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/10978028/diff/6003/content/browser/web_conten...
content/browser/web_contents/web_contents_impl.cc:2386: false);
On 2012/09/28 18:04:59, creis wrote:
> Why not pass storage_partition_id and persist_storage here?  It's ok if
they're
> unused in CreateGuest, but I don't see a need to pass fake values here.

Done.

https://codereview.chromium.org/10978028/diff/6003/content/common/browser_plu...
File content/common/browser_plugin_messages.h (right):

https://codereview.chromium.org/10978028/diff/6003/content/common/browser_plu...
content/common/browser_plugin_messages.h:30:
IPC_MESSAGE_ROUTED4(BrowserPluginHostMsg_CreateGuest,
On 2012/09/25 23:04:38, Fady Samuel wrote:
> Short description of what this message does please. 

Done.

[Charlie Reis, 2012-09-28 23:09:28]

Thanks.  LGTM, once you fix the compile issues.

[commit-bot: I haz the power, 2012-10-01 21:14:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nasko@chromium.org/10978028/27001

[commit-bot: I haz the power, 2012-10-01 23:03:39]

Change committed as 159578