Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: clean up context bridge scopes and add specs for internal bridge #23334

Merged
merged 2 commits into from
May 11, 2020
Merged

chore: clean up context bridge scopes and add specs for internal bridge #23334

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
33e37b8
chore: clean up context bridge context scopes
MarshallOfSound Apr 29, 2020
be5e28c
spec: add specs for internalContextBridge
MarshallOfSound Apr 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 6 additions & 2 deletions lib/renderer/api/context-bridge.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ const checkContextIsolationEnabled = () => {
if (!contextIsolationEnabled) throw new Error('contextBridge API can only be used when contextIsolation is enabled');
};

const contextBridge = {
const contextBridge: Electron.ContextBridge = {
exposeInMainWorld: (key: string, api: Record<string, any>) => {
checkContextIsolationEnabled();
return binding.exposeAPIInMainWorld(key, api);
},
debugGC: () => binding._debugGCMaps({})
};
} as any;

if (!binding._debugGCMaps) delete contextBridge.debugGC;

Expand All @@ -32,3 +32,7 @@ export const internalContextBridge = {
},
isInMainWorld: () => binding._isCalledFromMainWorld() as boolean
};

if (binding._debugGCMaps) {
contextBridge.internalContextBridge = internalContextBridge;
}
235 changes: 113 additions & 122 deletions shell/renderer/api/electron_api_context_bridge.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,14 +151,12 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
int recursion_depth) {
if (recursion_depth >= kMaxRecursion) {
v8::Context::Scope source_scope(source_context);
{
source_context->GetIsolate()->ThrowException(v8::Exception::TypeError(
gin::StringToV8(source_context->GetIsolate(),
"Electron contextBridge recursion depth exceeded. "
"Nested objects "
"deeper than 1000 are not supported.")));
return v8::MaybeLocal<v8::Value>();
}
source_context->GetIsolate()->ThrowException(v8::Exception::TypeError(
gin::StringToV8(source_context->GetIsolate(),
"Electron contextBridge recursion depth exceeded. "
"Nested objects "
"deeper than 1000 are not supported.")));
return v8::MaybeLocal<v8::Value>();
}
// Check Cache
auto cached_value = object_cache->GetCachedProxiedObject(value);
Expand All @@ -177,8 +175,8 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
size_t func_id = store->take_func_id();
store->functions()[func_id] =
std::make_tuple(std::move(global_func), std::move(global_source));
v8::Context::Scope destination_scope(destination_context);
{
v8::Context::Scope destination_scope(destination_context);
v8::Local<v8::Value> proxy_func = gin_helper::CallbackToV8Leaked(
destination_context->GetIsolate(),
base::BindRepeating(&ProxyFunctionWrapper, store, func_id,
Expand All @@ -194,85 +192,81 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
// Proxy promises as they have a safe and guaranteed memory lifecycle
if (value->IsPromise()) {
v8::Context::Scope destination_scope(destination_context);
{
auto source_promise = v8::Local<v8::Promise>::Cast(value);
// Make the promise a shared_ptr so that when the original promise is
// freed the proxy promise is correctly freed as well instead of being
// left dangling
std::shared_ptr<gin_helper::Promise<v8::Local<v8::Value>>>
proxied_promise(new gin_helper::Promise<v8::Local<v8::Value>>(
destination_context->GetIsolate()));
v8::Local<v8::Promise> proxied_promise_handle =
proxied_promise->GetHandle();

v8::Global<v8::Context> global_then_source_context(
source_context->GetIsolate(), source_context);
v8::Global<v8::Context> global_then_destination_context(
destination_context->GetIsolate(), destination_context);
global_then_source_context.SetWeak();
global_then_destination_context.SetWeak();
auto then_cb = base::BindOnce(
[](std::shared_ptr<gin_helper::Promise<v8::Local<v8::Value>>>
proxied_promise,
v8::Isolate* isolate,
v8::Global<v8::Context> global_source_context,
v8::Global<v8::Context> global_destination_context,
context_bridge::RenderFrameFunctionStore* store,
v8::Local<v8::Value> result) {
if (global_source_context.IsEmpty() ||
global_destination_context.IsEmpty())
return;
context_bridge::ObjectCache object_cache;
auto val =
PassValueToOtherContext(global_source_context.Get(isolate),
global_destination_context.Get(isolate),
result, store, &object_cache, false, 0);
if (!val.IsEmpty())
proxied_promise->Resolve(val.ToLocalChecked());
},
proxied_promise, destination_context->GetIsolate(),
std::move(global_then_source_context),
std::move(global_then_destination_context), store);

v8::Global<v8::Context> global_catch_source_context(
source_context->GetIsolate(), source_context);
v8::Global<v8::Context> global_catch_destination_context(
destination_context->GetIsolate(), destination_context);
global_catch_source_context.SetWeak();
global_catch_destination_context.SetWeak();
auto catch_cb = base::BindOnce(
[](std::shared_ptr<gin_helper::Promise<v8::Local<v8::Value>>>
proxied_promise,
v8::Isolate* isolate,
v8::Global<v8::Context> global_source_context,
v8::Global<v8::Context> global_destination_context,
context_bridge::RenderFrameFunctionStore* store,
v8::Local<v8::Value> result) {
if (global_source_context.IsEmpty() ||
global_destination_context.IsEmpty())
return;
context_bridge::ObjectCache object_cache;
auto val =
PassValueToOtherContext(global_source_context.Get(isolate),
global_destination_context.Get(isolate),
result, store, &object_cache, false, 0);
if (!val.IsEmpty())
proxied_promise->Reject(val.ToLocalChecked());
},
proxied_promise, destination_context->GetIsolate(),
std::move(global_catch_source_context),
std::move(global_catch_destination_context), store);

ignore_result(source_promise->Then(
source_context,
v8::Local<v8::Function>::Cast(
gin::ConvertToV8(destination_context->GetIsolate(), then_cb)),
v8::Local<v8::Function>::Cast(
gin::ConvertToV8(destination_context->GetIsolate(), catch_cb))));

object_cache->CacheProxiedObject(value, proxied_promise_handle);
return v8::MaybeLocal<v8::Value>(proxied_promise_handle);
}
auto source_promise = v8::Local<v8::Promise>::Cast(value);
// Make the promise a shared_ptr so that when the original promise is
// freed the proxy promise is correctly freed as well instead of being
// left dangling
std::shared_ptr<gin_helper::Promise<v8::Local<v8::Value>>> proxied_promise(
new gin_helper::Promise<v8::Local<v8::Value>>(
destination_context->GetIsolate()));
v8::Local<v8::Promise> proxied_promise_handle =
proxied_promise->GetHandle();

v8::Global<v8::Context> global_then_source_context(
source_context->GetIsolate(), source_context);
v8::Global<v8::Context> global_then_destination_context(
destination_context->GetIsolate(), destination_context);
global_then_source_context.SetWeak();
global_then_destination_context.SetWeak();
auto then_cb = base::BindOnce(
[](std::shared_ptr<gin_helper::Promise<v8::Local<v8::Value>>>
proxied_promise,
v8::Isolate* isolate, v8::Global<v8::Context> global_source_context,
v8::Global<v8::Context> global_destination_context,
context_bridge::RenderFrameFunctionStore* store,
v8::Local<v8::Value> result) {
if (global_source_context.IsEmpty() ||
global_destination_context.IsEmpty())
return;
context_bridge::ObjectCache object_cache;
auto val =
PassValueToOtherContext(global_source_context.Get(isolate),
global_destination_context.Get(isolate),
result, store, &object_cache, false, 0);
if (!val.IsEmpty())
proxied_promise->Resolve(val.ToLocalChecked());
},
proxied_promise, destination_context->GetIsolate(),
std::move(global_then_source_context),
std::move(global_then_destination_context), store);

v8::Global<v8::Context> global_catch_source_context(
source_context->GetIsolate(), source_context);
v8::Global<v8::Context> global_catch_destination_context(
destination_context->GetIsolate(), destination_context);
global_catch_source_context.SetWeak();
global_catch_destination_context.SetWeak();
auto catch_cb = base::BindOnce(
[](std::shared_ptr<gin_helper::Promise<v8::Local<v8::Value>>>
proxied_promise,
v8::Isolate* isolate, v8::Global<v8::Context> global_source_context,
v8::Global<v8::Context> global_destination_context,
context_bridge::RenderFrameFunctionStore* store,
v8::Local<v8::Value> result) {
if (global_source_context.IsEmpty() ||
global_destination_context.IsEmpty())
return;
context_bridge::ObjectCache object_cache;
auto val =
PassValueToOtherContext(global_source_context.Get(isolate),
global_destination_context.Get(isolate),
result, store, &object_cache, false, 0);
if (!val.IsEmpty())
proxied_promise->Reject(val.ToLocalChecked());
},
proxied_promise, destination_context->GetIsolate(),
std::move(global_catch_source_context),
std::move(global_catch_destination_context), store);

ignore_result(source_promise->Then(
source_context,
v8::Local<v8::Function>::Cast(
gin::ConvertToV8(destination_context->GetIsolate(), then_cb)),
v8::Local<v8::Function>::Cast(
gin::ConvertToV8(destination_context->GetIsolate(), catch_cb))));

object_cache->CacheProxiedObject(value, proxied_promise_handle);
return v8::MaybeLocal<v8::Value>(proxied_promise_handle);
}

// Errors aren't serializable currently, we need to pull the message out and
Expand All @@ -289,27 +283,25 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
// promises are proxied correctly.
if (IsPlainArray(value)) {
v8::Context::Scope destination_context_scope(destination_context);
{
v8::Local<v8::Array> arr = v8::Local<v8::Array>::Cast(value);
size_t length = arr->Length();
v8::Local<v8::Array> cloned_arr =
v8::Array::New(destination_context->GetIsolate(), length);
for (size_t i = 0; i < length; i++) {
auto value_for_array = PassValueToOtherContext(
source_context, destination_context,
arr->Get(source_context, i).ToLocalChecked(), store, object_cache,
support_dynamic_properties, recursion_depth + 1);
if (value_for_array.IsEmpty())
return v8::MaybeLocal<v8::Value>();

if (!IsTrue(cloned_arr->Set(destination_context, static_cast<int>(i),
value_for_array.ToLocalChecked()))) {
return v8::MaybeLocal<v8::Value>();
}
v8::Local<v8::Array> arr = v8::Local<v8::Array>::Cast(value);
size_t length = arr->Length();
v8::Local<v8::Array> cloned_arr =
v8::Array::New(destination_context->GetIsolate(), length);
for (size_t i = 0; i < length; i++) {
auto value_for_array = PassValueToOtherContext(
source_context, destination_context,
arr->Get(source_context, i).ToLocalChecked(), store, object_cache,
support_dynamic_properties, recursion_depth + 1);
if (value_for_array.IsEmpty())
return v8::MaybeLocal<v8::Value>();

if (!IsTrue(cloned_arr->Set(destination_context, static_cast<int>(i),
value_for_array.ToLocalChecked()))) {
return v8::MaybeLocal<v8::Value>();
}
object_cache->CacheProxiedObject(value, cloned_arr);
return v8::MaybeLocal<v8::Value>(cloned_arr);
}
object_cache->CacheProxiedObject(value, cloned_arr);
return v8::MaybeLocal<v8::Value>(cloned_arr);
}

// Proxy all objects
Expand All @@ -327,15 +319,13 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
blink::CloneableMessage ret;
{
v8::Context::Scope source_context_scope(source_context);
{
// V8 serializer will throw an error if required
if (!gin::ConvertFromV8(source_context->GetIsolate(), value, &ret))
return v8::MaybeLocal<v8::Value>();
}
// V8 serializer will throw an error if required
if (!gin::ConvertFromV8(source_context->GetIsolate(), value, &ret))
return v8::MaybeLocal<v8::Value>();
}

v8::Context::Scope destination_context_scope(destination_context);
{
v8::Context::Scope destination_context_scope(destination_context);
v8::Local<v8::Value> cloned_value =
gin::ConvertToV8(destination_context->GetIsolate(), ret);
object_cache->CacheProxiedObject(value, cloned_value);
Expand All @@ -354,9 +344,10 @@ v8::Local<v8::Value> ProxyFunctionWrapper(
v8::Local<v8::Context> func_owning_context =
std::get<1>(store->functions()[func_id]).Get(args->isolate());

v8::Context::Scope func_owning_context_scope(func_owning_context);
context_bridge::ObjectCache object_cache;
{
v8::Context::Scope func_owning_context_scope(func_owning_context);
context_bridge::ObjectCache object_cache;

v8::Local<v8::Function> func =
(std::get<0>(store->functions()[func_id])).Get(args->isolate());

Expand Down Expand Up @@ -396,10 +387,8 @@ v8::Local<v8::Value> ProxyFunctionWrapper(

if (did_error) {
v8::Context::Scope calling_context_scope(calling_context);
{
args->ThrowError(error_message);
return v8::Local<v8::Object>();
}
args->ThrowError(error_message);
return v8::Local<v8::Object>();
}

if (maybe_return_value.IsEmpty())
Expand All @@ -424,8 +413,9 @@ v8::MaybeLocal<v8::Object> CreateProxyForAPI(
bool support_dynamic_properties,
int recursion_depth) {
gin_helper::Dictionary api(source_context->GetIsolate(), api_object);
v8::Context::Scope destination_context_scope(destination_context);

{
v8::Context::Scope destination_context_scope(destination_context);
gin_helper::Dictionary proxy =
gin::Dictionary::CreateEmpty(destination_context->GetIsolate());
object_cache->CacheProxiedObject(api.GetHandle(), proxy.GetHandle());
Expand Down Expand Up @@ -536,9 +526,10 @@ void ExposeAPIInMainWorld(const std::string& key,
v8::Local<v8::Context> isolated_context =
frame->WorldScriptContext(args->isolate(), WorldIDs::ISOLATED_WORLD_ID);

context_bridge::ObjectCache object_cache;
v8::Context::Scope main_context_scope(main_context);
{
context_bridge::ObjectCache object_cache;
v8::Context::Scope main_context_scope(main_context);

v8::MaybeLocal<v8::Object> maybe_proxy =
CreateProxyForAPI(api_object, isolated_context, main_context, store,
&object_cache, false, 0);
Expand Down