Reland: Plugin Placeholders: Refactor for platforms that don't support plugins

gin/wrappable.h

Index: gin/wrappable.h
diff --git a/gin/wrappable.h b/gin/wrappable.h
index cd4d30e91552702650b8ec995bedaea1183f1145..67512e2239f4b1f028cffc4b2740aa5bc79ed977 100644
--- a/gin/wrappable.h
+++ b/gin/wrappable.h
@@ -5,6 +5,7 @@
 #ifndef GIN_WRAPPABLE_H_
 #define GIN_WRAPPABLE_H_
 
+#include "base/logging.h"
 #include "base/template_util.h"
 #include "gin/converter.h"
 #include "gin/gin_export.h"
@@ -12,15 +13,6 @@
 
 namespace gin {
 
-namespace internal {
-
-GIN_EXPORT void* FromV8Impl(v8::Isolate* isolate,
-                            v8::Local<v8::Value> val,
-                            WrapperInfo* info);
-
-}  // namespace internal
-
-
 // Wrappable is a base class for C++ objects that have corresponding v8 wrapper
 // objects. To retain a Wrappable object on the stack, use a gin::Handle.
 //
@@ -51,8 +43,6 @@ GIN_EXPORT void* FromV8Impl(v8::Isolate* isolate,
 // wrapper for the object. If clients fail to create a wrapper for a wrappable
 // object, the object will leak because we use the weak callback from the
 // wrapper as the signal to delete the wrapped object.
-template<typename T>
-class Wrappable;
 
 class ObjectTemplateBuilder;
 
@@ -65,7 +55,7 @@ class GIN_EXPORT WrappableBase {
   virtual ObjectTemplateBuilder GetObjectTemplateBuilder(v8::Isolate* isolate);
 
   v8::Local<v8::Object> GetWrapperImpl(v8::Isolate* isolate,
-                                        WrapperInfo* wrapper_info);
+                                       WrapperInfo* wrapper_info);
 
  private:
   static void FirstWeakCallback(
@@ -82,11 +72,16 @@ class GIN_EXPORT WrappableBase {
 template<typename T>
 class Wrappable : public WrappableBase {
  public:
+  // Subclasses derived from T that override GetObjectTemplateBuilder must
+  // also override this method. Otherwise, they will have the same WrapperInfo
+  // as T, and share a cached v8::ObjectTemplate. That will lead to crashes.
+  virtual WrapperInfo* GetWrapperInfo() { return &T::kWrapperInfo; }
+
   // Retrieve (or create) the v8 wrapper object cooresponding to this object.
   // To customize the wrapper created for a subclass, override GetWrapperInfo()
   // instead of overriding this function.
   v8::Local<v8::Object> GetWrapper(v8::Isolate* isolate) {
-    return GetWrapperImpl(isolate, &T::kWrapperInfo);
+    return GetWrapperImpl(isolate, GetWrapperInfo());
   }
 
  protected:
@@ -97,7 +92,6 @@ class Wrappable : public WrappableBase {
   DISALLOW_COPY_AND_ASSIGN(Wrappable);
 };
 
-
 // This converter handles any subclass of Wrappable.
 template<typename T>
 struct Converter<T*, typename base::enable_if<
@@ -107,9 +101,28 @@ struct Converter<T*, typename base::enable_if<
   }
 
   static bool FromV8(v8::Isolate* isolate, v8::Local<v8::Value> val, T** out) {

[tommycli, 2015/06/02 22:01:23]

These changes are inspired by reverting some changes of a year ago:
https://codereview.chromium.org/105743007/diff/100001/gin/wrappable.h

-    *out = static_cast<T*>(static_cast<WrappableBase*>(
-        internal::FromV8Impl(isolate, val, &T::kWrapperInfo)));
-    return *out != NULL;
+    if (!val->IsObject())
+      return false;
+    v8::Local<v8::Object> obj = v8::Local<v8::Object>::Cast(val);
+    WrapperInfo* info = WrapperInfo::From(obj);
+
+    // If this fails, the object is not managed by Gin. It is either a normal JS
+    // object that's not wrapping any external C++ object, or it is wrapping
+    // some C++ object, but that object isn't managed by Gin (maybe Blink).
+    if (!info)
+      return false;
+
+    // The stored pointer points to the WrappableBase portion of the object,
+    // so we must convert to WrappableBase first, then downcast, to correctly
+    // adjust the pointer to point to the the T object.
+    WrappableBase* pointer = static_cast<WrappableBase*>(
+        obj->GetAlignedPointerFromInternalField(kEncodedValueIndex));
+
+    // There is no way to detect if pointer is really of type T without RTTI.
+    // We cannot compare to &T::kWrapperInfo, as subclasses of T can override
+    // GetWrapperInfo(). This is unfortunate, but needed to support subclasses.

[tommycli, 2015/06/02 22:01:23]

I tried various methods for hours, but my conclusion is: We cannot
simultaneously support subclassing T and also be able to detect if the stored
pointer is really an object that derives from T, since GetWrapperInfo() can be
overridden.

This is lame, but I think most calls into this function are created via the
ObjectTemplateBuilder, which won't make this mistake...

+    *out = static_cast<T*>(pointer);
+    return true;
   }
 };