Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1216)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: extensions/renderer/messaging_bindings.cc

Issue 2547753002: [Extensions] Extension Port Ids and Initialization 2.0 (Closed)
Patch Set: Rob's Created 4 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« extensions/renderer/messaging_bindings.h ('K') | « extensions/renderer/messaging_bindings.h ('k') | third_party/WebKit/Source/web/WebDocument.cpp » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: extensions/renderer/messaging_bindings.cc
diff --git a/extensions/renderer/messaging_bindings.cc b/extensions/renderer/messaging_bindings.cc
index b82e84b113e571242a45d951dc5e0db6c9fb67e4..35b59503fbdba69f42bbdd653d6e7918aa65ca2d 100644
--- a/extensions/renderer/messaging_bindings.cc
+++ b/extensions/renderer/messaging_bindings.cc
@@ -13,6 +13,7 @@
#include "base/bind_helpers.h"
#include "base/callback.h"
#include "base/callback_helpers.h"
+#include "base/guid.h"
#include "base/lazy_instance.h"
#include "base/message_loop/message_loop.h"
#include "base/metrics/histogram_macros.h"
@@ -22,6 +23,7 @@
#include "content/public/renderer/render_frame.h"
#include "content/public/renderer/render_thread.h"
#include "extensions/common/api/messaging/message.h"
+#include "extensions/common/api/messaging/port_id.h"
#include "extensions/common/extension_messages.h"
#include "extensions/common/manifest_handlers/externally_connectable.h"
#include "extensions/renderer/extension_frame_helper.h"
@@ -55,7 +57,7 @@ namespace {
base::LazyInstance<std::map<ScriptContext*, MessagingBindings*>>
g_messaging_map = LAZY_INSTANCE_INITIALIZER;
-void HasMessagePort(int global_port_id,
+void HasMessagePort(const PortId& port_id,
bool* has_port,
ScriptContext* script_context) {
if (*has_port)
@@ -63,12 +65,12 @@ void HasMessagePort(int global_port_id,
MessagingBindings* bindings = g_messaging_map.Get()[script_context];
DCHECK(bindings);
- if (bindings->GetPortWithGlobalId(global_port_id))
- *has_port = true;
+ // No need for |=; we know this is false right now from above.
+ *has_port = bindings->GetPortWithId(port_id) != nullptr;
}
void DispatchOnConnectToScriptContext(
- int global_target_port_id,
+ const PortId& target_port_id,
const std::string& channel_name,
const ExtensionMsg_TabConnectionInfo* source,
const ExtensionMsg_ExternalConnectionInfo& info,
@@ -78,17 +80,17 @@ void DispatchOnConnectToScriptContext(
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))
+ // If the channel was opened by this same context, ignore it. This should only
+ // happen when messages are sent to an entire process (rather than a single
+ // frame) as an optimization; otherwise the browser process filters this out.
+ if (bindings->context_id() == target_port_id.context_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();
+ ExtensionPort* port = bindings->CreateNewPortWithId(target_port_id);
// Remove the port.
base::ScopedClosureRunner remove_port(
- base::Bind(&MessagingBindings::RemovePortWithLocalId,
- bindings->GetWeakPtr(), local_port_id));
+ base::Bind(&MessagingBindings::RemovePortWithJsId, bindings->GetWeakPtr(),
+ port->js_id()));
v8::Isolate* isolate = script_context->isolate();
v8::HandleScope handle_scope(isolate);
@@ -141,7 +143,7 @@ void DispatchOnConnectToScriptContext(
v8::Local<v8::Value> arguments[] = {
// portId
- v8::Integer::New(isolate, local_port_id),
+ v8::Integer::New(isolate, port->js_id()),
// channelName
v8_channel_name,
// sourceTab
@@ -178,11 +180,11 @@ void DispatchOnConnectToScriptContext(
}
void DeliverMessageToScriptContext(const Message& message,
- int global_target_port_id,
+ const PortId& target_port_id,
ScriptContext* script_context) {
MessagingBindings* bindings = g_messaging_map.Get()[script_context];
DCHECK(bindings);
- ExtensionPort* port = bindings->GetPortWithGlobalId(global_target_port_id);
+ ExtensionPort* port = bindings->GetPortWithId(target_port_id);
if (!port)
return;
@@ -190,7 +192,7 @@ void DeliverMessageToScriptContext(const Message& message,
v8::HandleScope handle_scope(isolate);
v8::Local<v8::Value> port_id_handle =
- v8::Integer::New(isolate, port->local_id());
+ v8::Integer::New(isolate, port->js_id());
v8::Local<v8::String> v8_data;
if (!ToV8String(isolate, message.data.c_str(), &v8_data))
@@ -217,12 +219,12 @@ void DeliverMessageToScriptContext(const Message& message,
"messaging", "dispatchOnMessage", &arguments);
}
-void DispatchOnDisconnectToScriptContext(int global_port_id,
+void DispatchOnDisconnectToScriptContext(const PortId& 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);
+ ExtensionPort* port = bindings->GetPortWithId(port_id);
if (!port)
return;
@@ -230,7 +232,7 @@ void DispatchOnDisconnectToScriptContext(int global_port_id,
v8::HandleScope handle_scope(isolate);
std::vector<v8::Local<v8::Value>> arguments;
- arguments.push_back(v8::Integer::New(isolate, port->local_id()));
+ arguments.push_back(v8::Integer::New(isolate, port->js_id()));
v8::Local<v8::String> v8_error_message;
if (!error_message.empty())
ToV8String(isolate, error_message.c_str(), &v8_error_message);
@@ -247,7 +249,9 @@ void DispatchOnDisconnectToScriptContext(int global_port_id,
} // namespace
MessagingBindings::MessagingBindings(ScriptContext* context)
- : ObjectBackedNativeHandler(context), weak_ptr_factory_(this) {
+ : ObjectBackedNativeHandler(context),
+ context_id_(base::GenerateGUID()),
+ weak_ptr_factory_(this) {
g_messaging_map.Get()[context] = this;
RouteFunction("CloseChannel", base::Bind(&MessagingBindings::CloseChannel,
base::Unretained(this)));
@@ -286,32 +290,32 @@ MessagingBindings::~MessagingBindings() {
// static
void MessagingBindings::ValidateMessagePort(
const ScriptContextSet& context_set,
- int global_port_id,
+ const PortId& port_id,
content::RenderFrame* render_frame) {
int routing_id = render_frame->GetRoutingID();
bool has_port = false;
context_set.ForEach(render_frame,
- base::Bind(&HasMessagePort, global_port_id, &has_port));
+ base::Bind(&HasMessagePort, port_id, &has_port));
// 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,
- global_port_id, false));
+ new ExtensionHostMsg_CloseMessagePort(routing_id, port_id, false));
}
}
// static
void MessagingBindings::DispatchOnConnect(
const ScriptContextSet& context_set,
- int target_port_id,
+ const PortId& target_port_id,
const std::string& channel_name,
const ExtensionMsg_TabConnectionInfo& source,
const ExtensionMsg_ExternalConnectionInfo& info,
const std::string& tls_channel_id,
content::RenderFrame* restrict_to_render_frame) {
+ DCHECK(!target_port_id.is_opener);
int routing_id = restrict_to_render_frame
? restrict_to_render_frame->GetRoutingID()
: MSG_ROUTING_NONE;
@@ -334,7 +338,7 @@ void MessagingBindings::DispatchOnConnect(
// static
void MessagingBindings::DeliverMessage(
const ScriptContextSet& context_set,
- int target_port_id,
+ const PortId& target_port_id,
const Message& message,
content::RenderFrame* restrict_to_render_frame) {
context_set.ForEach(
@@ -345,7 +349,7 @@ void MessagingBindings::DeliverMessage(
// static
void MessagingBindings::DispatchOnDisconnect(
const ScriptContextSet& context_set,
- int port_id,
+ const PortId& port_id,
const std::string& error_message,
content::RenderFrame* restrict_to_render_frame) {
context_set.ForEach(
@@ -353,27 +357,23 @@ void MessagingBindings::DispatchOnDisconnect(
base::Bind(&DispatchOnDisconnectToScriptContext, port_id, error_message));
}
-ExtensionPort* MessagingBindings::GetPortWithGlobalId(int id) {
+ExtensionPort* MessagingBindings::GetPortWithId(const PortId& id) {
for (const auto& key_value : ports_) {
- if (key_value.second->global_id() == id)
+ if (key_value.second->id() == id)
return key_value.second.get();
}
return nullptr;
}
-ExtensionPort* MessagingBindings::CreateNewPortWithGlobalId(int global_id) {
- int local_id = GetNextLocalId();
- ExtensionPort* port =
- ports_
- .insert(std::make_pair(
- local_id, base::MakeUnique<ExtensionPort>(context(), local_id)))
- .first->second.get();
- port->SetGlobalId(global_id);
- return port;
+ExtensionPort* MessagingBindings::CreateNewPortWithId(const PortId& id) {
+ int js_id = GetNextJsId();
+ auto port = base::MakeUnique<ExtensionPort>(context(), id, js_id);
+ return ports_.insert(std::make_pair(js_id, std::move(port)))
+ .first->second.get();
}
-void MessagingBindings::RemovePortWithLocalId(int local_id) {
- ports_.erase(local_id);
+void MessagingBindings::RemovePortWithJsId(int js_id) {
+ ports_.erase(js_id);
}
base::WeakPtr<MessagingBindings> MessagingBindings::GetWeakPtr() {
@@ -387,8 +387,8 @@ void MessagingBindings::PostMessage(
CHECK(args[0]->IsInt32());
CHECK(args[1]->IsString());
- int port_id = args[0].As<v8::Int32>()->Value();
- auto iter = ports_.find(port_id);
+ int js_port_id = args[0].As<v8::Int32>()->Value();
+ auto iter = ports_.find(js_port_id);
if (iter != ports_.end()) {
iter->second->PostExtensionMessage(base::MakeUnique<Message>(
*v8::String::Utf8Value(args[1]),
@@ -403,9 +403,9 @@ void MessagingBindings::CloseChannel(
CHECK(args[0]->IsInt32());
CHECK(args[1]->IsBoolean());
- int port_id = args[0].As<v8::Int32>()->Value();
+ int js_port_id = args[0].As<v8::Int32>()->Value();
bool force_close = args[1].As<v8::Boolean>()->Value();
- ClosePort(port_id, force_close);
+ ClosePort(js_port_id, force_close);
}
void MessagingBindings::BindToGC(
@@ -414,16 +414,16 @@ void MessagingBindings::BindToGC(
CHECK(args[0]->IsObject());
CHECK(args[1]->IsFunction());
CHECK(args[2]->IsInt32());
- int local_port_id = args[2].As<v8::Int32>()->Value();
+ int js_port_id = args[2].As<v8::Int32>()->Value();
base::Closure fallback = base::Bind(&base::DoNothing);
- if (local_port_id >= 0) {
+ if (js_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(), local_port_id,
+ weak_ptr_factory_.GetWeakPtr(), js_port_id,
false /* force_close */);
}
// Destroys itself when the object is GC'd or context is invalidated.
@@ -443,8 +443,9 @@ void MessagingBindings::OpenChannelToExtension(
CHECK(args[1]->IsString());
CHECK(args[2]->IsBoolean());
- int local_id = GetNextLocalId();
- ports_[local_id] = base::MakeUnique<ExtensionPort>(context(), local_id);
+ int js_id = GetNextJsId();
+ PortId port_id(context_id_, js_id, true);
+ ports_[js_id] = base::MakeUnique<ExtensionPort>(context(), port_id, js_id);
ExtensionMsg_ExternalConnectionInfo info;
// For messaging APIs, hosted apps should be considered a web page so hide
@@ -460,34 +461,15 @@ void MessagingBindings::OpenChannelToExtension(
bool include_tls_channel_id =
args.Length() > 2 ? args[2]->BooleanValue() : false;
- ExtensionFrameHelper* frame_helper = ExtensionFrameHelper::Get(render_frame);
- DCHECK(frame_helper);
-
- blink::WebLocalFrame* web_frame = render_frame->GetWebFrame();
- // If the frame is unloading, we need to assign the port id synchronously to
- // avoid dropping the message on the floor; see crbug.com/660706.
- // TODO(devlin): Investigate whether we need to continue supporting this long-
- // term, and, if so, find an alternative that doesn't require synchronous
- // IPCs.
- if (!web_frame->document().isNull() &&
- (web_frame->document().unloadStartedDoNotUse() ||
- web_frame->document().processingBeforeUnloadDoNotUse())) {
- ports_created_in_before_unload_ +=
- web_frame->document().processingBeforeUnloadDoNotUse() ? 1 : 0;
- ports_created_in_unload_ +=
- web_frame->document().unloadStartedDoNotUse() ? 1 : 0;
- int global_id = frame_helper->RequestSyncPortId(info, channel_name,
- include_tls_channel_id);
- ports_[local_id]->SetGlobalId(global_id);
- } else {
- ++ports_created_normal_;
- frame_helper->RequestPortId(
- info, channel_name, include_tls_channel_id,
- base::Bind(&MessagingBindings::SetGlobalPortId,
- weak_ptr_factory_.GetWeakPtr(), local_id));
+ {
+ SCOPED_UMA_HISTOGRAM_TIMER(
+ "Extensions.Messaging.GetPortIdSyncTime.Extension");
+ render_frame->Send(new ExtensionHostMsg_OpenChannelToExtension(
+ render_frame->GetRoutingID(), info, channel_name,
+ include_tls_channel_id, port_id));
}
- args.GetReturnValue().Set(static_cast<int32_t>(local_id));
+ args.GetReturnValue().Set(static_cast<int32_t>(js_id));
}
void MessagingBindings::OpenChannelToNativeApp(
@@ -504,16 +486,18 @@ void MessagingBindings::OpenChannelToNativeApp(
std::string native_app_name = *v8::String::Utf8Value(args[0]);
- int local_id = GetNextLocalId();
- ports_[local_id] = base::MakeUnique<ExtensionPort>(context(), local_id);
+ int js_id = GetNextJsId();
+ PortId port_id(context_id_, js_id, true);
+ ports_[js_id] = base::MakeUnique<ExtensionPort>(context(), port_id, js_id);
- ExtensionFrameHelper* frame_helper = ExtensionFrameHelper::Get(render_frame);
- DCHECK(frame_helper);
- frame_helper->RequestNativeAppPortId(
- native_app_name,
- base::Bind(&MessagingBindings::SetGlobalPortId,
- weak_ptr_factory_.GetWeakPtr(), local_id));
- args.GetReturnValue().Set(static_cast<int32_t>(local_id));
+ {
+ SCOPED_UMA_HISTOGRAM_TIMER(
+ "Extensions.Messaging.GetPortIdSyncTime.NativeApp");
+ render_frame->Send(new ExtensionHostMsg_OpenChannelToNativeApp(
+ render_frame->GetRoutingID(), native_app_name, port_id));
+ }
+
+ args.GetReturnValue().Set(static_cast<int32_t>(js_id));
}
void MessagingBindings::OpenChannelToTab(
@@ -535,8 +519,9 @@ void MessagingBindings::OpenChannelToTab(
CHECK(args[2]->IsString());
CHECK(args[3]->IsString());
- int local_id = GetNextLocalId();
- ports_[local_id] = base::MakeUnique<ExtensionPort>(context(), local_id);
+ int js_id = GetNextJsId();
+ PortId port_id(context_id_, js_id, true);
+ ports_[js_id] = base::MakeUnique<ExtensionPort>(context(), port_id, js_id);
ExtensionMsg_TabTargetConnectionInfo info;
info.tab_id = args[0]->Int32Value();
@@ -547,48 +532,28 @@ void MessagingBindings::OpenChannelToTab(
ExtensionFrameHelper* frame_helper = ExtensionFrameHelper::Get(render_frame);
DCHECK(frame_helper);
- frame_helper->RequestTabPortId(
- info, extension_id, channel_name,
- base::Bind(&MessagingBindings::SetGlobalPortId,
- weak_ptr_factory_.GetWeakPtr(), local_id));
- args.GetReturnValue().Set(static_cast<int32_t>(local_id));
+ {
+ SCOPED_UMA_HISTOGRAM_TIMER("Extensions.Messaging.GetPortIdSyncTime.Tab");
+ render_frame->Send(new ExtensionHostMsg_OpenChannelToTab(
+ render_frame->GetRoutingID(), info, extension_id, channel_name,
+ port_id));
+ }
+
+ args.GetReturnValue().Set(static_cast<int32_t>(js_id));
}
-void MessagingBindings::ClosePort(int local_port_id, bool force_close) {
+void MessagingBindings::ClosePort(int js_port_id, bool force_close) {
// TODO(robwu): Merge this logic with CloseChannel once the TODO in BindToGC
// has been addressed.
- auto iter = ports_.find(local_port_id);
+ auto iter = ports_.find(js_port_id);
if (iter != ports_.end()) {
std::unique_ptr<ExtensionPort> port = std::move(iter->second);
ports_.erase(iter);
port->Close(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);
}
-int MessagingBindings::GetNextLocalId() { return next_local_id_++; }
+int MessagingBindings::GetNextJsId() { return next_js_id_++; }
} // namespace extensions
« extensions/renderer/messaging_bindings.h ('K') | « extensions/renderer/messaging_bindings.h ('k') | third_party/WebKit/Source/web/WebDocument.cpp » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698