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

6 files changed, 109 insertions, 11 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);
   };
 
diff --git a/ppapi/shared_impl/var_tracker.cc b/ppapi/shared_impl/var_tracker.cc
index 9e1abe8..a564fed 100644
--- a/ppapi/shared_impl/var_tracker.cc
+++ b/ppapi/shared_impl/var_tracker.cc
@@ -33,10 +33,14 @@ VarTracker::~VarTracker() {
 }
 
 int32 VarTracker::AddVar(Var* var) {
+  DCHECK(CalledOnValidThread());
+
   return AddVarInternal(var, ADD_VAR_TAKE_ONE_REFERENCE);
 }
 
 Var* VarTracker::GetVar(int32 var_id) const {
+  DCHECK(CalledOnValidThread());
+
   VarMap::const_iterator result = live_vars_.find(var_id);
   if (result == live_vars_.end())
     return NULL;
@@ -44,12 +48,16 @@ Var* VarTracker::GetVar(int32 var_id) const {
 }
 
 Var* VarTracker::GetVar(const PP_Var& var) const {
+  DCHECK(CalledOnValidThread());
+
   if (!IsVarTypeRefcounted(var.type))
     return NULL;
   return GetVar(static_cast<int32>(var.value.as_id));
 }
 
 bool VarTracker::AddRefVar(int32 var_id) {
+  DCHECK(CalledOnValidThread());
+
   DLOG_IF(ERROR, !CheckIdType(var_id, PP_ID_TYPE_VAR))
       << var_id << " is not a PP_Var ID.";
   VarMap::iterator found = live_vars_.find(var_id);
@@ -73,12 +81,16 @@ bool VarTracker::AddRefVar(int32 var_id) {
 }
 
 bool VarTracker::AddRefVar(const PP_Var& var) {
+  DCHECK(CalledOnValidThread());
+
   if (!IsVarTypeRefcounted(var.type))
     return false;
   return AddRefVar(static_cast<int32>(var.value.as_id));
 }
 
 bool VarTracker::ReleaseVar(int32 var_id) {
+  DCHECK(CalledOnValidThread());
+
   DLOG_IF(ERROR, !CheckIdType(var_id, PP_ID_TYPE_VAR))
       << var_id << " is not a PP_Var ID.";
   VarMap::iterator found = live_vars_.find(var_id);
@@ -110,6 +122,8 @@ bool VarTracker::ReleaseVar(int32 var_id) {
 }
 
 bool VarTracker::ReleaseVar(const PP_Var& var) {
+  DCHECK(CalledOnValidThread());
+
   if (!IsVarTypeRefcounted(var.type))
     return false;
   return ReleaseVar(static_cast<int32>(var.value.as_id));
@@ -132,6 +146,8 @@ VarTracker::VarMap::iterator VarTracker::GetLiveVar(int32 id) {
 }
 
 int VarTracker::GetRefCountForObject(const PP_Var& plugin_object) {
+  DCHECK(CalledOnValidThread());
+
   VarMap::iterator found = GetLiveVar(plugin_object);
   if (found == live_vars_.end())
     return -1;
@@ -140,6 +156,8 @@ int VarTracker::GetRefCountForObject(const PP_Var& plugin_object) {
 
 int VarTracker::GetTrackedWithNoReferenceCountForObject(
     const PP_Var& plugin_object) {
+  DCHECK(CalledOnValidThread());
+
   VarMap::iterator found = GetLiveVar(plugin_object);
   if (found == live_vars_.end())
     return -1;
@@ -160,6 +178,8 @@ bool VarTracker::IsVarTypeRefcounted(PP_VarType type) const {
 }
 
 PP_Var VarTracker::MakeArrayBufferPPVar(uint32 size_in_bytes) {
+  DCHECK(CalledOnValidThread());
+
   scoped_refptr<ArrayBufferVar> array_buffer(CreateArrayBuffer(size_in_bytes));
   if (!array_buffer)
     return PP_MakeNull();
@@ -168,6 +188,8 @@ PP_Var VarTracker::MakeArrayBufferPPVar(uint32 size_in_bytes) {
 
 PP_Var VarTracker::MakeArrayBufferPPVar(uint32 size_in_bytes,
                                         const void* data) {
+  DCHECK(CalledOnValidThread());
+
   scoped_refptr<ArrayBufferVar> array_buffer(CreateArrayBuffer(size_in_bytes));
   if (!array_buffer)
     return PP_MakeNull();
@@ -176,6 +198,8 @@ PP_Var VarTracker::MakeArrayBufferPPVar(uint32 size_in_bytes,
 }
 
 std::vector<PP_Var> VarTracker::GetLiveVars() {
+  DCHECK(CalledOnValidThread());
+
   std::vector<PP_Var> var_vector;
   var_vector.reserve(live_vars_.size());
   for (VarMap::const_iterator iter = live_vars_.begin();
diff --git a/ppapi/shared_impl/var_tracker.h b/ppapi/shared_impl/var_tracker.h
index 575b710..4df2444 100644
--- a/ppapi/shared_impl/var_tracker.h
+++ b/ppapi/shared_impl/var_tracker.h
@@ -10,6 +10,7 @@
 #include "base/basictypes.h"
 #include "base/hash_tables.h"
 #include "base/memory/ref_counted.h"
+#include "base/threading/non_thread_safe.h"
 #include "ppapi/c/pp_module.h"
 #include "ppapi/c/pp_var.h"
 #include "ppapi/shared_impl/ppapi_shared_export.h"
@@ -31,7 +32,8 @@ class Var;
 // This class maintains the "track_with_no_reference_count" but doesn't do
 // anything with it other than call virtual functions. The interesting parts
 // are added by the PluginObjectVar derived from this class.
-class PPAPI_SHARED_EXPORT VarTracker {
+class PPAPI_SHARED_EXPORT VarTracker
+    : NON_EXPORTED_BASE(public base::NonThreadSafe) {
  public:
   VarTracker();
   virtual ~VarTracker();
diff --git a/webkit/plugins/ppapi/host_var_tracker.cc b/webkit/plugins/ppapi/host_var_tracker.cc
index 57bc92c..97db0fc 100644
--- a/webkit/plugins/ppapi/host_var_tracker.cc
+++ b/webkit/plugins/ppapi/host_var_tracker.cc
@@ -27,6 +27,8 @@ ArrayBufferVar* HostVarTracker::CreateArrayBuffer(uint32 size_in_bytes) {
 }
 
 void HostVarTracker::AddNPObjectVar(NPObjectVar* object_var) {
+  DCHECK(CalledOnValidThread());
+
   InstanceMap::iterator found_instance = instance_map_.find(
       object_var->pp_instance());
   if (found_instance == instance_map_.end()) {
@@ -46,6 +48,8 @@ void HostVarTracker::AddNPObjectVar(NPObjectVar* object_var) {
 }
 
 void HostVarTracker::RemoveNPObjectVar(NPObjectVar* object_var) {
+  DCHECK(CalledOnValidThread());
+
   InstanceMap::iterator found_instance = instance_map_.find(
       object_var->pp_instance());
   if (found_instance == instance_map_.end()) {
@@ -73,6 +77,8 @@ void HostVarTracker::RemoveNPObjectVar(NPObjectVar* object_var) {
 
 NPObjectVar* HostVarTracker::NPObjectVarForNPObject(PP_Instance instance,
                                                     NPObject* np_object) {
+  DCHECK(CalledOnValidThread());
+
   InstanceMap::iterator found_instance = instance_map_.find(instance);
   if (found_instance == instance_map_.end())
     return NULL;  // No such instance.
@@ -86,6 +92,8 @@ NPObjectVar* HostVarTracker::NPObjectVarForNPObject(PP_Instance instance,
 }
 
 int HostVarTracker::GetLiveNPObjectVarsForInstance(PP_Instance instance) const {
+  DCHECK(CalledOnValidThread());
+
   InstanceMap::const_iterator found = instance_map_.find(instance);
   if (found == instance_map_.end())
     return 0;
@@ -93,6 +101,8 @@ int HostVarTracker::GetLiveNPObjectVarsForInstance(PP_Instance instance) const {
 }
 
 void HostVarTracker::ForceFreeNPObjectsForInstance(PP_Instance instance) {
+  DCHECK(CalledOnValidThread());
+
   InstanceMap::iterator found_instance = instance_map_.find(instance);
   if (found_instance == instance_map_.end())
     return;  // Nothing to do.