Avoid accessing VarTracker from multiple threads.

BUG=118223,118226
TEST=None


Review URL: http://codereview.chromium.org/9786001

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

3 files changed, 72 insertions, 10 deletions

diff --git a/ppapi/proxy/plugin_var_tracker.cc b/ppapi/proxy/plugin_var_tracker.cc
index 525b1a3..74cd557 100644
--- a/ppapi/proxy/plugin_var_tracker.cc
+++ b/ppapi/proxy/plugin_var_tracker.cc
@@ -38,6 +38,7 @@ PluginVarTracker::~PluginVarTracker() {
 
 PP_Var PluginVarTracker::ReceiveObjectPassRef(const PP_Var& host_var,
                                               PluginDispatcher* dispatcher) {
+  DCHECK(CalledOnValidThread());
   DCHECK(host_var.type == PP_VARTYPE_OBJECT);
 
   // Get the object.
@@ -63,6 +64,7 @@ PP_Var PluginVarTracker::ReceiveObjectPassRef(const PP_Var& host_var,
 PP_Var PluginVarTracker::TrackObjectWithNoReference(
     const PP_Var& host_var,
     PluginDispatcher* dispatcher) {
+  DCHECK(CalledOnValidThread());
   DCHECK(host_var.type == PP_VARTYPE_OBJECT);
 
   // Get the object.
@@ -80,7 +82,9 @@ PP_Var PluginVarTracker::TrackObjectWithNoReference(
 
 void PluginVarTracker::StopTrackingObjectWithNoReference(
     const PP_Var& plugin_var) {
+  DCHECK(CalledOnValidThread());
   DCHECK(plugin_var.type == PP_VARTYPE_OBJECT);
+
   VarMap::iterator found = GetLiveVar(plugin_var);
   if (found == live_vars_.end()) {
     NOTREACHED();
@@ -93,6 +97,8 @@ void PluginVarTracker::StopTrackingObjectWithNoReference(
 }
 
 PP_Var PluginVarTracker::GetHostObject(const PP_Var& plugin_object) const {
+  DCHECK(CalledOnValidThread());
+
   if (plugin_object.type != PP_VARTYPE_OBJECT) {
     NOTREACHED();
     return PP_MakeUndefined();
@@ -113,6 +119,8 @@ PP_Var PluginVarTracker::GetHostObject(const PP_Var& plugin_object) const {
 
 PluginDispatcher* PluginVarTracker::DispatcherForPluginObject(
     const PP_Var& plugin_object) const {
+  DCHECK(CalledOnValidThread());
+
   if (plugin_object.type != PP_VARTYPE_OBJECT)
     return NULL;
 
@@ -128,8 +136,10 @@ PluginDispatcher* PluginVarTracker::DispatcherForPluginObject(
 
 void PluginVarTracker::ReleaseHostObject(PluginDispatcher* dispatcher,
                                          const PP_Var& host_object) {
-  // Convert the host object to a normal var valid in the plugin.
+  DCHECK(CalledOnValidThread());
   DCHECK(host_object.type == PP_VARTYPE_OBJECT);
+
+  // Convert the host object to a normal var valid in the plugin.
   HostVarToPluginVarMap::iterator found = host_var_to_plugin_var_.find(
       HostVar(dispatcher, static_cast<int32>(host_object.value.as_id)));
   if (found == host_var_to_plugin_var_.end()) {
diff --git a/ppapi/proxy/serialized_var.cc b/ppapi/proxy/serialized_var.cc
index f090e92..e648406 100644
--- a/ppapi/proxy/serialized_var.cc
+++ b/ppapi/proxy/serialized_var.cc
@@ -50,8 +50,10 @@ SerializedVar::Inner::~Inner() {
   }
 }
 
-PP_Var SerializedVar::Inner::GetVar() const {
+PP_Var SerializedVar::Inner::GetVar() {
   DCHECK(serialization_rules_);
+
+  ConvertRawVarData();
   return var_;
 }
 
@@ -60,14 +62,16 @@ void SerializedVar::Inner::SetVar(PP_Var var) {
   // serialization rules pointer already.
   DCHECK(serialization_rules_);
   var_ = var;
+  raw_var_data_.reset(NULL);
 }
 
 void SerializedVar::Inner::ForceSetVarValueForTest(PP_Var value) {
   var_ = value;
+  raw_var_data_.reset(NULL);
 }
 
 void SerializedVar::Inner::WriteToMessage(IPC::Message* m) const {
-  // When writing to the IPC messages, a serization rules handler should
+  // When writing to the IPC messages, a serialization rules handler should
   // always have been set.
   //
   // When sending a message, it should be difficult to trigger this if you're
@@ -86,6 +90,7 @@ void SerializedVar::Inner::WriteToMessage(IPC::Message* m) const {
   has_been_serialized_ = true;
 #endif
 
+  DCHECK(!raw_var_data_.get());
   m->WriteInt(static_cast<int>(var_.type));
   switch (var_.type) {
     case PP_VARTYPE_UNDEFINED:
@@ -182,9 +187,11 @@ bool SerializedVar::Inner::ReadFromMessage(const IPC::Message* m,
       success = IPC::ParamTraits<double>::Read(m, iter, &var_.value.as_double);
       break;
     case PP_VARTYPE_STRING: {
-      std::string string_from_ipc;
-      success = m->ReadString(iter, &string_from_ipc);
-      var_ = StringVar::SwapValidatedUTF8StringIntoPPVar(&string_from_ipc);
+      raw_var_data_.reset(new RawVarData);
+      raw_var_data_->type = PP_VARTYPE_STRING;
+      success = m->ReadString(iter, &raw_var_data_->data);
+      if (!success)
+        raw_var_data_.reset(NULL);
       break;
     }
     case PP_VARTYPE_ARRAY_BUFFER: {
@@ -192,8 +199,9 @@ bool SerializedVar::Inner::ReadFromMessage(const IPC::Message* m,
       const char* message_bytes = NULL;
       success = m->ReadData(iter, &message_bytes, &length);
       if (success) {
-        var_ = PpapiGlobals::Get()->GetVarTracker()->MakeArrayBufferPPVar(
-            length, message_bytes);
+        raw_var_data_.reset(new RawVarData);
+        raw_var_data_->type = PP_VARTYPE_ARRAY_BUFFER;
+        raw_var_data_->data.assign(message_bytes, length);
       }
       break;
     }
@@ -213,7 +221,9 @@ bool SerializedVar::Inner::ReadFromMessage(const IPC::Message* m,
   // All success cases get here. We avoid writing the type above so that the
   // output param is untouched (defaults to VARTYPE_UNDEFINED) even in the
   // failure case.
-  if (success)
+  // We also don't write the type if |raw_var_data_| is set. |var_| will be
+  // updated lazily when GetVar() is called.
+  if (success && !raw_var_data_.get())
     var_.type = static_cast<PP_VarType>(type);
   return success;
 }
@@ -226,6 +236,28 @@ void SerializedVar::Inner::SetCleanupModeToEndReceiveCallerOwned() {
   cleanup_mode_ = END_RECEIVE_CALLER_OWNED;
 }
 
+void SerializedVar::Inner::ConvertRawVarData() {
+  if (!raw_var_data_.get())
+    return;
+
+  DCHECK_EQ(PP_VARTYPE_UNDEFINED, var_.type);
+  switch (raw_var_data_->type) {
+    case PP_VARTYPE_STRING: {
+      var_ = StringVar::SwapValidatedUTF8StringIntoPPVar(
+          &raw_var_data_->data);
+      break;
+    }
+    case PP_VARTYPE_ARRAY_BUFFER: {
+      var_ = PpapiGlobals::Get()->GetVarTracker()->MakeArrayBufferPPVar(
+          raw_var_data_->data.size(), raw_var_data_->data.data());
+      break;
+    }
+    default:
+      NOTREACHED();
+  }
+  raw_var_data_.reset(NULL);
+}
+
 // SerializedVar ---------------------------------------------------------------
 
 SerializedVar::SerializedVar() : inner_(new Inner) {
diff --git a/ppapi/proxy/serialized_var.h b/ppapi/proxy/serialized_var.h
index aec557d..3b74a3e 100644
--- a/ppapi/proxy/serialized_var.h
+++ b/ppapi/proxy/serialized_var.h
@@ -101,7 +101,7 @@ class PPAPI_PROXY_EXPORT SerializedVar {
     }
 
     // See outer class's declarations above.
-    PP_Var GetVar() const;
+    PP_Var GetVar();
     void SetVar(PP_Var var);
 
     // For the SerializedVarTestConstructor, this writes the Var value as if
@@ -128,6 +128,21 @@ class PPAPI_PROXY_EXPORT SerializedVar {
       END_RECEIVE_CALLER_OWNED
     };
 
+    // ReadFromMessage() may be called on the I/O thread, e.g., when reading the
+    // reply to a sync message. We cannot use the var tracker on the I/O thread,
+    // which means we cannot create PP_Var for PP_VARTYPE_STRING and
+    // PP_VARTYPE_ARRAY_BUFFER in ReadFromMessage(). So we save the raw var data
+    // and create PP_Var later when GetVar() is called, which should happen on
+    // the main thread.
+    struct RawVarData {
+      PP_VarType type;
+      std::string data;
+    };
+
+    // Converts |raw_var_data_| to |var_|. It is a no-op if |raw_var_data_| is
+    // NULL.
+    void ConvertRawVarData();
+
     // Rules for serializing and deserializing vars for this process type.
     // This may be NULL, but must be set before trying to serialize to IPC when
     // sending, or before converting back to a PP_Var when receiving.
@@ -152,6 +167,11 @@ class PPAPI_PROXY_EXPORT SerializedVar {
     mutable bool has_been_deserialized_;
 #endif
 
+    // It will be non-NULL if there is PP_VARTYPE_STRING or
+    // PP_VARTYPE_ARRAY_BUFFER data from ReadFromMessage() that hasn't been
+    // converted to PP_Var.
+    scoped_ptr<RawVarData> raw_var_data_;
+
     DISALLOW_COPY_AND_ASSIGN(Inner);
   };