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

ppapi/proxy/serialized_var.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 "ppapi/proxy/serialized_var.h"
 
 #include "base/logging.h"
 #include "ipc/ipc_message_utils.h"
 #include "ppapi/c/pp_instance.h"
 #include "ppapi/proxy/dispatcher.h"
 #include "ppapi/proxy/interface_proxy.h"
 #include "ppapi/proxy/ppapi_param_traits.h"
 #include "ppapi/proxy/ppb_buffer_proxy.h"
 #include "ppapi/shared_impl/ppapi_globals.h"
 #include "ppapi/shared_impl/var.h"
 #include "ppapi/thunk/enter.h"
 
+namespace {
+const char kSerializationError[] = "Failed to serialize a PP_Var. It may have "
+    "cycles or be of an unsupported type: ";
+}  // namespace
+
 namespace ppapi {
 namespace proxy {
 
 // SerializedVar::Inner --------------------------------------------------------
 
 SerializedVar::Inner::Inner()
     : serialization_rules_(NULL),
       var_(PP_MakeUndefined()),
       instance_(0),
       cleanup_mode_(CLEANUP_NONE) {
@@ skipping 72 matching lines @@
   // that returns a var. This means the message handler didn't write to the
   // output parameter, or possibly you used the wrong helper class
   // (normally SerializedVarReturnValue).
   DCHECK(serialization_rules_.get());
 
 #ifndef NDEBUG
   // We should only be serializing something once.
   DCHECK(!has_been_serialized_);
   has_been_serialized_ = true;
 #endif
-  RawVarDataGraph::Create(var_, instance_)->Write(m);
+  scoped_ptr<RawVarDataGraph> data =
+      RawVarDataGraph::Create(var_, instance_, false);
+  if (data) {
+    data->Write(m);
+  } else {
+    // TODO(raymes): It's not ideal to log here. We should probably propagate
+    // the error all the way through to the location at which the var is used.
+    // However, SerializedVar isn't set up to propagate errors well. This case
+    // should only occur if the the PP_Var passed in is somehow corrupt or a var
+    // with reference cycles is passed in.
+    PpapiGlobals::Get()->LogWithSource(
+        instance_, PP_LOGLEVEL_ERROR, std::string(),
+        kSerializationError + ppapi::Var::PPVarToLogString(var_));

[dmichael (off chromium), 2013/06/04 16:59:10]

Yeah...  this is going to result in sending a message while inside the
serialization for another message. It probably works fine, but it seems like
something we ought to try to avoid.

I wonder if we could make an invalid form of a serialized Var, so that the
recipient will receive the invalid Var, know something went wrong, and can take
appropriate action there (log and don't send a message, in the case of
Messaging). [This would be purely an implementation detail; we wouldn't expose
the invalid kind of var to plugins]

[raymes, 2013/06/04 19:36:06]

I actually went down this road first and then backtracked on it because it felt
a bit nasty as well :P But I do prefer it. I've changed it to do that.

+    RawVarDataGraph::Create(PP_MakeUndefined(), instance_, false)->Write(m);
+  }
 }
 
 bool SerializedVar::Inner::ReadFromMessage(const IPC::Message* m,
                                            PickleIterator* iter) {
 #ifndef NDEBUG
   // We should only deserialize something once or will end up with leaked
   // references.
   //
   // One place this has happened in the past is using
   // std::vector<SerializedVar>.resize(). If you're doing this manually instead
@@ skipping 306 matching lines @@
     const std::string& str) {
   inner_->ForceSetVarValueForTest(StringVar::StringToPPVar(str));
 }
 
 SerializedVarTestReader::SerializedVarTestReader(const SerializedVar& var)
     : SerializedVar(var) {
 }
 
 }  // namespace proxy
 }  // namespace ppapi