commit e6113f28d68b72e280d5c23e7ce44906906b708d
author Tom Sepez <tsepez@chromium.org> Fri Mar 10 16:14:16 2023 +0000
committer Pdfium LUCI CQ <pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Mar 10 16:14:16 2023 +0000
tree 0eebcc163ce58c5ba2ecedadc82a9a80815333ca
parent 1c6ab7fa5324b91c17d216c03be795462f66cc59

Store global proxy object in CJS_Document

Avoid leaking actual global object back to V8 during callbacks. This
triggered a DCHECK() in some recent V8 versions.

Bug: chromium:1418955
Change-Id: Iad6173dcd2ac8dd49fb3d6c95825fa7fe9889edb
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/104630
Reviewed-by: Nigi <nigi@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>

fxjs/cfxjs_engine.cpp

diff --git a/fxjs/cfxjs_engine.cpp b/fxjs/cfxjs_engine.cpp
index 294452a..2ff6301 100644
--- a/fxjs/cfxjs_engine.cpp
+++ b/fxjs/cfxjs_engine.cpp
@@ -236,9 +236,11 @@
     return scope.Escape(m_Signature.Get(GetIsolate()));
   }
 
-  void RunConstructor(CFXJS_Engine* pEngine, v8::Local<v8::Object> obj) {
+  void RunConstructor(CFXJS_Engine* pEngine,
+                      v8::Local<v8::Object> obj,
+                      v8::Local<v8::Object> proxy) {
     if (m_pConstructor)
-      m_pConstructor(pEngine, obj);
+      m_pConstructor(pEngine, obj, proxy);
   }
 
   void RunDestructor(v8::Local<v8::Object> obj) {
@@ -522,15 +524,8 @@
   for (uint32_t i = 1; i <= maxID; ++i) {
     CFXJS_ObjDefinition* pObjDef = pIsolateData->ObjDefinitionForID(i);
     if (pObjDef->GetObjType() == FXJSOBJTYPE_GLOBAL) {
-      CFXJS_PerObjectData::SetInObject(new CFXJS_PerObjectData(i),
-                                       v8Context->Global()
-                                           ->GetPrototype()
-                                           ->ToObject(v8Context)
-                                           .ToLocalChecked());
-      pObjDef->RunConstructor(this, v8Context->Global()
-                                        ->GetPrototype()
-                                        ->ToObject(v8Context)
-                                        .ToLocalChecked());
+      CFXJS_PerObjectData::SetInObject(new CFXJS_PerObjectData(i), pThis);
+      pObjDef->RunConstructor(this, pThis, pThisProxy);
     } else if (pObjDef->GetObjType() == FXJSOBJTYPE_STATIC) {
       v8::Local<v8::String> pObjName = NewString(pObjDef->GetObjName());
       v8::Local<v8::Object> obj = NewFXJSBoundObject(i, FXJSOBJTYPE_STATIC);
@@ -625,7 +620,7 @@
 
   CFXJS_PerObjectData* pObjData = new CFXJS_PerObjectData(nObjDefnID);
   CFXJS_PerObjectData::SetInObject(pObjData, obj);
-  pObjDef->RunConstructor(this, obj);
+  pObjDef->RunConstructor(this, obj, obj);
   if (type == FXJSOBJTYPE_DYNAMIC) {
     auto* pIsolateData = FXJS_PerIsolateData::Get(GetIsolate());
     V8TemplateMap* pObjsMap = pIsolateData->GetDynamicObjsMap();

fxjs/cfxjs_engine.h

diff --git a/fxjs/cfxjs_engine.h b/fxjs/cfxjs_engine.h
index 3be3662..b0011fc 100644
--- a/fxjs/cfxjs_engine.h
+++ b/fxjs/cfxjs_engine.h
@@ -85,8 +85,9 @@
   explicit CFXJS_Engine(v8::Isolate* pIsolate);
   ~CFXJS_Engine() override;
 
-  using Constructor =
-      std::function<void(CFXJS_Engine* pEngine, v8::Local<v8::Object> obj)>;
+  using Constructor = std::function<void(CFXJS_Engine* pEngine,
+                                         v8::Local<v8::Object> obj,
+                                         v8::Local<v8::Object> proxy)>;
   using Destructor = std::function<void(v8::Local<v8::Object> obj)>;
 
   static uint32_t GetObjDefnID(v8::Local<v8::Object> pObj);

fxjs/cfxjs_engine_unittest.cpp

diff --git a/fxjs/cfxjs_engine_unittest.cpp b/fxjs/cfxjs_engine_unittest.cpp
index c6f01ed..8d6c36b 100644
--- a/fxjs/cfxjs_engine_unittest.cpp
+++ b/fxjs/cfxjs_engine_unittest.cpp
@@ -49,9 +49,10 @@
   // Object: 1
   engine()->DefineObj(
       "perm", FXJSOBJTYPE_DYNAMIC,
-      [](CFXJS_Engine* pEngine, v8::Local<v8::Object> obj) {
+      [](CFXJS_Engine* pEngine, v8::Local<v8::Object> obj,
+         v8::Local<v8::Object> proxy) {
         pEngine->SetObjectPrivate(obj,
-                                  std::make_unique<CJS_Object>(obj, nullptr));
+                                  std::make_unique<CJS_Object>(proxy, nullptr));
         perm_created = true;
       },
       [](v8::Local<v8::Object> obj) {
@@ -62,9 +63,10 @@
   // Object: 2
   engine()->DefineObj(
       "temp", FXJSOBJTYPE_DYNAMIC,
-      [](CFXJS_Engine* pEngine, v8::Local<v8::Object> obj) {
+      [](CFXJS_Engine* pEngine, v8::Local<v8::Object> obj,
+         v8::Local<v8::Object> proxy) {
         pEngine->SetObjectPrivate(obj,
-                                  std::make_unique<CJS_Object>(obj, nullptr));
+                                  std::make_unique<CJS_Object>(proxy, nullptr));
         temp_created = true;
       },
       [](v8::Local<v8::Object> obj) {

fxjs/js_define.h

diff --git a/fxjs/js_define.h b/fxjs/js_define.h
index 1228c10..5b6573a 100644
--- a/fxjs/js_define.h
+++ b/fxjs/js_define.h
@@ -42,9 +42,11 @@
 // to construct native object state.
 
 template <class T>
-static void JSConstructor(CFXJS_Engine* pEngine, v8::Local<v8::Object> obj) {
+static void JSConstructor(CFXJS_Engine* pEngine,
+                          v8::Local<v8::Object> obj,
+                          v8::Local<v8::Object> proxy) {
   pEngine->SetObjectPrivate(
-      obj, std::make_unique<T>(obj, static_cast<CJS_Runtime*>(pEngine)));
+      obj, std::make_unique<T>(proxy, static_cast<CJS_Runtime*>(pEngine)));
 }
 
 // CJS_Object has virtual dtor, template not required.