<webview>: Allocate the view instance ID from the WebView shim

This CL allocates a creates unique instance id for each
<webview> in an embedder RenderView/shim instead of in BrowserPlugin. In a subsequent CL, this will permit
usage of the WebRequest API prior to attachment to the DOM.

This CL also fixes a potential crash on edit commands if the instance ID passed did not exist: Perhaps open two <webview> apps and then close the first one, then send edit commands to the second one.

BUG=178663
Test=WebViewTest.*, WebViewInteractiveTest.*
TBR=cdn@chromium.org for deleted field in browser_plugin_messages.h

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

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

chrome/browser/chrome_content_browser_client.cc

@@ -831,7 +831,7 @@ void ChromeContentBrowserClient::GuestWebContentsCreated(
     return;
   }
   std::string api_type;
-  extra_params->GetString(guestview::kAttributeApi, &api_type);
+  extra_params->GetString(guestview::kParameterApi, &api_type);
 
   if (api_type == "adview") {
     *guest_delegate  = new AdViewGuest(guest_web_contents);
@@ -845,7 +845,6 @@ void ChromeContentBrowserClient::GuestWebContentsCreated(
 void ChromeContentBrowserClient::GuestWebContentsAttached(
     WebContents* guest_web_contents,
     WebContents* embedder_web_contents,
-    int browser_plugin_instance_id,
     const base::DictionaryValue& extra_params) {
   Profile* profile = Profile::FromBrowserContext(
       embedder_web_contents->GetBrowserContext());
@@ -873,7 +872,6 @@ void ChromeContentBrowserClient::GuestWebContentsAttached(
   }
   guest->Attach(embedder_web_contents,
                 extension->id(),
-                browser_plugin_instance_id,
                 extra_params);
 }
 

chrome/browser/chrome_content_browser_client.h

@@ -67,7 +67,6 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
   virtual void GuestWebContentsAttached(
       content::WebContents* guest_web_contents,
       content::WebContents* embedder_web_contents,
-      int browser_plugin_instance_id,
       const base::DictionaryValue& extra_params) OVERRIDE;
   virtual void RenderProcessHostCreated(
       content::RenderProcessHost* host) OVERRIDE;

chrome/browser/extensions/extension_renderer_state.h

@@ -21,7 +21,6 @@ class ExtensionRendererState {
   struct WebViewInfo {
     int embedder_process_id;
     int embedder_routing_id;
-    int guest_instance_id;
     int instance_id;
   };
 

chrome/browser/guestview/guestview.cc

@@ -67,13 +67,12 @@ GuestView* GuestView::From(int embedder_process_id, int guest_instance_id) {
 
 void GuestView::Attach(content::WebContents* embedder_web_contents,
                        const std::string& extension_id,
-                       int view_instance_id,
                        const base::DictionaryValue& args) {
   embedder_web_contents_ = embedder_web_contents;
   embedder_render_process_id_ =
       embedder_web_contents->GetRenderProcessHost()->GetID();
   extension_id_ = extension_id;
-  view_instance_id_ = view_instance_id;
+  args.GetInteger(guestview::kParameterInstanceId, &view_instance_id_);
 
   std::pair<int, int> key(embedder_render_process_id_, guest_instance_id_);
   embedder_guestview_map.Get().insert(std::make_pair(key, this));

chrome/browser/guestview/guestview.h

@@ -48,7 +48,6 @@ class GuestView : public content::BrowserPluginGuestDelegate {
 
   virtual void Attach(content::WebContents* embedder_web_contents,
                       const std::string& extension_id,
-                      int view_instance_id,
                       const base::DictionaryValue& args);
 
   content::WebContents* embedder_web_contents() const {

chrome/browser/guestview/guestview_constants.cc

@@ -11,8 +11,9 @@ const char kIsTopLevel[] = "isTopLevel";
 const char kReason[] = "reason";
 const char kUrl[] = "url";
 
-// Attributes.
-const char kAttributeApi[] = "api";
+// Initialization parameters.
+const char kParameterApi[] = "api";
+const char kParameterInstanceId[] = "instanceId";
 
 // Other.
 const int kInstanceIDNone = 0;

chrome/browser/guestview/guestview_constants.h

@@ -14,8 +14,9 @@ extern const char kIsTopLevel[];
 extern const char kReason[];
 extern const char kUrl[];
 
-// Attributes.
-extern const char kAttributeApi[];
+// Initialization parameters.
+extern const char kParameterApi[];
+extern const char kParameterInstanceId[];
 
 // Other.
 extern const int kInstanceIDNone;

chrome/browser/guestview/webview/webview_guest.cc

@@ -80,10 +80,9 @@ WebViewGuest* WebViewGuest::From(int embedder_process_id,
 
 void WebViewGuest::Attach(WebContents* embedder_web_contents,
                           const std::string& extension_id,
-                          int view_instance_id,
                           const base::DictionaryValue& args) {
   GuestView::Attach(
-      embedder_web_contents, extension_id, view_instance_id, args);
+      embedder_web_contents, extension_id, args);
 
   AddWebViewToExtensionRendererState();
 }
@@ -317,7 +316,6 @@ void WebViewGuest::AddWebViewToExtensionRendererState() {
   ExtensionRendererState::WebViewInfo webview_info;
   webview_info.embedder_process_id = embedder_render_process_id();
   webview_info.embedder_routing_id = embedder_web_contents()->GetRoutingID();
-  webview_info.guest_instance_id = guest_instance_id();
   webview_info.instance_id = view_instance_id();
 
   content::BrowserThread::PostTask(

chrome/browser/guestview/webview/webview_guest.h

@@ -33,7 +33,6 @@ class WebViewGuest : public GuestView,
   // GuestView implementation.
   virtual void Attach(content::WebContents* embedder_web_contents,
                       const std::string& extension_id,
-                      int view_instance_id,
                       const base::DictionaryValue& args) OVERRIDE;
   virtual GuestView::Type GetViewType() const OVERRIDE;
   virtual WebViewGuest* AsWebView() OVERRIDE;

chrome/renderer/resources/extensions/web_view.js

@@ -18,6 +18,8 @@ var WEB_VIEW_EVENTS = {
   'sizechanged': ['oldHeight', 'oldWidth', 'newHeight', 'newWidth'],
 };
 
+var webViewInstanceIdCounter = 0;
+
 var createEvent = function(name) {
   var eventOpts = {supportsListeners: true, supportsFilters: true};
   return new eventBindings.Event(name, undefined, eventOpts);
@@ -343,11 +345,13 @@ WebView.prototype.handleBrowserPluginAttributeMutation_ = function(mutation) {
  */
 WebView.prototype.setupWebviewNodeEvents_ = function() {
   var self = this;
+  this.viewInstanceId_ = ++webViewInstanceIdCounter;
   var onInstanceIdAllocated = function(e) {
     var detail = e.detail ? JSON.parse(e.detail) : {};
     self.instanceId_ = detail.windowId;
     var params = {
-      'api': 'webview'
+      'api': 'webview',
+      'instanceId': self.viewInstanceId_
     };
     self.browserPluginNode_['-internal-attach'](params);
 

chrome/renderer/resources/extensions/web_view_experimental.js

@@ -38,7 +38,7 @@ WebView.prototype.setupWebRequestEvents_ = function() {
                 'webview.' + webRequestEvent.name,
                 webRequestEvent.parameters,
                 webRequestEvent.extraParameters, null,
-                self.browserPluginNode_.getInstanceId());
+                self.viewInstanceId_);
       }
       return self[webRequestEvent.name + '_'];
     }

content/browser/browser_plugin/browser_plugin_embedder.cc

@@ -187,7 +187,6 @@ void BrowserPluginEmbedder::OnAttach(
     GetContentClient()->browser()->GuestWebContentsAttached(
         guest->GetWebContents(),
         web_contents(),
-        params.browser_plugin_instance_id,
         extra_params);
     guest->Attach(static_cast<WebContentsImpl*>(web_contents()), params);
     return;
@@ -202,7 +201,6 @@ void BrowserPluginEmbedder::OnAttach(
     GetContentClient()->browser()->GuestWebContentsAttached(
         guest->GetWebContents(),
         web_contents(),
-        params.browser_plugin_instance_id,
         extra_params);
     guest->Initialize(static_cast<WebContentsImpl*>(web_contents()), params);
   }

content/common/browser_plugin/browser_plugin_messages.h

@@ -64,7 +64,6 @@ IPC_STRUCT_BEGIN(BrowserPluginHostMsg_ResizeGuest_Params)
 IPC_STRUCT_END()
 
 IPC_STRUCT_BEGIN(BrowserPluginHostMsg_Attach_Params)
-  IPC_STRUCT_MEMBER(int, browser_plugin_instance_id)
   IPC_STRUCT_MEMBER(std::string, storage_partition_id)
   IPC_STRUCT_MEMBER(bool, persist_storage)
   IPC_STRUCT_MEMBER(bool, focused)

content/public/browser/content_browser_client.h

@@ -163,7 +163,6 @@ class CONTENT_EXPORT ContentBrowserClient {
   virtual void GuestWebContentsAttached(
       WebContents* guest_web_contents,
       WebContents* embedder_web_contents,
-      int browser_plugin_instance_id,
       const base::DictionaryValue& extra_params) {}
 
   // Notifies that a RenderProcessHost has been created. This is called before

content/renderer/browser_plugin/browser_plugin.cc

@@ -91,10 +91,8 @@ static base::LazyInstance<PluginContainerMap> g_plugin_container_map =
 BrowserPlugin::BrowserPlugin(
     RenderViewImpl* render_view,
     WebKit::WebFrame* frame,
-    const WebPluginParams& params,
-    int instance_id)
+    const WebPluginParams& params)
     : guest_instance_id_(browser_plugin::kInstanceIDNone),
-      instance_id_(instance_id),
       render_view_(render_view->AsWeakPtr()),
       render_view_routing_id_(render_view->GetRoutingID()),
       container_(NULL),
@@ -397,7 +395,6 @@ void BrowserPlugin::OnInstanceIDAllocated(int guest_instance_id) {
 
 void BrowserPlugin::Attach(scoped_ptr<base::DictionaryValue> extra_params) {
   BrowserPluginHostMsg_Attach_Params attach_params;
-  attach_params.browser_plugin_instance_id = instance_id_;
   attach_params.focused = ShouldGuestBeFocused();
   attach_params.visible = visible_;
   attach_params.name = GetNameAttribute();

content/renderer/browser_plugin/browser_plugin.h

@@ -40,7 +40,6 @@ class CONTENT_EXPORT BrowserPlugin :
   RenderViewImpl* render_view() const { return render_view_.get(); }
   int render_view_routing_id() const { return render_view_routing_id_; }
   int guest_instance_id() const { return guest_instance_id_; }
-  int instance_id() const { return instance_id_; }
 
   static BrowserPlugin* FromContainer(WebKit::WebPluginContainer* container);
 
@@ -219,8 +218,7 @@ class CONTENT_EXPORT BrowserPlugin :
   BrowserPlugin(
       RenderViewImpl* render_view,
       WebKit::WebFrame* frame,
-      const WebKit::WebPluginParams& params,
-      int instance_id);
+      const WebKit::WebPluginParams& params);
 
   virtual ~BrowserPlugin();
 
@@ -336,9 +334,6 @@ class CONTENT_EXPORT BrowserPlugin :
   // This is the browser-process-allocated instance ID that uniquely identifies
   // a guest WebContents.
   int guest_instance_id_;
-  // This is a render-process-allocated instance ID that uniquely identifies a
-  // BrowserPlugin.
-  int instance_id_;
   base::WeakPtr<RenderViewImpl> render_view_;
   // We cache the |render_view_|'s routing ID because we need it on destruction.
   // If the |render_view_| is destroyed before the BrowserPlugin is destroyed

content/renderer/browser_plugin/browser_plugin_bindings.cc

@@ -284,26 +284,6 @@ class BrowserPluginBindingAttachWindowTo : public BrowserPluginMethodBinding {
   DISALLOW_COPY_AND_ASSIGN(BrowserPluginBindingAttachWindowTo);
 };
 
-// Note: This is a method that is used internally by the <webview> shim only.
-// This should not be exposed to developers.
-class BrowserPluginBindingGetInstanceID : public BrowserPluginMethodBinding {
- public:
-  BrowserPluginBindingGetInstanceID()
-      : BrowserPluginMethodBinding(browser_plugin::kMethodGetInstanceId, 0) {
-  }
-
-  virtual bool Invoke(BrowserPluginBindings* bindings,
-                      const NPVariant* args,
-                      NPVariant* result) OVERRIDE {
-    int instance_id = bindings->instance()->instance_id();
-    INT32_TO_NPVARIANT(instance_id, *result);
-    return true;
-  }
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(BrowserPluginBindingGetInstanceID);
-};
-
 // Note: This is a method that is used internally by the <webview> shim only.
 // This should not be exposed to developers.
 class BrowserPluginBindingGetGuestInstanceID :
@@ -731,7 +711,6 @@ BrowserPluginBindings::BrowserPluginBindings(BrowserPlugin* instance)
 
   method_bindings_.push_back(new BrowserPluginBindingAttach);
   method_bindings_.push_back(new BrowserPluginBindingAttachWindowTo);
-  method_bindings_.push_back(new BrowserPluginBindingGetInstanceID);
   method_bindings_.push_back(new BrowserPluginBindingGetGuestInstanceID);
   method_bindings_.push_back(new BrowserPluginBindingSetPermission);
   method_bindings_.push_back(new BrowserPluginBindingTrackObjectLifetime);

content/renderer/browser_plugin/browser_plugin_browsertest.cc

@@ -192,7 +192,7 @@ TEST_F(BrowserPluginTest, InitialResize) {
   EXPECT_EQ(480, params.resize_guest_params.view_rect.height());
   ASSERT_TRUE(browser_plugin);
   // Now the browser plugin is expecting a UpdateRect resize.
-  int instance_id = browser_plugin->instance_id();
+  int instance_id = browser_plugin->guest_instance_id();
   EXPECT_TRUE(browser_plugin->pending_damage_buffer_.get());
 
   // Send the BrowserPlugin an UpdateRect equal to its container size with
@@ -289,7 +289,7 @@ TEST_F(BrowserPluginTest, ResizeFlowControl) {
   LoadHTML(GetHTMLForBrowserPluginObject().c_str());
   MockBrowserPlugin* browser_plugin = GetCurrentPlugin();
   ASSERT_TRUE(browser_plugin);
-  int instance_id = browser_plugin->instance_id();
+  int instance_id = browser_plugin->guest_instance_id();
   EXPECT_TRUE(browser_plugin->pending_damage_buffer_.get());
   // Send an UpdateRect to the BrowserPlugin to make it use the pending damage
   // buffer.

content/renderer/browser_plugin/browser_plugin_manager_impl.cc

@@ -16,7 +16,7 @@ namespace content {
 BrowserPluginManagerImpl::BrowserPluginManagerImpl(
     RenderViewImpl* render_view)
     : BrowserPluginManager(render_view),
-      browser_plugin_instance_id_counter_(0) {
+      request_id_counter_(0) {
 }
 
 BrowserPluginManagerImpl::~BrowserPluginManagerImpl() {
@@ -26,17 +26,16 @@ BrowserPlugin* BrowserPluginManagerImpl::CreateBrowserPlugin(
     RenderViewImpl* render_view,
     WebKit::WebFrame* frame,
     const WebKit::WebPluginParams& params) {
-  return new BrowserPlugin(render_view, frame, params,
-      ++browser_plugin_instance_id_counter_);
+  return new BrowserPlugin(render_view, frame, params);
 }
 
 void BrowserPluginManagerImpl::AllocateInstanceID(
     BrowserPlugin* browser_plugin) {
-  int instance_id = browser_plugin->instance_id();
+  int request_id = ++request_id_counter_;
   pending_allocate_guest_instance_id_requests_.AddWithID(browser_plugin,
-                                                         instance_id);
+                                                         request_id);
   Send(new BrowserPluginHostMsg_AllocateInstanceID(
-      browser_plugin->render_view_routing_id(), instance_id));
+      browser_plugin->render_view_routing_id(), request_id));
 }
 
 bool BrowserPluginManagerImpl::Send(IPC::Message* msg) {
@@ -78,15 +77,13 @@ void BrowserPluginManagerImpl::DidCommitCompositorFrame() {
 
 void BrowserPluginManagerImpl::OnAllocateInstanceIDACK(
     const IPC::Message& message,
-    int browser_plugin_instance_id,
+    int request_id,
     int guest_instance_id) {
   BrowserPlugin* plugin =
-    pending_allocate_guest_instance_id_requests_.Lookup(
-          browser_plugin_instance_id);
+    pending_allocate_guest_instance_id_requests_.Lookup(request_id);
   if (!plugin)
     return;
-  pending_allocate_guest_instance_id_requests_.Remove(
-      browser_plugin_instance_id);
+  pending_allocate_guest_instance_id_requests_.Remove(request_id);
   plugin->OnInstanceIDAllocated(guest_instance_id);
 }
 

content/renderer/browser_plugin/browser_plugin_manager_impl.h

@@ -45,7 +45,7 @@ class BrowserPluginManagerImpl : public BrowserPluginManager {
                                  int request_id,
                                  const gfx::Point& position);
 
-  int browser_plugin_instance_id_counter_;
+  int request_id_counter_;
   IDMap<BrowserPlugin> pending_allocate_guest_instance_id_requests_;
 
   DISALLOW_COPY_AND_ASSIGN(BrowserPluginManagerImpl);

content/renderer/browser_plugin/mock_browser_plugin.cc

@@ -9,9 +9,8 @@ namespace content {
 
 MockBrowserPlugin::MockBrowserPlugin(RenderViewImpl* render_view,
                                      WebKit::WebFrame* frame,
-                                     const WebKit::WebPluginParams& params,
-                                     int instance_id)
-    : BrowserPlugin(render_view, frame, params, instance_id) {
+                                     const WebKit::WebPluginParams& params)
+    : BrowserPlugin(render_view, frame, params) {
 }
 
 MockBrowserPlugin::~MockBrowserPlugin() {}

content/renderer/browser_plugin/mock_browser_plugin.h

@@ -13,8 +13,7 @@ class MockBrowserPlugin : public BrowserPlugin {
  public:
   MockBrowserPlugin(RenderViewImpl* render_view,
                     WebKit::WebFrame* frame,
-                    const WebKit::WebPluginParams& params,
-                    int instance_id);
+                    const WebKit::WebPluginParams& params);
 
   virtual ~MockBrowserPlugin();
 

content/renderer/browser_plugin/mock_browser_plugin_manager.cc

@@ -14,7 +14,6 @@ namespace content {
 MockBrowserPluginManager::MockBrowserPluginManager(
     RenderViewImpl* render_view)
     : BrowserPluginManager(render_view),
-      browser_plugin_instance_id_counter_(0),
       guest_instance_id_counter_(0) {
 }
 
@@ -25,8 +24,7 @@ BrowserPlugin* MockBrowserPluginManager::CreateBrowserPlugin(
     RenderViewImpl* render_view,
     WebKit::WebFrame* frame,
     const WebKit::WebPluginParams& params) {
-  int instance_id = ++browser_plugin_instance_id_counter_;
-  return new MockBrowserPlugin(render_view, frame, params, instance_id);
+  return new MockBrowserPlugin(render_view, frame, params);
 }
 
 void MockBrowserPluginManager::AllocateInstanceID(

content/renderer/browser_plugin/mock_browser_plugin_manager.h

@@ -40,7 +40,6 @@ class MockBrowserPluginManager : public BrowserPluginManager {
   // The last known good deserializer for sync messages.
   scoped_ptr<IPC::MessageReplyDeserializer> reply_deserializer_;
 
-  int browser_plugin_instance_id_counter_;
   int guest_instance_id_counter_;
 
   DISALLOW_COPY_AND_ASSIGN(MockBrowserPluginManager);