commit fe28729547a33ee5188ba9c73502834e0cd4261e
author Tom Sepez <tsepez@chromium.org> Fri Jun 03 21:05:30 2022 +0000
committer Pdfium LUCI CQ <pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Jun 03 21:05:30 2022 +0000
tree c8734f522c2670f33f64f007c99b6d58a107119f
parent d0fb7bc17c5a254cf093e16d98b858de725fe715

Fix HeapUnitTest.DISABLED_Bloat and re-enable.

Make FXGCUnitTest use the isolate/platform from the test environment
as set up globally in unit_test_main.cpp, rather than the isolate
created on a per-test basis in FXV8UnitTest. This now pumps the
correct message loop, so that the secheduled GC is executed as
expected, and the test can pass without running out of memory. Note
this affects unit tests only, pdfium_test (and chrome) are not affected.

-- Centralize some code from  GCedTreeNodeUnitTest

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

fxjs/gc/gced_tree_node_mixin_unittest.cpp

diff --git a/fxjs/gc/gced_tree_node_mixin_unittest.cpp b/fxjs/gc/gced_tree_node_mixin_unittest.cpp
index eba8c15..835a09c 100644
--- a/fxjs/gc/gced_tree_node_mixin_unittest.cpp
+++ b/fxjs/gc/gced_tree_node_mixin_unittest.cpp
@@ -53,11 +53,6 @@
         heap()->GetAllocationHandle());
   }
 
-  void ForceGCAndPump() {
-    FXGC_ForceGarbageCollection(heap());
-    V8TestEnvironment::PumpPlatformMessageLoop(isolate());
-  }
-
   void AddClutterToFront(ObservableGCedTreeNodeMixinForTest* parent) {
     for (int i = 0; i < 4; ++i) {
       parent->AppendFirstChild(

fxjs/gc/gced_tree_node_unittest.cpp

diff --git a/fxjs/gc/gced_tree_node_unittest.cpp b/fxjs/gc/gced_tree_node_unittest.cpp
index dc7a28c..00158ce 100644
--- a/fxjs/gc/gced_tree_node_unittest.cpp
+++ b/fxjs/gc/gced_tree_node_unittest.cpp
@@ -47,11 +47,6 @@
         heap()->GetAllocationHandle());
   }
 
-  void ForceGCAndPump() {
-    FXGC_ForceGarbageCollection(heap());
-    V8TestEnvironment::PumpPlatformMessageLoop(isolate());
-  }
-
   void AddClutterToFront(ObservableGCedTreeNodeForTest* parent) {
     for (int i = 0; i < 4; ++i) {
       parent->AppendFirstChild(

fxjs/gc/heap_unittest.cpp

diff --git a/fxjs/gc/heap_unittest.cpp b/fxjs/gc/heap_unittest.cpp
index 16d331a..f9de39f 100644
--- a/fxjs/gc/heap_unittest.cpp
+++ b/fxjs/gc/heap_unittest.cpp
@@ -166,12 +166,10 @@
   EXPECT_EQ(1u, PseudoCollectible::DeadCount());
 }
 
-// TODO(tsepez): find conditions that will trigger a GC.
-TEST_F(HeapUnitTest, DISABLED_Bloat) {
-  FXGCScopedHeap heap1 = FXGC_CreateHeap();
-  ASSERT_TRUE(heap1);
+TEST_F(HeapUnitTest, Bloat) {
+  ASSERT_TRUE(heap());
   for (int i = 0; i < 100000; ++i) {
-    cppgc::MakeGarbageCollected<Bloater>(heap1->GetAllocationHandle());
-    V8TestEnvironment::PumpPlatformMessageLoop(isolate());
+    cppgc::MakeGarbageCollected<Bloater>(heap()->GetAllocationHandle());
+    Pump();  // Do not force GC, must happen implicitly when space required.
   }
 }

testing/fxgc_unittest.cpp

diff --git a/testing/fxgc_unittest.cpp b/testing/fxgc_unittest.cpp
index 5b0a493..f7378a9 100644
--- a/testing/fxgc_unittest.cpp
+++ b/testing/fxgc_unittest.cpp
@@ -14,7 +14,7 @@
 FXGCUnitTest::~FXGCUnitTest() = default;
 
 void FXGCUnitTest::SetUp() {
-  FXV8UnitTest::SetUp();
+  ::testing::Test::SetUp();
   auto* env = V8TestEnvironment::GetInstance();
   FXGC_Initialize(env->platform(), env->isolate());
   heap_ = FXGC_CreateHeap();
@@ -22,12 +22,18 @@
 }
 
 void FXGCUnitTest::TearDown() {
-  FXGC_ForceGarbageCollection(heap_.get());
-  auto* env = V8TestEnvironment::GetInstance();
-  while (v8::platform::PumpMessageLoop(env->platform(), env->isolate()))
-    continue;
-
+  ForceGCAndPump();
   heap_.reset();
   FXGC_Release();
-  FXV8UnitTest::TearDown();
+  ::testing::Test::TearDown();
+}
+
+void FXGCUnitTest::ForceGCAndPump() {
+  FXGC_ForceGarbageCollection(heap_.get());
+  Pump();
+}
+
+void FXGCUnitTest::Pump() {
+  V8TestEnvironment::PumpPlatformMessageLoop(
+      V8TestEnvironment::GetInstance()->isolate());
 }

testing/fxgc_unittest.h

diff --git a/testing/fxgc_unittest.h b/testing/fxgc_unittest.h
index 0e6515b..ac5a603 100644
--- a/testing/fxgc_unittest.h
+++ b/testing/fxgc_unittest.h
@@ -6,18 +6,20 @@
 #define TESTING_FXGC_UNITTEST_H_
 
 #include "fxjs/gc/heap.h"
-#include "testing/fxv8_unittest.h"
+#include "testing/gtest/include/gtest/gtest.h"
 
-class FXGCUnitTest : public FXV8UnitTest {
+class FXGCUnitTest : public ::testing::Test {
  public:
   FXGCUnitTest();
   ~FXGCUnitTest() override;
 
-  // FXV8UnitTest:
+  // testing::Test:
   void SetUp() override;
   void TearDown() override;
 
   cppgc::Heap* heap() const { return heap_.get(); }
+  void ForceGCAndPump();
+  void Pump();
 
  private:
   FXGCScopedHeap heap_;