BrowserPlugin: Simplify guest access

Now that it's impossible for content to create guests and it's impossible
for a compromised embedder to access a guest it does not have permission to
access from content, BrowserPluginGuestManager::MaybeGetGuestByInstanceIDOrKill
seems a bit heavy-handed. This CL simplifies the API a bit.

This is also a prerequiste to support accessibility in BrowserPlugin.

BUG=226145

Review URL: https://codereview.chromium.org/511643002

Cr-Commit-Position: refs/heads/master@{#292060}

content/browser/browser_plugin/browser_plugin_embedder.cc

@@ -132,27 +132,17 @@ void BrowserPluginEmbedder::OnUpdateDragCursor(bool* handled) {
   *handled = (guest_dragging_over_.get() != NULL);
 }
 
-void BrowserPluginEmbedder::OnGuestCallback(
-    int browser_plugin_instance_id,
-    const BrowserPluginHostMsg_Attach_Params& params,
-    WebContents* guest_web_contents) {
-  BrowserPluginGuest* guest = guest_web_contents ?
-      static_cast<WebContentsImpl*>(guest_web_contents)->
-          GetBrowserPluginGuest() : NULL;
-  if (guest)
-    guest->Attach(browser_plugin_instance_id, GetWebContents(), params);
-}
-
 void BrowserPluginEmbedder::OnAttach(
     int browser_plugin_instance_id,
     const BrowserPluginHostMsg_Attach_Params& params) {
-  GetBrowserPluginGuestManager()->MaybeGetGuestByInstanceIDOrKill(
-      web_contents(),
-      browser_plugin_instance_id,
-      base::Bind(&BrowserPluginEmbedder::OnGuestCallback,
-                 base::Unretained(this),
-                 browser_plugin_instance_id,
-                 params));
+  WebContents* guest_web_contents =
+      GetBrowserPluginGuestManager()->GetGuestByInstanceID(
+          GetWebContents(), browser_plugin_instance_id);
+  if (!guest_web_contents)
+    return;
+  BrowserPluginGuest* guest = static_cast<WebContentsImpl*>(guest_web_contents)
+                                  ->GetBrowserPluginGuest();
+  guest->Attach(browser_plugin_instance_id, GetWebContents(), params);
 }
 
 bool BrowserPluginEmbedder::HandleKeyboardEvent(

content/browser/browser_plugin/browser_plugin_embedder.h

@@ -83,14 +83,7 @@ class CONTENT_EXPORT BrowserPluginEmbedder : public WebContentsObserver {
 
   bool UnlockMouseIfNecessaryCallback(bool* mouse_unlocked, WebContents* guest);
 
-  // Called by the content embedder when a guest exists with the provided
-  // |instance_id|.
-  void OnGuestCallback(int instance_id,
-                       const BrowserPluginHostMsg_Attach_Params& params,
-                       WebContents* guest_web_contents);
-
   // Message handlers.
-
   void OnAttach(int instance_id,
                 const BrowserPluginHostMsg_Attach_Params& params);
   void OnPluginAtPositionResponse(int instance_id,

content/browser/browser_plugin/browser_plugin_message_filter.cc

@@ -59,14 +59,6 @@ void BrowserPluginMessageFilter::OverrideThreadForMessage(
     *thread = BrowserThread::UI;
 }
 
-static void BrowserPluginGuestMessageCallback(const IPC::Message& message,
-                                              WebContents* guest_web_contents) {
-  if (!guest_web_contents)
-    return;
-  static_cast<WebContentsImpl*>(guest_web_contents)->GetBrowserPluginGuest()->
-      OnMessageReceivedFromEmbedder(message);
-}
-
 void BrowserPluginMessageFilter::ForwardMessageToGuest(
     const IPC::Message& message) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -82,12 +74,17 @@ void BrowserPluginMessageFilter::ForwardMessageToGuest(
   PickleIterator iter(message);
   bool success = iter.ReadInt(&browser_plugin_instance_id);
   DCHECK(success);
-  embedder_web_contents->GetBrowserContext()->GetGuestManager()->
-      MaybeGetGuestByInstanceIDOrKill(
-          embedder_web_contents,
-          browser_plugin_instance_id,
-          base::Bind(&BrowserPluginGuestMessageCallback,
-                     message));
+  WebContents* guest_web_contents =
+      embedder_web_contents->GetBrowserContext()
+          ->GetGuestManager()
+          ->GetGuestByInstanceID(embedder_web_contents,
+                                 browser_plugin_instance_id);
+  if (!guest_web_contents)
+    return;
+
+  static_cast<WebContentsImpl*>(guest_web_contents)
+      ->GetBrowserPluginGuest()
+      ->OnMessageReceivedFromEmbedder(message);
 }
 
 void BrowserPluginMessageFilter::OnSwapBuffersACK(

content/public/browser/browser_plugin_guest_manager.cc

@@ -8,6 +8,12 @@
 
 namespace content {
 
+WebContents* BrowserPluginGuestManager::GetGuestByInstanceID(
+    WebContents* embedder_web_contents,
+    int browser_plugin_instance_id) {
+  return NULL;
+}
+
 bool BrowserPluginGuestManager::ForEachGuest(
     WebContents* embedder_web_contents,
     const GuestCallback& callback) {

content/public/browser/browser_plugin_guest_manager.h

@@ -29,15 +29,10 @@ class CONTENT_EXPORT BrowserPluginGuestManager {
 
   typedef base::Callback<void(WebContents*)> GuestByInstanceIDCallback;
   // Requests a guest WebContents associated with the provided
-  // |guest_instance_id|. If a guest associated with the provided ID
-  // does not exist, then the |callback| will be called with a NULL
-  // WebContents. If the provided |embedder_render_process_id| does
-  // not own the requested guest, then the embedder will be killed,
-  // and the |callback| will not be called.
-  virtual void MaybeGetGuestByInstanceIDOrKill(
-      WebContents* embedder_web_contents,
-      int browser_plugin_instance_id,
-      const GuestByInstanceIDCallback& callback) {}
+  // |browser_plugin_instance_id|.
+  // Returns the guest associated with the provided ID if one exists.
+  virtual WebContents* GetGuestByInstanceID(WebContents* embedder_web_contents,
+                                            int browser_plugin_instance_id);
 
   // Iterates over all WebContents belonging to a given |embedder_web_contents|,
   // calling |callback| for each. If one of the callbacks returns true, then

extensions/browser/guest_view/guest_view_manager.cc

@@ -148,27 +148,18 @@ content::WebContents* GuestViewManager::CreateGuestWithWebContentsParams(
   return guest_web_contents;
 }
 
-void GuestViewManager::MaybeGetGuestByInstanceIDOrKill(
+content::WebContents* GuestViewManager::GetGuestByInstanceID(
     content::WebContents* embedder_web_contents,
-    int element_instance_id,
-    const GuestByInstanceIDCallback& callback) {
-  int guest_instance_id = GetGuestInstanceIDForPluginID(embedder_web_contents,
-                                                        element_instance_id);
+    int element_instance_id) {
+  int guest_instance_id = GetGuestInstanceIDForElementID(embedder_web_contents,
+                                                         element_instance_id);
   if (guest_instance_id == guestview::kInstanceIDNone)
-    return;
-  int embedder_render_process_id =
-      embedder_web_contents->GetRenderProcessHost()->GetID();
-  if (!CanEmbedderAccessInstanceIDMaybeKill(embedder_render_process_id,
-                                            guest_instance_id)) {
-    // If we kill the embedder, then don't bother calling back.
-    return;
-  }
-  content::WebContents* guest_web_contents =
-      GetGuestByInstanceID(guest_instance_id);
-  callback.Run(guest_web_contents);
+    return NULL;
+
+  return GetGuestByInstanceID(guest_instance_id);
 }
 
-int GuestViewManager::GetGuestInstanceIDForPluginID(
+int GuestViewManager::GetGuestInstanceIDForElementID(
     content::WebContents* embedder_web_contents,
     int element_instance_id) {
   GuestInstanceIDMap::iterator iter = instance_id_map_.find(

extensions/browser/guest_view/guest_view_manager.h

@@ -57,11 +57,10 @@ class GuestViewManager : public content::BrowserPluginGuestManager,
                    const base::DictionaryValue& attach_params);
 
   int GetNextInstanceID();
-  int GetGuestInstanceIDForPluginID(
+  int GetGuestInstanceIDForElementID(
       content::WebContents* embedder_web_contents,
       int element_instance_id);
 
-
   typedef base::Callback<void(content::WebContents*)>
       WebContentsCreatedCallback;
   void CreateGuest(const std::string& view_type,
@@ -80,10 +79,9 @@ class GuestViewManager : public content::BrowserPluginGuestManager,
       const GURL& guest_site);
 
   // BrowserPluginGuestManager implementation.
-  virtual void MaybeGetGuestByInstanceIDOrKill(
+  virtual content::WebContents* GetGuestByInstanceID(
       content::WebContents* embedder_web_contents,
-      int element_instance_id,
-      const GuestByInstanceIDCallback& callback) OVERRIDE;
+      int element_instance_id) OVERRIDE;
   virtual bool ForEachGuest(content::WebContents* embedder_web_contents,
                             const GuestCallback& callback) OVERRIDE;
  protected: