Proxy PPB_Var, fix o-o-p string var id tracking.

Note this doesn't need to use IPC at all, so it's a little strange.

Made test for pp::Var/PPB_Var that does only strings (copied from test_var_deprecated.cc).  Fixed string var tracking so test can pass out-of-process (aside from invalid UTF8 checking, which is still not implemented o-o-p).

BUG=85236
TEST=test_var.cc, run tests manually out-of-process.

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

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

8 files changed, 223 insertions, 59 deletions

diff --git a/ppapi/proxy/dispatcher.cc b/ppapi/proxy/dispatcher.cc
index 498aadc..8a1fa74 100644
--- a/ppapi/proxy/dispatcher.cc
+++ b/ppapi/proxy/dispatcher.cc
@@ -34,6 +34,7 @@
 #include "ppapi/c/ppb_url_loader.h"
 #include "ppapi/c/ppb_url_request_info.h"
 #include "ppapi/c/ppb_url_response_info.h"
+#include "ppapi/c/ppb_var.h"
 #include "ppapi/c/ppp_instance.h"
 #include "ppapi/c/private/ppb_flash.h"
 #include "ppapi/c/private/ppb_flash_clipboard.h"
@@ -77,6 +78,7 @@
 #include "ppapi/proxy/ppb_url_response_info_proxy.h"
 #include "ppapi/proxy/ppb_url_util_proxy.h"
 #include "ppapi/proxy/ppb_var_deprecated_proxy.h"
+#include "ppapi/proxy/ppb_var_proxy.h"
 #include "ppapi/proxy/ppp_class_proxy.h"
 #include "ppapi/proxy/ppp_graphics_3d_proxy.h"
 #include "ppapi/proxy/ppp_instance_private_proxy.h"
@@ -144,6 +146,7 @@ InterfaceList::InterfaceList() {
   AddPPB(PPB_URLResponseInfo_Proxy::GetInfo());
   AddPPB(PPB_URLUtil_Proxy::GetInfo());
   AddPPB(PPB_Var_Deprecated_Proxy::GetInfo());
+  AddPPB(PPB_Var_Proxy::GetInfo());
 
 #ifdef ENABLE_FLAPPER_HACKS
   AddPPB(PPB_Flash_NetConnector_Proxy::GetInfo());
diff --git a/ppapi/proxy/plugin_var_serialization_rules.cc b/ppapi/proxy/plugin_var_serialization_rules.cc
index a3d8231..8ba5574 100644
--- a/ppapi/proxy/plugin_var_serialization_rules.cc
+++ b/ppapi/proxy/plugin_var_serialization_rules.cc
@@ -24,10 +24,14 @@ PP_Var PluginVarSerializationRules::SendCallerOwned(const PP_Var& var,
   if (var.type == PP_VARTYPE_OBJECT)
     return var_tracker_->GetHostObject(var);
 
-  // Nothing to do since we manage the refcount, other than retrieve the string
-  // to use for IPC.
-  if (var.type == PP_VARTYPE_STRING)
-    *str_val = var_tracker_->GetString(var);
+  // Retrieve the string to use for IPC.
+  if (var.type == PP_VARTYPE_STRING) {
+    const std::string* var_string = var_tracker_->GetExistingString(var);
+    if (var_string)
+      *str_val = *var_string;
+    else
+      NOTREACHED() << "Trying to send unknown string over IPC.";
+  }
   return var;
 }
 
@@ -119,8 +123,13 @@ PP_Var PluginVarSerializationRules::BeginSendPassRef(const PP_Var& var,
   if (var.type == PP_VARTYPE_OBJECT)
     return var_tracker_->GetHostObject(var);
 
-  if (var.type == PP_VARTYPE_STRING)
-    *str_val = var_tracker_->GetString(var);
+  if (var.type == PP_VARTYPE_STRING) {
+    const std::string* var_string = var_tracker_->GetExistingString(var);
+    if (var_string)
+      *str_val = *var_string;
+    else
+      NOTREACHED() << "Trying to send unknown string over IPC.";
+  }
   return var;
 }
 
diff --git a/ppapi/proxy/plugin_var_tracker.cc b/ppapi/proxy/plugin_var_tracker.cc
index 151fd2b..5734f96 100644
--- a/ppapi/proxy/plugin_var_tracker.cc
+++ b/ppapi/proxy/plugin_var_tracker.cc
@@ -19,28 +19,6 @@ namespace {
 // When non-NULL, this object overrides the VarTrackerSingleton.
 PluginVarTracker* var_tracker_override = NULL;
 
-class RefCountedString : public base::RefCounted<RefCountedString> {
- public:
-  RefCountedString() {
-  }
-  RefCountedString(const std::string& str) : value_(str) {
-  }
-  RefCountedString(const char* data, size_t len)
-      : value_(data, len) {
-  }
-
-  const std::string& value() const { return value_; }
-
- private:
-  std::string value_;
-};
-
-// When running in the plugin, this will convert the string ID to the object
-// using casting. No validity checking is done.
-RefCountedString* PluginStringFromID(PluginVarTracker::VarID id) {
-  return reinterpret_cast<RefCountedString*>(static_cast<intptr_t>(id));
-}
-
 }  // namespace
 
 PluginVarTracker::HostVar::HostVar(PluginDispatcher* d, VarID i)
@@ -62,7 +40,7 @@ PluginVarTracker::PluginVarInfo::PluginVarInfo(const HostVar& host_var)
       track_with_no_reference_count(0) {
 }
 
-PluginVarTracker::PluginVarTracker() : last_plugin_object_id_(0) {
+PluginVarTracker::PluginVarTracker() : last_plugin_var_id_(0) {
 }
 
 PluginVarTracker::~PluginVarTracker() {
@@ -81,33 +59,45 @@ PluginVarTracker* PluginVarTracker::GetInstance() {
 }
 
 PluginVarTracker::VarID PluginVarTracker::MakeString(const std::string& str) {
-  RefCountedString* out = new RefCountedString(str);
-  out->AddRef();
-  return static_cast<VarID>(reinterpret_cast<intptr_t>(out));
+  return MakeString(str.c_str(), str.length());
 }
 
 PluginVarTracker::VarID PluginVarTracker::MakeString(const char* str,
                                                      uint32_t len) {
-  RefCountedString* out = new RefCountedString(str, len);
-  out->AddRef();
-  return static_cast<VarID>(reinterpret_cast<intptr_t>(out));
-}
-
-std::string PluginVarTracker::GetString(const PP_Var& var) const {
-  return PluginStringFromID(var.value.as_id)->value();
+  std::pair<VarIDStringMap::iterator, bool>
+      iter_success_pair(var_id_to_string_.end(), false);
+  VarID new_id(0);
+  RefCountedStringPtr str_ptr(new RefCountedString(str, len));
+  // Pick new IDs until one is successfully inserted. This loop is very unlikely
+  // to ever run a 2nd time, since we have ~2^63 possible IDs to exhaust.
+  while (!iter_success_pair.second) {
+    new_id = GetNewVarID();
+    iter_success_pair =
+        var_id_to_string_.insert(VarIDStringMap::value_type(new_id, str_ptr));
+  }
+  iter_success_pair.first->second->AddRef();
+  return new_id;
 }
 
 const std::string* PluginVarTracker::GetExistingString(
     const PP_Var& var) const {
   if (var.type != PP_VARTYPE_STRING)
     return NULL;
-  RefCountedString* str = PluginStringFromID(var.value.as_id);
-  return &str->value();
+  VarIDStringMap::const_iterator found =
+      var_id_to_string_.find(var.value.as_id);
+  if (found != var_id_to_string_.end())
+    return &found->second->value();
+  return NULL;
 }
 
 void PluginVarTracker::AddRef(const PP_Var& var) {
   if (var.type == PP_VARTYPE_STRING) {
-    PluginStringFromID(var.value.as_id)->AddRef();
+    VarIDStringMap::iterator found = var_id_to_string_.find(var.value.as_id);
+    if (found == var_id_to_string_.end()) {
+      NOTREACHED() << "Requesting to addref an unknown string.";
+      return;
+    }
+    found->second->AddRef();
   } else if (var.type == PP_VARTYPE_OBJECT && var.value.as_id != 0) {
     PluginVarInfoMap::iterator found = plugin_var_info_.find(var.value.as_id);
     if (found == plugin_var_info_.end()) {
@@ -131,7 +121,16 @@ void PluginVarTracker::AddRef(const PP_Var& var) {
 
 void PluginVarTracker::Release(const PP_Var& var) {
   if (var.type == PP_VARTYPE_STRING) {
-    PluginStringFromID(var.value.as_id)->Release();
+    VarIDStringMap::iterator found = var_id_to_string_.find(var.value.as_id);
+    if (found == var_id_to_string_.end()) {
+      NOTREACHED() << "Requesting to release an unknown string.";
+      return;
+    }
+    found->second->Release();
+    // If there is only 1 reference left, it's the map's reference. Erase it
+    // from the map, which will delete the string.
+    if (found->second->HasOneRef())
+      var_id_to_string_.erase(found);
   } else if (var.type == PP_VARTYPE_OBJECT) {
     PluginVarInfoMap::iterator found = plugin_var_info_.find(var.value.as_id);
     if (found == plugin_var_info_.end()) {
@@ -308,7 +307,7 @@ PluginVarTracker::FindOrMakePluginVarFromHostVar(const PP_Var& var,
   }
 
   // Make a new var, adding references to both maps.
-  VarID new_plugin_var_id = ++last_plugin_object_id_;
+  VarID new_plugin_var_id = GetNewVarID();
   host_var_to_plugin_var_[host_var] = new_plugin_var_id;
   return plugin_var_info_.insert(
       std::make_pair(new_plugin_var_id, PluginVarInfo(host_var))).first;
@@ -327,5 +326,11 @@ void PluginVarTracker::DeletePluginVarInfoIfNecessary(
   plugin_var_info_.erase(iter);
 }
 
+PluginVarTracker::VarID PluginVarTracker::GetNewVarID() {
+  if (last_plugin_var_id_ == std::numeric_limits<VarID>::max())
+    last_plugin_var_id_ = 0;
+  return ++last_plugin_var_id_;
+}
+
 }  // namesace proxy
 }  // namespace pp
diff --git a/ppapi/proxy/plugin_var_tracker.h b/ppapi/proxy/plugin_var_tracker.h
index 24f6818..1587736 100644
--- a/ppapi/proxy/plugin_var_tracker.h
+++ b/ppapi/proxy/plugin_var_tracker.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -8,6 +8,7 @@
 #include <map>
 #include <string>
 
+#include "base/memory/ref_counted.h"
 #include "ppapi/c/pp_stdint.h"
 #include "ppapi/c/pp_var.h"
 
@@ -51,10 +52,6 @@ class PluginVarTracker {
   VarID MakeString(const std::string& str);
   VarID MakeString(const char* str, uint32_t len);
 
-  // Returns the string associated with the given string var. The var must be
-  // of type string and must be valid or this function will crash.
-  std::string GetString(const PP_Var& plugin_var) const;
-
   // Returns a pointer to the given string if it exists, or NULL if the var
   // isn't a string var.
   const std::string* GetExistingString(const PP_Var& plugin_var) const;
@@ -93,6 +90,27 @@ class PluginVarTracker {
   friend struct DefaultSingletonTraits<PluginVarTracker>;
   friend class PluginProxyTest;
 
+  class RefCountedString : public base::RefCounted<RefCountedString> {
+   public:
+    RefCountedString() {
+    }
+    RefCountedString(const std::string& str) : value_(str) {
+    }
+    RefCountedString(const char* data, size_t len)
+        : value_(data, len) {
+    }
+
+    const std::string& value() const { return value_; }
+
+   private:
+    std::string value_;
+
+    // Ensure only base::RefCounted can delete a RefCountedString.
+    friend void base::RefCounted<RefCountedString>::Release() const;
+    virtual ~RefCountedString() {}
+  };
+  typedef scoped_refptr<RefCountedString> RefCountedStringPtr;
+
   // Represents a var as received from the host.
   struct HostVar {
     HostVar(PluginDispatcher* d, int64_t i);
@@ -160,9 +178,16 @@ class PluginVarTracker {
   typedef std::map<HostVar, VarID> HostVarToPluginVarMap;
   HostVarToPluginVarMap host_var_to_plugin_var_;
 
-  // The last plugin object ID we've handed out. This must be unique for the
+  // Maps plugin var IDs to ref counted strings.
+  typedef std::map<VarID, RefCountedStringPtr> VarIDStringMap;
+  VarIDStringMap var_id_to_string_;
+
+  // The last plugin PP_Var ID we've handed out. This must be unique for the
   // process.
-  VarID last_plugin_object_id_;
+  VarID last_plugin_var_id_;
+
+  // Get a new Var ID and increment last_plugin_var_id_.
+  VarID GetNewVarID();
 };
 
 }  // namespace proxy
diff --git a/ppapi/proxy/plugin_var_tracker_unittest.cc b/ppapi/proxy/plugin_var_tracker_unittest.cc
index 3f7e220..08fba9b 100644
--- a/ppapi/proxy/plugin_var_tracker_unittest.cc
+++ b/ppapi/proxy/plugin_var_tracker_unittest.cc
@@ -58,10 +58,11 @@ TEST_F(PluginVarTrackerTest, Strings) {
   EXPECT_NE(0, str_id2);
 
   // Make sure the strings come out the other end.
-  std::string result = var_tracker().GetString(MakeString(str_id1));
-  EXPECT_EQ(str, result);
-  result = var_tracker().GetString(MakeString(str_id2));
-  EXPECT_EQ(str, result);
+  const std::string* result =
+      var_tracker().GetExistingString(MakeString(str_id1));
+  EXPECT_EQ(str, *result);
+  result = var_tracker().GetExistingString(MakeString(str_id2));
+  EXPECT_EQ(str, *result);
 }
 
 TEST_F(PluginVarTrackerTest, GetHostObject) {
diff --git a/ppapi/proxy/ppb_var_deprecated_proxy.h b/ppapi/proxy/ppb_var_deprecated_proxy.h
index 17e3a53..1e6a70c 100644
--- a/ppapi/proxy/ppb_var_deprecated_proxy.h
+++ b/ppapi/proxy/ppb_var_deprecated_proxy.h
@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef PPAPI_PPB_VAR_PROXY_H_
-#define PPAPI_PPB_VAR_PROXY_H_
+#ifndef PPAPI_PPB_VAR_DEPRECATED_PROXY_H_
+#define PPAPI_PPB_VAR_DEPRECATED_PROXY_H_
 
 #include <vector>
 
@@ -33,7 +33,7 @@ class PPB_Var_Deprecated_Proxy : public InterfaceProxy {
   static const Info* GetInfo();
 
   const PPB_Var_Deprecated* ppb_var_target() const {
-    return reinterpret_cast<const PPB_Var_Deprecated*>(target_interface());
+    return static_cast<const PPB_Var_Deprecated*>(target_interface());
   }
 
   // InterfaceProxy implementation.
@@ -101,4 +101,4 @@ class PPB_Var_Deprecated_Proxy : public InterfaceProxy {
 }  // namespace proxy
 }  // namespace pp
 
-#endif  // PPAPI_PPB_VAR_PROXY_H_
+#endif  // PPAPI_PPB_VAR_DEPRECATED_PROXY_H_
diff --git a/ppapi/proxy/ppb_var_proxy.cc b/ppapi/proxy/ppb_var_proxy.cc
new file mode 100644
index 0000000..063d4d3
--- /dev/null
+++ b/ppapi/proxy/ppb_var_proxy.cc
@@ -0,0 +1,85 @@
+// Copyright (c) 2011 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/ppb_var_proxy.h"
+
+#include "ppapi/c/pp_var.h"
+#include "ppapi/c/ppb_var.h"
+#include "ppapi/proxy/plugin_var_tracker.h"
+
+namespace pp {
+namespace proxy {
+
+namespace {
+
+// PPP_Var plugin --------------------------------------------------------------
+
+void AddRefVar(PP_Var var) {
+  PluginVarTracker::GetInstance()->AddRef(var);
+}
+
+void ReleaseVar(PP_Var var) {
+  PluginVarTracker::GetInstance()->Release(var);
+}
+
+PP_Var VarFromUtf8(PP_Module module, const char* data, uint32_t len) {
+  PP_Var ret = {};
+  ret.type = PP_VARTYPE_STRING;
+  ret.value.as_id = PluginVarTracker::GetInstance()->MakeString(data, len);
+  return ret;
+}
+
+const char* VarToUtf8(PP_Var var, uint32_t* len) {
+  const std::string* str =
+      PluginVarTracker::GetInstance()->GetExistingString(var);
+  if (str) {
+    *len = static_cast<uint32_t>(str->size());
+    return str->c_str();
+  }
+  *len = 0;
+  return NULL;
+}
+
+const PPB_Var var_interface = {
+  &AddRefVar,
+  &ReleaseVar,
+  &VarFromUtf8,
+  &VarToUtf8
+};
+
+InterfaceProxy* CreateVarProxy(Dispatcher* dispatcher,
+                               const void* target_interface) {
+  return new PPB_Var_Proxy(dispatcher, target_interface);
+}
+
+}  // namespace
+
+PPB_Var_Proxy::PPB_Var_Proxy(Dispatcher* dispatcher,
+                             const void* target_interface)
+    : InterfaceProxy(dispatcher, target_interface) {
+}
+
+PPB_Var_Proxy::~PPB_Var_Proxy() {
+}
+
+// static
+const InterfaceProxy::Info* PPB_Var_Proxy::GetInfo() {
+  static const Info info = {
+    &var_interface,
+    PPB_VAR_INTERFACE,
+    INTERFACE_ID_PPB_VAR,
+    false,
+    &CreateVarProxy,
+  };
+  return &info;
+}
+
+bool PPB_Var_Proxy::OnMessageReceived(const IPC::Message& msg) {
+  // All PPB_Var calls are handled locally; there is no need to send or receive
+  // messages here.
+  return false;
+}
+
+}  // namespace proxy
+}  // namespace pp
diff --git a/ppapi/proxy/ppb_var_proxy.h b/ppapi/proxy/ppb_var_proxy.h
new file mode 100644
index 0000000..e527aab
--- /dev/null
+++ b/ppapi/proxy/ppb_var_proxy.h
@@ -0,0 +1,36 @@
+// Copyright (c) 2011 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.
+
+#ifndef PPAPI_PPB_VAR_PROXY_H_
+#define PPAPI_PPB_VAR_PROXY_H_
+
+#include "ppapi/proxy/interface_proxy.h"
+
+struct PPB_Var;
+
+namespace pp {
+namespace proxy {
+
+class PPB_Var_Proxy : public InterfaceProxy {
+ public:
+  PPB_Var_Proxy(Dispatcher* dispatcher,
+                const void* target_interface);
+  virtual ~PPB_Var_Proxy();
+
+  static const Info* GetInfo();
+
+  const PPB_Var* ppb_var_target() const {
+    return static_cast<const PPB_Var*>(target_interface());
+  }
+
+  // InterfaceProxy implementation. In this case, no messages are sent or
+  // received, so this always returns false.
+  virtual bool OnMessageReceived(const IPC::Message& msg);
+
+};
+
+}  // namespace proxy
+}  // namespace pp
+
+#endif  // PPAPI_PPB_VAR_PROXY_H_