Refactor GuestViewContainer::AttachRequest to allow for other request types

In a subsequent CL, DetachGuest will be introduced, to allow a GuestView to
detach itself from its container. This CL performs the necessary refactor to allow
for new operations to be easily introduced.

This CL also makes BrowserPlugin IDs unique per process. This allows addressing attachment, and soon detachment, across JavaScript contexts.

BUG=434226
TBR=lazyboy@chromium.org

Committed: https://crrev.com/d397750f3e2e53f414a4798c4e5039837a8c00ea
Cr-Commit-Position: refs/heads/master@{#306025}

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

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

6 files changed, 143 insertions, 127 deletions

diff --git a/content/renderer/browser_plugin/browser_plugin_manager.cc b/content/renderer/browser_plugin/browser_plugin_manager.cc
index 16cc98e..7676aef 100644
--- a/content/renderer/browser_plugin/browser_plugin_manager.cc
+++ b/content/renderer/browser_plugin/browser_plugin_manager.cc
@@ -23,7 +23,6 @@ BrowserPluginManager* BrowserPluginManager::Create(
 
 BrowserPluginManager::BrowserPluginManager(RenderViewImpl* render_view)
     : RenderViewObserver(render_view),
-      current_instance_id_(browser_plugin::kInstanceIDNone),
       render_view_(render_view->AsWeakPtr()) {
 }
 
@@ -46,7 +45,7 @@ BrowserPlugin* BrowserPluginManager::GetBrowserPlugin(
 }
 
 int BrowserPluginManager::GetNextInstanceID() {
-  return ++current_instance_id_;
+  return RenderThread::Get()->GenerateRoutingID();
 }
 
 void BrowserPluginManager::UpdateDeviceScaleFactor() {
diff --git a/content/renderer/browser_plugin/browser_plugin_manager.h b/content/renderer/browser_plugin/browser_plugin_manager.h
index 23cde18..cedaa5f0 100644
--- a/content/renderer/browser_plugin/browser_plugin_manager.h
+++ b/content/renderer/browser_plugin/browser_plugin_manager.h
@@ -51,6 +51,9 @@ class CONTENT_EXPORT BrowserPluginManager
   void UpdateDeviceScaleFactor();
   void UpdateFocusState();
   RenderViewImpl* render_view() const { return render_view_.get(); }
+
+  // Returns a new instance ID to be used by BrowserPlugin. Instance IDs are
+  // unique per process.
   int GetNextInstanceID();
 
   // RenderViewObserver override. Call on render thread.
@@ -75,7 +78,6 @@ class CONTENT_EXPORT BrowserPluginManager
 
   // This map is keyed by guest instance IDs.
   IDMap<BrowserPlugin> instances_;
-  int current_instance_id_;
   base::WeakPtr<RenderViewImpl> render_view_;
 
   DISALLOW_COPY_AND_ASSIGN(BrowserPluginManager);
diff --git a/extensions/renderer/guest_view/extensions_guest_view_container.cc b/extensions/renderer/guest_view/extensions_guest_view_container.cc
index 3790d56..82204ec 100644
--- a/extensions/renderer/guest_view/extensions_guest_view_container.cc
+++ b/extensions/renderer/guest_view/extensions_guest_view_container.cc
@@ -13,8 +13,7 @@
 #include "third_party/WebKit/public/web/WebView.h"
 
 namespace {
-typedef std::pair<int, int> GuestViewID;
-typedef std::map<GuestViewID, extensions::ExtensionsGuestViewContainer*>
+typedef std::map<int, extensions::ExtensionsGuestViewContainer*>
     ExtensionsGuestViewContainerMap;
 static base::LazyInstance<ExtensionsGuestViewContainerMap>
     g_guest_view_container_map = LAZY_INSTANCE_INITIALIZER;
@@ -22,31 +21,89 @@ static base::LazyInstance<ExtensionsGuestViewContainerMap>
 
 namespace extensions {
 
-ExtensionsGuestViewContainer::AttachRequest::AttachRequest(
-    int element_instance_id,
-    int guest_instance_id,
-    scoped_ptr<base::DictionaryValue> params,
+ExtensionsGuestViewContainer::Request::Request(
+    GuestViewContainer* container,
     v8::Handle<v8::Function> callback,
     v8::Isolate* isolate)
-    : element_instance_id_(element_instance_id),
-      guest_instance_id_(guest_instance_id),
-      params_(params.Pass()),
+    : container_(container),
       callback_(callback),
       isolate_(isolate) {
 }
 
-ExtensionsGuestViewContainer::AttachRequest::~AttachRequest() {
+ExtensionsGuestViewContainer::Request::~Request() {
 }
 
-bool ExtensionsGuestViewContainer::AttachRequest::HasCallback() const {
+bool ExtensionsGuestViewContainer::Request::HasCallback() const {
   return !callback_.IsEmpty();
 }
 
 v8::Handle<v8::Function>
-ExtensionsGuestViewContainer::AttachRequest::GetCallback() const {
+ExtensionsGuestViewContainer::Request::GetCallback() const {
   return callback_.NewHandle(isolate_);
 }
 
+ExtensionsGuestViewContainer::AttachRequest::AttachRequest(
+    GuestViewContainer* container,
+    int guest_instance_id,
+    scoped_ptr<base::DictionaryValue> params,
+    v8::Handle<v8::Function> callback,
+    v8::Isolate* isolate)
+    : Request(container, callback, isolate),
+      guest_instance_id_(guest_instance_id),
+      params_(params.Pass()) {
+}
+
+ExtensionsGuestViewContainer::AttachRequest::~AttachRequest() {
+}
+
+void ExtensionsGuestViewContainer::AttachRequest::PerformRequest() {
+  // Step 1, send the attach params to extensions/.
+  container()->render_frame()->Send(
+      new ExtensionHostMsg_AttachGuest(container()->render_view_routing_id(),
+                                       container()->element_instance_id(),
+                                       guest_instance_id_,
+                                       *params_));
+
+  // Step 2, attach plugin through content/.
+  container()->render_frame()->AttachGuest(container()->element_instance_id());
+}
+
+void ExtensionsGuestViewContainer::AttachRequest::HandleResponse(
+    const IPC::Message& message) {
+  ExtensionMsg_GuestAttached::Param param;
+  if (!ExtensionMsg_GuestAttached::Read(&message, &param))
+    return;
+
+  // If we don't have a callback then there's nothing more to do.
+  if (!HasCallback())
+    return;
+
+  content::RenderView* guest_proxy_render_view =
+      content::RenderView::FromRoutingID(param.b);
+  // TODO(fsamuel): Should we be reporting an error to JavaScript or DCHECKing?
+  if (!guest_proxy_render_view)
+    return;
+
+  v8::HandleScope handle_scope(isolate());
+  v8::Handle<v8::Function> callback = GetCallback();
+  v8::Handle<v8::Context> context = callback->CreationContext();
+  if (context.IsEmpty())
+    return;
+
+  blink::WebFrame* frame = guest_proxy_render_view->GetWebView()->mainFrame();
+  v8::Local<v8::Value> window = frame->mainWorldScriptContext()->Global();
+
+  const int argc = 1;
+  v8::Handle<v8::Value> argv[argc] = { window };
+
+  v8::Context::Scope context_scope(context);
+  blink::WebScopedMicrotaskSuppression suppression;
+
+  // Call the AttachGuest API's callback with the guest proxy as the first
+  // parameter.
+  callback->Call(context->Global(), argc, argv);
+}
+
 ExtensionsGuestViewContainer::ExtensionsGuestViewContainer(
       content::RenderFrame* render_frame)
     : GuestViewContainer(render_frame),
@@ -55,127 +112,79 @@ ExtensionsGuestViewContainer::ExtensionsGuestViewContainer(
 
 ExtensionsGuestViewContainer::~ExtensionsGuestViewContainer() {
   if (element_instance_id() != guestview::kInstanceIDNone) {
-    g_guest_view_container_map.Get().erase(
-        GuestViewID(render_view_routing_id(), element_instance_id()));
+    g_guest_view_container_map.Get().erase(element_instance_id());
   }
 }
 
 ExtensionsGuestViewContainer* ExtensionsGuestViewContainer::FromID(
-    int render_view_routing_id,
     int element_instance_id) {
   ExtensionsGuestViewContainerMap* guest_view_containers =
       g_guest_view_container_map.Pointer();
-  ExtensionsGuestViewContainerMap::iterator it = guest_view_containers->find(
-      GuestViewID(render_view_routing_id, element_instance_id));
+  ExtensionsGuestViewContainerMap::iterator it =
+      guest_view_containers->find(element_instance_id);
   return it == guest_view_containers->end() ? NULL : it->second;
 }
 
-void ExtensionsGuestViewContainer::AttachGuest(
-    linked_ptr<AttachRequest> request) {
-  EnqueueAttachRequest(request);
-  PerformPendingAttachRequest();
+void ExtensionsGuestViewContainer::IssueRequest(linked_ptr<Request> request) {
+  EnqueueRequest(request);
+  PerformPendingRequest();
 }
 
 void ExtensionsGuestViewContainer::SetElementInstanceID(
     int element_instance_id) {
   GuestViewContainer::SetElementInstanceID(element_instance_id);
 
-  GuestViewID guest_view_id(render_view_routing_id(), element_instance_id);
-  DCHECK(g_guest_view_container_map.Get().find(guest_view_id) ==
+  DCHECK(g_guest_view_container_map.Get().find(element_instance_id) ==
             g_guest_view_container_map.Get().end());
-  g_guest_view_container_map.Get().insert(std::make_pair(guest_view_id, this));
+  g_guest_view_container_map.Get().insert(
+      std::make_pair(element_instance_id, this));
 }
 
 void ExtensionsGuestViewContainer::Ready() {
   ready_ = true;
   CHECK(!pending_response_.get());
-  PerformPendingAttachRequest();
+  PerformPendingRequest();
 }
 
 bool ExtensionsGuestViewContainer::HandlesMessage(const IPC::Message& message) {
-  return message.type() == ExtensionMsg_GuestAttached::ID;
+  return (message.type() == ExtensionMsg_GuestAttached::ID);
 }
 
 bool ExtensionsGuestViewContainer::OnMessage(const IPC::Message& message) {
-  bool handled = false;
-  IPC_BEGIN_MESSAGE_MAP(ExtensionsGuestViewContainer, message)
-    IPC_MESSAGE_HANDLER(ExtensionMsg_GuestAttached, OnGuestAttached)
-    IPC_MESSAGE_UNHANDLED(handled = false)
-  IPC_END_MESSAGE_MAP()
-  return handled;
+  if (message.type() == ExtensionMsg_GuestAttached::ID) {
+    OnHandleCallback(message);
+    return true;
+  }
+  return false;
 }
 
-void ExtensionsGuestViewContainer::OnGuestAttached(int /* unused */,
-                                                   int guest_proxy_routing_id) {
+void ExtensionsGuestViewContainer::OnHandleCallback(
+    const IPC::Message& message) {
   // Handle the callback for the current request with a pending response.
-  HandlePendingResponseCallback(guest_proxy_routing_id);
+  HandlePendingResponseCallback(message);
   // Perform the subsequent attach request if one exists.
-  PerformPendingAttachRequest();
-}
-
-void ExtensionsGuestViewContainer::AttachGuestInternal(
-    linked_ptr<AttachRequest> request) {
-  CHECK(!pending_response_.get());
-  // Step 1, send the attach params to chrome/.
-  render_frame()->Send(
-      new ExtensionHostMsg_AttachGuest(render_view_routing_id(),
-                                       request->element_instance_id(),
-                                       request->guest_instance_id(),
-                                       *request->attach_params()));
-
-  // Step 2, attach plugin through content/.
-  render_frame()->AttachGuest(request->element_instance_id());
-
-  pending_response_ = request;
+  PerformPendingRequest();
 }
 
-void ExtensionsGuestViewContainer::EnqueueAttachRequest(
-    linked_ptr<AttachRequest> request) {
+void ExtensionsGuestViewContainer::EnqueueRequest(linked_ptr<Request> request) {
   pending_requests_.push_back(request);
 }
 
-void ExtensionsGuestViewContainer::PerformPendingAttachRequest() {
+void ExtensionsGuestViewContainer::PerformPendingRequest() {
   if (!ready_ || pending_requests_.empty() || pending_response_.get())
     return;
 
-  linked_ptr<AttachRequest> pending_request = pending_requests_.front();
+  linked_ptr<Request> pending_request = pending_requests_.front();
   pending_requests_.pop_front();
-  AttachGuestInternal(pending_request);
+  pending_request->PerformRequest();
+  pending_response_ = pending_request;
 }
 
 void ExtensionsGuestViewContainer::HandlePendingResponseCallback(
-    int guest_proxy_routing_id) {
+    const IPC::Message& message) {
   CHECK(pending_response_.get());
-  linked_ptr<AttachRequest> pending_response(pending_response_.release());
-
-  // If we don't have a callback then there's nothing more to do.
-  if (!pending_response->HasCallback())
-    return;
-
-  content::RenderView* guest_proxy_render_view =
-      content::RenderView::FromRoutingID(guest_proxy_routing_id);
-  // TODO(fsamuel): Should we be reporting an error to JavaScript or DCHECKing?
-  if (!guest_proxy_render_view)
-    return;
-
-  v8::HandleScope handle_scope(pending_response->isolate());
-  v8::Handle<v8::Function> callback = pending_response->GetCallback();
-  v8::Handle<v8::Context> context = callback->CreationContext();
-  if (context.IsEmpty())
-    return;
-
-  blink::WebFrame* frame = guest_proxy_render_view->GetWebView()->mainFrame();
-  v8::Local<v8::Value> window = frame->mainWorldScriptContext()->Global();
-
-  const int argc = 1;
-  v8::Handle<v8::Value> argv[argc] = { window };
-
-  v8::Context::Scope context_scope(context);
-  blink::WebScopedMicrotaskSuppression suppression;
-
-  // Call the AttachGuest API's callback with the guest proxy as the first
-  // parameter.
-  callback->Call(context->Global(), argc, argv);
+  linked_ptr<Request> pending_response(pending_response_.release());
+  pending_response->HandleResponse(message);
 }
 
 }  // namespace extensions
diff --git a/extensions/renderer/guest_view/extensions_guest_view_container.h b/extensions/renderer/guest_view/extensions_guest_view_container.h
index 9b1424d..0efec26 100644
--- a/extensions/renderer/guest_view/extensions_guest_view_container.h
+++ b/extensions/renderer/guest_view/extensions_guest_view_container.h
@@ -17,48 +17,58 @@ namespace extensions {
 
 class ExtensionsGuestViewContainer : public GuestViewContainer {
  public:
+
+  class Request {
+   public:
+    Request(GuestViewContainer* container,
+            v8::Handle<v8::Function> callback,
+            v8::Isolate* isolate);
+    virtual ~Request();
+
+    virtual void PerformRequest() = 0;
+    virtual void HandleResponse(const IPC::Message& message) = 0;
+
+    GuestViewContainer* container() const { return container_; }
+
+    bool HasCallback() const;
+
+    v8::Handle<v8::Function> GetCallback() const;
+
+    v8::Isolate* isolate() const { return isolate_; }
+
+   private:
+    GuestViewContainer* container_;
+    ScopedPersistent<v8::Function> callback_;
+    v8::Isolate* const isolate_;
+  };
+
   // This class represents an AttachGuest request from Javascript. It includes
   // the input parameters and the callback function. The Attach operation may
   // not execute immediately, if the container is not ready or if there are
   // other attach operations in flight.
-  class AttachRequest {
+  class AttachRequest : public Request {
    public:
-    AttachRequest(int element_instance_id,
+    AttachRequest(GuestViewContainer* container,
                   int guest_instance_id,
                   scoped_ptr<base::DictionaryValue> params,
                   v8::Handle<v8::Function> callback,
                   v8::Isolate* isolate);
-    ~AttachRequest();
-
-    int element_instance_id() const { return element_instance_id_; }
+    ~AttachRequest() override;
 
-    int guest_instance_id() const { return guest_instance_id_; }
-
-    base::DictionaryValue* attach_params() const {
-      return params_.get();
-    }
-
-    bool HasCallback() const;
-
-    v8::Handle<v8::Function> GetCallback() const;
-
-    v8::Isolate* isolate() const { return isolate_; }
+    void PerformRequest() override;
+    void HandleResponse(const IPC::Message& message) override;
 
    private:
-    const int element_instance_id_;
     const int guest_instance_id_;
     scoped_ptr<base::DictionaryValue> params_;
-    ScopedPersistent<v8::Function> callback_;
-    v8::Isolate* const isolate_;
   };
 
   explicit ExtensionsGuestViewContainer(content::RenderFrame* render_frame);
   ~ExtensionsGuestViewContainer() override;
 
-  static ExtensionsGuestViewContainer* FromID(int render_view_routing_id,
-                                              int element_instance_id);
+  static ExtensionsGuestViewContainer* FromID(int element_instance_id);
 
-  void AttachGuest(linked_ptr<AttachRequest> request);
+  void IssueRequest(linked_ptr<Request> request);
 
   // BrowserPluginDelegate implementation.
   void SetElementInstanceID(int element_instance_id) override;
@@ -69,18 +79,16 @@ class ExtensionsGuestViewContainer : public GuestViewContainer {
   bool OnMessage(const IPC::Message& message) override;
 
  private:
-  void OnGuestAttached(int element_instance_id,
-                       int guest_proxy_routing_id);
+  void OnHandleCallback(const IPC::Message& message);
 
-  void AttachGuestInternal(linked_ptr<AttachRequest> request);
-  void EnqueueAttachRequest(linked_ptr<AttachRequest> request);
-  void PerformPendingAttachRequest();
-  void HandlePendingResponseCallback(int guest_proxy_routing_id);
+  void EnqueueRequest(linked_ptr<Request> request);
+  void PerformPendingRequest();
+  void HandlePendingResponseCallback(const IPC::Message& message);
 
   bool ready_;
 
-  std::deque<linked_ptr<AttachRequest> > pending_requests_;
-  linked_ptr<AttachRequest> pending_response_;
+  std::deque<linked_ptr<Request> > pending_requests_;
+  linked_ptr<Request> pending_response_;
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionsGuestViewContainer);
 };
diff --git a/extensions/renderer/guest_view/guest_view_container.h b/extensions/renderer/guest_view/guest_view_container.h
index 96fd637..965dfd6 100644
--- a/extensions/renderer/guest_view/guest_view_container.h
+++ b/extensions/renderer/guest_view/guest_view_container.h
@@ -27,7 +27,6 @@ class GuestViewContainer : public content::BrowserPluginDelegate,
   virtual bool HandlesMessage(const IPC::Message& message) = 0;
   virtual bool OnMessage(const IPC::Message& message) = 0;
 
- protected:
   int element_instance_id() const { return element_instance_id_; }
   int render_view_routing_id() const { return render_view_routing_id_; }
 
diff --git a/extensions/renderer/guest_view/guest_view_internal_custom_bindings.cc b/extensions/renderer/guest_view/guest_view_internal_custom_bindings.cc
index 69f70e8..86ba4ed 100644
--- a/extensions/renderer/guest_view/guest_view_internal_custom_bindings.cc
+++ b/extensions/renderer/guest_view/guest_view_internal_custom_bindings.cc
@@ -44,8 +44,7 @@ void GuestViewInternalCustomBindings::AttachGuest(
   // An element instance ID uniquely identifies a ExtensionsGuestViewContainer
   // within a RenderView.
   ExtensionsGuestViewContainer* guest_view_container =
-      ExtensionsGuestViewContainer::FromID(
-          context()->GetRenderView()->GetRoutingID(), element_instance_id);
+      ExtensionsGuestViewContainer::FromID(element_instance_id);
 
   // TODO(fsamuel): Should we be reporting an error if the element instance ID
   // is invalid?
@@ -64,15 +63,15 @@ void GuestViewInternalCustomBindings::AttachGuest(
         static_cast<base::DictionaryValue*>(params_as_value.release()));
   }
 
-  linked_ptr<ExtensionsGuestViewContainer::AttachRequest> request(
+  linked_ptr<ExtensionsGuestViewContainer::Request> request(
       new ExtensionsGuestViewContainer::AttachRequest(
-          element_instance_id,
+          guest_view_container,
           guest_instance_id,
           params.Pass(),
           args.Length() == 4 ? args[3].As<v8::Function>() :
               v8::Handle<v8::Function>(),
           args.GetIsolate()));
-  guest_view_container->AttachGuest(request);
+  guest_view_container->IssueRequest(request);
 
   args.GetReturnValue().Set(v8::Boolean::New(context()->isolate(), true));
 }