[Extensions] Begin making Extension port initialization asynchronous

extensions/renderer/messaging_bindings.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "extensions/renderer/messaging_bindings.h"
 
 #include <stdint.h>
 
 #include <map>
 #include <string>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
+#include "base/lazy_instance.h"
 #include "base/message_loop/message_loop.h"
 #include "base/values.h"
 #include "content/public/child/v8_value_converter.h"
 #include "content/public/common/child_process_host.h"
 #include "content/public/renderer/render_frame.h"
 #include "content/public/renderer/render_thread.h"
 #include "extensions/common/api/messaging/message.h"
 #include "extensions/common/extension_messages.h"
 #include "extensions/common/manifest_handlers/externally_connectable.h"
 #include "extensions/renderer/extension_frame_helper.h"
+#include "extensions/renderer/extension_port.h"
 #include "extensions/renderer/gc_callback.h"
 #include "extensions/renderer/script_context.h"
 #include "extensions/renderer/script_context_set.h"
 #include "extensions/renderer/v8_helpers.h"
 #include "third_party/WebKit/public/web/WebDocument.h"
 #include "third_party/WebKit/public/web/WebLocalFrame.h"
 #include "third_party/WebKit/public/web/WebScopedUserGesture.h"
 #include "third_party/WebKit/public/web/WebScopedWindowFocusAllowedIndicator.h"
 #include "third_party/WebKit/public/web/WebUserGestureIndicator.h"
 #include "v8/include/v8.h"
 
 // Message passing API example (in a content script):
 // var extension =

[lazyboy, 2016/09/08 00:05:40]

(unrelated), should we remove |extension| from this example?

[Devlin, 2016/09/08 19:15:31]

Yep, I have no idea why that was there.  Removed.

 //    new chrome.Extension('00123456789abcdef0123456789abcdef0123456');
 // var port = runtime.connect();
 // port.postMessage('Can you hear me now?');
 // port.onmessage.addListener(function(msg, port) {
 //   alert('response=' + msg);
 //   port.postMessage('I got your reponse');
 // });
 
 namespace extensions {
 
 using v8_helpers::ToV8String;
 using v8_helpers::IsEmptyOrUndefied;
 
 namespace {
 
-void HasMessagePort(int port_id,
+// A simple wrapper to run a given closure when the object goes out of scope,
+// unless reset.
+class RunOnDestruction {

[lazyboy, 2016/09/08 00:05:40]

Can we use ScopedClosureRunner instead?

[Devlin, 2016/09/08 19:15:30]

That exists?  I should have known there are no more original ideas for handy
utilities in chromium. ;)  Done.  I slightly miss the presence of a Reset() that
doesn't warn about the release, but ah well.

+ public:
+  RunOnDestruction(const base::Closure& closure) : closure_(closure) {}
+  ~RunOnDestruction() {
+    if (!closure_.is_null())
+      closure_.Run();
+  }
+
+  void Reset() { closure_.Reset(); }
+
+ private:
+  base::Closure closure_;
+
+  DISALLOW_COPY_AND_ASSIGN(RunOnDestruction);
+};
+
+// A global map between ScriptContext and MessagingBindings.
+base::LazyInstance<std::map<ScriptContext*, MessagingBindings*>>
+    g_messaging_map = LAZY_INSTANCE_INITIALIZER;
+
+void HasMessagePort(int global_port_id,
                     bool* has_port,
                     ScriptContext* script_context) {
   if (*has_port)
     return;  // Stop checking if the port was found.
 
-  v8::Isolate* isolate = script_context->isolate();
-  v8::HandleScope handle_scope(isolate);
-
-  v8::Local<v8::Value> port_id_handle = v8::Integer::New(isolate, port_id);
-  v8::Local<v8::Value> v8_has_port =
-      script_context->module_system()->CallModuleMethod("messaging", "hasPort",
-                                                        1, &port_id_handle);
-  if (IsEmptyOrUndefied(v8_has_port))
-    return;
-  CHECK(v8_has_port->IsBoolean());
-  if (!v8_has_port.As<v8::Boolean>()->Value())
-    return;
-  *has_port = true;
+  MessagingBindings* bindings = g_messaging_map.Get()[script_context];
+  DCHECK(bindings);
+  if (bindings->GetPortWithGlobalId(global_port_id))
+    *has_port = true;
 }
 
 void DispatchOnConnectToScriptContext(
-    int target_port_id,
+    int global_target_port_id,
     const std::string& channel_name,
     const ExtensionMsg_TabConnectionInfo* source,
     const ExtensionMsg_ExternalConnectionInfo& info,
     const std::string& tls_channel_id,
     bool* port_created,

[lazyboy, 2016/09/08 00:05:40]

See comment in related to HasMessagePort, I think we should be able to simplify
this too.

[Devlin, 2016/09/08 19:15:31]

See reply in HasMessagePort() comment. :)

     ScriptContext* script_context) {
+  MessagingBindings* bindings = g_messaging_map.Get()[script_context];
+  DCHECK(bindings);
+
+  int opposite_port_id = global_target_port_id ^ 1;
+  if (bindings->GetPortWithGlobalId(opposite_port_id))
+    return;  // The channel was opened by this same context; ignore it.
+
+  ExtensionPort* port =
+      bindings->CreateNewPortWithGlobalId(global_target_port_id);
+  int local_port_id = port->local_id();
+  // Remove the port
+  RunOnDestruction remove_port(
+      base::Bind(&MessagingBindings::RemovePortWithLocalId,
+                 bindings->GetWeakPtr(), local_port_id));
+
   v8::Isolate* isolate = script_context->isolate();
   v8::HandleScope handle_scope(isolate);
 
-  std::unique_ptr<content::V8ValueConverter> converter(
-      content::V8ValueConverter::create());
 
   const std::string& source_url_spec = info.source_url.spec();
   std::string target_extension_id = script_context->GetExtensionID();
   const Extension* extension = script_context->extension();
 
   v8::Local<v8::Value> tab = v8::Null(isolate);
   v8::Local<v8::Value> tls_channel_id_value = v8::Undefined(isolate);
   v8::Local<v8::Value> guest_process_id = v8::Undefined(isolate);
   v8::Local<v8::Value> guest_render_frame_routing_id = v8::Undefined(isolate);
 
   if (extension) {
-    if (!source->tab.empty() && !extension->is_platform_app())
+    if (!source->tab.empty() && !extension->is_platform_app()) {
+      std::unique_ptr<content::V8ValueConverter> converter(
+          content::V8ValueConverter::create());
       tab = converter->ToV8Value(&source->tab, script_context->v8_context());
+    }
 
     ExternallyConnectableInfo* externally_connectable =
         ExternallyConnectableInfo::Get(extension);
     if (externally_connectable &&
         externally_connectable->accepts_tls_channel_id) {
       v8::Local<v8::String> v8_tls_channel_id;
       if (ToV8String(isolate, tls_channel_id.c_str(), &v8_tls_channel_id))
         tls_channel_id_value = v8_tls_channel_id;
     }
 
@@ skipping 12 matching lines @@
       !ToV8String(isolate, info.source_id.c_str(), &v8_source_id) ||
       !ToV8String(isolate, target_extension_id.c_str(),
                   &v8_target_extension_id) ||
       !ToV8String(isolate, source_url_spec.c_str(), &v8_source_url_spec)) {
     NOTREACHED() << "dispatchOnConnect() passed non-string argument";
     return;
   }
 
   v8::Local<v8::Value> arguments[] = {
       // portId
-      v8::Integer::New(isolate, target_port_id),
+      v8::Integer::New(isolate, local_port_id),
       // channelName
       v8_channel_name,
       // sourceTab
       tab,
       // source_frame_id
       v8::Integer::New(isolate, source->frame_id),
       // guestProcessId
       guest_process_id,
       // guestRenderFrameRoutingId
       guest_render_frame_routing_id,
       // sourceExtensionId
       v8_source_id,
       // targetExtensionId
       v8_target_extension_id,
       // sourceUrl
       v8_source_url_spec,
       // tlsChannelId
       tls_channel_id_value,
   };
 
   v8::Local<v8::Value> retval =
       script_context->module_system()->CallModuleMethod(
           "messaging", "dispatchOnConnect", arraysize(arguments), arguments);
 
   if (!IsEmptyOrUndefied(retval)) {
     CHECK(retval->IsBoolean());
-    *port_created |= retval.As<v8::Boolean>()->Value();
+    bool used = retval.As<v8::Boolean>()->Value();
+    *port_created |= used;
+    if (used)
+      remove_port.Reset();  // Port was used; don't remove it.
   } else {
     LOG(ERROR) << "Empty return value from dispatchOnConnect.";
   }
 }
 
 void DeliverMessageToScriptContext(const Message& message,
-                                   int target_port_id,
+                                   int global_target_port_id,
                                    ScriptContext* script_context) {
+  MessagingBindings* bindings = g_messaging_map.Get()[script_context];
+  DCHECK(bindings);
+  ExtensionPort* port = bindings->GetPortWithGlobalId(global_target_port_id);
+  if (!port)
+    return;
+
   v8::Isolate* isolate = script_context->isolate();
   v8::HandleScope handle_scope(isolate);
 
-  // Check to see whether the context has this port before bothering to create
-  // the message.
   v8::Local<v8::Value> port_id_handle =
-      v8::Integer::New(isolate, target_port_id);
-  v8::Local<v8::Value> has_port =
-      script_context->module_system()->CallModuleMethod("messaging", "hasPort",
-                                                        1, &port_id_handle);
-  // Could be empty/undefined if an exception was thrown.
-  // TODO(kalman): Should this be built into CallModuleMethod?
-  if (IsEmptyOrUndefied(has_port))
-    return;
-  CHECK(has_port->IsBoolean());
-  if (!has_port.As<v8::Boolean>()->Value())
-    return;
+      v8::Integer::New(isolate, port->local_id());
 
   v8::Local<v8::String> v8_data;
   if (!ToV8String(isolate, message.data.c_str(), &v8_data))
     return;
   std::vector<v8::Local<v8::Value>> arguments;
   arguments.push_back(v8_data);
   arguments.push_back(port_id_handle);
 
   std::unique_ptr<blink::WebScopedUserGesture> web_user_gesture;
   std::unique_ptr<blink::WebScopedWindowFocusAllowedIndicator>
       allow_window_focus;
   if (message.user_gesture) {
     web_user_gesture.reset(new blink::WebScopedUserGesture);
 
     if (script_context->web_frame()) {
       blink::WebDocument document = script_context->web_frame()->document();
       allow_window_focus.reset(new blink::WebScopedWindowFocusAllowedIndicator(
           &document));
     }
   }
 
   script_context->module_system()->CallModuleMethod(
       "messaging", "dispatchOnMessage", &arguments);
 }
 
-void DispatchOnDisconnectToScriptContext(int port_id,
+void DispatchOnDisconnectToScriptContext(int global_port_id,
                                          const std::string& error_message,
                                          ScriptContext* script_context) {
+  MessagingBindings* bindings = g_messaging_map.Get()[script_context];
+  DCHECK(bindings);
+  ExtensionPort* port = bindings->GetPortWithGlobalId(global_port_id);
+  if (!port)
+    return;
+
   v8::Isolate* isolate = script_context->isolate();
   v8::HandleScope handle_scope(isolate);
 
   std::vector<v8::Local<v8::Value>> arguments;
-  arguments.push_back(v8::Integer::New(isolate, port_id));
+  arguments.push_back(v8::Integer::New(isolate, port->local_id()));
   v8::Local<v8::String> v8_error_message;
   if (!error_message.empty())
     ToV8String(isolate, error_message.c_str(), &v8_error_message);
   if (!v8_error_message.IsEmpty()) {
     arguments.push_back(v8_error_message);
   } else {
     arguments.push_back(v8::Null(isolate));
   }
 
   script_context->module_system()->CallModuleMethod(
       "messaging", "dispatchOnDisconnect", &arguments);
 }
 
 }  // namespace
 
 MessagingBindings::MessagingBindings(ScriptContext* context)
     : ObjectBackedNativeHandler(context), weak_ptr_factory_(this) {
+  g_messaging_map.Get()[context] = this;
   RouteFunction("CloseChannel", base::Bind(&MessagingBindings::CloseChannel,
                                            base::Unretained(this)));
   RouteFunction("PostMessage", base::Bind(&MessagingBindings::PostMessage,
                                           base::Unretained(this)));
   // TODO(fsamuel, kalman): Move BindToGC out of messaging natives.
   RouteFunction("BindToGC", base::Bind(&MessagingBindings::BindToGC,
                                        base::Unretained(this)));
+  RouteFunction("OpenChannelToExtension", "runtime.connect",
+                base::Bind(&MessagingBindings::OpenChannelToExtension,
+                           base::Unretained(this)));
+  RouteFunction("OpenChannelToNativeApp", "runtime.connectNative",
+                base::Bind(&MessagingBindings::OpenChannelToNativeApp,
+                           base::Unretained(this)));
+  RouteFunction(
+      "OpenChannelToTab",
+      base::Bind(&MessagingBindings::OpenChannelToTab, base::Unretained(this)));
 }
 
-MessagingBindings::~MessagingBindings() {}
+MessagingBindings::~MessagingBindings() {
+  g_messaging_map.Get().erase(context());
+}
 
 // static
 void MessagingBindings::ValidateMessagePort(
     const ScriptContextSet& context_set,
     int port_id,
     content::RenderFrame* render_frame) {
   int routing_id = render_frame->GetRoutingID();
 
   bool has_port = false;
   context_set.ForEach(render_frame,

[lazyboy, 2016/09/08 00:05:40]

This and HasMessagePort() can be simplified now, right?

[Devlin, 2016/09/08 19:15:30]

How?  I suppose we could just loop over the map of MessagingBindings, but then
we have to reimplement the restrict_to_render_frame logic from
ScriptContextSet::ForEach() (or check in every binding).  I'm not sure either
one of those is strictly better or simpler.

[lazyboy, 2016/09/08 21:59:16]

I see, I was envisioning a simple "bool HasMessagePort()".
ScritpContextSet::ForEach feels heavy given it tries to mitigate against
ScriptContext-s going away if we call JavaScript functions within its callbacks.
Please ignore this comment as hand rolling a loop with manual check might open a
way for future bugs...

                       base::Bind(&HasMessagePort, port_id, &has_port));
   // Note: HasMessagePort invokes a JavaScript function. If the runtime of the

[lazyboy, 2016/09/08 00:05:40]

This isn't true anymore.

[Devlin, 2016/09/08 19:15:31]

Done.

   // extension bindings in JS have been compromised, then |render_frame| may be
   // invalid at this point.
 
   // A reply is only sent if the port is missing, because the port is assumed to
   // exist unless stated otherwise.
   if (!has_port) {
     content::RenderThread::Get()->Send(
         new ExtensionHostMsg_CloseMessagePort(routing_id, port_id, false));
   }
 }
@@ skipping 41 matching lines @@
 void MessagingBindings::DispatchOnDisconnect(
     const ScriptContextSet& context_set,
     int port_id,
     const std::string& error_message,
     content::RenderFrame* restrict_to_render_frame) {
   context_set.ForEach(
       restrict_to_render_frame,
       base::Bind(&DispatchOnDisconnectToScriptContext, port_id, error_message));
 }
 
+ExtensionPort* MessagingBindings::GetPortWithGlobalId(int id) {
+  for (const auto& key_value : ports_) {
+    if (key_value.second->global_id() == id)
+      return key_value.second.get();
+  }
+  return nullptr;
+}
+
+ExtensionPort* MessagingBindings::CreateNewPortWithGlobalId(int global_id) {
+  int local_id = next_local_id_++;
+  ExtensionPort* port =
+      ports_
+          .insert(std::make_pair(
+              local_id, base::MakeUnique<ExtensionPort>(context(), local_id)))
+          .first->second.get();
+  port->SetGlobalId(global_id);
+  return port;
+}
+
+void MessagingBindings::RemovePortWithLocalId(int local_id) {
+  ports_.erase(local_id);
+}
+
+base::WeakPtr<MessagingBindings> MessagingBindings::GetWeakPtr() {
+  return weak_ptr_factory_.GetWeakPtr();
+}
+
 void MessagingBindings::PostMessage(
     const v8::FunctionCallbackInfo<v8::Value>& args) {
   // Arguments are (int32_t port_id, string message).
   CHECK(args.Length() == 2 && args[0]->IsInt32() && args[1]->IsString());
 
   int port_id = args[0].As<v8::Int32>()->Value();
-
-  content::RenderFrame* render_frame = context()->GetRenderFrame();
-  if (render_frame) {
-    render_frame->Send(new ExtensionHostMsg_PostMessage(
-        render_frame->GetRoutingID(), port_id,
-        Message(*v8::String::Utf8Value(args[1]),
-                blink::WebUserGestureIndicator::isProcessingUserGesture())));
+  auto iter = ports_.find(port_id);
+  if (iter != ports_.end()) {
+    iter->second->PostExtensionMessage(base::MakeUnique<Message>(
+        *v8::String::Utf8Value(args[1]),
+        blink::WebUserGestureIndicator::isProcessingUserGesture()));
   }
 }
 
 void MessagingBindings::CloseChannel(
     const v8::FunctionCallbackInfo<v8::Value>& args) {
   // Arguments are (int32_t port_id, bool force_close).
   CHECK_EQ(2, args.Length());
   CHECK(args[0]->IsInt32());
   CHECK(args[1]->IsBoolean());
 
   int port_id = args[0].As<v8::Int32>()->Value();
   bool force_close = args[1].As<v8::Boolean>()->Value();
   ClosePort(port_id, force_close);
 }
 
 void MessagingBindings::BindToGC(
     const v8::FunctionCallbackInfo<v8::Value>& args) {
   CHECK(args.Length() == 3 && args[0]->IsObject() && args[1]->IsFunction() &&
         args[2]->IsInt32());
   int port_id = args[2].As<v8::Int32>()->Value();

[lazyboy, 2016/09/08 00:05:40]

local_port_id or local_id?

[Devlin, 2016/09/08 19:15:30]

Done.

   base::Closure fallback = base::Bind(&base::DoNothing);
   if (port_id >= 0) {
     // TODO(robwu): Falling back to closing the port shouldn't be needed. If
     // the script context is destroyed, then the frame has navigated. But that
     // is already detected by the browser, so this logic is redundant. Remove
     // this fallback (and move BindToGC out of messaging because it is also
     // used in other places that have nothing to do with messaging...).
     fallback = base::Bind(&MessagingBindings::ClosePort,
                           weak_ptr_factory_.GetWeakPtr(), port_id,
                           false /* force_close */);
   }
   // Destroys itself when the object is GC'd or context is invalidated.
   new GCCallback(context(), args[0].As<v8::Object>(),
                  args[1].As<v8::Function>(), fallback);
 }
 
+void MessagingBindings::OpenChannelToExtension(
+    const v8::FunctionCallbackInfo<v8::Value>& args) {
+  content::RenderFrame* render_frame = context()->GetRenderFrame();
+  if (!render_frame)
+    return;
+
+  // The Javascript code should validate/fill the arguments.
+  CHECK_EQ(args.Length(), 3);
+  CHECK(args[0]->IsString());
+  CHECK(args[1]->IsString());
+  CHECK(args[2]->IsBoolean());
+
+  int local_id = next_local_id_++;

[lazyboy, 2016/09/08 00:05:40]

I'd wrap this in GetNextLocalId() { return next_local_id_++ } since this is
called from many places.

[Devlin, 2016/09/08 19:15:31]

Done.

+  ports_[local_id] = base::MakeUnique<ExtensionPort>(context(), local_id);
+
+  ExtensionMsg_ExternalConnectionInfo info;
+  // For messaging APIs, hosted apps should be considered a web page so hide
+  // its extension ID.
+  const Extension* extension = context()->extension();
+  if (extension && !extension->is_hosted_app())
+    info.source_id = extension->id();
+
+  info.target_id = *v8::String::Utf8Value(args[0]);
+  info.source_url = context()->url();
+  std::string channel_name = *v8::String::Utf8Value(args[1]);
+  bool include_tls_channel_id =
+      args.Length() > 2 ? args[2]->BooleanValue() : false;
+
+  ExtensionFrameHelper* frame_helper = ExtensionFrameHelper::Get(render_frame);
+  DCHECK(frame_helper);
+  frame_helper->RequestPortId(
+      info, channel_name, include_tls_channel_id,

[lazyboy, 2016/09/08 00:05:40]

generic: Why is |include_tls_channel_id| not part of |info|? 

[Devlin, 2016/09/08 19:15:30]

I really don't know.  Added a TODO.

+      base::Bind(&MessagingBindings::SetGlobalPortId,
+                 weak_ptr_factory_.GetWeakPtr(), local_id));
+
+  args.GetReturnValue().Set(static_cast<int32_t>(local_id));

[lazyboy, 2016/09/08 00:05:40]

Fingers crossed, hope no one is relying on the old global behavior.

[Devlin, 2016/09/08 19:15:30]

Port ids are an implementation detail, exposed only the pseudo-private PortImpl
JS object (public documentation [1] doesn't include it).  And it seems like all
our tests pass.  If anyone else was relying on somehow snagging the private
field, then I think we're okay breaking them.  Of course, there's the
possibility that we're relying on it somewhere that isn't tested, but I'm hoping
that's not the case.

[1] https://developer.chrome.com/extensions/runtime#type-Port

+}
+
+void MessagingBindings::OpenChannelToNativeApp(
+    const v8::FunctionCallbackInfo<v8::Value>& args) {
+  // The Javascript code should validate/fill the arguments.
+  CHECK_EQ(args.Length(), 1);
+  CHECK(args[0]->IsString());
+  CHECK(context()->GetAvailability("runtime.connectNative").is_available());
+
+  content::RenderFrame* render_frame = context()->GetRenderFrame();
+  if (!render_frame)
+    return;
+
+  std::string native_app_name = *v8::String::Utf8Value(args[0]);
+
+  int local_id = next_local_id_++;
+  ExtensionPort* port =
+      ports_
+          .insert(std::make_pair(
+              local_id, base::MakeUnique<ExtensionPort>(context(), local_id)))
+          .first->second.get();
+
+  int global_id = -1;
+  // TODO(devlin): Make this async.

[lazyboy, 2016/09/08 00:05:40]

Bug ID (here and below)

[Devlin, 2016/09/08 19:15:30]

Done.

+  render_frame->Send(new ExtensionHostMsg_OpenChannelToNativeApp(
+      render_frame->GetRoutingID(), native_app_name, &global_id));
+  port->SetGlobalId(global_id);
+  args.GetReturnValue().Set(static_cast<int32_t>(local_id));
+}
+
+void MessagingBindings::OpenChannelToTab(
+    const v8::FunctionCallbackInfo<v8::Value>& args) {
+  content::RenderFrame* render_frame = context()->GetRenderFrame();
+  if (!render_frame)
+    return;
+
+  // tabs_custom_bindings.js unwraps arguments to tabs.connect/sendMessage and
+  // passes them to OpenChannelToTab, in the following order:
+  // - |tab_id| - Positive number that specifies the destination of the channel.
+  // - |frame_id| - Target frame(s) in the tab where onConnect is dispatched:
+  //   -1 for all frames, 0 for the main frame, >0 for a child frame.
+  // - |extension_id| - Extension ID of sender and destination.

[lazyboy, 2016/09/08 00:05:40]

I find "sender and destination" a bit confusing.

[Devlin, 2016/09/08 19:15:30]

Me too (this was copy-pasted). Replaced with "id of the initiating extension."

+  // - |channel_name| - A user-defined channel name.
+  CHECK(args.Length() >= 4 && args[0]->IsInt32() && args[1]->IsInt32() &&

[lazyboy, 2016/09/08 00:05:40]

Curious: why is this >= 4 and not == 4?

[Devlin, 2016/09/08 19:15:30]

Shouldn't be (copy-paste).  Updated to be == and also to have separate CHECK
statements so we get useful output from any crashes.

+        args[2]->IsString() && args[3]->IsString());
+
+  int local_id = next_local_id_++;
+  ExtensionPort* port =
+      ports_
+          .insert(std::make_pair(
+              local_id, base::MakeUnique<ExtensionPort>(context(), local_id)))
+          .first->second.get();
+
+  ExtensionMsg_TabTargetConnectionInfo info;
+  info.tab_id = args[0]->Int32Value();
+  info.frame_id = args[1]->Int32Value();
+  std::string extension_id = *v8::String::Utf8Value(args[2]);
+  std::string channel_name = *v8::String::Utf8Value(args[3]);
+  int global_id = -1;
+  // TODO(devlin): Make this async.
+  render_frame->Send(new ExtensionHostMsg_OpenChannelToTab(
+      render_frame->GetRoutingID(), info, extension_id, channel_name,
+      &global_id));
+  port->SetGlobalId(global_id);
+  args.GetReturnValue().Set(static_cast<int32_t>(local_id));
+}
+
 void MessagingBindings::ClosePort(int port_id, bool force_close) {

[lazyboy, 2016/09/08 00:05:40]

s/port_id/local_port_id

[Devlin, 2016/09/08 19:15:30]

Done.

   // TODO(robwu): Merge this logic with CloseChannel once the TODO in BindToGC
   // has been addressed.
-  content::RenderFrame* render_frame = context()->GetRenderFrame();
-  if (render_frame) {
-    render_frame->Send(new ExtensionHostMsg_CloseMessagePort(
-        render_frame->GetRoutingID(), port_id, force_close));
+  auto iter = ports_.find(port_id);
+  if (iter != ports_.end()) {
+    std::unique_ptr<ExtensionPort> port = std::move(iter->second);
+    ports_.erase(iter);
+    port->ClosePort(force_close);
+    // If the port hasn't been initialized, we can't delete it just yet, because
+    // we need to wait for it to send any pending messages. This is important
+    // to support the flow:
+    //  var port = chrome.runtime.connect();
+    //  port.postMessage({message});
+    //  port.disconnect();
+    if (!port->initialized())
+      disconnected_ports_[port->local_id()] = std::move(port);
   }
 }
 
+void MessagingBindings::SetGlobalPortId(int local_id, int global_id) {
+  auto iter = ports_.find(local_id);
+  if (iter != ports_.end()) {
+    iter->second->SetGlobalId(global_id);
+    return;
+  }
+
+  iter = disconnected_ports_.find(local_id);
+  DCHECK(iter != disconnected_ports_.end());
+  iter->second->SetGlobalId(global_id);
+  // Setting the global id dispatches pending messages, so we can delete the
+  // port now.
+  disconnected_ports_.erase(iter);
+}
+
 }  // namespace extensions