Don't send PP_Vars/V8 values with cycles across PostMessage

webkit/plugins/ppapi/message_channel.cc

 // Copyright (c) 2012 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 "webkit/plugins/ppapi/message_channel.h"
 
 #include <cstdlib>
 #include <string>
 
 #include "base/bind.h"
@@ skipping 27 matching lines @@
 using WebKit::WebPluginContainer;
 using WebKit::WebSerializedScriptValue;
 
 namespace webkit {
 
 namespace ppapi {
 
 namespace {
 
 const char kPostMessage[] = "postMessage";
+const char kV8ToVarConversionError[] = "Failed to convert a PostMessage "
+    "argument from a JavaScript value to a PP_Var. It may have cycles or be of "
+    "an unsupported type.";
+const char kVarToV8ConversionError[] = "Failed to convert a PostMessage "
+    "argument from a PP_Var to a Javascript value. It may have cycles or be of "
+    "an unsupported type.";
 
 // Helper function to get the MessageChannel that is associated with an
 // NPObject*.
 MessageChannel* ToMessageChannel(NPObject* object) {
   return static_cast<MessageChannel::MessageChannelNPObject*>(object)->
       message_channel.get();
 }
 
 NPObject* ToPassThroughObject(NPObject* object) {
   MessageChannel* channel = ToMessageChannel(object);
@@ skipping 20 matching lines @@
       *result = PP_MakeInt32(NPVARIANT_TO_INT32(*variant));
       return true;
     case NPVariantType_Double:
       *result = PP_MakeDouble(NPVARIANT_TO_DOUBLE(*variant));
       return true;
     case NPVariantType_String:
       *result = StringVar::StringToPPVar(
           NPVARIANT_TO_STRING(*variant).UTF8Characters,
           NPVARIANT_TO_STRING(*variant).UTF8Length);
       return true;
-    case NPVariantType_Object:
-      V8VarConverter converter;
+    case NPVariantType_Object: {
       // Calling WebBindings::toV8Value creates a wrapper around NPVariant so it
       // shouldn't result in a deep copy.
-      return converter.FromV8Value(WebBindings::toV8Value(variant),
-                                   v8::Context::GetCurrent(), result);
+      v8::Handle<v8::Value> v8_value = WebBindings::toV8Value(variant);
+      if (!V8VarConverter::FromV8Value(v8_value, v8::Context::GetCurrent(),
+                                       result)) {
+        return false;
+      }
+      return true;
+    }
   }
   return false;
 }
 
 // Copy a PP_Var in to a PP_Var that is appropriate for sending via postMessage.
 // This currently just copies the value.  For a string Var, the result is a
 // PP_Var with the a copy of |var|'s string contents and a reference count of 1.
 PP_Var CopyPPVar(const PP_Var& var) {
   switch (var.type) {
     case PP_VARTYPE_UNDEFINED:
@@ skipping 71 matching lines @@
                           NPVariant* result) {
   if (!np_obj)
     return false;
 
   // We only handle a function called postMessage.
   if (IdentifierIsPostMessage(name) && (arg_count == 1)) {
     MessageChannel* message_channel = ToMessageChannel(np_obj);
     if (message_channel) {
       PP_Var argument = PP_MakeUndefined();
       if (!NPVariantToPPVar(&args[0], &argument)) {
-        NOTREACHED();
+        PpapiGlobals::Get()->LogWithSource(
+            message_channel->instance()->pp_instance(),
+            PP_LOGLEVEL_ERROR, std::string(), kV8ToVarConversionError);
         return false;
       }
       message_channel->PostMessageToNative(argument);
       PpapiGlobals::Get()->GetVarTracker()->ReleaseVar(argument);
       return true;
     } else {
       return false;
     }
   }
   // Other method calls we will pass to the passthrough object, if we have one.
@@ skipping 143 matching lines @@
   WebPluginContainer* container = instance_->container();
   // It's possible that container() is NULL if the plugin has been removed from
   // the DOM (but the PluginInstance is not destroyed yet).
   if (!container)
     return;
 
   v8::Local<v8::Context> context =
       container->element().document().frame()->mainWorldScriptContext();
   v8::Context::Scope context_scope(context);
 
-  v8::Local<v8::Value> v8_val;
-  V8VarConverter converter;
-  if (!converter.ToV8Value(message_data, context, &v8_val)) {
-    NOTREACHED();
+  v8::Handle<v8::Value> v8_val;
+  if (!V8VarConverter::ToV8Value(message_data, context, &v8_val)) {
+    PpapiGlobals::Get()->LogWithSource(instance_->pp_instance(),
+        PP_LOGLEVEL_ERROR, std::string(), kVarToV8ConversionError);
     return;
   }
 
   // This is for backward compatibility. It usually makes sense for us to return
   // a string object rather than a string primitive because it allows multiple
   // references to the same string (as with PP_Var strings). However, prior to
   // implementing dictionary and array, vars we would return a string primitive
   // here. Changing it to an object now will break existing code that uses
   // strict comparisons for strings returned from PostMessage. e.g. x === "123"
   // will no longer return true. So if the only value to return is a string
@@ skipping 133 matching lines @@
   // invokes:
   //   SetPassthroughObject(passthrough_object());
   if (passthrough_object_)
     WebBindings::releaseObject(passthrough_object_);
 
   passthrough_object_ = passthrough;
 }
 
 }  // namespace ppapi
 }  // namespace webkit