Skip to content

Commit

Permalink
[Gin] Add Arguments::GetHolderCreationContext()
Browse files Browse the repository at this point in the history
Currently, in order to get at the creation context of the holder, one
has to do the following:
v8::Local<v8::Object> holder;
arguments->GetHolder(&holder);
v8::Local<v8::Context> context = holder->CreationContext();

This isn't terrible, but it's a little verbose when all we really want
is FunctionCallbackInfo::Holder::CreationContext(). Add a method to get
at it directly.

BUG=None

Review-Url: https://codereview.chromium.org/2765853004
Cr-Commit-Position: refs/heads/master@{#459251}
  • Loading branch information
rdcronin authored and Commit bot committed Mar 23, 2017
1 parent aaee891 commit d982fdf
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 10 deletions.
13 changes: 3 additions & 10 deletions extensions/renderer/chrome_setting.cc
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,7 @@ void ChromeSetting::Get(gin::Arguments* arguments) {
void ChromeSetting::Set(gin::Arguments* arguments) {
v8::Isolate* isolate = arguments->isolate();
v8::HandleScope handle_scope(isolate);

v8::Local<v8::Object> holder;
CHECK(arguments->GetHolder(&holder));
v8::Local<v8::Context> context = holder->CreationContext();
v8::Local<v8::Context> context = arguments->GetHolderCreationContext();

v8::Local<v8::Value> value = arguments->PeekNext();
// The set schema included in the Schema object is generic, since it varies
Expand All @@ -129,9 +126,7 @@ void ChromeSetting::Clear(gin::Arguments* arguments) {
v8::Local<v8::Value> ChromeSetting::GetOnChangeEvent(
gin::Arguments* arguments) {
v8::Isolate* isolate = arguments->isolate();
v8::Local<v8::Object> holder;
CHECK(arguments->GetHolder(&holder));
v8::Local<v8::Context> context = holder->CreationContext();
v8::Local<v8::Context> context = arguments->GetHolderCreationContext();
v8::Local<v8::Object> wrapper = GetWrapper(isolate);
v8::Local<v8::Private> key = v8::Private::ForApi(
isolate, gin::StringToSymbol(isolate, "onChangeEvent"));
Expand Down Expand Up @@ -161,9 +156,7 @@ void ChromeSetting::HandleFunction(const std::string& method_name,
gin::Arguments* arguments) {
v8::Isolate* isolate = arguments->isolate();
v8::HandleScope handle_scope(isolate);
v8::Local<v8::Object> holder;
CHECK(arguments->GetHolder(&holder));
v8::Local<v8::Context> context = holder->CreationContext();
v8::Local<v8::Context> context = arguments->GetHolderCreationContext();

std::vector<v8::Local<v8::Value>> argument_list;
if (arguments->Length() > 0) {
Expand Down
1 change: 1 addition & 0 deletions gin/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ source_set("gin_test") {

test("gin_unittests") {
sources = [
"arguments_unittest.cc",
"converter_unittest.cc",
"interceptor_unittest.cc",
"modules/module_registry_unittest.cc",
Expand Down
4 changes: 4 additions & 0 deletions gin/arguments.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ v8::Local<v8::Value> Arguments::PeekNext() const {
return (*info_)[next_];
}

v8::Local<v8::Context> Arguments::GetHolderCreationContext() {
return info_->Holder()->CreationContext();
}

std::string V8TypeAsString(v8::Local<v8::Value> value) {
if (value.IsEmpty())
return "<empty handle>";
Expand Down
3 changes: 3 additions & 0 deletions gin/arguments.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ class GIN_EXPORT Arguments {
info_->GetReturnValue().Set(v8_value);
}

// Returns the creation context of the Holder.
v8::Local<v8::Context> GetHolderCreationContext();

// Always check the return value whether the handle is empty before
// dereferencing the handle.
v8::Local<v8::Value> PeekNext() const;
Expand Down
67 changes: 67 additions & 0 deletions gin/arguments_unittest.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
// Copyright 2017 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 "gin/arguments.h"

#include "base/bind.h"
#include "gin/converter.h"
#include "gin/object_template_builder.h"
#include "gin/public/isolate_holder.h"
#include "gin/test/v8_test.h"
#include "v8/include/v8.h"

namespace gin {

using ArgumentsTest = V8Test;

// Test that Arguments::GetHolderCreationContext returns the proper context.
TEST_F(ArgumentsTest, TestArgumentsHolderCreationContext) {
v8::Isolate* isolate = instance_->isolate();
v8::HandleScope handle_scope(isolate);

v8::Local<v8::Context> creation_context = context_.Get(instance_->isolate());

auto check_creation_context = [](v8::Local<v8::Context> expected_context,
gin::Arguments* arguments) {
EXPECT_EQ(expected_context, arguments->GetHolderCreationContext());
};

// Create an object that will compare GetHolderCreationContext() with
// |creation_context|.
v8::Local<v8::ObjectTemplate> object_template =
ObjectTemplateBuilder(isolate)
.SetMethod("checkCreationContext",
base::Bind(check_creation_context, creation_context))
.Build();

v8::Local<v8::Object> object =
object_template->NewInstance(creation_context).ToLocalChecked();

// Call checkCreationContext() on the generated object using the passed-in
// context as the current context.
auto test_context = [object, isolate](v8::Local<v8::Context> context) {
v8::Context::Scope context_scope(context);
const char kCallFunction[] = "(function(o) { o.checkCreationContext(); })";
v8::Local<v8::Script> script =
v8::Script::Compile(context, StringToV8(isolate, kCallFunction))
.ToLocalChecked();
v8::Local<v8::Function> function;
ASSERT_TRUE(ConvertFromV8(isolate, script->Run(), &function));
v8::Local<v8::Value> args[] = {object};
function->Call(v8::Undefined(isolate), arraysize(args), args);
};

// Test calling in the creation context.
test_context(creation_context);

{
// Create a second context, and test calling in that. The creation context
// should be the same (even though the current context has changed).
v8::Local<v8::Context> second_context =
v8::Context::New(isolate, nullptr, v8::Local<v8::ObjectTemplate>());
test_context(second_context);
}
}

} // namespace gin

0 comments on commit d982fdf

Please sign in to comment.