Revert 157025 - Make V8ValueConverter.FromV8Value be more consistent with JSON.stringify: don't

serialize undefined as null, don't serialize functions as objects, omit values
from objects when they don't serialize, and insert null into arrays when they
don't serialize.

It is now possible for FromV8Value to return NULL; previously it would return
Value::CreateNullValue on failure.

This is needed for the Storage API, where we promise that the values that are
passed in are serialized as JSON, yet the value conversion doesn't work in a
way that allows it.

However, the null-stripping behavior needs to be configurable so that existing
extension APIs (which only expect null/undefined to appear for optional values)
still work.

BUG=145081

Review URL: https://codereview.chromium.org/10890002

TBR=kalman@chromium.org
Review URL: https://codereview.chromium.org/10911327

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@157031 0039d316-1c4b-4281-b951-d872f2087c98

14 files changed, 147 insertions, 283 deletions

diff --git a/chrome/common/extensions/api/context_menus.json b/chrome/common/extensions/api/context_menus.json
index 7bc03f8..5a333eb 100644
--- a/chrome/common/extensions/api/context_menus.json
+++ b/chrome/common/extensions/api/context_menus.json
@@ -77,7 +77,6 @@
       {
         "name": "create",
         "type": "function",
-        "allow_functions_in_objects": true,
         "description": "Creates a new context menu item. Note that if an error occurs during creation, you may not find out until the creation callback fires (the details will be in chrome.extension.lastError).",
         "returns": {
           "choices": [
@@ -178,7 +177,6 @@
       {
         "name": "update",
         "type": "function",
-        "allow_functions_in_objects": true,
         "description": "Updates a previously created context menu item.",
         "parameters": [
           {
diff --git a/chrome/renderer/extensions/event_bindings.cc b/chrome/renderer/extensions/event_bindings.cc
index 94258db..329a70b 100644
--- a/chrome/renderer/extensions/event_bindings.cc
+++ b/chrome/renderer/extensions/event_bindings.cc
@@ -191,8 +191,6 @@ 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 bba13bf..a5a8834 100644
--- a/chrome/renderer/extensions/send_request_natives.cc
+++ b/chrome/renderer/extensions/send_request_natives.cc
@@ -37,15 +37,14 @@ 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();
-  bool allow_functions_in_objects = args[6]->BooleanValue();
 
   scoped_ptr<V8ValueConverter> converter(V8ValueConverter::create());
 
-  if (!preserve_null_in_objects)
-    converter->SetStripNullFromObjects(true);
-  if (allow_functions_in_objects)
-    converter->SetFunctionAllowed(true);
+  // 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);
 
   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 fec5a98..3d98aea 100644
--- a/chrome/renderer/extensions/user_script_scheduler.cc
+++ b/chrome/renderer/extensions/user_script_scheduler.cc
@@ -203,6 +203,7 @@ 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);
@@ -222,10 +223,7 @@ void UserScriptScheduler::ExecuteCodeImpl(
       if (!script_value.IsEmpty()) {
         base::Value* base_val =
             v8_converter->FromV8Value(script_value, context);
-        // 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());
+        execution_results.Append(base_val);
         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 9bc788c..9e4016a 100644
--- a/chrome/renderer/resources/extensions/context_menus_custom_bindings.js
+++ b/chrome/renderer/resources/extensions/context_menus_custom_bindings.js
@@ -48,14 +48,10 @@ chromeHidden.registerCustomHook('contextMenus', function(bindingsAPI) {
     var args = arguments;
     var id = GetNextContextMenuId();
     args[0].generatedId = id;
-    var optArgs = {
-      customCallback: this.customCallback,
-      // Functions aren't usually serializable, but the browser needs to know
-      // if there's an onclick handler, so convert it to an object.
-      // TODO(kalman): write this custom binding in such a way that we don't.
-      allowFunctionsInObjects: true
-    };
-    sendRequest(this.name, args, this.definition.parameters, optArgs);
+    sendRequest(this.name,
+                args,
+                this.definition.parameters,
+                {customCallback: this.customCallback});
     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 321bcc5..ecf46f3 100644
--- a/chrome/renderer/resources/extensions/schema_generated_bindings.js
+++ b/chrome/renderer/resources/extensions/schema_generated_bindings.js
@@ -311,13 +311,9 @@
             if (this.handleRequest) {
               retval = this.handleRequest.apply(this, args);
             } else {
-              var optArgs = {
-                customCallback: this.customCallback,
-                allowFunctionsInObjects: functionDef.allow_functions_in_objects
-              };
               retval = sendRequest(this.name, args,
                                    this.definition.parameters,
-                                   optArgs);
+                                   {customCallback: this.customCallback});
             }
 
             // 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 dacf4af..1d7231f 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.
-// |optArgs| is an object with optional parameters as follows:
+// |opt_args| 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,15 +85,12 @@ function prepareRequest(args, argSchemas) {
 //   StartRequest if missing.
 // - forIOThread: true if this function should be handled on the browser IO
 //   thread.
-// - preserveNullInObjects: true if it is safe for null to be in objects.
-// - allowFunctionsInObjects: true if functions should be converted to
-//   empty objects (apart from the callback parameter!).
-function sendRequest(functionName, args, argSchemas, optArgs) {
-  if (!optArgs)
-    optArgs = {};
+function sendRequest(functionName, args, argSchemas, opt_args) {
+  if (!opt_args)
+    opt_args = {};
   var request = prepareRequest(args, argSchemas);
-  if (optArgs.customCallback) {
-    request.customCallback = optArgs.customCallback;
+  if (opt_args.customCallback) {
+    request.customCallback = opt_args.customCallback;
   }
   // JSON.stringify doesn't support a root object which is undefined.
   if (request.args === undefined)
@@ -102,23 +99,19 @@ function sendRequest(functionName, args, argSchemas, optArgs) {
   // TODO(asargent) - convert all optional native functions to accept raw
   // v8 values instead of expecting JSON strings.
   var doStringify = false;
-  if (optArgs.nativeFunction && !optArgs.noStringify)
+  if (opt_args.nativeFunction && !opt_args.noStringify)
     doStringify = true;
   var requestArgs = doStringify ?
       chromeHidden.JSON.stringify(request.args) : request.args;
-  var nativeFunction = optArgs.nativeFunction || natives.StartRequest;
+  var nativeFunction = opt_args.nativeFunction || natives.StartRequest;
 
   var requestId = natives.GetNextRequestId();
   request.id = requestId;
   requests[requestId] = request;
-  var hasCallback = request.callback || optArgs.customCallback;
-  return nativeFunction(functionName,
-                        requestArgs,
-                        requestId,
-                        hasCallback,
-                        optArgs.forIOThread,
-                        optArgs.preserveNullInObjects,
-                        optArgs.allowFunctionsInObjects);
+  var hasCallback =
+      (request.callback || opt_args.customCallback) ? true : false;
+  return nativeFunction(functionName, requestArgs, requestId, hasCallback,
+                        opt_args.forIOThread);
 }
 
 exports.sendRequest = sendRequest;
diff --git a/chrome/renderer/resources/extensions/storage_custom_bindings.js b/chrome/renderer/resources/extensions/storage_custom_bindings.js
index d230099..9b0ece5 100644
--- a/chrome/renderer/resources/extensions/storage_custom_bindings.js
+++ b/chrome/renderer/resources/extensions/storage_custom_bindings.js
@@ -33,8 +33,7 @@ chromeHidden.registerCustomType('storage.StorageArea', function() {
         return sendRequest(
             'storage.' + functionName,
             [namespace].concat(args),
-            extendSchema(funSchema.definition.parameters),
-            {preserveNullInObjects: true});
+            extendSchema(funSchema.definition.parameters));
       };
     }
     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 02fe318..0ac0b5e 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,10 +2,6 @@
 // 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,
@@ -23,15 +19,15 @@ chrome.test.runTests([
       this.get('foo', stage1.bind(this));
     }
     function stage1(settings) {
-      assertEq({}, settings);
+      chrome.test.assertEq({}, settings);
       this.get(['foo', 'bar'], stage2.bind(this));
     }
     function stage2(settings) {
-      assertEq({}, settings);
+      chrome.test.assertEq({}, settings);
       this.get(undefined, stage3.bind(this));
     }
     function stage3(settings) {
-      assertEq({}, settings);
+      chrome.test.assertEq({}, settings);
       this.succeed();
     }
     test(stage0);
@@ -49,21 +45,21 @@ chrome.test.runTests([
       this.get(['foo', 'baz'], stage2.bind(this));
     }
     function stage2(settings) {
-      assertEq({
+      chrome.test.assertEq({
         'foo': 'bar',
         'baz': 'qux'
       }, settings);
       this.get(['nothing', 'baz', 'hello', 'ignore'], stage3.bind(this));
     }
     function stage3(settings) {
-      assertEq({
+      chrome.test.assertEq({
         'baz'  : 'qux',
         'hello': 'world'
       }, settings);
       this.get(null, stage4.bind(this));
     }
     function stage4(settings) {
-      assertEq({
+      chrome.test.assertEq({
         'foo'  : 'bar',
         'baz'  : 'qux',
         'hello': 'world'
@@ -98,7 +94,7 @@ chrome.test.runTests([
       this.get(null, stage3.bind(this));
     }
     function stage3(settings) {
-      assertEq({
+      chrome.test.assertEq({
         'baz'  : 'qux',
         'hello': 'world'
       }, settings);
@@ -108,7 +104,7 @@ chrome.test.runTests([
       this.get(null, stage5.bind(this));
     }
     function stage5(settings) {
-      assertEq({
+      chrome.test.assertEq({
         'hello': 'world'
       }, settings);
       this.remove('hello', stage6.bind(this));
@@ -117,7 +113,7 @@ chrome.test.runTests([
       this.get(null, stage7.bind(this));
     }
     function stage7(settings) {
-      assertEq({}, settings);
+      chrome.test.assertEq({}, settings);
       this.succeed();
     }
     test(stage0);
@@ -141,7 +137,7 @@ chrome.test.runTests([
       this.get(null, stage3.bind(this));
     }
     function stage3(settings) {
-      assertEq({
+      chrome.test.assertEq({
         'foo'  : 'otherBar',
         'baz'  : 'otherQux',
         'hello': 'world'
@@ -156,7 +152,7 @@ chrome.test.runTests([
       this.get(null, stage5.bind(this));
     }
     function stage5(settings) {
-      assertEq({
+      chrome.test.assertEq({
         'foo'  : 'otherBar',
         'baz'  : 'anotherQux',
         'hello': 'otherWorld',
@@ -175,7 +171,7 @@ chrome.test.runTests([
       this.get(null, stage2.bind(this));
     }
     function stage2(settings) {
-      assertEq({}, settings);
+      chrome.test.assertEq({}, settings);
       this.succeed();
     }
     test(stage0);
@@ -196,7 +192,7 @@ chrome.test.runTests([
       this.get(null, stage3.bind(this));
     }
     function stage3(settings) {
-      assertEq({}, settings);
+      chrome.test.assertEq({}, settings);
       this.succeed();
     }
     test(stage0);
@@ -213,21 +209,21 @@ chrome.test.runTests([
       this.get(['foo.bar', 'one'], stage2.bind(this));
     }
     function stage2(settings) {
-      assertEq({
+      chrome.test.assertEq({
         'foo.bar' : 'baz',
         'one'     : {'two': 'three'}
       }, settings);
       this.get('one.two', stage3.bind(this));
     }
     function stage3(settings) {
-      assertEq({}, settings);
+      chrome.test.assertEq({}, settings);
       this.remove(['foo.bar', 'one.two'], stage4.bind(this));
     }
     function stage4() {
       this.get(null, stage5.bind(this));
     }
     function stage5(settings) {
-      assertEq({
+      chrome.test.assertEq({
         'one'     : {'two': 'three'}
       }, settings);
       this.succeed();
@@ -243,14 +239,14 @@ chrome.test.runTests([
       }, stage1.bind(this));
     }
     function stage1(settings) {
-      assertEq({
+      chrome.test.assertEq({
         'foo': 'defaultBar',
         'baz': [1, 2, 3]
       }, settings);
       this.get(null, stage2.bind(this));
     }
     function stage2(settings) {
-      assertEq({}, settings);
+      chrome.test.assertEq({}, settings);
       this.set({'foo': 'bar'}, stage3.bind(this));
     }
     function stage3() {
@@ -260,7 +256,7 @@ chrome.test.runTests([
       }, stage4.bind(this));
     }
     function stage4(settings) {
-      assertEq({
+      chrome.test.assertEq({
         'foo': 'bar',
         'baz': [1, 2, 3]
       }, settings);
@@ -273,7 +269,7 @@ chrome.test.runTests([
       }, stage6.bind(this));
     }
     function stage6(settings) {
-      assertEq({
+      chrome.test.assertEq({
         'foo': 'bar',
         'baz': {}
       }, settings);
@@ -286,7 +282,7 @@ chrome.test.runTests([
       }, stage8.bind(this));
     }
     function stage8(settings) {
-      assertEq({
+      chrome.test.assertEq({
         'foo': 'defaultBar',
         'baz': {}
       }, settings);
@@ -299,66 +295,38 @@ 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.
-    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.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.local.QUOTA_BYTES > 0);
-    assertEq('undefined', typeof chrome.storage.local.QUOTA_BYTES_PER_ITEM);
-    assertEq('undefined', typeof chrome.storage.local.MAX_ITEMS);
+    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);
 
     var area = chrome.storage.sync;
     function stage0() {
       area.getBytesInUse(null, stage1);
     }
     function stage1(bytesInUse) {
-      assertEq(0, bytesInUse);
+      chrome.test.assertEq(0, bytesInUse);
       area.set({ a: 42, b: 43, c: 44 }, stage2);
     }
     function stage2() {
       area.getBytesInUse(null, stage3);
     }
     function stage3(bytesInUse) {
-      assertEq(9, bytesInUse);
+      chrome.test.assertEq(9, bytesInUse);
       area.getBytesInUse('a', stage4);
     }
     function stage4(bytesInUse) {
-      assertEq(3, bytesInUse);
+      chrome.test.assertEq(3, bytesInUse);
       area.getBytesInUse(['a', 'b'], stage5);
     }
     function stage5(bytesInUse) {
-      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();
+      chrome.test.assertEq(6, bytesInUse);
+      chrome.test.succeed();
     }
     area.clear(stage0);
   },
@@ -398,5 +366,5 @@ chrome.test.runTests([
         }));
       }));
     }));
-  },
+  }
 ]);
diff --git a/content/public/renderer/v8_value_converter.h b/content/public/renderer/v8_value_converter.h
index b71b32d..be45ea9 100644
--- a/content/public/renderer/v8_value_converter.h
+++ b/content/public/renderer/v8_value_converter.h
@@ -28,48 +28,31 @@ class CONTENT_EXPORT V8ValueConverter {
 
   virtual ~V8ValueConverter() {}
 
-  // 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;
+  // Use the following setters to support additional types other than the
+  // default ones.
+  virtual bool GetUndefinedAllowed() const = 0;
+  virtual void SetUndefinedAllowed(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;
+  virtual bool GetDateAllowed() const = 0;
+  virtual void SetDateAllowed(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;
+  virtual bool GetRegexpAllowed() const = 0;
+  virtual void SetRegexpAllowed(bool val) = 0;
 
-  // If true, null values are stripped from objects. This is often useful when
-  // converting arguments to extension APIs.
+  // Gets/sets whether to treat undefined or null in objects as nonexistent.
+  virtual bool GetStripNullFromObjects() const = 0;
   virtual void SetStripNullFromObjects(bool val) = 0;
 
-  // 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.
+  // 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.
   virtual v8::Handle<v8::Value> ToV8Value(
       const base::Value* value,
       v8::Handle<v8::Context> context) const = 0;
 
-  // 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).
+  // 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.
   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 224cb16..8e473184 100644
--- a/content/renderer/render_view_impl.cc
+++ b/content/renderer/render_view_impl.cc
@@ -4219,6 +4219,7 @@ WebKit::WebPlugin* RenderViewImpl::CreatePlugin(
         pepper_module.get(), params, pepper_delegate_.AsWeakPtr());
   }
 
+
 #if defined(USE_AURA) && !defined(OS_WIN)
   return NULL;
 #else
@@ -4243,10 +4244,8 @@ void RenderViewImpl::EvaluateScript(const string16& frame_xpath,
       v8::Context::Scope context_scope(context);
       V8ValueConverterImpl converter;
       converter.SetDateAllowed(true);
-      converter.SetRegExpAllowed(true);
-      base::Value* result_value = converter.FromV8Value(result, context);
-      list.Set(0, result_value ? result_value :
-                                 base::Value::CreateNullValue());
+      converter.SetRegexpAllowed(true);
+      list.Set(0, converter.FromV8Value(result, context));
     } 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 4c2130d..8d55ecc 100644
--- a/content/renderer/v8_value_converter_impl.cc
+++ b/content/renderer/v8_value_converter_impl.cc
@@ -27,22 +27,38 @@ V8ValueConverter* V8ValueConverter::create() {
 }  // namespace content
 
 V8ValueConverterImpl::V8ValueConverterImpl()
-    : date_allowed_(false),
-      reg_exp_allowed_(false),
-      function_allowed_(false),
+    : undefined_allowed_(false),
+      date_allowed_(false),
+      regexp_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;
 }
 
-void V8ValueConverterImpl::SetRegExpAllowed(bool val) {
-  reg_exp_allowed_ = val;
+bool V8ValueConverterImpl::GetRegexpAllowed() const {
+  return regexp_allowed_;
+}
+
+void V8ValueConverterImpl::SetRegexpAllowed(bool val) {
+  regexp_allowed_ = val;
 }
 
-void V8ValueConverterImpl::SetFunctionAllowed(bool val) {
-  function_allowed_ = val;
+bool V8ValueConverterImpl::GetStripNullFromObjects() const {
+  return strip_null_from_objects_;
 }
 
 void V8ValueConverterImpl::SetStripNullFromObjects(bool val) {
@@ -184,37 +200,23 @@ Value* V8ValueConverterImpl::FromV8ValueImpl(v8::Handle<v8::Value> val,
     return Value::CreateStringValue(std::string(*utf8, utf8.length()));
   }
 
-  if (val->IsUndefined())
-    // JSON.stringify ignores undefined.
-    return NULL;
+  if (undefined_allowed_ && val->IsUndefined())
+    return Value::CreateNullValue();
 
-  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);
+  if (date_allowed_ && val->IsDate()) {
     v8::Date* date = v8::Date::Cast(*val);
     return Value::CreateDoubleValue(date->NumberValue() / 1000.0);
   }
 
-  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()));
+  if (regexp_allowed_ && val->IsRegExp()) {
+    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) {
@@ -223,9 +225,8 @@ Value* V8ValueConverterImpl::FromV8ValueImpl(v8::Handle<v8::Value> val,
       return FromV8Object(val->ToObject(), unique_set);
     }
   }
-
   LOG(ERROR) << "Unexpected v8 value type encountered.";
-  return NULL;
+  return Value::CreateNullValue();
 }
 
 Value* V8ValueConverterImpl::FromV8Array(v8::Handle<v8::Array> val,
@@ -257,12 +258,9 @@ Value* V8ValueConverterImpl::FromV8Array(v8::Handle<v8::Array> val,
       continue;
 
     Value* child = FromV8ValueImpl(child_v8, unique_set);
-    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());
+    CHECK(child);
+
+    result->Append(child);
   }
   return result;
 }
@@ -337,10 +335,7 @@ Value* V8ValueConverterImpl::FromV8Object(
     }
 
     scoped_ptr<Value> child(FromV8ValueImpl(child_v8, unique_set));
-    if (!child.get())
-      // JSON.stringify skips properties whose values don't serialize, for
-      // example undefined and functions. Emulate that behavior.
-      continue;
+    CHECK(child.get());
 
     // 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 c6b9511..baf061a 100644
--- a/content/renderer/v8_value_converter_impl.h
+++ b/content/renderer/v8_value_converter_impl.h
@@ -23,9 +23,13 @@ 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 void SetRegExpAllowed(bool val) OVERRIDE;
-  virtual void SetFunctionAllowed(bool val) OVERRIDE;
+  virtual bool GetRegexpAllowed() const OVERRIDE;
+  virtual void SetRegexpAllowed(bool val) OVERRIDE;
+  virtual bool GetStripNullFromObjects() const OVERRIDE;
   virtual void SetStripNullFromObjects(bool val) OVERRIDE;
   virtual v8::Handle<v8::Value> ToV8Value(
       const base::Value* value,
@@ -54,14 +58,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 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, we will convet RegExp JavaScript objects to string.
+  bool regexp_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 b4b12bd..9146fb3 100644
--- a/content/renderer/v8_value_converter_impl_unittest.cc
+++ b/content/renderer/v8_value_converter_impl_unittest.cc
@@ -10,8 +10,6 @@
 #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.
@@ -114,14 +112,10 @@ class V8ValueConverterImplTest : public testing::Test {
                      base::Value::Type expected_type,
                      scoped_ptr<Value> expected_value) {
     scoped_ptr<Value> raw(converter.FromV8Value(val, context_));
-
-    if (expected_value.get()) {
-      ASSERT_TRUE(raw.get());
+    ASSERT_TRUE(raw.get());
+    EXPECT_EQ(expected_type, raw->GetType());
+    if (expected_value.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);
@@ -130,14 +124,11 @@ class V8ValueConverterImplTest : public testing::Test {
             converter.FromV8Value(object, context_)));
     ASSERT_TRUE(dictionary.get());
 
-    if (expected_value.get()) {
-      Value* temp = NULL;
-      ASSERT_TRUE(dictionary->Get("test", &temp));
-      EXPECT_EQ(expected_type, temp->GetType());
+    Value* temp = NULL;
+    ASSERT_TRUE(dictionary->Get("test", &temp));
+    EXPECT_EQ(expected_type, temp->GetType());
+    if (expected_value.get())
       EXPECT_TRUE(expected_value->Equals(temp));
-    } else {
-      EXPECT_FALSE(dictionary->HasKey("test"));
-    }
 
     v8::Handle<v8::Array> array(v8::Array::New());
     array->Set(0, val);
@@ -145,18 +136,10 @@ class V8ValueConverterImplTest : public testing::Test {
         static_cast<ListValue*>(
             converter.FromV8Value(array, context_)));
     ASSERT_TRUE(list.get());
-    if (expected_value.get()) {
-      Value* temp = NULL;
-      ASSERT_TRUE(list->Get(0, &temp));
-      EXPECT_EQ(expected_type, temp->GetType());
+    ASSERT_TRUE(list->Get(0, &temp));
+    EXPECT_EQ(expected_type, temp->GetType());
+    if (expected_value.get())
       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.
@@ -317,29 +300,23 @@ TEST_F(V8ValueConverterImplTest, WeirdTypes) {
       v8::RegExp::New(v8::String::New("."), v8::RegExp::kNone));
 
   V8ValueConverterImpl converter;
-  TestWeirdType(converter,
-                v8::Undefined(),
-                Value::TYPE_NULL,  // Arbitrary type, result is NULL.
+  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,
                 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("/./")));
 }
 
@@ -376,6 +353,7 @@ TEST_F(V8ValueConverterImplTest, StripNullFromObjects) {
   ASSERT_FALSE(object.IsEmpty());
 
   V8ValueConverterImpl converter;
+  converter.SetUndefinedAllowed(true);
   converter.SetStripNullFromObjects(true);
 
   scoped_ptr<DictionaryValue> result(
@@ -507,43 +485,3 @@ 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()));
-}