Browser Plugin: New Implementation (Browser Side)

content/browser/web_contents/web_contents_impl.h

Index: content/browser/web_contents/web_contents_impl.h
diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h
index 654ec64056872131c49dc9162a68c8c2562040b6..2483d44304fd9ae1b6de0935a7d030fa523600c9 100644
--- a/content/browser/web_contents/web_contents_impl.h
+++ b/content/browser/web_contents/web_contents_impl.h
@@ -38,6 +38,8 @@ class WebContentsImpl;
 struct ViewMsg_PostMessage_Params;
 
 namespace content {
+class BrowserPluginEmbedder;
+class BrowserPluginGuest;
 class ColorChooser;
 class DownloadItem;
 class JavaScriptDialogCreator;
@@ -161,6 +163,15 @@ class CONTENT_EXPORT WebContentsImpl
   // Expose the render manager for testing.
   RenderViewHostManager* GetRenderManagerForTesting();
 
+  // Sets guest to this WebContents embedder.

[awong, 2012/09/06 00:23:27]

I expect a SetXXX() function to take an object that gets put into a member
variable and to be symmetrical with the GetXXX functions.

But this function seems to be creating the guest.  Is it more like
AssignBrowserPluingGuest?

The comment also doesn't quite explain the functionality. BTW, is it okay to
call this function twice?

[lazyboy, 2012/09/06 04:33:53]

Renamed the function.
There should be no side effect calling it > 1. We don't expect it to be called
more than once for a given web_contents though.

[awong, 2012/09/06 19:55:26]

Ah, I see.  Though looking at this more, this seems like it should be a
construction. A WebContents never switches between being a guest, and being
{normal,embedder}.

What about making a WebContents::CreateGuest() constructor?  You wouldn't even
need the base_web_contents and could move the messy custom SiteInstance creation
logic into there so you just take the host's GURL.

[lazyboy, 2012/09/07 19:33:19]

Embedder and guest can be seen as roles A WC can play, it can have none, any or
both role in theory.
A guest can be an embedder that way, although for first cut we will disable
recursive embedding.
+1, So to be clear, we would have:
gwc = WebContentsImpl* WebContentsImpl::CreateGuest(
    BrowserContext*, string& url, int guest_instance_id);
remove function WebContentsImpl::Assign/SetBrowserPluginGuest()
work with gwc->GetBrowserPluginGuest() in embedder.
?
I can do the change.

[awong, 2012/09/07 20:51:03]

Yep. I think constructing the right state from the get go makes a class of
possible problems impossible. :)

[lazyboy, 2012/09/08 02:12:22]

Done.

+  content::BrowserPluginGuest* SetBrowserPluginGuest(int instance_id);
+  // Returns guest browser plugin object, or NULL if this WebContents is not a
+  // guest.
+  content::BrowserPluginGuest* GetBrowserPluginGuest();
+  // Returns embedder browser plugin object, or NULL if this WebContents is not
+  // an embedder.
+  content::BrowserPluginEmbedder* GetBrowserPluginEmbedder();
+
   // content::WebContents ------------------------------------------------------
   virtual content::WebContentsDelegate* GetDelegate() OVERRIDE;
   virtual void SetDelegate(content::WebContentsDelegate* delegate) OVERRIDE;
@@ -542,6 +553,10 @@ class CONTENT_EXPORT WebContentsImpl
   void OnRequestPpapiBrokerPermission(int request_id,
                                       const GURL& url,
                                       const FilePath& plugin_path);
+  void OnBrowserPluginNavigateGuest(int instance_id,
+                                    int64 frame_id,
+                                    std::string src,

[awong, 2012/09/06 00:23:27]

Use a const std::string&?

[lazyboy, 2012/09/06 04:33:53]

Done.

+                                    gfx::Size size);
 
   // Changes the IsLoading state and notifies delegate as needed
   // |details| is used to provide details on the load that just finished
@@ -647,6 +662,10 @@ class CONTENT_EXPORT WebContentsImpl
                                     std::string* embedder_channel_name,
                                     int* embedder_container_id);
 
+  // Removes browser plugin embedder if there is one and its associated
+  // RenderViewHost has been swapped out.
+  void RemoveSwappedOutBrowserPluginEmbedder();
+
   // Data for core operation ---------------------------------------------------
 
   // Delegate for notifying our owner about stuff. Not owned by us.
@@ -694,7 +713,7 @@ class CONTENT_EXPORT WebContentsImpl
       java_bridge_dispatcher_host_manager_;
 
   // TODO(fsamuel): Remove this once upstreaming of the new browser plugin
-  // implmentation is complete.
+  // implementation is complete.
   // Manages the browser plugin instances hosted by this WebContents.
   scoped_ptr<content::old::BrowserPluginHost> old_browser_plugin_host_;
 
@@ -813,6 +832,13 @@ class CONTENT_EXPORT WebContentsImpl
   // Color chooser that was opened by this tab.
   content::ColorChooser* color_chooser_;
 
+  // Manages the embedder state for browser plugins, if this WebContents is an
+  // embedder; NULL otherwise.
+  scoped_ptr<content::BrowserPluginEmbedder> browser_plugin_embedder_;
+  // Manages the guest state for browser plugin, if this WebContents is a guest;
+  // NULL otherwise.
+  scoped_ptr<content::BrowserPluginGuest> browser_plugin_guest_;
+
   // This must be at the end, or else we might get notifications and use other
   // member variables that are gone.
   content::NotificationRegistrar registrar_;