Refactor V8System constructors/init

Author: ketanhwr

src/workerd/jsg/jsg.c++

@@ -4,7 +4,7 @@
 
 #include "jsg.h"
 
-#include "libplatform/libplatform.h"
+#include <libplatform/libplatform.h>
 #include "setup.h"
 
 #include <workerd/jsg/util.h>
@@ -273,7 +273,7 @@ void Lock::terminateExecution() {
 
 void Lock::pumpMessageLoop() {
   auto platform = IsolateBase::from(v8Isolate).getDefaultPlatform();
-  while (v8::platform::PumpMessageLoop(platform, v8Isolate)) {}
+  while (v8::platform::PumpMessageLoop(platform, v8Isolate, v8::platform::MessageLoopBehavior::kDoNotWait)) {}
 }
 
 Name Lock::newSymbol(kj::StringPtr symbol) {

src/workerd/jsg/jsg.h

@@ -2302,8 +2302,6 @@ class ExternalMemoryAdjustment final {
 // Isolate<TypeWrapper>::Lock. Usually this is only done in top-level code, and the Lock is
 // passed down to everyone else from there. See setup.h for details.
 
-class V8System;
-
 class Lock {
  public:
   // The underlying V8 isolate, useful for directly calling V8 APIs. Hopefully, this is rarely

src/workerd/jsg/setup.c++

@@ -74,22 +74,27 @@ static kj::Own<v8::Platform> userPlatform(v8::Platform& platform) {
   return kj::Own<v8::Platform>(&platform, kj::NullDisposer::instance);
 }
 
-V8System::V8System(): V8System(defaultPlatform(0), nullptr) {}
-V8System::V8System(kj::ArrayPtr<const kj::StringPtr> flags): V8System(defaultPlatform(0), flags) {}
-V8System::V8System(v8::Platform& platformParam): V8System(platformParam, nullptr) {}
+V8System::V8System(kj::ArrayPtr<const kj::StringPtr> flags) {
+  auto platform = defaultPlatform(0);
+  auto defaultPlatformPtr = platform.get();
+  init(kj::mv(platform), flags, defaultPlatformPtr);
+}
+
 V8System::V8System(v8::Platform& platformParam,
     kj::ArrayPtr<const kj::StringPtr> flags,
-    v8::Platform* defaultPlatformPtr)
-    : V8System(userPlatform(platformParam), flags, defaultPlatformPtr) {}
-V8System::V8System(kj::Own<v8::Platform> platformParam,
+    v8::Platform* defaultPlatformPtr) {
+  init(userPlatform(platformParam), flags, defaultPlatformPtr);
+}
+
+void V8System::init(kj::Own<v8::Platform> platformParam,
     kj::ArrayPtr<const kj::StringPtr> flags,
-    v8::Platform* defaultPlatformPtr)
-    : platformInner(kj::mv(platformParam)),
-      platformWrapper(*platformInner),
-      defaultPlatformPtr_{defaultPlatformPtr} {
-  if (!defaultPlatformPtr_) {
-    defaultPlatformPtr_ = platformInner.get();
-  }
+    v8::Platform* defaultPlatformPtr) {
+  platformInner = kj::mv(platformParam);
+  platformWrapper = kj::heap<V8PlatformWrapper>(*platformInner);
+  defaultPlatformPtr_ = defaultPlatformPtr;
+
+  KJ_ASSERT(defaultPlatformPtr_ != nullptr);
+
 #if V8_HAS_STACK_START_MARKER
   v8::StackStartMarker::EnableForProcess();
 #endif
@@ -157,9 +162,9 @@ V8System::V8System(kj::Own<v8::Platform> platformParam,
   v8::V8::InitializeICUDefaultLocation(nullptr);
 #endif
 
-  v8::V8::InitializePlatform(&platformWrapper);
+  v8::V8::InitializePlatform(platformWrapper.get());
   v8::V8::Initialize();
-  cppgc::InitializeProcess(platformWrapper.GetPageAllocator());
+  cppgc::InitializeProcess(platformWrapper->GetPageAllocator());
   v8Initialized = true;
 }
 
@@ -337,8 +342,8 @@ static v8::Isolate* newIsolate(v8::Isolate::CreateParams&& params, v8::CppHeap*
 
 IsolateBase::IsolateBase(
     V8System& system, v8::Isolate::CreateParams&& createParams, kj::Own<IsolateObserver> observer)
-    : defaultPlatform(&system.getDefaultPlatform()),
-      cppHeap(newCppHeap(const_cast<V8PlatformWrapper*>(&system.platformWrapper))),
+    : defaultPlatform(system.defaultPlatformPtr_),
+      cppHeap(newCppHeap(const_cast<V8PlatformWrapper*>(system.platformWrapper.get()))),
       ptr(newIsolate(kj::mv(createParams), cppHeap.release())),
       envAsyncContextKey(kj::refcounted<AsyncContextFrame::StorageKey>()),
       heapTracer(ptr),

src/workerd/jsg/setup.h

@@ -42,42 +42,30 @@ kj::Own<v8::Platform> defaultPlatform(uint backgroundThreadCount);
 // library.
 class V8System {
  public:
-  // Use the default v8::Platform implementation, as if by:
+  // Uses the default v8::Platform implementation, as if by:
   //   auto v8Platform = jsg::defaultPlatform();
-  //   auto v8System = V8System(*v8Platform);
-  V8System();
-
-  // `flags` is a list of command-line flags to pass to V8, like "--expose-gc" or
+  //   auto v8System = V8System(*v8Platform, flags);
+  // (Optional) `flags` is a list of command-line flags to pass to V8, like "--expose-gc" or
   // "--single_threaded_gc". An exception will be thrown if any flags are not recognized.
-  explicit V8System(kj::ArrayPtr<const kj::StringPtr> flags);
-
-  // Use a possibly-custom v8::Platform implementation. Use this if you need to override any
-  // functionality provided by the v8::Platform API.
-  explicit V8System(v8::Platform& platform);
+  explicit V8System(kj::ArrayPtr<const kj::StringPtr> flags = nullptr);
 
   // Use a possibly-custom v8::Platform implementation, and apply flags.
   explicit V8System(v8::Platform& platform,
       kj::ArrayPtr<const kj::StringPtr> flags,
-      v8::Platform* defaultPlatformPtr = nullptr);
+      v8::Platform* defaultPlatformPtr);
 
   ~V8System() noexcept(false);
 
   typedef void FatalErrorCallback(kj::StringPtr location, kj::StringPtr message);
   static void setFatalErrorCallback(FatalErrorCallback* callback);
 
-  auto& getDefaultPlatform() {
-    return *defaultPlatformPtr_;
-  }
-
  private:
   kj::Own<v8::Platform> platformInner;
-  V8PlatformWrapper platformWrapper;
-  friend class IsolateBase;
+  kj::Own<V8PlatformWrapper> platformWrapper;
   v8::Platform* defaultPlatformPtr_;
+  friend class IsolateBase;
 
-  explicit V8System(kj::Own<v8::Platform>,
-      kj::ArrayPtr<const kj::StringPtr>,
-      v8::Platform* defaultPlatformPtr = nullptr);
+  void init(kj::Own<v8::Platform>, kj::ArrayPtr<const kj::StringPtr>, v8::Platform*);
 };
 
 // Base class of Isolate<T> containing parts that don't need to be templated, to avoid code