Introduce class EmbedderTestEnvironment
Then initialize the library once per binary execution rather than
once per test. Should a clean library initialization be required for
a specific test, then unload/reload the library in that specific test.
Consequently, isolate creation takes place in EmbedderTestEnvironment,
rather than in EmbedderTest.
This lays the groundwork for making some of the XFA tests faster,
but more importantly will allow for a split in CFGAS between a
per-module singleton and a GC'd font mgr down the road.
-- Make fx_win32_device_embeddertest a proper embedder test
Change-Id: I528005fbecd061453060ff33f077414020f5987f
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/73014
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fxge/fx_ge_text_embeddertest.cpp b/core/fxge/fx_ge_text_embeddertest.cpp
index 77d591b..89269c5 100644
--- a/core/fxge/fx_ge_text_embeddertest.cpp
+++ b/core/fxge/fx_ge_text_embeddertest.cpp
@@ -6,9 +6,21 @@
#include "public/cpp/fpdf_scopers.h"
#include "testing/embedder_test.h"
+#include "testing/embedder_test_environment.h"
#include "testing/gtest/include/gtest/gtest.h"
-class FXGETextEmbedderTest : public EmbedderTest {};
+class FXGETextEmbedderTest : public EmbedderTest {
+ public:
+ void TearDown() override {
+ EmbedderTest::TearDown();
+
+ // TODO(tsepez): determine how this is changing the environment,
+ // such that FPDFAnnotEmbedderTest.BUG_1206 will diff if run
+ // after this.
+ EmbedderTestEnvironment::GetInstance()->TearDown();
+ EmbedderTestEnvironment::GetInstance()->SetUp();
+ }
+};
TEST_F(FXGETextEmbedderTest, BadItalic) {
// Shouldn't crash.
diff --git a/core/fxge/win32/fx_win32_device_embeddertest.cpp b/core/fxge/win32/fx_win32_device_embeddertest.cpp
index bca965f..68e43ad 100644
--- a/core/fxge/win32/fx_win32_device_embeddertest.cpp
+++ b/core/fxge/win32/fx_win32_device_embeddertest.cpp
@@ -9,6 +9,7 @@
#include <memory>
#include "core/fxge/cfx_fillrenderoptions.h"
+#include "testing/embedder_test.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace {
@@ -17,13 +18,14 @@
} // namespace
-class CFX_WindowsRenderDeviceTest : public testing::Test {
+class CFX_WindowsRenderDeviceTest : public EmbedderTest {
public:
void SetUp() override {
+ EmbedderTest::SetUp();
+
// Get a device context with Windows GDI.
m_hDC = CreateCompatibleDC(nullptr);
ASSERT_TRUE(m_hDC);
- CFX_GEModule::Create(nullptr);
m_driver = std::make_unique<CFX_WindowsRenderDevice>(m_hDC, nullptr);
m_driver->SaveState();
}
@@ -31,8 +33,8 @@
void TearDown() override {
m_driver->RestoreState(false);
m_driver.reset();
- CFX_GEModule::Destroy();
DeleteDC(m_hDC);
+ EmbedderTest::TearDown();
}
protected:
diff --git a/fpdfsdk/fpdf_sysfontinfo_embeddertest.cpp b/fpdfsdk/fpdf_sysfontinfo_embeddertest.cpp
index e67ec55..e9c828d 100644
--- a/fpdfsdk/fpdf_sysfontinfo_embeddertest.cpp
+++ b/fpdfsdk/fpdf_sysfontinfo_embeddertest.cpp
@@ -7,6 +7,7 @@
#include <set>
#include "testing/embedder_test.h"
+#include "testing/embedder_test_environment.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/base/stl_util.h"
@@ -75,6 +76,15 @@
FPDF_SetSystemFontInfo(&font_info_);
}
+ void TearDown() override {
+ EmbedderTest::TearDown();
+
+ // Bouncing the library is the only reliable way to undo the
+ // FPDF_SetSystemFontInfo() call at the momemt.
+ EmbedderTestEnvironment::GetInstance()->TearDown();
+ EmbedderTestEnvironment::GetInstance()->SetUp();
+ }
+
FPDF_SYSFONTINFO font_info_;
};
@@ -92,7 +102,15 @@
void TearDown() override {
EmbedderTest::TearDown();
+
+ // Bouncing the library is the only reliable way to undo the
+ // FPDF_SetSystemFontInfo() call at the momemt.
+ EmbedderTestEnvironment::GetInstance()->TearDown();
+
+ // After shutdown, it is safe to release the font info.
FPDF_FreeDefaultSystemFontInfo(font_info_);
+
+ EmbedderTestEnvironment::GetInstance()->SetUp();
}
FPDF_SYSFONTINFO* font_info_;
diff --git a/fpdfsdk/fpdf_view_embeddertest.cpp b/fpdfsdk/fpdf_view_embeddertest.cpp
index a5acb83..563710a 100644
--- a/fpdfsdk/fpdf_view_embeddertest.cpp
+++ b/fpdfsdk/fpdf_view_embeddertest.cpp
@@ -16,6 +16,7 @@
#include "public/fpdfview.h"
#include "testing/embedder_test.h"
#include "testing/embedder_test_constants.h"
+#include "testing/embedder_test_environment.h"
#include "testing/fx_string_testhelpers.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "testing/utils/file_util.h"
@@ -312,7 +313,7 @@
}
TEST_F(FPDFViewEmbedderTest, MultipleInitDestroy) {
- FPDF_InitLibrary(); // Redundant given call in SetUp(), but safe.
+ FPDF_InitLibrary(); // Redundant given SetUp() in environment, but safe.
FPDF_InitLibrary(); // Doubly-redundant even, but safe.
ASSERT_TRUE(OpenDocument("about_blank.pdf"));
@@ -320,8 +321,11 @@
CloseDocument(); // Redundant given above, but safe.
CloseDocument(); // Doubly-redundant even, but safe.
- FPDF_DestroyLibrary(); // Doubly-redundant even, but safe.
- FPDF_DestroyLibrary(); // Redundant given call in TearDown(), but safe.
+ FPDF_DestroyLibrary(); // Doubly-Redundant even, but safe.
+ FPDF_DestroyLibrary(); // Redundant given call to TearDown(), but safe.
+
+ EmbedderTestEnvironment::GetInstance()->TearDown();
+ EmbedderTestEnvironment::GetInstance()->SetUp();
}
TEST_F(FPDFViewEmbedderTest, Document) {
diff --git a/testing/BUILD.gn b/testing/BUILD.gn
index f448f45..215a8ed 100644
--- a/testing/BUILD.gn
+++ b/testing/BUILD.gn
@@ -78,6 +78,7 @@
"v8_test_environment.h",
]
deps += [
+ "../fxjs",
"//v8",
"//v8:v8_libplatform",
]
@@ -130,6 +131,8 @@
"embedder_test.h",
"embedder_test_constants.cpp",
"embedder_test_constants.h",
+ "embedder_test_environment.cpp",
+ "embedder_test_environment.h",
"embedder_test_mock_delegate.h",
"embedder_test_timer_handling_delegate.h",
"fake_file_access.cpp",
diff --git a/testing/embedder_test.cpp b/testing/embedder_test.cpp
index ccfd40c..f910d6d 100644
--- a/testing/embedder_test.cpp
+++ b/testing/embedder_test.cpp
@@ -62,25 +62,11 @@
EmbedderTest::~EmbedderTest() = default;
void EmbedderTest::SetUp() {
- FPDF_LIBRARY_CONFIG config;
- config.version = 3;
- config.m_pUserFontPaths = nullptr;
- config.m_v8EmbedderSlot = 0;
- config.m_pIsolate = external_isolate_;
-#ifdef PDF_ENABLE_V8
- config.m_pPlatform = V8TestEnvironment::GetInstance()->platform();
-#else // PDF_ENABLE_V8
- config.m_pPlatform = nullptr;
-#endif // PDF_ENABLE_V8
-
- FPDF_InitLibraryWithConfig(&config);
-
UNSUPPORT_INFO* info = static_cast<UNSUPPORT_INFO*>(this);
memset(info, 0, sizeof(UNSUPPORT_INFO));
info->version = 1;
info->FSDK_UnSupport_Handler = UnsupportedHandlerTrampoline;
FSDK_SetUnSpObjProcessHandler(info);
-
saved_document_ = nullptr;
}
@@ -89,21 +75,13 @@
// possible. This can fail when an ASSERT test fails in a test case.
EXPECT_EQ(0U, page_map_.size());
EXPECT_EQ(0U, saved_page_map_.size());
-
if (document_)
CloseDocument();
FPDFAvail_Destroy(avail_);
- FPDF_DestroyLibrary();
loader_.reset();
}
-#ifdef PDF_ENABLE_V8
-void EmbedderTest::SetExternalIsolate(void* isolate) {
- external_isolate_ = static_cast<v8::Isolate*>(isolate);
-}
-#endif // PDF_ENABLE_V8
-
bool EmbedderTest::CreateEmptyDocument() {
document_ = FPDF_CreateNewDocument();
if (!document_)
@@ -244,9 +222,8 @@
JavaScriptOption javascript_option) {
IPDF_JSPLATFORM* platform = static_cast<IPDF_JSPLATFORM*>(this);
memset(platform, '\0', sizeof(IPDF_JSPLATFORM));
- platform->version = 2;
+ platform->version = 3;
platform->app_alert = AlertTrampoline;
- platform->m_isolate = external_isolate_;
FPDF_FORMFILLINFO* formfillinfo = static_cast<FPDF_FORMFILLINFO*>(this);
memset(formfillinfo, 0, sizeof(FPDF_FORMFILLINFO));
diff --git a/testing/embedder_test.h b/testing/embedder_test.h
index a1fbac8..2242355 100644
--- a/testing/embedder_test.h
+++ b/testing/embedder_test.h
@@ -93,11 +93,6 @@
void SetUp() override;
void TearDown() override;
-#ifdef PDF_ENABLE_V8
- // Call before SetUp to pass shared isolate, otherwise PDFium creates one.
- void SetExternalIsolate(void* isolate);
-#endif // PDF_ENABLE_V8
-
void SetDelegate(Delegate* delegate) {
delegate_ = delegate ? delegate : default_delegate_.get();
}
@@ -285,7 +280,6 @@
FPDF_FILEACCESS file_access_; // must outlive |avail_|.
std::unique_ptr<FakeFileAccess> fake_file_access_; // must outlive |avail_|.
- void* external_isolate_ = nullptr;
std::unique_ptr<TestLoader> loader_;
size_t file_length_ = 0;
std::unique_ptr<char, pdfium::FreeDeleter> file_contents_;
diff --git a/testing/embedder_test_environment.cpp b/testing/embedder_test_environment.cpp
new file mode 100644
index 0000000..0b10a93
--- /dev/null
+++ b/testing/embedder_test_environment.cpp
@@ -0,0 +1,54 @@
+// Copyright 2020 PDFium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "testing/embedder_test_environment.h"
+
+#include "core/fxcrt/fx_system.h"
+#include "public/fpdfview.h"
+
+#ifdef PDF_ENABLE_V8
+#include "testing/v8_test_environment.h"
+#endif // PDF_ENABLE_V8
+
+namespace {
+
+EmbedderTestEnvironment* g_environment = nullptr;
+
+} // namespace
+
+EmbedderTestEnvironment::EmbedderTestEnvironment() {
+ ASSERT(!g_environment);
+ g_environment = this;
+}
+
+EmbedderTestEnvironment::~EmbedderTestEnvironment() {
+ ASSERT(g_environment);
+ g_environment = nullptr;
+}
+
+// static
+EmbedderTestEnvironment* EmbedderTestEnvironment::GetInstance() {
+ return g_environment;
+}
+
+void EmbedderTestEnvironment::SetUp() {
+ FPDF_LIBRARY_CONFIG config;
+ config.version = 3;
+ config.m_pUserFontPaths = nullptr;
+ config.m_v8EmbedderSlot = 0;
+ config.m_pPlatform = nullptr;
+#ifdef PDF_ENABLE_V8
+ config.m_pIsolate = V8TestEnvironment::GetInstance()->isolate();
+ config.m_pPlatform = V8TestEnvironment::GetInstance()->platform();
+#else // PDF_ENABLE_V8
+ config.m_pIsolate = nullptr;
+ config.m_pPlatform = nullptr;
+#endif // PDF_ENABLE_V8
+
+ FPDF_InitLibraryWithConfig(&config);
+}
+
+void EmbedderTestEnvironment::TearDown() {
+ FPDF_DestroyLibrary();
+}
diff --git a/testing/embedder_test_environment.h b/testing/embedder_test_environment.h
new file mode 100644
index 0000000..19e0808
--- /dev/null
+++ b/testing/embedder_test_environment.h
@@ -0,0 +1,25 @@
+// Copyright 2020 PDFium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef TESTING_EMBEDDER_TEST_ENVIRONMENT_H_
+#define TESTING_EMBEDDER_TEST_ENVIRONMENT_H_
+
+#include "public/fpdfview.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+class EmbedderTestEnvironment : public testing::Environment {
+ public:
+ EmbedderTestEnvironment();
+ ~EmbedderTestEnvironment() override;
+
+ // Note: GetInstance() does not create one if it does not exist,
+ // so the main program must explicitly add this enviroment.
+ static EmbedderTestEnvironment* GetInstance();
+
+ // testing::Environment:
+ void SetUp() override;
+ void TearDown() override;
+};
+
+#endif // TESTING_EMBEDDER_TEST_ENVIRONMENT_H_
diff --git a/testing/embedder_test_main.cpp b/testing/embedder_test_main.cpp
index 798c9cf..193ee6e 100644
--- a/testing/embedder_test_main.cpp
+++ b/testing/embedder_test_main.cpp
@@ -4,6 +4,7 @@
#include "build/build_config.h"
#include "core/fxcrt/fx_memory.h"
+#include "testing/embedder_test_environment.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
@@ -22,6 +23,9 @@
AddGlobalTestEnvironment(new V8TestEnvironment(argv[0]));
#endif // PDF_ENABLE_V8
+ // The env will be deleted by gtest.
+ AddGlobalTestEnvironment(new EmbedderTestEnvironment);
+
testing::InitGoogleTest(&argc, argv);
testing::InitGoogleMock(&argc, argv);
diff --git a/testing/external_engine_embedder_test.cpp b/testing/external_engine_embedder_test.cpp
index d9dfbbd..9f9a84b 100644
--- a/testing/external_engine_embedder_test.cpp
+++ b/testing/external_engine_embedder_test.cpp
@@ -5,13 +5,14 @@
#include "testing/external_engine_embedder_test.h"
#include "fxjs/cfxjs_engine.h"
+#include "testing/v8_test_environment.h"
ExternalEngineEmbedderTest::ExternalEngineEmbedderTest() = default;
ExternalEngineEmbedderTest::~ExternalEngineEmbedderTest() = default;
void ExternalEngineEmbedderTest::SetUp() {
- JSEmbedderTest::SetUp();
+ EmbedderTest::SetUp();
v8::Isolate::Scope isolate_scope(isolate());
v8::HandleScope handle_scope(isolate());
diff --git a/testing/js_embedder_test.cpp b/testing/js_embedder_test.cpp
index e0a4881..b11f315 100644
--- a/testing/js_embedder_test.cpp
+++ b/testing/js_embedder_test.cpp
@@ -4,24 +4,12 @@
#include "testing/js_embedder_test.h"
-#include "fxjs/cfxjs_engine.h"
+#include "testing/v8_test_environment.h"
-JSEmbedderTest::JSEmbedderTest()
- : m_pArrayBufferAllocator(std::make_unique<CFX_V8ArrayBufferAllocator>()) {}
+JSEmbedderTest::JSEmbedderTest() = default;
JSEmbedderTest::~JSEmbedderTest() = default;
-void JSEmbedderTest::SetUp() {
- v8::Isolate::CreateParams params;
- params.array_buffer_allocator = m_pArrayBufferAllocator.get();
- m_pIsolate.reset(v8::Isolate::New(params));
-
- EmbedderTest::SetExternalIsolate(m_pIsolate.get());
- EmbedderTest::SetUp();
-}
-
-void JSEmbedderTest::TearDown() {
- EmbedderTest::TearDown();
- EmbedderTest::SetExternalIsolate(nullptr);
- m_pIsolate.reset();
+v8::Isolate* JSEmbedderTest::isolate() const {
+ return V8TestEnvironment::GetInstance()->isolate();
}
diff --git a/testing/js_embedder_test.h b/testing/js_embedder_test.h
index cf59c67..ec2ebb4 100644
--- a/testing/js_embedder_test.h
+++ b/testing/js_embedder_test.h
@@ -7,24 +7,18 @@
#include <memory>
-#include "fxjs/cfx_v8.h"
#include "testing/embedder_test.h"
-#include "v8/include/v8.h"
+
+namespace v8 {
+class Isolate;
+} // namespace v8
class JSEmbedderTest : public EmbedderTest {
public:
JSEmbedderTest();
~JSEmbedderTest() override;
- // EmbedderTest:
- void SetUp() override;
- void TearDown() override;
-
- v8::Isolate* isolate() const { return m_pIsolate.get(); }
-
- private:
- std::unique_ptr<CFX_V8ArrayBufferAllocator> m_pArrayBufferAllocator;
- std::unique_ptr<v8::Isolate, CFX_V8IsolateDeleter> m_pIsolate;
+ v8::Isolate* isolate() const;
};
#endif // TESTING_JS_EMBEDDER_TEST_H_
diff --git a/testing/v8_test_environment.cpp b/testing/v8_test_environment.cpp
index 78cc453..94203c2 100644
--- a/testing/v8_test_environment.cpp
+++ b/testing/v8_test_environment.cpp
@@ -4,6 +4,7 @@
#include "testing/v8_test_environment.h"
+#include <memory>
#include <string>
#include "core/fxcrt/fx_system.h"
@@ -19,19 +20,21 @@
} // namespace
V8TestEnvironment::V8TestEnvironment(const char* exe_name)
- : exe_path_(exe_name) {
+ : exe_path_(exe_name),
+ array_buffer_allocator_(std::make_unique<CFX_V8ArrayBufferAllocator>()) {
ASSERT(!g_environment);
g_environment = this;
}
V8TestEnvironment::~V8TestEnvironment() {
ASSERT(g_environment);
- g_environment = nullptr;
#ifdef V8_USE_EXTERNAL_STARTUP_DATA
- if (v8_snapshot_)
- free(const_cast<char*>(v8_snapshot_->data));
+ if (startup_data_)
+ free(const_cast<char*>(startup_data_->data));
#endif // V8_USE_EXTERNAL_STARTUP_DATA
+
+ g_environment = nullptr;
}
// static
@@ -48,19 +51,24 @@
void V8TestEnvironment::SetUp() {
#ifdef V8_USE_EXTERNAL_STARTUP_DATA
- if (v8_snapshot_) {
+ if (startup_data_) {
platform_ = InitializeV8ForPDFiumWithStartupData(exe_path_, std::string(),
std::string(), nullptr);
} else {
- v8_snapshot_ = std::make_unique<v8::StartupData>();
+ startup_data_ = std::make_unique<v8::StartupData>();
platform_ = InitializeV8ForPDFiumWithStartupData(
- exe_path_, std::string(), std::string(), v8_snapshot_.get());
+ exe_path_, std::string(), std::string(), startup_data_.get());
}
#else
platform_ = InitializeV8ForPDFium(exe_path_);
#endif // V8_USE_EXTERNAL_STARTUP_DATA
+
+ v8::Isolate::CreateParams params;
+ params.array_buffer_allocator = array_buffer_allocator_.get();
+ isolate_.reset(v8::Isolate::New(params));
}
void V8TestEnvironment::TearDown() {
+ isolate_.reset();
v8::V8::ShutdownPlatform();
}
diff --git a/testing/v8_test_environment.h b/testing/v8_test_environment.h
index b5dc49b..3b122fa 100644
--- a/testing/v8_test_environment.h
+++ b/testing/v8_test_environment.h
@@ -5,20 +5,19 @@
#ifndef TESTING_V8_TEST_ENVIRONMENT_H_
#define TESTING_V8_TEST_ENVIRONMENT_H_
-#include <memory>
-
-#include "testing/gtest/include/gtest/gtest.h"
-
#ifndef PDF_ENABLE_V8
#error "V8 must be enabled"
#endif // PDF_ENABLE_V8
+#include <memory>
+
+#include "fxjs/cfx_v8.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
namespace v8 {
class Isolate;
class Platform;
-#ifdef V8_USE_EXTERNAL_STARTUP_DATA
class StartupData;
-#endif // V8_USE_EXTERNAL_STARTUP_DATA
} // namespace v8
class TestLoader;
@@ -28,7 +27,8 @@
explicit V8TestEnvironment(const char* exe_path);
~V8TestEnvironment() override;
- // Note: GetInstance() does not create one if it does not exist.
+ // Note: GetInstance() does not create one if it does not exist,
+ // so the main program must explicitly add this enviroment.
static V8TestEnvironment* GetInstance();
static void PumpPlatformMessageLoop(v8::Isolate* pIsolate);
@@ -37,13 +37,14 @@
void TearDown() override;
v8::Platform* platform() const { return platform_.get(); }
+ v8::Isolate* isolate() const { return isolate_.get(); }
private:
const char* const exe_path_;
-#ifdef V8_USE_EXTERNAL_STARTUP_DATA
- std::unique_ptr<v8::StartupData> v8_snapshot_;
-#endif // V8_USE_EXTERNAL_STARTUP_DATA
+ std::unique_ptr<v8::StartupData> startup_data_;
std::unique_ptr<v8::Platform> platform_;
+ std::unique_ptr<CFX_V8ArrayBufferAllocator> array_buffer_allocator_;
+ std::unique_ptr<v8::Isolate, CFX_V8IsolateDeleter> isolate_;
};
#endif // TESTING_V8_TEST_ENVIRONMENT_H_
