Use phantom handles instead of weak handles
authorJochen Eisinger <jochen@chromium.org>
Thu, 14 May 2015 00:00:44 +0000 (02:00 +0200)
committerJochen Eisinger <jochen@chromium.org>
Thu, 14 May 2015 00:00:44 +0000 (02:00 +0200)
Phantom handles allow for freeing objects with one pass of GC. However,
this means that by the time the callback is invoked, the v8 object already
does no longer exist. To avoid accidential access to the dead object, there
are now two callbacks, where the first must only reset the handle, and the
second does the clean-up work.

R=tsepez@chromium.org
BUG=

Review URL: https://codereview.chromium.org/1129253004

fpdfsdk/include/javascript/JS_Object.h
fpdfsdk/include/jsapi/fxjs_v8.h
fpdfsdk/src/javascript/JS_Object.cpp
fpdfsdk/src/jsapi/fxjs_v8.cpp

index 72cd990..acbff84 100644 (file)
@@ -46,6 +46,7 @@ public:
        virtual ~CJS_Object(void);
 
        void                                            MakeWeak();
+        void                                            Dispose();
 
        virtual FX_BOOL                         IsType(FX_LPCSTR sClassName){return TRUE;};
        virtual CFX_ByteString          GetClassName(){return "";};
index a32ace6..7f23380 100644 (file)
@@ -82,6 +82,7 @@ void                                                  JS_SetPrivate(IJS_Runtime* pJSRuntime, v8::Handle<v8::Object> pObj, v
 void*                                                  JS_GetPrivate(IJS_Runtime* pJSRuntime, v8::Handle<v8::Object> pObj);
 void                                                   JS_SetPrivate(v8::Handle<v8::Object> pObj, void* p);
 void*                                                  JS_GetPrivate(v8::Handle<v8::Object> pObj);
+void                                                   JS_FreePrivate(void* p);
 void                                                   JS_FreePrivate(v8::Handle<v8::Object> pObj);
 v8::Handle<v8::Value>                  JS_GetObjectValue(v8::Handle<v8::Object> pObj);
 v8::Handle<v8::Value>                  JS_GetObjectElement(IJS_Runtime* pJSRuntime, v8::Handle<v8::Object> pObj,const wchar_t* PropertyName);
index 6c0c868..66fd2b6 100644 (file)
@@ -88,16 +88,19 @@ void CJS_EmbedObj::EndTimer(CJS_Timer* pTimer)
 }
 
 /* ---------------------------------  CJS_Object --------------------------------- */
-void  FreeObject(const v8::WeakCallbackData<v8::Object, CJS_Object>& data)
+void  FreeObject(const v8::WeakCallbackInfo<CJS_Object>& data)
 {
        CJS_Object* pJSObj  = data.GetParameter();
-       if(pJSObj)
-       {
-               pJSObj->ExitInstance();
-               delete pJSObj;
-       }
-       v8::Local<v8::Object> obj = data.GetValue();
-       JS_FreePrivate(obj);
+        pJSObj->ExitInstance();
+        delete pJSObj;
+       JS_FreePrivate(data.GetInternalField(0));
+}
+
+void  DisposeObject(const v8::WeakCallbackInfo<CJS_Object>& data)
+{
+       CJS_Object* pJSObj  = data.GetParameter();
+        pJSObj->Dispose();
+        data.SetSecondPassCallback(FreeObject);
 }
 
 CJS_Object::CJS_Object(JSFXObject pObject) :m_pEmbedObj(NULL)
@@ -117,7 +120,13 @@ CJS_Object::~CJS_Object(void)
 
 void   CJS_Object::MakeWeak()
 {
-       m_pObject.SetWeak(this, FreeObject);
+       m_pObject.SetWeak(
+            this, DisposeObject, v8::WeakCallbackType::kInternalFields);
+}
+
+void    CJS_Object::Dispose()
+{
+        m_pObject.Reset();
 }
 
 CPDFSDK_PageView* CJS_Object::JSGetPageView(IFXJS_Context* cc)
index 7af8237..59c99ac 100644 (file)
@@ -47,7 +47,7 @@ public:
                  v8::HandleScope handle_scope(isolate);
 
                  v8::Handle<v8::ObjectTemplate> objTemplate = v8::ObjectTemplate::New(isolate);
-                 objTemplate->SetInternalFieldCount(1);
+                 objTemplate->SetInternalFieldCount(2);
                  m_objTemplate.Reset(isolate, objTemplate);
 
                 //Document as the global object.
@@ -527,10 +527,15 @@ void* JS_GetPrivate(IJS_Runtime* pJSRuntime, v8::Handle<v8::Object> pObj)
        return pPrivateData->pPrivate;
 }
 
+void JS_FreePrivate(void* pPrivateData)
+{
+        delete (CJS_PrivateData*)pPrivateData;
+}
+
 void JS_FreePrivate(v8::Handle<v8::Object> pObj)
 {
        if(pObj.IsEmpty() || !pObj->InternalFieldCount()) return;
-       delete (CJS_PrivateData*)pObj->GetAlignedPointerFromInternalField(0);
+       JS_FreePrivate(pObj->GetAlignedPointerFromInternalField(0));
        pObj->SetAlignedPointerInInternalField(0, NULL);
 }