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

content/browser/browser_plugin/browser_plugin_guest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/browser_plugin/browser_plugin_guest.h"
 
 #include <algorithm>
 
 #include "base/message_loop.h"
 #include "base/string_util.h"
@@ skipping 102 matching lines @@
       damage_buffer_sequence_id_(0),
       damage_buffer_size_(0),
       damage_buffer_scale_factor_(1.0f),
       guest_hang_timeout_(
           base::TimeDelta::FromMilliseconds(kHungRendererDelayMs)),
       focused_(false),
       mouse_locked_(false),
       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_,
                                                              GetWebContents());
 }
 
 void BrowserPluginGuest::DestroyUnattachedWindows() {
   // Destroy() reaches in and removes the BrowserPluginGuest from its opener's
   // pending_new_windows_ set. To avoid mutating the set while iterating, we
   // create a copy of the pending new windows set and iterate over the copy.
@@ skipping 50 matching lines @@
 void BrowserPluginGuest::Initialize(
     WebContentsImpl* embedder_web_contents,
     const BrowserPluginHostMsg_CreateGuest_Params& params) {
   focused_ = params.focused;
   guest_visible_ = params.visible;
   if (!params.name.empty())
     name_ = params.name;
   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.
   embedder_web_contents_ = embedder_web_contents;
 
   // |render_view_host| manages the ownership of this BrowserPluginGuestHelper.
   new BrowserPluginGuestHelper(this, GetWebContents()->GetRenderViewHost());
 
   RendererPreferences* renderer_prefs =
       GetWebContents()->GetMutableRendererPrefs();
@@ skipping 486 matching lines @@
     // processed the move.  The browser may have ignored the move, but it
     // finished processing.  This is used because the renderer keeps a temporary
     // cache of the widget position while these asynchronous operations are in
     // progress.
     Send(new ViewMsg_Move_ACK(web_contents()->GetRoutingID()));
   }
   // Once a new guest is attached to the DOM of the embedder page, then the
   // 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.
   if (!name_.empty())
     params.name.clear();
 
   Initialize(embedder_web_contents, params);
 
   // We initialize the RenderViewHost after a BrowserPlugin has been attached
   // to it and is ready to receive pixels. Until a RenderViewHost is
   // initialized, it will not allow any resize requests.
   if (!GetWebContents()->GetRenderViewHost()->IsRenderViewLive()) {
     static_cast<RenderViewHostImpl*>(
         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,
                                             WebKit::WebDragStatus drag_status,
                                             const WebDropData& drop_data,
                                             WebKit::WebDragOperationsMask mask,
                                             const gfx::Point& location) {
   RenderViewHost* host = GetWebContents()->GetRenderViewHost();
   switch (drag_status) {
     case WebKit::WebDragStatusEnter:
@@ skipping 499 matching lines @@
     LOG(INFO) << "Guest not found. Instance ID: " << instance_id;
     return;
   }
   if (!should_allow)
     guest->Destroy();
   // If we do not destroy the guest then we allow the new window.
   new_window_request_map_.erase(new_window_request_iter);
 }
 
 }  // namespace content