Browser Plugin: <webview> should inherit partition attribute of opener on attachment.

content/browser/browser_plugin/browser_plugin_guest.cc

Index: content/browser/browser_plugin/browser_plugin_guest.cc
diff --git a/content/browser/browser_plugin/browser_plugin_guest.cc b/content/browser/browser_plugin/browser_plugin_guest.cc
index 973408fbd476f1fa1a1d651c24368d9a7ca8a06c..b3f4dfec0cf7e7d499e2f4afe010a1c357fecb79 100644
--- a/content/browser/browser_plugin/browser_plugin_guest.cc
+++ b/content/browser/browser_plugin/browser_plugin_guest.cc
@@ -120,7 +120,8 @@ BrowserPluginGuest::BrowserPluginGuest(
       pending_lock_request_(false),
       embedder_visible_(true),
       opener_(NULL),
-      next_permission_request_id_(0) {
+      next_permission_request_id_(0),
+      persist_storage_(false) {
   DCHECK(web_contents);
   web_contents->SetDelegate(this);
   GetWebContents()->GetBrowserPluginGuestManager()->AddGuest(instance_id_,
@@ -191,6 +192,10 @@ void BrowserPluginGuest::Initialize(
   auto_size_enabled_ = params.auto_size_params.enable;
   max_auto_size_ = params.auto_size_params.max_size;
   min_auto_size_ = params.auto_size_params.min_size;
+  if (!params.storage_partition_id.empty()) {
+    storage_partition_id_ = params.storage_partition_id;
+    persist_storage_ = params.persist_storage;
+  }
 
   // Once a BrowserPluginGuest has an embedder WebContents, it's considered to
   // be attached.
@@ -697,6 +702,10 @@ void BrowserPluginGuest::Attach(
   // lifetime of the new guest is no longer managed by the opener guest.
   opener()->pending_new_windows_.erase(this);
 
+  // The new window inherits the opener's storage partition.
+  storage_partition_id_ = opener()->storage_partition_id_;
+  persist_storage_ = opener()->persist_storage_;

[Charlie Reis, 2013/03/26 17:02:32]

The partition name and whether it is persistent is stored in our SiteInstance,
which was inherited from the opener.  (See
BrowserPluginGuestManager::CreateGuest.)  Do we need to keep a separate copy of
it?

[Fady Samuel, 2013/03/26 17:15:40]

I thought about that. The issue is BrowserPluginGuestManager::CreateGuest
formats the partition as a URL and so we'd need to parse out these bits out of
the URL. If the URL format changes, the parsing would also need to change so
this is a bit brittle.

[Charlie Reis, 2013/03/26 17:49:30]

I'm not happy with having a separate copy of these that could diverge. 
Conceptually, they're a property of the SiteInstance and process, and it's too
easy to end up with bugs if they differ.

The information is accessible as GURL::path() and GURL::query(), and I'd be fine
having helper methods on BrowserPluginGuest for getting to them.  I think it's
important to read them from the guest's SiteInstance, though.

+
   // The guest's frame name takes precedence over the BrowserPlugin's name.
   // The guest's frame name is assigned in
   // BrowserPluginGuest::WebContentsCreated.
@@ -713,11 +722,13 @@ void BrowserPluginGuest::Attach(
         GetWebContents()->GetRenderViewHost())->Init();
   }
 
-  // Inform the embedder BrowserPlugin of the attached guest.
-  if (!name_.empty()) {
-    SendMessageToEmbedder(
-        new BrowserPluginMsg_UpdatedName(instance_id_, name_));
-  }
+  // Inform the embedder of the guest's information.
+  BrowserPluginMsg_Attach_ACK_Params ack_params;
+  ack_params.storage_partition_id = storage_partition_id_;
+  ack_params.persist_storage = persist_storage_;
+  ack_params.name = name_;
+  SendMessageToEmbedder(
+      new BrowserPluginMsg_Attach_ACK(instance_id_, ack_params));

[Charlie Reis, 2013/03/26 17:02:32]

I don't understand why we need to send these in their own ACK message.

What is the timeline for window.open?  Is there another message that tells the
renderer to create the BrowserPlugin for the new guest?  Seems like these would
belong on that message, rather than happening after the fact.

[Fady Samuel, 2013/03/26 17:15:40]

No, the BrowserPlugin is a pre-existing BrowserPlugin, it is not created on
window.open. We simply attach a new guest to a pre-existing BrowserPlugin.

[Charlie Reis, 2013/03/26 17:49:30]

So the guest is already created in the correct partition, and we're just telling
it so that it can set its DOM attribute accordingly?  Ok, that sounds alright to
me.

 }
 
 void BrowserPluginGuest::OnDragStatusUpdate(int instance_id,