Browser Plugin: Unify Attach IPC

BrowserPluginHostMsg_CreateGuest, and BrowserPluginHostMsg_Attach are two very similar IPCs that served slightly different purposes.

CreateGuest was used to create a new guest renderer.
Attach was used to attach to an existing (unattached) guest renderer.

We might, in the future, wish to expose this as an internal API to enable shims to use BrowserPlugin as their underlying infrastructure.

By unifying the two operations and adding additional security checks in BrowserPluginGuestManager, we reduce the likelihood of abuse for this API and fully define its behavior.

BUG=166165

Review URL: https://chromiumcodereview.appspot.com/14327007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@197526 0039d316-1c4b-4281-b951-d872f2087c98

12 files changed, 159 insertions, 123 deletions

diff --git a/content/browser/browser_plugin/browser_plugin_embedder.cc b/content/browser/browser_plugin/browser_plugin_embedder.cc
index 10df740f..01c1c34 100644
--- a/content/browser/browser_plugin/browser_plugin_embedder.cc
+++ b/content/browser/browser_plugin/browser_plugin_embedder.cc
@@ -62,7 +62,6 @@ bool BrowserPluginEmbedder::OnMessageReceived(const IPC::Message& message) {
     IPC_MESSAGE_HANDLER(BrowserPluginHostMsg_AllocateInstanceID,
                         OnAllocateInstanceID)
     IPC_MESSAGE_HANDLER(BrowserPluginHostMsg_Attach, OnAttach)
-    IPC_MESSAGE_HANDLER(BrowserPluginHostMsg_CreateGuest, OnCreateGuest)
     IPC_MESSAGE_HANDLER(BrowserPluginHostMsg_PluginAtPositionResponse,
                         OnPluginAtPositionResponse)
     IPC_MESSAGE_UNHANDLED(handled = false)
@@ -97,18 +96,21 @@ void BrowserPluginEmbedder::OnAllocateInstanceID(int request_id) {
 
 void BrowserPluginEmbedder::OnAttach(
     int instance_id,
-    const BrowserPluginHostMsg_CreateGuest_Params& params) {
+    const BrowserPluginHostMsg_Attach_Params& params) {
+  if (!GetBrowserPluginGuestManager()->CanEmbedderAccessInstanceIDMaybeKill(
+          web_contents()->GetRenderProcessHost()->GetID(), instance_id))
+    return;
+
   BrowserPluginGuest* guest =
       GetBrowserPluginGuestManager()->GetGuestByInstanceID(
           instance_id, web_contents()->GetRenderProcessHost()->GetID());
-  if (guest)
+
+  if (guest) {
     guest->Attach(static_cast<WebContentsImpl*>(web_contents()), params);
-}
+    return;
+  }
 
-void BrowserPluginEmbedder::OnCreateGuest(
-    int instance_id,
-    const BrowserPluginHostMsg_CreateGuest_Params& params) {
-  BrowserPluginGuest* guest = GetBrowserPluginGuestManager()->CreateGuest(
+  guest = GetBrowserPluginGuestManager()->CreateGuest(
       web_contents()->GetSiteInstance(), instance_id, params);
   if (guest)
     guest->Initialize(static_cast<WebContentsImpl*>(web_contents()), params);
diff --git a/content/browser/browser_plugin/browser_plugin_embedder.h b/content/browser/browser_plugin/browser_plugin_embedder.h
index ddcf6f1..9c04a1f 100644
--- a/content/browser/browser_plugin/browser_plugin_embedder.h
+++ b/content/browser/browser_plugin/browser_plugin_embedder.h
@@ -19,7 +19,7 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
 
-struct BrowserPluginHostMsg_CreateGuest_Params;
+struct BrowserPluginHostMsg_Attach_Params;
 struct BrowserPluginHostMsg_ResizeGuest_Params;
 
 namespace gfx {
@@ -69,9 +69,7 @@ class CONTENT_EXPORT BrowserPluginEmbedder : public WebContentsObserver {
 
   void OnAllocateInstanceID(int request_id);
   void OnAttach(int instance_id,
-                const BrowserPluginHostMsg_CreateGuest_Params& params);
-  void OnCreateGuest(int instance_id,
-                     const BrowserPluginHostMsg_CreateGuest_Params& params);
+                const BrowserPluginHostMsg_Attach_Params& params);
   void OnPluginAtPositionResponse(int instance_id,
                                   int request_id,
                                   const gfx::Point& position);
diff --git a/content/browser/browser_plugin/browser_plugin_guest.cc b/content/browser/browser_plugin/browser_plugin_guest.cc
index 8627e1e..18a25c0 100644
--- a/content/browser/browser_plugin/browser_plugin_guest.cc
+++ b/content/browser/browser_plugin/browser_plugin_guest.cc
@@ -224,7 +224,7 @@ bool BrowserPluginGuest::OnMessageReceivedFromEmbedder(
 
 void BrowserPluginGuest::Initialize(
     WebContentsImpl* embedder_web_contents,
-    const BrowserPluginHostMsg_CreateGuest_Params& params) {
+    const BrowserPluginHostMsg_Attach_Params& params) {
   focused_ = params.focused;
   guest_visible_ = params.visible;
   if (!params.name.empty())
@@ -743,7 +743,15 @@ bool BrowserPluginGuest::OnMessageReceived(const IPC::Message& message) {
 
 void BrowserPluginGuest::Attach(
     WebContentsImpl* embedder_web_contents,
-    BrowserPluginHostMsg_CreateGuest_Params params) {
+    BrowserPluginHostMsg_Attach_Params params) {
+  if (attached())
+    return;
+
+  // Clear parameters that get inherited from the opener.
+  params.storage_partition_id.clear();
+  params.persist_storage = false;
+  params.src.clear();
+
   const std::string target_url = opener()->pending_new_windows_[this];
   if (!GetWebContents()->opener()) {
     // For guests that have a suppressed opener, we navigate now.
diff --git a/content/browser/browser_plugin/browser_plugin_guest.h b/content/browser/browser_plugin/browser_plugin_guest.h
index c335c4b..526d183 100644
--- a/content/browser/browser_plugin/browser_plugin_guest.h
+++ b/content/browser/browser_plugin/browser_plugin_guest.h
@@ -42,7 +42,7 @@
 #include "ui/surface/transport_dib.h"
 
 struct BrowserPluginHostMsg_AutoSize_Params;
-struct BrowserPluginHostMsg_CreateGuest_Params;
+struct BrowserPluginHostMsg_Attach_Params;
 struct BrowserPluginHostMsg_ResizeGuest_Params;
 struct ViewHostMsg_CreateWindow_Params;
 #if defined(OS_MACOSX)
@@ -107,7 +107,7 @@ class CONTENT_EXPORT BrowserPluginGuest : public NotificationObserver,
   bool OnMessageReceivedFromEmbedder(const IPC::Message& message);
 
   void Initialize(WebContentsImpl* embedder_web_contents,
-                  const BrowserPluginHostMsg_CreateGuest_Params& params);
+                  const BrowserPluginHostMsg_Attach_Params& params);
 
   void set_guest_hang_timeout_for_testing(const base::TimeDelta& timeout) {
     guest_hang_timeout_ = timeout;
@@ -220,7 +220,7 @@ class CONTENT_EXPORT BrowserPluginGuest : public NotificationObserver,
   // to an embedder implies that this guest's lifetime is no longer managed
   // by its opener, and it can begin loading resources.
   void Attach(WebContentsImpl* embedder_web_contents,
-              BrowserPluginHostMsg_CreateGuest_Params params);
+              BrowserPluginHostMsg_Attach_Params params);
 
   // Requests geolocation permission through Embedder JavaScript API.
   void AskEmbedderForGeolocationPermission(int bridge_id,
diff --git a/content/browser/browser_plugin/browser_plugin_guest_manager.cc b/content/browser/browser_plugin/browser_plugin_guest_manager.cc
index 6a41b50..081f332 100644
--- a/content/browser/browser_plugin/browser_plugin_guest_manager.cc
+++ b/content/browser/browser_plugin/browser_plugin_guest_manager.cc
@@ -40,14 +40,8 @@ BrowserPluginGuestManager* BrowserPluginGuestManager::Create() {
 BrowserPluginGuest* BrowserPluginGuestManager::CreateGuest(
     SiteInstance* embedder_site_instance,
     int instance_id,
-    const BrowserPluginHostMsg_CreateGuest_Params& params) {
+    const BrowserPluginHostMsg_Attach_Params& params) {
   SiteInstance* guest_site_instance = NULL;
-  int embedder_render_process_id =
-      embedder_site_instance->GetProcess()->GetID();
-  BrowserPluginGuest* guest =
-      GetGuestByInstanceID(instance_id, embedder_render_process_id);
-  CHECK(!guest);
-
   // Validate that the partition id coming from the renderer is valid UTF-8,
   // since we depend on this in other parts of the code, such as FilePath
   // creation. If the validation fails, treat it as a bad message and kill the
@@ -110,22 +104,15 @@ BrowserPluginGuest* BrowserPluginGuestManager::CreateGuest(
 BrowserPluginGuest* BrowserPluginGuestManager::GetGuestByInstanceID(
     int instance_id,
     int embedder_render_process_id) const {
+  if (!CanEmbedderAccessInstanceIDMaybeKill(embedder_render_process_id,
+                                            instance_id)) {
+    return NULL;
+  }
   GuestInstanceMap::const_iterator it =
       guest_web_contents_by_instance_id_.find(instance_id);
   if (it == guest_web_contents_by_instance_id_.end())
     return NULL;
-
-  BrowserPluginGuest* guest =
-      static_cast<WebContentsImpl*>(it->second)->GetBrowserPluginGuest();
-  if (!CanEmbedderAccessGuest(embedder_render_process_id, guest)) {
-    // The embedder process is trying to access a guest it does not own.
-    content::RecordAction(UserMetricsAction("BadMessageTerminate_BPGM"));
-    base::KillProcess(
-        RenderProcessHost::FromID(embedder_render_process_id)->GetHandle(),
-        content::RESULT_CODE_KILLED_BAD_MESSAGE, false);
-    return NULL;
-  }
-  return guest;
+  return static_cast<WebContentsImpl*>(it->second)->GetBrowserPluginGuest();
 }
 
 void BrowserPluginGuestManager::AddGuest(int instance_id,
@@ -141,6 +128,20 @@ void BrowserPluginGuestManager::RemoveGuest(int instance_id) {
   guest_web_contents_by_instance_id_.erase(instance_id);
 }
 
+bool BrowserPluginGuestManager::CanEmbedderAccessInstanceIDMaybeKill(
+    int embedder_render_process_id,
+    int instance_id) const {
+  if (!CanEmbedderAccessInstanceID(embedder_render_process_id, instance_id)) {
+    // The embedder process is trying to access a guest it does not own.
+    content::RecordAction(UserMetricsAction("BadMessageTerminate_BPGM"));
+    base::KillProcess(
+        RenderProcessHost::FromID(embedder_render_process_id)->GetHandle(),
+        content::RESULT_CODE_KILLED_BAD_MESSAGE, false);
+    return false;
+  }
+  return true;
+}
+
 void BrowserPluginGuestManager::OnMessageReceived(const IPC::Message& message,
                                                   int render_process_id) {
   if (BrowserPluginGuest::ShouldForwardToBrowserPluginGuest(message)) {
@@ -160,11 +161,11 @@ void BrowserPluginGuestManager::OnMessageReceived(const IPC::Message& message,
   IPC_END_MESSAGE_MAP()
 }
 
-//static
+// static
 bool BrowserPluginGuestManager::CanEmbedderAccessGuest(
     int embedder_render_process_id,
     BrowserPluginGuest* guest) {
-  // An embedder can access |guest| if the guest has not been attached and its
+  // The embedder can access the guest if it has not been attached and its
   // opener's embedder lives in the same process as the given embedder.
   if (!guest->attached()) {
     if (!guest->opener())
@@ -179,6 +180,31 @@ bool BrowserPluginGuestManager::CanEmbedderAccessGuest(
       guest->embedder_web_contents()->GetRenderProcessHost()->GetID();
 }
 
+bool BrowserPluginGuestManager::CanEmbedderAccessInstanceID(
+    int embedder_render_process_id,
+    int instance_id) const {
+  // The embedder is trying to access a guest with a negative or zero
+  // instance ID.
+  if (instance_id <= browser_plugin::kInstanceIDNone)
+    return false;
+
+  // The embedder is trying to access an instance ID that has not yet been
+  // allocated by BrowserPluginGuestManager. This could cause instance ID
+  // collisions in the future, and potentially give one embedder access to a
+  // guest it does not own.
+  if (instance_id > next_instance_id_)
+    return false;
+
+  GuestInstanceMap::const_iterator it =
+      guest_web_contents_by_instance_id_.find(instance_id);
+  if (it == guest_web_contents_by_instance_id_.end())
+    return true;
+  BrowserPluginGuest* guest =
+      static_cast<WebContentsImpl*>(it->second)->GetBrowserPluginGuest();
+
+  return CanEmbedderAccessGuest(embedder_render_process_id, guest);
+}
+
 SiteInstance* BrowserPluginGuestManager::GetGuestSiteInstance(
     const GURL& guest_site) {
   for (GuestInstanceMap::const_iterator it =
diff --git a/content/browser/browser_plugin/browser_plugin_guest_manager.h b/content/browser/browser_plugin/browser_plugin_guest_manager.h
index 4f5bb9b..4e1ba28 100644
--- a/content/browser/browser_plugin/browser_plugin_guest_manager.h
+++ b/content/browser/browser_plugin/browser_plugin_guest_manager.h
@@ -14,7 +14,7 @@
 #include "content/common/content_export.h"
 #include "ipc/ipc_message.h"
 
-struct BrowserPluginHostMsg_CreateGuest_Params;
+struct BrowserPluginHostMsg_Attach_Params;
 struct BrowserPluginHostMsg_ResizeGuest_Params;
 class GURL;
 
@@ -58,19 +58,27 @@ class CONTENT_EXPORT BrowserPluginGuestManager :
   BrowserPluginGuest* CreateGuest(
       SiteInstance* embedder_site_instance,
       int instance_id,
-      const BrowserPluginHostMsg_CreateGuest_Params& params);
+      const BrowserPluginHostMsg_Attach_Params& params);
 
-  // Returns a BrowserPluginGuest given an instance ID. Returns NULL if the
-  // guest wasn't found.  If the guest doesn't belong to the given embedder,
-  // then NULL is returned and the embedder is killed.
+  // Returns a BrowserPluginGuest given an |instance_id|. Returns NULL if the
+  // guest wasn't found.  If the embedder is not permitted to access the given
+  // |instance_id|, the embedder is killed, and NULL is returned.
   BrowserPluginGuest* GetGuestByInstanceID(
       int instance_id,
       int embedder_render_process_id) const;
 
   // Adds a new |guest_web_contents| to the embedder (overridable in test).
   virtual void AddGuest(int instance_id, WebContentsImpl* guest_web_contents);
+
+  // Removes the guest with the given |instance_id| from this
+  // BrowserPluginGuestManager.
   void RemoveGuest(int instance_id);
 
+  // Returns whether the specified embedder is permitted to access the given
+  // |instance_id|, and kills the embedder if not.
+  bool CanEmbedderAccessInstanceIDMaybeKill(int embedder_render_process_id,
+                                            int instance_id) const;
+
   void OnMessageReceived(const IPC::Message& message, int render_process_id);
 
  private:
@@ -78,10 +86,20 @@ class CONTENT_EXPORT BrowserPluginGuestManager :
 
   BrowserPluginGuestManager();
 
-  // Returns whether the given embedder process is allowed to access |guest|.
+  // Returns whether the given embedder process is allowed to access the
+  // provided |guest|.
   static bool CanEmbedderAccessGuest(int embedder_render_process_id,
                                      BrowserPluginGuest* guest);
 
+  // Returns whether the given embedder process is allowed to use the provided
+  // |instance_id| or access the guest associated with the |instance_id|. If the
+  // embedder can, the method returns true. If the guest does not exist but the
+  // embedder can use that |instance_id|, then it returns true. If the embedder
+  // is not permitted to use that instance ID or access the associated guest,
+  // then it returns false.
+  bool CanEmbedderAccessInstanceID(int embedder_render_process_id,
+                                   int instance_id) const;
+
   // Returns an existing SiteInstance if the current profile has a guest of the
   // given |guest_site|.
   SiteInstance* GetGuestSiteInstance(const GURL& guest_site);
diff --git a/content/common/browser_plugin/browser_plugin_messages.h b/content/common/browser_plugin/browser_plugin_messages.h
index 67ec484..2d97618 100644
--- a/content/common/browser_plugin/browser_plugin_messages.h
+++ b/content/common/browser_plugin/browser_plugin_messages.h
@@ -60,7 +60,7 @@ IPC_STRUCT_BEGIN(BrowserPluginHostMsg_ResizeGuest_Params)
   IPC_STRUCT_MEMBER(bool, repaint)
 IPC_STRUCT_END()
 
-IPC_STRUCT_BEGIN(BrowserPluginHostMsg_CreateGuest_Params)
+IPC_STRUCT_BEGIN(BrowserPluginHostMsg_Attach_Params)
   IPC_STRUCT_MEMBER(std::string, storage_partition_id)
   IPC_STRUCT_MEMBER(bool, persist_storage)
   IPC_STRUCT_MEMBER(bool, focused)
@@ -154,21 +154,14 @@ IPC_MESSAGE_ROUTED3(
     BrowserPluginHostMsg_AutoSize_Params /* auto_size_params */,
     BrowserPluginHostMsg_ResizeGuest_Params /* resize_guest_params */)
 
-
-// This message is sent to the browser process to create the browser plugin
-// embedder and helper. It is sent once prior to sending the first
-// BrowserPluginHostMsg_NavigateGuest message.
-IPC_MESSAGE_ROUTED2(BrowserPluginHostMsg_CreateGuest,
-                    int /* instance_id */,
-                    BrowserPluginHostMsg_CreateGuest_Params /* params */)
-
 // This message is sent to the browser process to indicate that a BrowserPlugin
 // has taken ownership of the lifetime of the guest of the given |instance_id|.
-// |params| is the size information of the BrowserPlugin taking ownership of
-// the guest.
+// |params| is the state of the BrowserPlugin taking ownership of
+// the guest. If a guest doesn't already exist with the given |instance_id|,
+// a new one will be created.
 IPC_MESSAGE_ROUTED2(BrowserPluginHostMsg_Attach,
                     int /* instance_id */,
-                    BrowserPluginHostMsg_CreateGuest_Params /* params */)
+                    BrowserPluginHostMsg_Attach_Params /* params */)
 
 // Tells the browser process to terminate the guest associated with the
 // browser plugin associated with the provided |instance_id|.
diff --git a/content/renderer/browser_plugin/browser_plugin.cc b/content/renderer/browser_plugin/browser_plugin.cc
index 4576d31..2a5d9e8 100644
--- a/content/renderer/browser_plugin/browser_plugin.cc
+++ b/content/renderer/browser_plugin/browser_plugin.cc
@@ -342,7 +342,7 @@ bool BrowserPlugin::ParseSrcAttribute(std::string* error_message) {
     // prevents BrowserPlugin from allocating more than one instance ID.
     // Upon receiving an instance ID from the browser process, we continue
     // the process of navigation by populating the
-    // BrowserPluginHostMsg_CreateGuest_Params with the current state of
+    // BrowserPluginHostMsg_Attach_Params with the current state of
     // BrowserPlugin and sending a BrowserPluginHostMsg_CreateGuest to the
     // browser process in order to create a new guest.
     if (before_first_navigation_) {
@@ -424,33 +424,25 @@ bool BrowserPlugin::UsesPendingDamageBuffer(
   return damage_buffer_sequence_id_ == params.damage_buffer_sequence_id;
 }
 
-void BrowserPlugin::SetInstanceID(int instance_id, bool new_guest) {
+void BrowserPlugin::Attach(int instance_id) {
   CHECK(instance_id != browser_plugin::kInstanceIDNone);
   before_first_navigation_ = false;
   instance_id_ = instance_id;
   browser_plugin_manager()->AddBrowserPlugin(instance_id, this);
 
-  BrowserPluginHostMsg_CreateGuest_Params create_guest_params;
+  BrowserPluginHostMsg_Attach_Params create_guest_params;
   create_guest_params.focused = ShouldGuestBeFocused();
   create_guest_params.visible = visible_;
   create_guest_params.name = GetNameAttribute();
-  GetDamageBufferWithSizeParams(&create_guest_params.auto_size_params,
-                                &create_guest_params.resize_guest_params);
-
-  if (!new_guest) {
-    browser_plugin_manager()->Send(
-        new BrowserPluginHostMsg_Attach(render_view_routing_id_,
-                                        instance_id_, create_guest_params));
-    return;
-  }
-
   create_guest_params.storage_partition_id = storage_partition_id_;
   create_guest_params.persist_storage = persist_storage_;
   create_guest_params.src = GetSrcAttribute();
+  GetDamageBufferWithSizeParams(&create_guest_params.auto_size_params,
+                                &create_guest_params.resize_guest_params);
+
   browser_plugin_manager()->Send(
-      new BrowserPluginHostMsg_CreateGuest(render_view_routing_id_,
-                                           instance_id_,
-                                           create_guest_params));
+      new BrowserPluginHostMsg_Attach(render_view_routing_id_,
+                                      instance_id_, create_guest_params));
 }
 
 void BrowserPlugin::DidCommitCompositorFrame() {
@@ -856,11 +848,11 @@ bool BrowserPlugin::AttachWindowTo(const WebKit::WebNode& node, int window_id) {
   // If the BrowserPlugin already has a guest attached to it then we probably
   // shouldn't allow attaching a different guest.
   // TODO(fsamuel): We may wish to support reattaching guests in the future:
-  // http://crbug.com/156219
+  // http://crbug.com/156219.
   if (browser_plugin->HasGuest())
     return false;
 
-  browser_plugin->SetInstanceID(window_id, false);
+  browser_plugin->Attach(window_id);
   return true;
 }
 
diff --git a/content/renderer/browser_plugin/browser_plugin.h b/content/renderer/browser_plugin/browser_plugin.h
index e90ce51..defe4fc 100644
--- a/content/renderer/browser_plugin/browser_plugin.h
+++ b/content/renderer/browser_plugin/browser_plugin.h
@@ -151,9 +151,12 @@ class CONTENT_EXPORT BrowserPlugin :
   // Called by browser plugin binding.
   void OnEmbedderDecidedPermission(int request_id, bool allow);
 
-  // Sets the instance ID of the BrowserPlugin and requests a guest from the
-  // browser process.
-  void SetInstanceID(int instance_id, bool new_guest);
+  // Attaches this BrowserPlugin to a guest with the provided |instance_id|.
+  // If the |instance_id| is not yet associated with a guest, a new guest
+  // will be created. If the |instance_id| has not yet been allocated or the
+  // embedder is not permitted access to that particular guest, then the
+  // embedder will be killed.
+  void Attach(int instance_id);
 
   // Notify the plugin about a compositor commit so that frame ACKs could be
   // sent, if needed.
@@ -294,10 +297,6 @@ class CONTENT_EXPORT BrowserPlugin :
   bool UsesPendingDamageBuffer(
       const BrowserPluginMsg_UpdateRect_Params& params);
 
-  // Sets the instance ID of the BrowserPlugin and requests a guest from the
-  // browser process.
-  void SetInstanceID(int instance_id);
-
   void AddPermissionRequestToMap(int request_id,
                                  BrowserPluginPermissionType type);
 
diff --git a/content/renderer/browser_plugin/browser_plugin_browsertest.cc b/content/renderer/browser_plugin/browser_plugin_browsertest.cc
index 626a8e5..7b6a373 100644
--- a/content/renderer/browser_plugin/browser_plugin_browsertest.cc
+++ b/content/renderer/browser_plugin/browser_plugin_browsertest.cc
@@ -159,15 +159,15 @@ bool BrowserPluginTest::ExecuteScriptAndReturnBool(
 // satisfy its resize request.
 TEST_F(BrowserPluginTest, InitialResize) {
   LoadHTML(GetHTMLForBrowserPluginObject().c_str());
-  // Verify that the information in CreateGuest is correct.
+  // Verify that the information in Attach is correct.
   int instance_id = 0;
   {
     const IPC::Message* msg =
         browser_plugin_manager()->sink().GetUniqueMessageMatching(
-            BrowserPluginHostMsg_CreateGuest::ID);
+            BrowserPluginHostMsg_Attach::ID);
     ASSERT_TRUE(msg);
-    BrowserPluginHostMsg_CreateGuest_Params params;
-    BrowserPluginHostMsg_CreateGuest::Read(msg, &instance_id, &params);
+    BrowserPluginHostMsg_Attach_Params params;
+    BrowserPluginHostMsg_Attach::Read(msg, &instance_id, &params);
     EXPECT_EQ(640, params.resize_guest_params.view_size.width());
     EXPECT_EQ(480, params.resize_guest_params.view_size.height());
   }
@@ -234,15 +234,15 @@ TEST_F(BrowserPluginTest, SrcAttribute) {
   // Verify that we're reporting the correct URL to navigate to based on the
   // src attribute.
   {
-    // Ensure we get a CreateGuest on the initial navigation.
+    // Ensure we get a Attach on the initial navigation.
     const IPC::Message* msg =
         browser_plugin_manager()->sink().GetUniqueMessageMatching(
-            BrowserPluginHostMsg_CreateGuest::ID);
+            BrowserPluginHostMsg_Attach::ID);
     ASSERT_TRUE(msg);
 
     int instance_id = 0;
-    BrowserPluginHostMsg_CreateGuest_Params params;
-    BrowserPluginHostMsg_CreateGuest::Read(msg, &instance_id, &params);
+    BrowserPluginHostMsg_Attach_Params params;
+    BrowserPluginHostMsg_Attach::Read(msg, &instance_id, &params);
     EXPECT_EQ("foo", params.src);
   }
 
@@ -252,10 +252,10 @@ TEST_F(BrowserPluginTest, SrcAttribute) {
   // Verify that the src attribute is updated as well.
   ExecuteJavaScript("document.getElementById('browserplugin').src = 'bar'");
   {
-    // Verify that we do not get a CreateGuest on subsequent navigations.
+    // Verify that we do not get a Attach on subsequent navigations.
     const IPC::Message* create_msg =
         browser_plugin_manager()->sink().GetUniqueMessageMatching(
-            BrowserPluginHostMsg_CreateGuest::ID);
+            BrowserPluginHostMsg_Attach::ID);
     ASSERT_FALSE(create_msg);
 
     const IPC::Message* msg =
@@ -278,14 +278,14 @@ TEST_F(BrowserPluginTest, ResizeFlowControl) {
   LoadHTML(GetHTMLForBrowserPluginObject().c_str());
   int instance_id = 0;
   {
-    // Ensure we get a CreateGuest on the initial navigation and grab the
+    // Ensure we get a Attach on the initial navigation and grab the
     // BrowserPlugin's instance_id from there.
     const IPC::Message* msg =
         browser_plugin_manager()->sink().GetUniqueMessageMatching(
-            BrowserPluginHostMsg_CreateGuest::ID);
+            BrowserPluginHostMsg_Attach::ID);
     ASSERT_TRUE(msg);
-    BrowserPluginHostMsg_CreateGuest_Params params;
-    BrowserPluginHostMsg_CreateGuest::Read(msg, &instance_id, &params);
+    BrowserPluginHostMsg_Attach_Params params;
+    BrowserPluginHostMsg_Attach::Read(msg, &instance_id, &params);
   }
   MockBrowserPlugin* browser_plugin =
       static_cast<MockBrowserPlugin*>(
@@ -373,15 +373,15 @@ TEST_F(BrowserPluginTest, ResizeFlowControl) {
 TEST_F(BrowserPluginTest, GuestCrash) {
   LoadHTML(GetHTMLForBrowserPluginObject().c_str());
 
-  // Grab the BrowserPlugin's instance ID from its CreateGuest message.
+  // Grab the BrowserPlugin's instance ID from its Attach message.
   int instance_id = 0;
   {
     const IPC::Message* msg =
         browser_plugin_manager()->sink().GetFirstMessageMatching(
-            BrowserPluginHostMsg_CreateGuest::ID);
+            BrowserPluginHostMsg_Attach::ID);
     ASSERT_TRUE(msg);
-    BrowserPluginHostMsg_CreateGuest_Params params;
-    BrowserPluginHostMsg_CreateGuest::Read(msg, &instance_id, &params);
+    BrowserPluginHostMsg_Attach_Params params;
+    BrowserPluginHostMsg_Attach::Read(msg, &instance_id, &params);
   }
   MockBrowserPlugin* browser_plugin =
       static_cast<MockBrowserPlugin*>(
@@ -482,11 +482,11 @@ TEST_F(BrowserPluginTest, CustomEvents) {
   // Grab the BrowserPlugin's instance ID from its resize message.
   const IPC::Message* msg =
       browser_plugin_manager()->sink().GetFirstMessageMatching(
-          BrowserPluginHostMsg_CreateGuest::ID);
+          BrowserPluginHostMsg_Attach::ID);
   ASSERT_TRUE(msg);
   int instance_id = 0;
-  BrowserPluginHostMsg_CreateGuest_Params params;
-  BrowserPluginHostMsg_CreateGuest::Read(msg, &instance_id, &params);
+  BrowserPluginHostMsg_Attach_Params params;
+  BrowserPluginHostMsg_Attach::Read(msg, &instance_id, &params);
 
   MockBrowserPlugin* browser_plugin =
       static_cast<MockBrowserPlugin*>(
@@ -652,12 +652,12 @@ TEST_F(BrowserPluginTest, ImmutableAttributesAfterNavigation) {
   {
     const IPC::Message* create_msg =
     browser_plugin_manager()->sink().GetUniqueMessageMatching(
-        BrowserPluginHostMsg_CreateGuest::ID);
+        BrowserPluginHostMsg_Attach::ID);
     ASSERT_TRUE(create_msg);
 
     int create_instance_id = 0;
-    BrowserPluginHostMsg_CreateGuest_Params params;
-    BrowserPluginHostMsg_CreateGuest::Read(
+    BrowserPluginHostMsg_Attach_Params params;
+    BrowserPluginHostMsg_Attach::Read(
         create_msg,
         &create_instance_id,
         &params);
@@ -703,14 +703,14 @@ TEST_F(BrowserPluginTest, RemoveEventListenerInEventListener) {
 
   LoadHTML(GetHTMLForBrowserPluginObject().c_str());
   ExecuteJavaScript(kAddEventListener);
-  // Grab the BrowserPlugin's instance ID from its CreateGuest message.
+  // Grab the BrowserPlugin's instance ID from its Attach message.
   const IPC::Message* msg =
       browser_plugin_manager()->sink().GetFirstMessageMatching(
-          BrowserPluginHostMsg_CreateGuest::ID);
+          BrowserPluginHostMsg_Attach::ID);
   ASSERT_TRUE(msg);
   int instance_id = 0;
-  BrowserPluginHostMsg_CreateGuest_Params params;
-  BrowserPluginHostMsg_CreateGuest::Read(msg, &instance_id, &params);
+  BrowserPluginHostMsg_Attach_Params params;
+  BrowserPluginHostMsg_Attach::Read(msg, &instance_id, &params);
 
   MockBrowserPlugin* browser_plugin =
       static_cast<MockBrowserPlugin*>(
@@ -760,14 +760,14 @@ TEST_F(BrowserPluginTest, MultipleEventListeners) {
 
   LoadHTML(GetHTMLForBrowserPluginObject().c_str());
   ExecuteJavaScript(kAddEventListener);
-  // Grab the BrowserPlugin's instance ID from its CreateGuest message.
+  // Grab the BrowserPlugin's instance ID from its Attach message.
   const IPC::Message* msg =
       browser_plugin_manager()->sink().GetFirstMessageMatching(
-          BrowserPluginHostMsg_CreateGuest::ID);
+          BrowserPluginHostMsg_Attach::ID);
   ASSERT_TRUE(msg);
   int instance_id = 0;
-  BrowserPluginHostMsg_CreateGuest_Params params;
-  BrowserPluginHostMsg_CreateGuest::Read(msg, &instance_id, &params);
+  BrowserPluginHostMsg_Attach_Params params;
+  BrowserPluginHostMsg_Attach::Read(msg, &instance_id, &params);
 
   MockBrowserPlugin* browser_plugin =
       static_cast<MockBrowserPlugin*>(
@@ -788,15 +788,15 @@ TEST_F(BrowserPluginTest, MultipleEventListeners) {
 TEST_F(BrowserPluginTest, RemoveBrowserPluginOnExit) {
   LoadHTML(GetHTMLForBrowserPluginObject().c_str());
 
-  // Grab the BrowserPlugin's instance ID from its CreateGuest message.
+  // Grab the BrowserPlugin's instance ID from its Attach message.
   int instance_id = 0;
   {
     const IPC::Message* msg =
         browser_plugin_manager()->sink().GetFirstMessageMatching(
-            BrowserPluginHostMsg_CreateGuest::ID);
+            BrowserPluginHostMsg_Attach::ID);
     ASSERT_TRUE(msg);
-    BrowserPluginHostMsg_CreateGuest_Params params;
-    BrowserPluginHostMsg_CreateGuest::Read(msg, &instance_id, &params);
+    BrowserPluginHostMsg_Attach_Params params;
+    BrowserPluginHostMsg_Attach::Read(msg, &instance_id, &params);
   }
 
   MockBrowserPlugin* browser_plugin =
@@ -843,18 +843,18 @@ TEST_F(BrowserPluginTest, AutoSizeAttributes) {
 
   int instance_id = 0;
   // Set some autosize parameters before navigating then navigate.
-  // Verify that the BrowserPluginHostMsg_CreateGuest message contains
+  // Verify that the BrowserPluginHostMsg_Attach message contains
   // the correct autosize parameters.
   ExecuteJavaScript(kSetAutoSizeParametersAndNavigate);
   ProcessPendingMessages();
   {
     const IPC::Message* create_msg =
     browser_plugin_manager()->sink().GetUniqueMessageMatching(
-        BrowserPluginHostMsg_CreateGuest::ID);
+        BrowserPluginHostMsg_Attach::ID);
     ASSERT_TRUE(create_msg);
 
-    BrowserPluginHostMsg_CreateGuest_Params params;
-    BrowserPluginHostMsg_CreateGuest::Read(
+    BrowserPluginHostMsg_Attach_Params params;
+    BrowserPluginHostMsg_Attach::Read(
         create_msg,
         &instance_id,
         &params);
diff --git a/content/renderer/browser_plugin/browser_plugin_manager_impl.cc b/content/renderer/browser_plugin/browser_plugin_manager_impl.cc
index c41c6ed..11b7be0 100644
--- a/content/renderer/browser_plugin/browser_plugin_manager_impl.cc
+++ b/content/renderer/browser_plugin/browser_plugin_manager_impl.cc
@@ -81,7 +81,7 @@ void BrowserPluginManagerImpl::OnAllocateInstanceIDACK(
       pending_allocate_instance_id_requests_.Lookup(request_id);
   pending_allocate_instance_id_requests_.Remove(request_id);
   if (plugin)
-    plugin->SetInstanceID(instance_id, true /* new_guest */);
+    plugin->Attach(instance_id);
 }
 
 void BrowserPluginManagerImpl::OnPluginAtPositionRequest(
diff --git a/content/renderer/browser_plugin/mock_browser_plugin_manager.cc b/content/renderer/browser_plugin/mock_browser_plugin_manager.cc
index a1d9abd..e6da3bc 100644
--- a/content/renderer/browser_plugin/mock_browser_plugin_manager.cc
+++ b/content/renderer/browser_plugin/mock_browser_plugin_manager.cc
@@ -41,7 +41,7 @@ void MockBrowserPluginManager::AllocateInstanceID(
 void MockBrowserPluginManager::AllocateInstanceIDACK(
     BrowserPlugin* browser_plugin,
     int instance_id) {
-  browser_plugin->SetInstanceID(instance_id, true);
+  browser_plugin->Attach(instance_id);
 }
 
 bool MockBrowserPluginManager::Send(IPC::Message* msg) {