Add a new C++ pointer wrapping API to External to not dilute the

External::Cast.

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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@1578 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

include/v8.h

@@ -1141,20 +1141,25 @@ class V8EXPORT Function : public Object {
 
 
 /**
- * A JavaScript value that wraps a c++ void*.  This type of value is
- * mainly used to associate c++ data structures with JavaScript
+ * A JavaScript value that wraps a C++ void*.  This type of value is
+ * mainly used to associate C++ data structures with JavaScript
  * objects.
+ *
+ * The Wrap function V8 will return the most optimal Value object wrapping the
+ * C++ void*. The type of the value is not guaranteed to be an External object
+ * and no assumptions about its type should be made. To access the wrapped
+ * value Unwrap should be used, all other operations on that object will lead
+ * to unpredictable results.
  */
 class V8EXPORT External : public Value {
  public:
+  static Local<Value> Wrap(void* data);
+  static void* Unwrap(Handle<Value> obj);
+
   static Local<External> New(void* value);
   static External* Cast(Value* obj);
   void* Value() const;
  private:
-  enum {
-    kAlignedPointerMask = 3,
-    kAlignedPointerShift = 2
-  };
   External();
 };
 

src/api.cc

@@ -1447,7 +1447,7 @@ Local<Integer> Value::ToInteger() const {
 External* External::Cast(v8::Value* that) {
   if (IsDeadCheck("v8::External::Cast()")) return 0;
   i::Handle<i::Object> obj = Utils::OpenHandle(that);
-  ApiCheck(obj->IsProxy() || obj->IsSmi(),
+  ApiCheck(obj->IsProxy(),
            "v8::External::Cast()",
            "Could not convert to external");
   return static_cast<External*>(that);
@@ -2226,18 +2226,6 @@ int32_t Int32::Value() const {
 }
 
 
-void* External::Value() const {
-  if (IsDeadCheck("v8::External::Value()")) return 0;
-  i::Handle<i::Object> obj = Utils::OpenHandle(this);
-  if (obj->IsSmi()) {
-    // The external value was an aligned pointer.
-    return reinterpret_cast<void*>(
-        i::Smi::cast(*obj)->value() << kAlignedPointerShift);
-  }
-  return reinterpret_cast<void*>(i::Proxy::cast(*obj)->proxy());
-}
-
-
 int v8::Object::InternalFieldCount() {
   if (IsDeadCheck("v8::Object::InternalFieldCount()")) return 0;
   i::Handle<i::JSObject> obj = Utils::OpenHandle(this);
@@ -2468,18 +2456,58 @@ bool FunctionTemplate::HasInstance(v8::Handle<v8::Value> value) {
 }
 
 
-Local<External> v8::External::New(void* data) {
+static Local<External> ExternalNewImpl(void* data) {
+  return Utils::ToLocal(i::Factory::NewProxy(static_cast<i::Address>(data)));
+}
+
+static void* ExternalValueImpl(i::Handle<i::Object> obj) {
+  return reinterpret_cast<void*>(i::Proxy::cast(*obj)->proxy());
+}
+
+
+static const intptr_t kAlignedPointerMask = 3;
+static const int kAlignedPointerShift = 2;
+
+
+Local<Value> v8::External::Wrap(void* data) {
   STATIC_ASSERT(sizeof(data) == sizeof(i::Address));
-  LOG_API("External::New");
-  EnsureInitialized("v8::External::New()");
+  LOG_API("External::Wrap");
+  EnsureInitialized("v8::External::Wrap()");
   if ((reinterpret_cast<intptr_t>(data) & kAlignedPointerMask) == 0) {
     uintptr_t data_ptr = reinterpret_cast<uintptr_t>(data);
     int data_value = static_cast<int>(data_ptr >> kAlignedPointerShift);
     STATIC_ASSERT(sizeof(data_ptr) == sizeof(data_value));
-    i::Handle<i::Smi> obj(i::Smi::FromInt(data_value));
+    i::Handle<i::Object> obj(i::Smi::FromInt(data_value));
     return Utils::ToLocal(obj);
   }
-  return Utils::ToLocal(i::Factory::NewProxy(static_cast<i::Address>(data)));
+  return ExternalNewImpl(data);
+}
+
+
+void* v8::External::Unwrap(v8::Handle<v8::Value> value) {
+  if (IsDeadCheck("v8::External::Unwrap()")) return 0;
+  i::Handle<i::Object> obj = Utils::OpenHandle(*value);
+  if (obj->IsSmi()) {
+    // The external value was an aligned pointer.
+    uintptr_t result = i::Smi::cast(*obj)->value() << kAlignedPointerShift;
+    return reinterpret_cast<void*>(result);
+  }
+  return ExternalValueImpl(obj);
+}
+
+
+Local<External> v8::External::New(void* data) {
+  STATIC_ASSERT(sizeof(data) == sizeof(i::Address));
+  LOG_API("External::New");
+  EnsureInitialized("v8::External::New()");
+  return ExternalNewImpl(data);
+}
+
+
+void* External::Value() const {
+  if (IsDeadCheck("v8::External::Value()")) return 0;
+  i::Handle<i::Object> obj = Utils::OpenHandle(this);
+  return ExternalValueImpl(obj);
 }
 
 

src/api.h

@@ -181,8 +181,6 @@ class Utils {
       v8::internal::Handle<v8::internal::JSArray> obj);
   static inline Local<External> ToLocal(
       v8::internal::Handle<v8::internal::Proxy> obj);
-  static inline Local<External> ToLocal(
-      v8::internal::Handle<v8::internal::Smi> obj);
   static inline Local<Message> MessageToLocal(
       v8::internal::Handle<v8::internal::Object> obj);
   static inline Local<Number> NumberToLocal(
@@ -258,7 +256,6 @@ MAKE_TO_LOCAL(ToLocal, String, String)
 MAKE_TO_LOCAL(ToLocal, JSObject, Object)
 MAKE_TO_LOCAL(ToLocal, JSArray, Array)
 MAKE_TO_LOCAL(ToLocal, Proxy, External)
-MAKE_TO_LOCAL(ToLocal, Smi, External)
 MAKE_TO_LOCAL(ToLocal, FunctionTemplateInfo, FunctionTemplate)
 MAKE_TO_LOCAL(ToLocal, ObjectTemplateInfo, ObjectTemplate)
 MAKE_TO_LOCAL(ToLocal, SignatureInfo, Signature)

test/cctest/test-api.cc

@@ -1342,18 +1342,18 @@ THREADED_TEST(External) {
 
   // Make sure unaligned pointers are wrapped properly.
   char* data = i::StrDup("0123456789");
-  Local<v8::External> zero = v8::External::New(&data[0]);
-  Local<v8::External> one = v8::External::New(&data[1]);
-  Local<v8::External> two = v8::External::New(&data[2]);
-  Local<v8::External> three = v8::External::New(&data[3]);
+  Local<v8::Value> zero = v8::External::Wrap(&data[0]);
+  Local<v8::Value> one = v8::External::Wrap(&data[1]);
+  Local<v8::Value> two = v8::External::Wrap(&data[2]);
+  Local<v8::Value> three = v8::External::Wrap(&data[3]);
 
-  char* char_ptr = reinterpret_cast<char*>(zero->Value());
+  char* char_ptr = reinterpret_cast<char*>(v8::External::Unwrap(zero));
   CHECK_EQ('0', *char_ptr);
-  char_ptr = reinterpret_cast<char*>(one->Value());
+  char_ptr = reinterpret_cast<char*>(v8::External::Unwrap(one));
   CHECK_EQ('1', *char_ptr);
-  char_ptr = reinterpret_cast<char*>(two->Value());
+  char_ptr = reinterpret_cast<char*>(v8::External::Unwrap(two));
   CHECK_EQ('2', *char_ptr);
-  char_ptr = reinterpret_cast<char*>(three->Value());
+  char_ptr = reinterpret_cast<char*>(v8::External::Unwrap(three));
   CHECK_EQ('3', *char_ptr);
   i::DeleteArray(data);
 }