diff options
13 files changed, 274 insertions, 147 deletions
diff --git a/chrome/renderer/extensions/event_bindings.cc b/chrome/renderer/extensions/event_bindings.cc index 329a70b..94258db 100644 --- a/chrome/renderer/extensions/event_bindings.cc +++ b/chrome/renderer/extensions/event_bindings.cc @@ -191,6 +191,8 @@ class ExtensionImpl : public ChromeV8Extension { base::DictionaryValue* filter_dict = NULL; base::Value* filter_value = converter->FromV8Value(args[1]->ToObject(), v8::Context::GetCurrent()); + if (!filter_value) + return v8::Integer::New(-1); if (!filter_value->GetAsDictionary(&filter_dict)) { delete filter_value; return v8::Integer::New(-1); diff --git a/chrome/renderer/extensions/send_request_natives.cc b/chrome/renderer/extensions/send_request_natives.cc index a5a8834..b788a8a 100644 --- a/chrome/renderer/extensions/send_request_natives.cc +++ b/chrome/renderer/extensions/send_request_natives.cc @@ -37,14 +37,16 @@ v8::Handle<v8::Value> SendRequestNatives::StartRequest( int request_id = args[2]->Int32Value(); bool has_callback = args[3]->BooleanValue(); bool for_io_thread = args[4]->BooleanValue(); + bool preserve_null_in_objects = args[5]->BooleanValue(); scoped_ptr<V8ValueConverter> converter(V8ValueConverter::create()); - // Make "undefined" the same as "null" for optional arguments, but for objects - // strip nulls (like {foo: null}, and therefore {foo: undefined} as well) to - // make it easier for extension APIs to check for optional arguments. - converter->SetUndefinedAllowed(true); - converter->SetStripNullFromObjects(true); + // See http://crbug.com/149880. The context menus APIs relies on this, but + // we shouln't really be doing it (e.g. for the sake of the storage API). + converter->SetFunctionAllowed(true); + + if (!preserve_null_in_objects) + converter->SetStripNullFromObjects(true); scoped_ptr<Value> value_args( converter->FromV8Value(args[1], v8::Context::GetCurrent())); diff --git a/chrome/renderer/extensions/user_script_scheduler.cc b/chrome/renderer/extensions/user_script_scheduler.cc index 3d98aea..fec5a98 100644 --- a/chrome/renderer/extensions/user_script_scheduler.cc +++ b/chrome/renderer/extensions/user_script_scheduler.cc @@ -203,7 +203,6 @@ void UserScriptScheduler::ExecuteCodeImpl( scoped_ptr<content::V8ValueConverter> v8_converter( content::V8ValueConverter::create()); - v8_converter->SetUndefinedAllowed(true); v8::Handle<v8::Value> script_value; if (params.in_main_world) { script_value = child_frame->executeScriptAndReturnValue(source); @@ -223,7 +222,10 @@ void UserScriptScheduler::ExecuteCodeImpl( if (!script_value.IsEmpty()) { base::Value* base_val = v8_converter->FromV8Value(script_value, context); - execution_results.Append(base_val); + // Always append an execution result (i.e. no result == null result) so + // that |execution_results| lines up with the frames. + execution_results.Append(base_val ? base_val : + base::Value::CreateNullValue()); script_value.Clear(); } } else { diff --git a/chrome/renderer/resources/extensions/context_menus_custom_bindings.js b/chrome/renderer/resources/extensions/context_menus_custom_bindings.js index 9e4016a..e2629f7 100644 --- a/chrome/renderer/resources/extensions/context_menus_custom_bindings.js +++ b/chrome/renderer/resources/extensions/context_menus_custom_bindings.js @@ -48,10 +48,10 @@ chromeHidden.registerCustomHook('contextMenus', function(bindingsAPI) { var args = arguments; var id = GetNextContextMenuId(); args[0].generatedId = id; - sendRequest(this.name, - args, - this.definition.parameters, - {customCallback: this.customCallback}); + var optArgs = { + customCallback: this.customCallback, + }; + sendRequest(this.name, args, this.definition.parameters, optArgs); return chromeHidden.contextMenus.getIdFromCreateProperties(args[0]); }); diff --git a/chrome/renderer/resources/extensions/schema_generated_bindings.js b/chrome/renderer/resources/extensions/schema_generated_bindings.js index ecf46f3..76bda1e 100644 --- a/chrome/renderer/resources/extensions/schema_generated_bindings.js +++ b/chrome/renderer/resources/extensions/schema_generated_bindings.js @@ -311,9 +311,12 @@ if (this.handleRequest) { retval = this.handleRequest.apply(this, args); } else { + var optArgs = { + customCallback: this.customCallback + }; retval = sendRequest(this.name, args, this.definition.parameters, - {customCallback: this.customCallback}); + optArgs); } // Validate return value if defined - only in debug. diff --git a/chrome/renderer/resources/extensions/send_request.js b/chrome/renderer/resources/extensions/send_request.js index 1d7231f..5aeb995 100644 --- a/chrome/renderer/resources/extensions/send_request.js +++ b/chrome/renderer/resources/extensions/send_request.js @@ -77,7 +77,7 @@ function prepareRequest(args, argSchemas) { } // Send an API request and optionally register a callback. -// |opt_args| is an object with optional parameters as follows: +// |optArgs| is an object with optional parameters as follows: // - noStringify: true if we should not stringify the request arguments. // - customCallback: a callback that should be called instead of the standard // callback. @@ -85,12 +85,13 @@ function prepareRequest(args, argSchemas) { // StartRequest if missing. // - forIOThread: true if this function should be handled on the browser IO // thread. -function sendRequest(functionName, args, argSchemas, opt_args) { - if (!opt_args) - opt_args = {}; +// - preserveNullInObjects: true if it is safe for null to be in objects. +function sendRequest(functionName, args, argSchemas, optArgs) { + if (!optArgs) + optArgs = {}; var request = prepareRequest(args, argSchemas); - if (opt_args.customCallback) { - request.customCallback = opt_args.customCallback; + if (optArgs.customCallback) { + request.customCallback = optArgs.customCallback; } // JSON.stringify doesn't support a root object which is undefined. if (request.args === undefined) @@ -99,19 +100,22 @@ function sendRequest(functionName, args, argSchemas, opt_args) { // TODO(asargent) - convert all optional native functions to accept raw // v8 values instead of expecting JSON strings. var doStringify = false; - if (opt_args.nativeFunction && !opt_args.noStringify) + if (optArgs.nativeFunction && !optArgs.noStringify) doStringify = true; var requestArgs = doStringify ? chromeHidden.JSON.stringify(request.args) : request.args; - var nativeFunction = opt_args.nativeFunction || natives.StartRequest; + var nativeFunction = optArgs.nativeFunction || natives.StartRequest; var requestId = natives.GetNextRequestId(); request.id = requestId; requests[requestId] = request; - var hasCallback = - (request.callback || opt_args.customCallback) ? true : false; - return nativeFunction(functionName, requestArgs, requestId, hasCallback, - opt_args.forIOThread); + var hasCallback = request.callback || optArgs.customCallback; + return nativeFunction(functionName, + requestArgs, + requestId, + hasCallback, + optArgs.forIOThread, + optArgs.preserveNullInObjects); } exports.sendRequest = sendRequest; diff --git a/chrome/renderer/resources/extensions/storage_custom_bindings.js b/chrome/renderer/resources/extensions/storage_custom_bindings.js index 9b0ece5..d230099 100644 --- a/chrome/renderer/resources/extensions/storage_custom_bindings.js +++ b/chrome/renderer/resources/extensions/storage_custom_bindings.js @@ -33,7 +33,8 @@ chromeHidden.registerCustomType('storage.StorageArea', function() { return sendRequest( 'storage.' + functionName, [namespace].concat(args), - extendSchema(funSchema.definition.parameters)); + extendSchema(funSchema.definition.parameters), + {preserveNullInObjects: true}); }; } var apiFunctions = ['get', 'set', 'remove', 'clear', 'getBytesInUse']; diff --git a/chrome/test/data/extensions/api_test/settings/simple_test/background.js b/chrome/test/data/extensions/api_test/settings/simple_test/background.js index 0ac0b5e..02fe318 100644 --- a/chrome/test/data/extensions/api_test/settings/simple_test/background.js +++ b/chrome/test/data/extensions/api_test/settings/simple_test/background.js @@ -2,6 +2,10 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +var assertEq = chrome.test.assertEq; +var assertTrue = chrome.test.assertTrue; +var succeed = chrome.test.succeed; + function test(stage0) { var apis = [ chrome.storage.sync, @@ -19,15 +23,15 @@ chrome.test.runTests([ this.get('foo', stage1.bind(this)); } function stage1(settings) { - chrome.test.assertEq({}, settings); + assertEq({}, settings); this.get(['foo', 'bar'], stage2.bind(this)); } function stage2(settings) { - chrome.test.assertEq({}, settings); + assertEq({}, settings); this.get(undefined, stage3.bind(this)); } function stage3(settings) { - chrome.test.assertEq({}, settings); + assertEq({}, settings); this.succeed(); } test(stage0); @@ -45,21 +49,21 @@ chrome.test.runTests([ this.get(['foo', 'baz'], stage2.bind(this)); } function stage2(settings) { - chrome.test.assertEq({ + assertEq({ 'foo': 'bar', 'baz': 'qux' }, settings); this.get(['nothing', 'baz', 'hello', 'ignore'], stage3.bind(this)); } function stage3(settings) { - chrome.test.assertEq({ + assertEq({ 'baz' : 'qux', 'hello': 'world' }, settings); this.get(null, stage4.bind(this)); } function stage4(settings) { - chrome.test.assertEq({ + assertEq({ 'foo' : 'bar', 'baz' : 'qux', 'hello': 'world' @@ -94,7 +98,7 @@ chrome.test.runTests([ this.get(null, stage3.bind(this)); } function stage3(settings) { - chrome.test.assertEq({ + assertEq({ 'baz' : 'qux', 'hello': 'world' }, settings); @@ -104,7 +108,7 @@ chrome.test.runTests([ this.get(null, stage5.bind(this)); } function stage5(settings) { - chrome.test.assertEq({ + assertEq({ 'hello': 'world' }, settings); this.remove('hello', stage6.bind(this)); @@ -113,7 +117,7 @@ chrome.test.runTests([ this.get(null, stage7.bind(this)); } function stage7(settings) { - chrome.test.assertEq({}, settings); + assertEq({}, settings); this.succeed(); } test(stage0); @@ -137,7 +141,7 @@ chrome.test.runTests([ this.get(null, stage3.bind(this)); } function stage3(settings) { - chrome.test.assertEq({ + assertEq({ 'foo' : 'otherBar', 'baz' : 'otherQux', 'hello': 'world' @@ -152,7 +156,7 @@ chrome.test.runTests([ this.get(null, stage5.bind(this)); } function stage5(settings) { - chrome.test.assertEq({ + assertEq({ 'foo' : 'otherBar', 'baz' : 'anotherQux', 'hello': 'otherWorld', @@ -171,7 +175,7 @@ chrome.test.runTests([ this.get(null, stage2.bind(this)); } function stage2(settings) { - chrome.test.assertEq({}, settings); + assertEq({}, settings); this.succeed(); } test(stage0); @@ -192,7 +196,7 @@ chrome.test.runTests([ this.get(null, stage3.bind(this)); } function stage3(settings) { - chrome.test.assertEq({}, settings); + assertEq({}, settings); this.succeed(); } test(stage0); @@ -209,21 +213,21 @@ chrome.test.runTests([ this.get(['foo.bar', 'one'], stage2.bind(this)); } function stage2(settings) { - chrome.test.assertEq({ + assertEq({ 'foo.bar' : 'baz', 'one' : {'two': 'three'} }, settings); this.get('one.two', stage3.bind(this)); } function stage3(settings) { - chrome.test.assertEq({}, settings); + assertEq({}, settings); this.remove(['foo.bar', 'one.two'], stage4.bind(this)); } function stage4() { this.get(null, stage5.bind(this)); } function stage5(settings) { - chrome.test.assertEq({ + assertEq({ 'one' : {'two': 'three'} }, settings); this.succeed(); @@ -239,14 +243,14 @@ chrome.test.runTests([ }, stage1.bind(this)); } function stage1(settings) { - chrome.test.assertEq({ + assertEq({ 'foo': 'defaultBar', 'baz': [1, 2, 3] }, settings); this.get(null, stage2.bind(this)); } function stage2(settings) { - chrome.test.assertEq({}, settings); + assertEq({}, settings); this.set({'foo': 'bar'}, stage3.bind(this)); } function stage3() { @@ -256,7 +260,7 @@ chrome.test.runTests([ }, stage4.bind(this)); } function stage4(settings) { - chrome.test.assertEq({ + assertEq({ 'foo': 'bar', 'baz': [1, 2, 3] }, settings); @@ -269,7 +273,7 @@ chrome.test.runTests([ }, stage6.bind(this)); } function stage6(settings) { - chrome.test.assertEq({ + assertEq({ 'foo': 'bar', 'baz': {} }, settings); @@ -282,7 +286,7 @@ chrome.test.runTests([ }, stage8.bind(this)); } function stage8(settings) { - chrome.test.assertEq({ + assertEq({ 'foo': 'defaultBar', 'baz': {} }, settings); @@ -295,38 +299,66 @@ chrome.test.runTests([ function quota() { // Just check that the constants are defined; no need to be forced to // update them here as well if/when they change. - chrome.test.assertTrue(chrome.storage.sync.QUOTA_BYTES > 0); - chrome.test.assertTrue(chrome.storage.sync.QUOTA_BYTES_PER_ITEM > 0); - chrome.test.assertTrue(chrome.storage.sync.MAX_ITEMS > 0); + assertTrue(chrome.storage.sync.QUOTA_BYTES > 0); + assertTrue(chrome.storage.sync.QUOTA_BYTES_PER_ITEM > 0); + assertTrue(chrome.storage.sync.MAX_ITEMS > 0); - chrome.test.assertTrue(chrome.storage.local.QUOTA_BYTES > 0); - chrome.test.assertEq('undefined', - typeof chrome.storage.local.QUOTA_BYTES_PER_ITEM); - chrome.test.assertEq('undefined', - typeof chrome.storage.local.MAX_ITEMS); + assertTrue(chrome.storage.local.QUOTA_BYTES > 0); + assertEq('undefined', typeof chrome.storage.local.QUOTA_BYTES_PER_ITEM); + assertEq('undefined', typeof chrome.storage.local.MAX_ITEMS); var area = chrome.storage.sync; function stage0() { area.getBytesInUse(null, stage1); } function stage1(bytesInUse) { - chrome.test.assertEq(0, bytesInUse); + assertEq(0, bytesInUse); area.set({ a: 42, b: 43, c: 44 }, stage2); } function stage2() { area.getBytesInUse(null, stage3); } function stage3(bytesInUse) { - chrome.test.assertEq(9, bytesInUse); + assertEq(9, bytesInUse); area.getBytesInUse('a', stage4); } function stage4(bytesInUse) { - chrome.test.assertEq(3, bytesInUse); + assertEq(3, bytesInUse); area.getBytesInUse(['a', 'b'], stage5); } function stage5(bytesInUse) { - chrome.test.assertEq(6, bytesInUse); - chrome.test.succeed(); + assertEq(6, bytesInUse); + succeed(); + } + area.clear(stage0); + }, + + function nullsInArgs() { + var area = chrome.storage.local; + function stage0() { + area.get({ + foo: 'foo', + bar: null, + baz: undefined + }, stage1); + } + function stage1(values) { + assertEq({ + foo: 'foo', + bar: null, + }, values); + area.set({ + foo: 'foo', + bar: null, + baz: undefined + }, area.get.bind(area, stage2)); + } + function stage2(values) { + assertEq({ + foo: 'foo', + bar: null, + }, values); + succeed(); } area.clear(stage0); }, @@ -366,5 +398,5 @@ chrome.test.runTests([ })); })); })); - } + }, ]); diff --git a/content/public/renderer/v8_value_converter.h b/content/public/renderer/v8_value_converter.h index be45ea9..b71b32d 100644 --- a/content/public/renderer/v8_value_converter.h +++ b/content/public/renderer/v8_value_converter.h @@ -28,31 +28,48 @@ class CONTENT_EXPORT V8ValueConverter { virtual ~V8ValueConverter() {} - // Use the following setters to support additional types other than the - // default ones. - virtual bool GetUndefinedAllowed() const = 0; - virtual void SetUndefinedAllowed(bool val) = 0; - - virtual bool GetDateAllowed() const = 0; + // If true, Date objects are converted into DoubleValues with the number of + // seconds since Unix epoch. + // + // Otherwise they are converted into DictionaryValues with whatever additional + // properties has been set on them. virtual void SetDateAllowed(bool val) = 0; - virtual bool GetRegexpAllowed() const = 0; - virtual void SetRegexpAllowed(bool val) = 0; + // If true, RegExp objects are converted into StringValues with the regular + // expression between / and /, for example "/ab?c/". + // + // Otherwise they are converted into DictionaryValues with whatever additional + // properties has been set on them. + virtual void SetRegExpAllowed(bool val) = 0; + + // If true, Function objects are converted into DictionaryValues with whatever + // additional properties has been set on them. + // + // Otherwise they are treated as unsupported, see FromV8Value. + virtual void SetFunctionAllowed(bool val) = 0; - // Gets/sets whether to treat undefined or null in objects as nonexistent. - virtual bool GetStripNullFromObjects() const = 0; + // If true, null values are stripped from objects. This is often useful when + // converting arguments to extension APIs. virtual void SetStripNullFromObjects(bool val) = 0; - // Converts Value to v8::Value. Unsupported types are replaced with null. - // If an array or object throws while setting a value, that property or item - // is skipped, leaving a hole in the case of arrays. + // Converts a base::Value to a v8::Value. + // + // Unsupported types are replaced with null. If an array or object throws + // while setting a value, that property or item is skipped, leaving a hole in + // the case of arrays. virtual v8::Handle<v8::Value> ToV8Value( const base::Value* value, v8::Handle<v8::Context> context) const = 0; - // Converts v8::Value to Value. Unsupported types are replaced with null. - // If an array or object throws while getting a value, that property or item - // is replaced with null. + // Converts a v8::Value to base::Value. + // + // Unsupported types (unless explicitly configured) are not converted, so + // this method may return NULL -- the exception is when converting arrays, + // where unsupported types are converted to Value(TYPE_NULL). + // + // Likewise, if an object throws while converting a property it will not be + // converted, whereas if an array throws while converting an item it will be + // converted to Value(TYPE_NULL). virtual base::Value* FromV8Value(v8::Handle<v8::Value> value, v8::Handle<v8::Context> context) const = 0; }; diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc index 8e473184..224cb16 100644 --- a/content/renderer/render_view_impl.cc +++ b/content/renderer/render_view_impl.cc @@ -4219,7 +4219,6 @@ WebKit::WebPlugin* RenderViewImpl::CreatePlugin( pepper_module.get(), params, pepper_delegate_.AsWeakPtr()); } - #if defined(USE_AURA) && !defined(OS_WIN) return NULL; #else @@ -4244,8 +4243,10 @@ void RenderViewImpl::EvaluateScript(const string16& frame_xpath, v8::Context::Scope context_scope(context); V8ValueConverterImpl converter; converter.SetDateAllowed(true); - converter.SetRegexpAllowed(true); - list.Set(0, converter.FromV8Value(result, context)); + converter.SetRegExpAllowed(true); + base::Value* result_value = converter.FromV8Value(result, context); + list.Set(0, result_value ? result_value : + base::Value::CreateNullValue()); } else { list.Set(0, Value::CreateNullValue()); } diff --git a/content/renderer/v8_value_converter_impl.cc b/content/renderer/v8_value_converter_impl.cc index 8d55ecc..4c2130d 100644 --- a/content/renderer/v8_value_converter_impl.cc +++ b/content/renderer/v8_value_converter_impl.cc @@ -27,38 +27,22 @@ V8ValueConverter* V8ValueConverter::create() { } // namespace content V8ValueConverterImpl::V8ValueConverterImpl() - : undefined_allowed_(false), - date_allowed_(false), - regexp_allowed_(false), + : date_allowed_(false), + reg_exp_allowed_(false), + function_allowed_(false), strip_null_from_objects_(false) { } -bool V8ValueConverterImpl::GetUndefinedAllowed() const { - return undefined_allowed_; -} - -void V8ValueConverterImpl::SetUndefinedAllowed(bool val) { - undefined_allowed_ = val; -} - -bool V8ValueConverterImpl::GetDateAllowed() const { - return date_allowed_; -} - void V8ValueConverterImpl::SetDateAllowed(bool val) { date_allowed_ = val; } -bool V8ValueConverterImpl::GetRegexpAllowed() const { - return regexp_allowed_; -} - -void V8ValueConverterImpl::SetRegexpAllowed(bool val) { - regexp_allowed_ = val; +void V8ValueConverterImpl::SetRegExpAllowed(bool val) { + reg_exp_allowed_ = val; } -bool V8ValueConverterImpl::GetStripNullFromObjects() const { - return strip_null_from_objects_; +void V8ValueConverterImpl::SetFunctionAllowed(bool val) { + function_allowed_ = val; } void V8ValueConverterImpl::SetStripNullFromObjects(bool val) { @@ -200,23 +184,37 @@ Value* V8ValueConverterImpl::FromV8ValueImpl(v8::Handle<v8::Value> val, return Value::CreateStringValue(std::string(*utf8, utf8.length())); } - if (undefined_allowed_ && val->IsUndefined()) - return Value::CreateNullValue(); + if (val->IsUndefined()) + // JSON.stringify ignores undefined. + return NULL; - if (date_allowed_ && val->IsDate()) { + if (val->IsDate()) { + if (!date_allowed_) + // JSON.stringify would convert this to a string, but an object is more + // consistent within this class. + return FromV8Object(val->ToObject(), unique_set); v8::Date* date = v8::Date::Cast(*val); return Value::CreateDoubleValue(date->NumberValue() / 1000.0); } - if (regexp_allowed_ && val->IsRegExp()) { - return Value::CreateStringValue( - *v8::String::Utf8Value(val->ToString())); + if (val->IsRegExp()) { + if (!reg_exp_allowed_) + // JSON.stringify converts to an object. + return FromV8Object(val->ToObject(), unique_set); + return Value::CreateStringValue(*v8::String::Utf8Value(val->ToString())); } // v8::Value doesn't have a ToArray() method for some reason. if (val->IsArray()) return FromV8Array(val.As<v8::Array>(), unique_set); + if (val->IsFunction()) { + if (!function_allowed_) + // JSON.stringify refuses to convert function(){}. + return NULL; + return FromV8Object(val->ToObject(), unique_set); + } + if (val->IsObject()) { BinaryValue* binary_value = FromV8Buffer(val); if (binary_value) { @@ -225,8 +223,9 @@ Value* V8ValueConverterImpl::FromV8ValueImpl(v8::Handle<v8::Value> val, return FromV8Object(val->ToObject(), unique_set); } } + LOG(ERROR) << "Unexpected v8 value type encountered."; - return Value::CreateNullValue(); + return NULL; } Value* V8ValueConverterImpl::FromV8Array(v8::Handle<v8::Array> val, @@ -258,9 +257,12 @@ Value* V8ValueConverterImpl::FromV8Array(v8::Handle<v8::Array> val, continue; Value* child = FromV8ValueImpl(child_v8, unique_set); - CHECK(child); - - result->Append(child); + if (child) + result->Append(child); + else + // JSON.stringify puts null in places where values don't serialize, for + // example undefined and functions. Emulate that behavior. + result->Append(Value::CreateNullValue()); } return result; } @@ -335,7 +337,10 @@ Value* V8ValueConverterImpl::FromV8Object( } scoped_ptr<Value> child(FromV8ValueImpl(child_v8, unique_set)); - CHECK(child.get()); + if (!child.get()) + // JSON.stringify skips properties whose values don't serialize, for + // example undefined and functions. Emulate that behavior. + continue; // Strip null if asked (and since undefined is turned into null, undefined // too). The use case for supporting this is JSON-schema support, diff --git a/content/renderer/v8_value_converter_impl.h b/content/renderer/v8_value_converter_impl.h index baf061a..c6b9511 100644 --- a/content/renderer/v8_value_converter_impl.h +++ b/content/renderer/v8_value_converter_impl.h @@ -23,13 +23,9 @@ class CONTENT_EXPORT V8ValueConverterImpl : public content::V8ValueConverter { V8ValueConverterImpl(); // V8ValueConverter implementation. - virtual bool GetUndefinedAllowed() const OVERRIDE; - virtual void SetUndefinedAllowed(bool val) OVERRIDE; - virtual bool GetDateAllowed() const OVERRIDE; virtual void SetDateAllowed(bool val) OVERRIDE; - virtual bool GetRegexpAllowed() const OVERRIDE; - virtual void SetRegexpAllowed(bool val) OVERRIDE; - virtual bool GetStripNullFromObjects() const OVERRIDE; + virtual void SetRegExpAllowed(bool val) OVERRIDE; + virtual void SetFunctionAllowed(bool val) OVERRIDE; virtual void SetStripNullFromObjects(bool val) OVERRIDE; virtual v8::Handle<v8::Value> ToV8Value( const base::Value* value, @@ -58,14 +54,14 @@ class CONTENT_EXPORT V8ValueConverterImpl : public content::V8ValueConverter { base::Value* FromV8Object(v8::Handle<v8::Object> object, std::set<int>* unique_set) const; - // If true, we will convert undefined JavaScript values to null. - bool undefined_allowed_; - // If true, we will convert Date JavaScript objects to doubles. bool date_allowed_; - // If true, we will convet RegExp JavaScript objects to string. - bool regexp_allowed_; + // If true, we will convert RegExp JavaScript objects to string. + bool reg_exp_allowed_; + + // If true, we will convert Function JavaScript objects to dictionaries. + bool function_allowed_; // If true, undefined and null values are ignored when converting v8 objects // into Values. diff --git a/content/renderer/v8_value_converter_impl_unittest.cc b/content/renderer/v8_value_converter_impl_unittest.cc index 9146fb3..b4b12bd 100644 --- a/content/renderer/v8_value_converter_impl_unittest.cc +++ b/content/renderer/v8_value_converter_impl_unittest.cc @@ -10,6 +10,8 @@ #include "testing/gtest/include/gtest/gtest.h" #include "v8/include/v8.h" +using content::V8ValueConverter; + namespace { // A dumb getter for an object's named callback. @@ -112,10 +114,14 @@ class V8ValueConverterImplTest : public testing::Test { base::Value::Type expected_type, scoped_ptr<Value> expected_value) { scoped_ptr<Value> raw(converter.FromV8Value(val, context_)); - ASSERT_TRUE(raw.get()); - EXPECT_EQ(expected_type, raw->GetType()); - if (expected_value.get()) + + if (expected_value.get()) { + ASSERT_TRUE(raw.get()); EXPECT_TRUE(expected_value->Equals(raw.get())); + EXPECT_EQ(expected_type, raw->GetType()); + } else { + EXPECT_FALSE(raw.get()); + } v8::Handle<v8::Object> object(v8::Object::New()); object->Set(v8::String::New("test"), val); @@ -124,11 +130,14 @@ class V8ValueConverterImplTest : public testing::Test { converter.FromV8Value(object, context_))); ASSERT_TRUE(dictionary.get()); - Value* temp = NULL; - ASSERT_TRUE(dictionary->Get("test", &temp)); - EXPECT_EQ(expected_type, temp->GetType()); - if (expected_value.get()) + if (expected_value.get()) { + Value* temp = NULL; + ASSERT_TRUE(dictionary->Get("test", &temp)); + EXPECT_EQ(expected_type, temp->GetType()); EXPECT_TRUE(expected_value->Equals(temp)); + } else { + EXPECT_FALSE(dictionary->HasKey("test")); + } v8::Handle<v8::Array> array(v8::Array::New()); array->Set(0, val); @@ -136,10 +145,18 @@ class V8ValueConverterImplTest : public testing::Test { static_cast<ListValue*>( converter.FromV8Value(array, context_))); ASSERT_TRUE(list.get()); - ASSERT_TRUE(list->Get(0, &temp)); - EXPECT_EQ(expected_type, temp->GetType()); - if (expected_value.get()) + if (expected_value.get()) { + Value* temp = NULL; + ASSERT_TRUE(list->Get(0, &temp)); + EXPECT_EQ(expected_type, temp->GetType()); EXPECT_TRUE(expected_value->Equals(temp)); + } else { + // Arrays should preserve their length, and convert unconvertible + // types into null. + Value* temp = NULL; + ASSERT_TRUE(list->Get(0, &temp)); + EXPECT_EQ(Value::TYPE_NULL, temp->GetType()); + } } // Context for the JavaScript in the test. @@ -300,23 +317,29 @@ TEST_F(V8ValueConverterImplTest, WeirdTypes) { v8::RegExp::New(v8::String::New("."), v8::RegExp::kNone)); V8ValueConverterImpl converter; - TestWeirdType(converter, v8::Undefined(), Value::TYPE_NULL, - scoped_ptr<Value>(NULL)); - TestWeirdType(converter, v8::Date::New(1000), Value::TYPE_DICTIONARY, - scoped_ptr<Value>(NULL)); - TestWeirdType(converter, regex, Value::TYPE_DICTIONARY, - scoped_ptr<Value>(NULL)); - - converter.SetUndefinedAllowed(true); - TestWeirdType(converter, v8::Undefined(), Value::TYPE_NULL, + TestWeirdType(converter, + v8::Undefined(), + Value::TYPE_NULL, // Arbitrary type, result is NULL. scoped_ptr<Value>(NULL)); + TestWeirdType(converter, + v8::Date::New(1000), + Value::TYPE_DICTIONARY, + scoped_ptr<Value>(new DictionaryValue())); + TestWeirdType(converter, + regex, + Value::TYPE_DICTIONARY, + scoped_ptr<Value>(new DictionaryValue())); converter.SetDateAllowed(true); - TestWeirdType(converter, v8::Date::New(1000), Value::TYPE_DOUBLE, + TestWeirdType(converter, + v8::Date::New(1000), + Value::TYPE_DOUBLE, scoped_ptr<Value>(Value::CreateDoubleValue(1))); - converter.SetRegexpAllowed(true); - TestWeirdType(converter, regex, Value::TYPE_STRING, + converter.SetRegExpAllowed(true); + TestWeirdType(converter, + regex, + Value::TYPE_STRING, scoped_ptr<Value>(Value::CreateStringValue("/./"))); } @@ -353,7 +376,6 @@ TEST_F(V8ValueConverterImplTest, StripNullFromObjects) { ASSERT_FALSE(object.IsEmpty()); V8ValueConverterImpl converter; - converter.SetUndefinedAllowed(true); converter.SetStripNullFromObjects(true); scoped_ptr<DictionaryValue> result( @@ -485,3 +507,43 @@ TEST_F(V8ValueConverterImplTest, ArrayGetters) { ASSERT_TRUE(result.get()); EXPECT_EQ(2u, result->GetSize()); } + +TEST_F(V8ValueConverterImplTest, UndefinedValueBehavior) { + v8::Context::Scope context_scope(context_); + v8::HandleScope handle_scope; + + v8::Handle<v8::Object> object; + { + const char* source = "(function() {" + "return { foo: undefined, bar: null, baz: function(){} };" + "})();"; + v8::Handle<v8::Script> script(v8::Script::New(v8::String::New(source))); + object = script->Run().As<v8::Object>(); + ASSERT_FALSE(object.IsEmpty()); + } + + v8::Handle<v8::Array> array; + { + const char* source = "(function() {" + "return [ undefined, null, function(){} ];" + "})();"; + v8::Handle<v8::Script> script(v8::Script::New(v8::String::New(source))); + array = script->Run().As<v8::Array>(); + ASSERT_FALSE(array.IsEmpty()); + } + + V8ValueConverterImpl converter; + + DictionaryValue expected_object; + expected_object.Set("bar", Value::CreateNullValue()); + scoped_ptr<Value> actual_object(converter.FromV8Value(object, context_)); + EXPECT_TRUE(Value::Equals(&expected_object, actual_object.get())); + + ListValue expected_array; + // Everything is null because JSON stringification preserves array length. + expected_array.Append(Value::CreateNullValue()); + expected_array.Append(Value::CreateNullValue()); + expected_array.Append(Value::CreateNullValue()); + scoped_ptr<Value> actual_array(converter.FromV8Value(array, context_)); + EXPECT_TRUE(Value::Equals(&expected_array, actual_array.get())); +} |