Pass v8::Isolate to PDFium at init time.

Move the external isolate and embedder slot from the
IPDF_JSPlatforms struct supplied at the
FPDFDOC_InitFormFillEnvironment() call time to arguments to
the FPDF_InitLibraryWithConfig() call.

This has several benefits:
-- Avoids the crash that could happen if multiple
FPDFDOC_InitFormFillEnvironmen() calls should happen to be
made by an embedder with different slot values.
-- Down the road, for XFA, there may be XFA but no FormFill
environment.

We support both forms for the time being, until the chrome
side catches up, at which point we will deprecate the old
way.

R=thestig@chromium.org

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

9 files changed, 79 insertions, 48 deletions

diff --git a/fpdfsdk/include/javascript/JS_Runtime.h b/fpdfsdk/include/javascript/JS_Runtime.h
index 67ce9c0414..dd21e6d2f1 100644
--- a/fpdfsdk/include/javascript/JS_Runtime.h
+++ b/fpdfsdk/include/javascript/JS_Runtime.h
@@ -10,7 +10,6 @@
 #include <set>
 #include <utility>
 
-#include "../../../third_party/base/nonstd_unique_ptr.h"
 #include "../../../core/include/fxcrt/fx_basic.h"
 #include "../jsapi/fxjs_v8.h"
 #include "IJavaScript.h"
@@ -55,7 +54,6 @@ class CJS_Runtime : public IFXJS_Runtime {
   std::set<FieldEvent> m_FieldEventSet;
   v8::Isolate* m_isolate;
   bool m_isolateManaged;
-  nonstd::unique_ptr<FXJS_ArrayBufferAllocator> m_pArrayBufferAllocator;
   v8::Global<v8::Context> m_context;
 };
 
diff --git a/fpdfsdk/include/jsapi/fxjs_v8.h b/fpdfsdk/include/jsapi/fxjs_v8.h
index 60fcea6341..1705b894cf 100644
--- a/fpdfsdk/include/jsapi/fxjs_v8.h
+++ b/fpdfsdk/include/jsapi/fxjs_v8.h
@@ -62,9 +62,14 @@ using FXJS_CONSTRUCTOR = void (*)(IFXJS_Context* cc,
 using FXJS_DESTRUCTOR = void (*)(v8::Local<v8::Object> obj);
 
 // Call before making FXJS_PrepareIsolate call.
-void FXJS_Initialize(unsigned int embedderDataSlot);
+void FXJS_Initialize(unsigned int embedderDataSlot, v8::Isolate* pIsolate);
 void FXJS_Release();
 
+// Gets the global isolate set by FXJS_Initialize(), or makes a new one each
+// time if there is no such isolate. Returns true if a new isolate had to be
+// created.
+bool FXJS_GetIsolate(v8::Isolate** pResultIsolate);
+
 // Call before making FXJS_Define* calls. Resources allocated here are cleared
 // as part of FXJS_ReleaseRuntime().
 void FXJS_PrepareIsolate(v8::Isolate* pIsolate);
diff --git a/fpdfsdk/src/fpdfview.cpp b/fpdfsdk/src/fpdfview.cpp
index 3603af6d91..0bf6426b66 100644
--- a/fpdfsdk/src/fpdfview.cpp
+++ b/fpdfsdk/src/fpdfview.cpp
@@ -14,6 +14,7 @@
 #include "../include/fsdk_define.h"
 #include "../include/fsdk_mgr.h"
 #include "../include/fsdk_rendercontext.h"
+#include "../include/jsapi/fxjs_v8.h"
 
 CPDF_CustomAccess::CPDF_CustomAccess(FPDF_FILEACCESS* pFileAccess) {
   if (pFileAccess)
@@ -109,6 +110,10 @@ DLLEXPORT void STDCALL FPDF_InitLibraryWithConfig(
     pModuleMgr->LoadEmbeddedCNS1CMaps();
     pModuleMgr->LoadEmbeddedKorea1CMaps();
   }
+  if (cfg && cfg->version >= 2) {
+    FXJS_Initialize(cfg->m_v8EmbedderSlot,
+                    reinterpret_cast<v8::Isolate*>(cfg->m_pIsolate));
+  }
 }
 
 DLLEXPORT void STDCALL FPDF_DestroyLibrary() {
diff --git a/fpdfsdk/src/javascript/JS_Runtime.cpp b/fpdfsdk/src/javascript/JS_Runtime.cpp
index 1e1486df5c..5c463ce498 100644
--- a/fpdfsdk/src/javascript/JS_Runtime.cpp
+++ b/fpdfsdk/src/javascript/JS_Runtime.cpp
@@ -35,22 +35,17 @@ CJS_Runtime::CJS_Runtime(CPDFDoc_Environment* pApp)
       m_bBlocking(FALSE),
       m_isolate(NULL),
       m_isolateManaged(false) {
-  unsigned int embedderDataSlot = 0;
-  if (m_pApp->GetFormFillInfo()->m_pJsPlatform->version >= 2) {
-    m_isolate = reinterpret_cast<v8::Isolate*>(
-        m_pApp->GetFormFillInfo()->m_pJsPlatform->m_isolate);
-    embedderDataSlot = pApp->GetFormFillInfo()->m_pJsPlatform->m_v8EmbedderSlot;
-  }
-  if (!m_isolate) {
-    m_pArrayBufferAllocator.reset(new FXJS_ArrayBufferAllocator());
-
-    v8::Isolate::CreateParams params;
-    params.array_buffer_allocator = m_pArrayBufferAllocator.get();
-    m_isolate = v8::Isolate::New(params);
-    m_isolateManaged = true;
+  IPDF_JSPLATFORM* pPlatform = m_pApp->GetFormFillInfo()->m_pJsPlatform;
+  if (pPlatform->version <= 2) {
+    unsigned int embedderDataSlot = 0;
+    v8::Isolate* pExternalIsolate = nullptr;
+    if (pPlatform->version == 2) {
+      pExternalIsolate = reinterpret_cast<v8::Isolate*>(pPlatform->m_isolate);
+      embedderDataSlot = pPlatform->m_v8EmbedderSlot;
+    }
+    FXJS_Initialize(embedderDataSlot, pExternalIsolate);
   }
-
-  FXJS_Initialize(embedderDataSlot);
+  m_isolateManaged = FXJS_GetIsolate(&m_isolate);
   DefineJSObjects();
 
   CJS_Context* pContext = (CJS_Context*)NewContext();
diff --git a/fpdfsdk/src/jsapi/fxjs_v8.cpp b/fpdfsdk/src/jsapi/fxjs_v8.cpp
index 667132fde4..ff45a7a0ce 100644
--- a/fpdfsdk/src/jsapi/fxjs_v8.cpp
+++ b/fpdfsdk/src/jsapi/fxjs_v8.cpp
@@ -16,14 +16,14 @@ const wchar_t kFXJSValueNameFxobj[] = L"fxobj";
 const wchar_t kFXJSValueNameNull[] = L"null";
 const wchar_t kFXJSValueNameUndefined[] = L"undefined";
 
-static unsigned int g_embedderDataSlot = 1u;
-
 // Keep this consistent with the values defined in gin/public/context_holder.h
 // (without actually requiring a dependency on gin itself for the standalone
 // embedders of PDFIum). The value we want to use is:
 //   kPerContextDataStartIndex + kEmbedderPDFium, which is 3.
 static const unsigned int kPerContextDataIndex = 3u;
-
+static unsigned int g_embedderDataSlot = 1u;
+static v8::Isolate* g_isolate = nullptr;
+static FXJS_ArrayBufferAllocator* g_arrayBufferAllocator = nullptr;
 static v8::Global<v8::ObjectTemplate>* g_DefaultGlobalObjectTemplate = nullptr;
 
 class CFXJS_PrivateData {
@@ -131,6 +131,33 @@ void FXJS_ArrayBufferAllocator::Free(void* data, size_t length) {
   free(data);
 }
 
+void FXJS_Initialize(unsigned int embedderDataSlot, v8::Isolate* pIsolate) {
+  g_embedderDataSlot = embedderDataSlot;
+  g_isolate = pIsolate;
+}
+
+void FXJS_Release() {
+  g_DefaultGlobalObjectTemplate = nullptr;
+  g_isolate = nullptr;
+
+  delete g_arrayBufferAllocator;
+  g_arrayBufferAllocator = nullptr;
+}
+
+bool FXJS_GetIsolate(v8::Isolate** pResultIsolate) {
+  if (g_isolate) {
+    *pResultIsolate = g_isolate;
+    return false;
+  }
+  // Provide backwards compatibility when no external isolate.
+  if (!g_arrayBufferAllocator)
+    g_arrayBufferAllocator = new FXJS_ArrayBufferAllocator();
+  v8::Isolate::CreateParams params;
+  params.array_buffer_allocator = g_arrayBufferAllocator;
+  *pResultIsolate = v8::Isolate::New(params);
+  return true;
+}
+
 // static
 void FXJS_PerIsolateData::SetUp(v8::Isolate* pIsolate) {
   if (!pIsolate->GetData(g_embedderDataSlot))
@@ -143,13 +170,6 @@ FXJS_PerIsolateData* FXJS_PerIsolateData::Get(v8::Isolate* pIsolate) {
       pIsolate->GetData(g_embedderDataSlot));
 }
 
-void FXJS_Initialize(unsigned int embedderDataSlot) {
-  g_embedderDataSlot = embedderDataSlot;
-}
-
-void FXJS_Release() {
-}
-
 int FXJS_DefineObj(v8::Isolate* pIsolate,
                    const wchar_t* sObjName,
                    FXJSOBJTYPE eObjType,
diff --git a/fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp b/fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp
index 7726c40c5e..1c660397d1 100644
--- a/fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp
+++ b/fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp
@@ -30,7 +30,7 @@ class FXJSV8Embeddertest : public EmbedderTest {
 
     v8::Isolate::Scope isolate_scope(m_pIsolate);
     v8::HandleScope handle_scope(m_pIsolate);
-    FXJS_Initialize(0);
+    FXJS_Initialize(0, nullptr);
     FXJS_PerIsolateData::SetUp(m_pIsolate);
     FXJS_InitializeRuntime(m_pIsolate, nullptr, nullptr, m_pPersistentContext);
   }
diff --git a/public/fpdf_formfill.h b/public/fpdf_formfill.h
index 128c7ec818..baab18ab92 100644
--- a/public/fpdf_formfill.h
+++ b/public/fpdf_formfill.h
@@ -293,17 +293,12 @@ typedef struct _IPDF_JsPlatform {
 
   /* Version 2. */
 
-  /**
-  *   pointer to the v8::Isolate to use, or NULL to force PDFium to create one.
-  **/
-  void* m_isolate;
+  void* m_isolate;               /* Unused in v3, retain for compatibility. */
+  unsigned int m_v8EmbedderSlot; /* Unused in v3, retain for compatibility. */
+
+  /* Version 3. */
+  /* Version 3 moves m_Isolate and m_v8EmbedderSlot to FPDF_LIBRARY_CONFIG. */
 
-  /**
-   *   The embedder data slot to use in the v8::Isolate to store PDFium's
-   *   per-isolate data. The value needs to be between 0 and
-   *   v8::Internals::kNumIsolateDataLots (exclusive).
-   */
-  unsigned int m_v8EmbedderSlot;
 } IPDF_JSPLATFORM;
 
 // Flags for Cursor type
diff --git a/public/fpdfview.h b/public/fpdfview.h
index 9d199a1d2e..ea45c2912c 100644
--- a/public/fpdfview.h
+++ b/public/fpdfview.h
@@ -130,7 +130,7 @@ DLLEXPORT void STDCALL FPDF_InitLibrary();
 
 // Process-wide options for initializing library.
 typedef struct FPDF_LIBRARY_CONFIG_ {
-  // Version number of the interface. Currently must be 1.
+  // Version number of the interface. Currently must be 2.
   int version;
 
   // Array of paths to scan in place of the defaults when using built-in
@@ -138,6 +138,17 @@ typedef struct FPDF_LIBRARY_CONFIG_ {
   // The Array may be NULL itself to use the default paths. May be ignored
   // entirely depending upon the platform.
   const char** m_pUserFontPaths;
+
+  // Version 2.
+
+  // pointer to the v8::Isolate to use, or NULL to force PDFium to create one.
+  void* m_pIsolate;
+
+  // The embedder data slot to use in the v8::Isolate to store PDFium's
+  // per-isolate data. The value needs to be between 0 and
+  // v8::Internals::kNumIsolateDataLots (exclusive). Note that 0 is fine
+  // for most embedders.
+  unsigned int m_v8EmbedderSlot;
 } FPDF_LIBRARY_CONFIG;
 
 // Function: FPDF_InitLibraryWithConfig
diff --git a/samples/pdfium_test.cc b/samples/pdfium_test.cc
index 1d8a77a3ad..74e797b944 100644
--- a/samples/pdfium_test.cc
+++ b/samples/pdfium_test.cc
@@ -437,7 +437,7 @@ void RenderPdf(const std::string& name, const char* pBuf, size_t len,
 
   IPDF_JSPLATFORM platform_callbacks;
   memset(&platform_callbacks, '\0', sizeof(platform_callbacks));
-  platform_callbacks.version = 2;
+  platform_callbacks.version = 3;
   platform_callbacks.app_alert = ExampleAppAlert;
   platform_callbacks.Doc_gotoPage = ExampleDocGotoPage;
 
@@ -616,17 +616,19 @@ int main(int argc, const char* argv[]) {
   v8::V8::SetSnapshotDataBlob(&snapshot);
 #endif  // V8_USE_EXTERNAL_STARTUP_DATA
 
-  if (options.font_directory.empty()) {
-    FPDF_InitLibrary();
-  } else {
-    const char* path_array[2];
+  FPDF_LIBRARY_CONFIG config;
+  config.version = 2;
+  config.m_pUserFontPaths = nullptr;
+  config.m_pIsolate = nullptr;
+  config.m_v8EmbedderSlot = 0;
+
+  const char* path_array[2];
+  if (!options.font_directory.empty()) {
     path_array[0] = options.font_directory.c_str();
     path_array[1] = nullptr;
-    FPDF_LIBRARY_CONFIG config;
-    config.version = 1;
     config.m_pUserFontPaths = path_array;
-    FPDF_InitLibraryWithConfig(&config);
   }
+  FPDF_InitLibraryWithConfig(&config);
 
   UNSUPPORT_INFO unsuppored_info;
   memset(&unsuppored_info, '\0', sizeof(unsuppored_info));