Use phantom handles instead of weak handles

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

4 files changed, 27 insertions, 11 deletions

diff --git a/fpdfsdk/include/javascript/JS_Object.h b/fpdfsdk/include/javascript/JS_Object.h
index 72cd9904f5..acbff840f1 100644
--- a/fpdfsdk/include/javascript/JS_Object.h
+++ b/fpdfsdk/include/javascript/JS_Object.h
@@ -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 "";};
diff --git a/fpdfsdk/include/jsapi/fxjs_v8.h b/fpdfsdk/include/jsapi/fxjs_v8.h
index a32ace674b..7f23380406 100644
--- a/fpdfsdk/include/jsapi/fxjs_v8.h
+++ b/fpdfsdk/include/jsapi/fxjs_v8.h
@@ -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);
diff --git a/fpdfsdk/src/javascript/JS_Object.cpp b/fpdfsdk/src/javascript/JS_Object.cpp
index 6c0c868e4c..66fd2b682f 100644
--- a/fpdfsdk/src/javascript/JS_Object.cpp
+++ b/fpdfsdk/src/javascript/JS_Object.cpp
@@ -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)
diff --git a/fpdfsdk/src/jsapi/fxjs_v8.cpp b/fpdfsdk/src/jsapi/fxjs_v8.cpp
index 7af8237c2e..59c99acdfe 100644
--- a/fpdfsdk/src/jsapi/fxjs_v8.cpp
+++ b/fpdfsdk/src/jsapi/fxjs_v8.cpp
@@ -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);
 }