Revert "Revert "Revert "Introducing an event loop mechanism for d8."""

This reverts commit 7dcc8eff.

Reason for revert: Some flakes still (see comments) and breaks predictable testing:
https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20predictable/builds/11452

Original change's description:
> Revert "Revert "Introducing an event loop mechanism for d8.""
> 
> This reverts commit f7c25da6.
> 
> Reason for revert: Fixed
> 
> Original change's description:
> > Revert "Introducing an event loop mechanism for d8."
> > 
> > This reverts commit de964dbe.
> > 
> > Reason for revert:
> > https://build.chromium.org/p/client.v8/builders/V8%20Linux/builds/17958
> > 
> > Original change's description:
> > > Introducing an event loop mechanism for d8.
> > > 
> > > This mechanism ensures APIs like wasm async complete their work, 
> > > without requiring use of natives (%APIs).
> > > 
> > > The mechanism is similar to the one used in content_shell,
> > > which should allow us to easily port tests in that environment.
> > > 
> > > Review-Url: https://codereview.chromium.org/2842843005
> > > Cr-Original-Commit-Position: refs/heads/master@{#44908}
> > > Bug: 
> > > Change-Id: I9deee0d256a600c60b42902fc8ef8478e5546344
> > > Reviewed-on: https://chromium-review.googlesource.com/494968
> > > Commit-Queue: Mircea Trofin <mtrofin@google.com>
> > > Reviewed-by: Jochen Eisinger <jochen@chromium.org>
> > > Cr-Commit-Position: refs/heads/master@{#45165}
> > 
> > TBR=bradnelson@chromium.org,mtrofin@chromium.org,mtrofin@google.com,jochen@chromium.org
> > NOPRESUBMIT=true
> > NOTREECHECKS=true
> > NOTRY=true
> > 
> > Change-Id: Iafec2615d705d1990c57229cab3a988c00b5e12f
> > Reviewed-on: https://chromium-review.googlesource.com/498630
> > Reviewed-by: Michael Achenbach <machenbach@chromium.org>
> > Commit-Queue: Michael Achenbach <machenbach@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#45166}
> 
> TBR=bradnelson@chromium.org,machenbach@chromium.org,mtrofin@chromium.org,mtrofin@google.com,jochen@chromium.org,v8-reviews@googlegroups.com
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> 
> Change-Id: Ic3c782e918326e291a6cb9bb349c609e9a340b09
> Reviewed-on: https://chromium-review.googlesource.com/498430
> Reviewed-by: Mircea Trofin <mtrofin@chromium.org>
> Commit-Queue: Mircea Trofin <mtrofin@google.com>
> Cr-Commit-Position: refs/heads/master@{#45172}

TBR=bradnelson@chromium.org,machenbach@chromium.org,mtrofin@chromium.org,mtrofin@google.com,jochen@chromium.org,v8-reviews@googlegroups.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Change-Id: I21ffba7141db0bfb4a3275b6e1bf4fb399800ed2
Reviewed-on: https://chromium-review.googlesource.com/500128Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45177}

include/libplatform/libplatform.h

@@ -15,11 +15,6 @@ namespace platform {
 enum class IdleTaskSupport { kDisabled, kEnabled };
 enum class InProcessStackDumping { kDisabled, kEnabled };
 
-enum class MessageLoopBehavior : bool {
-  kDoNotWait = false,
-  kWaitForWork = true
-};
-
 /**
  * Returns a new instance of the default v8::Platform implementation.
  *
@@ -41,15 +36,11 @@ V8_PLATFORM_EXPORT v8::Platform* CreateDefaultPlatform(
  * Pumps the message loop for the given isolate.
  *
  * The caller has to make sure that this is called from the right thread.
- * Returns true if a task was executed, and false otherwise. Unless requested
- * through the |behavior| parameter, this call does not block if no task is
- * pending. The |platform| has to be created using |CreateDefaultPlatform|.
+ * Returns true if a task was executed, and false otherwise. This call does
+ * not block if no task is pending. The |platform| has to be created using
+ * |CreateDefaultPlatform|.
  */
-V8_PLATFORM_EXPORT bool PumpMessageLoop(
-    v8::Platform* platform, v8::Isolate* isolate,
-    MessageLoopBehavior behavior = MessageLoopBehavior::kDoNotWait);
-
-V8_PLATFORM_EXPORT void EnsureEventLoopInitialized(v8::Platform* platform,
+V8_PLATFORM_EXPORT bool PumpMessageLoop(v8::Platform* platform,
                                         v8::Isolate* isolate);
 
 /**

src/d8.cc

@@ -422,7 +422,6 @@ base::LazyMutex Shell::workers_mutex_;
 bool Shell::allow_new_workers_ = true;
 i::List<Worker*> Shell::workers_;
 std::vector<ExternalizedContents> Shell::externalized_contents_;
-std::map<v8::Isolate*, bool> Shell::isolate_status;
 
 Global<Context> Shell::evaluation_context_;
 ArrayBuffer::Allocator* Shell::array_buffer_allocator;
@@ -1346,18 +1345,6 @@ void Shell::Quit(const v8::FunctionCallbackInfo<v8::Value>& args) {
                  const_cast<v8::FunctionCallbackInfo<v8::Value>*>(&args));
 }
 
-void Shell::WaitUntilDone(const v8::FunctionCallbackInfo<v8::Value>& args) {
-  if (isolate_status.count(args.GetIsolate()) == 0) {
-    isolate_status.insert(std::make_pair(args.GetIsolate(), true));
-  } else {
-    isolate_status[args.GetIsolate()] = true;
-  }
-}
-
-void Shell::NotifyDone(const v8::FunctionCallbackInfo<v8::Value>& args) {
-  DCHECK_EQ(isolate_status.count(args.GetIsolate()), 1);
-  isolate_status[args.GetIsolate()] = false;
-}
 
 void Shell::Version(const v8::FunctionCallbackInfo<v8::Value>& args) {
   args.GetReturnValue().Set(
@@ -1595,19 +1582,6 @@ Local<ObjectTemplate> Shell::CreateGlobalTemplate(Isolate* isolate) {
             .ToLocalChecked(),
         FunctionTemplate::New(isolate, Quit));
   }
-  Local<ObjectTemplate> test_template = ObjectTemplate::New(isolate);
-  global_template->Set(
-      String::NewFromUtf8(isolate, "testRunner", NewStringType::kNormal)
-          .ToLocalChecked(),
-      test_template);
-  test_template->Set(
-      String::NewFromUtf8(isolate, "notifyDone", NewStringType::kNormal)
-          .ToLocalChecked(),
-      FunctionTemplate::New(isolate, NotifyDone));
-  test_template->Set(
-      String::NewFromUtf8(isolate, "waitUntilDone", NewStringType::kNormal)
-          .ToLocalChecked(),
-      FunctionTemplate::New(isolate, WaitUntilDone));
   global_template->Set(
       String::NewFromUtf8(isolate, "version", NewStringType::kNormal)
           .ToLocalChecked(),
@@ -2292,8 +2266,6 @@ void SourceGroup::ExecuteInThread() {
   create_params.host_import_module_dynamically_callback_ =
       Shell::HostImportModuleDynamically;
   Isolate* isolate = Isolate::New(create_params);
-
-  v8::platform::EnsureEventLoopInitialized(g_platform, isolate);
   D8Console console(isolate);
   debug::SetConsoleDelegate(isolate, &console);
   for (int i = 0; i < Shell::options.stress_runs; ++i) {
@@ -2314,7 +2286,6 @@ void SourceGroup::ExecuteInThread() {
         DisposeModuleEmbedderData(context);
       }
       Shell::CollectGarbage(isolate);
-      Shell::CompleteMessageLoop(isolate);
     }
     done_semaphore_.Signal();
   }
@@ -2675,7 +2646,6 @@ int Shell::RunMain(Isolate* isolate, int argc, char* argv[], bool last_run) {
     options.isolate_sources[i].StartExecuteInThread();
   }
   {
-    v8::platform::EnsureEventLoopInitialized(g_platform, isolate);
     if (options.lcov_file) {
       debug::Coverage::SelectMode(isolate, debug::Coverage::kPreciseCount);
     }
@@ -2698,7 +2668,6 @@ int Shell::RunMain(Isolate* isolate, int argc, char* argv[], bool last_run) {
     WriteLcovData(isolate, options.lcov_file);
   }
   CollectGarbage(isolate);
-  CompleteMessageLoop(isolate);
   for (int i = 1; i < options.num_isolates; ++i) {
     if (last_run) {
       options.isolate_sources[i].JoinThread();
@@ -2726,28 +2695,24 @@ void Shell::CollectGarbage(Isolate* isolate) {
   }
 }
 
-void Shell::CompleteMessageLoop(Isolate* isolate) {
-  while (v8::platform::PumpMessageLoop(
-      g_platform, isolate,
-      Shell::isolate_status[isolate]
-          ? platform::MessageLoopBehavior::kWaitForWork
-          : platform::MessageLoopBehavior::kDoNotWait)) {
-    isolate->RunMicrotasks();
-  }
-  v8::platform::RunIdleTasks(g_platform, isolate,
-                             50.0 / base::Time::kMillisecondsPerSecond);
-}
-
 void Shell::EmptyMessageQueues(Isolate* isolate) {
   if (i::FLAG_verify_predictable) return;
+  while (true) {
     // Pump the message loop until it is empty.
-  while (v8::platform::PumpMessageLoop(
-      g_platform, isolate, platform::MessageLoopBehavior::kDoNotWait)) {
+    while (v8::platform::PumpMessageLoop(g_platform, isolate)) {
       isolate->RunMicrotasks();
     }
     // Run the idle tasks.
     v8::platform::RunIdleTasks(g_platform, isolate,
                                50.0 / base::Time::kMillisecondsPerSecond);
+    // If there are still outstanding waiters, sleep a little (to wait for
+    // background tasks) and then try everything again.
+    if (reinterpret_cast<i::Isolate*>(isolate)->GetWaitCountForTesting() > 0) {
+      base::OS::Sleep(base::TimeDelta::FromMilliseconds(1));
+    } else {
+      break;
+    }
+  }
 }
 
 class Serializer : public ValueSerializer::Delegate {

src/d8.h

@@ -6,7 +6,6 @@
 #define V8_D8_H_
 
 #include <iterator>
-#include <map>
 #include <memory>
 #include <string>
 #include <vector>
@@ -357,7 +356,6 @@ class Shell : public i::AllStatic {
   static void OnExit(Isolate* isolate);
   static void CollectGarbage(Isolate* isolate);
   static void EmptyMessageQueues(Isolate* isolate);
-  static void CompleteMessageLoop(Isolate* isolate);
 
   static std::unique_ptr<SerializationData> SerializeValue(
       Isolate* isolate, Local<Value> value, Local<Value> transfer);
@@ -393,8 +391,6 @@ class Shell : public i::AllStatic {
   static void Print(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void PrintErr(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void Write(const v8::FunctionCallbackInfo<v8::Value>& args);
-  static void WaitUntilDone(const v8::FunctionCallbackInfo<v8::Value>& args);
-  static void NotifyDone(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void QuitOnce(v8::FunctionCallbackInfo<v8::Value>* args);
   static void Quit(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void Version(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -458,7 +454,6 @@ class Shell : public i::AllStatic {
   static const char* kPrompt;
   static ShellOptions options;
   static ArrayBuffer::Allocator* array_buffer_allocator;
-  static std::map<Isolate*, bool> isolate_status;
 
  private:
   static Global<Context> evaluation_context_;

src/isolate.h

@@ -1291,6 +1291,11 @@ class Isolate {
   // reset to nullptr.
   void UnregisterFromReleaseAtTeardown(ManagedObjectFinalizer** finalizer_ptr);
 
+  // Used by mjsunit tests to force d8 to wait for certain things to run.
+  inline void IncrementWaitCountForTesting() { wait_count_++; }
+  inline void DecrementWaitCountForTesting() { wait_count_--; }
+  inline int GetWaitCountForTesting() { return wait_count_; }
+
  protected:
   explicit Isolate(bool enable_serializer);
   bool IsArrayOrObjectPrototype(Object* object);
@@ -1577,6 +1582,8 @@ class Isolate {
 
   size_t total_regexp_code_generated_;
 
+  int wait_count_ = 0;
+
   friend class ExecutionAccess;
   friend class HandleScopeImplementer;
   friend class HeapTester;

src/libplatform/default-platform.cc

@@ -41,15 +41,9 @@ v8::Platform* CreateDefaultPlatform(
   return platform;
 }
 
-bool PumpMessageLoop(v8::Platform* platform, v8::Isolate* isolate,
-                     MessageLoopBehavior behavior) {
-  return reinterpret_cast<DefaultPlatform*>(platform)->PumpMessageLoop(
-      isolate, behavior);
-}
 
-void EnsureEventLoopInitialized(v8::Platform* platform, v8::Isolate* isolate) {
-  return reinterpret_cast<DefaultPlatform*>(platform)
-      ->EnsureEventLoopInitialized(isolate);
+bool PumpMessageLoop(v8::Platform* platform, v8::Isolate* isolate) {
+  return reinterpret_cast<DefaultPlatform*>(platform)->PumpMessageLoop(isolate);
 }
 
 void RunIdleTasks(v8::Platform* platform, v8::Isolate* isolate,
@@ -164,23 +158,7 @@ IdleTask* DefaultPlatform::PopTaskInMainThreadIdleQueue(v8::Isolate* isolate) {
   return task;
 }
 
-void DefaultPlatform::EnsureEventLoopInitialized(v8::Isolate* isolate) {
-  if (event_loop_control_.count(isolate) == 0) {
-    event_loop_control_.insert(std::make_pair(
-        isolate, std::unique_ptr<base::Semaphore>(new base::Semaphore(0))));
-  }
-}
-
-void DefaultPlatform::WaitForForegroundWork(v8::Isolate* isolate) {
-  DCHECK_EQ(event_loop_control_.count(isolate), 1);
-  event_loop_control_[isolate]->Wait();
-}
-
-bool DefaultPlatform::PumpMessageLoop(v8::Isolate* isolate,
-                                      MessageLoopBehavior behavior) {
-  if (behavior == MessageLoopBehavior::kWaitForWork) {
-    WaitForForegroundWork(isolate);
-  }
+bool DefaultPlatform::PumpMessageLoop(v8::Isolate* isolate) {
   Task* task = NULL;
   {
     base::LockGuard<base::Mutex> guard(&lock_);
@@ -188,14 +166,14 @@ bool DefaultPlatform::PumpMessageLoop(v8::Isolate* isolate,
     // Move delayed tasks that hit their deadline to the main queue.
     task = PopTaskInMainThreadDelayedQueue(isolate);
     while (task != NULL) {
-      ScheduleOnForegroundThread(isolate, task);
+      main_thread_queue_[isolate].push(task);
       task = PopTaskInMainThreadDelayedQueue(isolate);
     }
 
     task = PopTaskInMainThreadQueue(isolate);
 
     if (task == NULL) {
-      return behavior == MessageLoopBehavior::kWaitForWork;
+      return false;
     }
   }
   task->Run();
@@ -228,17 +206,10 @@ void DefaultPlatform::CallOnBackgroundThread(Task* task,
   queue_.Append(task);
 }
 
-void DefaultPlatform::ScheduleOnForegroundThread(v8::Isolate* isolate,
-                                                 Task* task) {
-  main_thread_queue_[isolate].push(task);
-  if (event_loop_control_.count(isolate) != 0) {
-    event_loop_control_[isolate]->Signal();
-  }
-}
 
 void DefaultPlatform::CallOnForegroundThread(v8::Isolate* isolate, Task* task) {
   base::LockGuard<base::Mutex> guard(&lock_);
-  ScheduleOnForegroundThread(isolate, task);
+  main_thread_queue_[isolate].push(task);
 }
 
 

src/libplatform/default-platform.h

@@ -41,10 +41,7 @@ class V8_PLATFORM_EXPORT DefaultPlatform : public NON_EXPORTED_BASE(Platform) {
 
   void EnsureInitialized();
 
-  bool PumpMessageLoop(
-      v8::Isolate* isolate,
-      MessageLoopBehavior behavior = MessageLoopBehavior::kDoNotWait);
-  void EnsureEventLoopInitialized(v8::Isolate* isolate);
+  bool PumpMessageLoop(v8::Isolate* isolate);
 
   void RunIdleTasks(v8::Isolate* isolate, double idle_time_in_seconds);
 
@@ -84,9 +81,6 @@ class V8_PLATFORM_EXPORT DefaultPlatform : public NON_EXPORTED_BASE(Platform) {
   Task* PopTaskInMainThreadDelayedQueue(v8::Isolate* isolate);
   IdleTask* PopTaskInMainThreadIdleQueue(v8::Isolate* isolate);
 
-  void WaitForForegroundWork(v8::Isolate* isolate);
-  void ScheduleOnForegroundThread(v8::Isolate* isolate, Task* task);
-
   base::Mutex lock_;
   bool initialized_;
   int thread_pool_size_;
@@ -95,7 +89,6 @@ class V8_PLATFORM_EXPORT DefaultPlatform : public NON_EXPORTED_BASE(Platform) {
   TaskQueue queue_;
   std::map<v8::Isolate*, std::queue<Task*>> main_thread_queue_;
   std::map<v8::Isolate*, std::queue<IdleTask*>> main_thread_idle_queue_;
-  std::map<v8::Isolate*, std::unique_ptr<base::Semaphore>> event_loop_control_;
 
   typedef std::pair<double, Task*> DelayedEntry;
   std::map<v8::Isolate*,

src/runtime/runtime-test.cc

@@ -1019,5 +1019,15 @@ RUNTIME_FUNCTION(Runtime_RedirectToWasmInterpreter) {
   return isolate->heap()->undefined_value();
 }
 
+RUNTIME_FUNCTION(Runtime_IncrementWaitCount) {
+  isolate->IncrementWaitCountForTesting();
+  return isolate->heap()->undefined_value();
+}
+
+RUNTIME_FUNCTION(Runtime_DecrementWaitCount) {
+  isolate->DecrementWaitCountForTesting();
+  return isolate->heap()->undefined_value();
+}
+
 }  // namespace internal
 }  // namespace v8

src/runtime/runtime.h

@@ -471,7 +471,9 @@ namespace internal {
   F(PromiseRevokeReject, 1, 1)              \
   F(PromiseResult, 1, 1)                    \
   F(PromiseStatus, 1, 1)                    \
-  F(ReportPromiseReject, 2, 1)
+  F(ReportPromiseReject, 2, 1)              \
+  F(IncrementWaitCount, 0, 1)               \
+  F(DecrementWaitCount, 0, 1)
 
 #define FOR_EACH_INTRINSIC_PROXY(F)     \
   F(IsJSProxy, 1, 1)                    \

test/debugger/debug/debug-stepin-accessor.js

@@ -233,8 +233,7 @@ function testProtoSetter1_2() {
 }
 
 for (var n in this) {
-  if (n.substr(0, 4) != 'test' ||
-     n == 'testRunner') {
+  if (n.substr(0, 4) != 'test') {
     continue;
   }
   state = 1;

test/mjsunit/basic-promise.js

@@ -8,6 +8,14 @@
 // exceptions which are swallowed in a then clause.
 failWithMessage = (msg) => %AbortJS(msg);
 
+let decrement = () => { %DecrementWaitCount(); }
+let increment = () => { %IncrementWaitCount(); }
+
+function WaitForPromise(p) {
+  increment();
+  p.then(decrement, decrement);
+}
+
 function newPromise() {
   var outerResolve;
   var outerReject;
@@ -15,7 +23,7 @@ function newPromise() {
     outerResolve = resolve;
     outerReject = reject;
   });
-  Promise.resolve(promise);
+  WaitForPromise(promise);  // explicitly wait for promise to resolve.
   return {
     resolve: outerResolve,
     reject: outerReject,

test/mjsunit/mjsunit.js

@@ -123,9 +123,6 @@ var assertMatches;
 // Assert the result of a promise.
 var assertPromiseResult;
 
-var promiseTestChain;
-var promiseTestCount = 0;
-
 // These bits must be in sync with bits defined in Runtime_GetOptimizationStatus
 var V8OptimizationStatus = {
   kIsFunction: 1 << 0,
@@ -502,35 +499,21 @@ var failWithMessage;
     // We have to patch mjsunit because normal assertion failures just throw
     // exceptions which are swallowed in a then clause.
     // We use eval here to avoid parsing issues with the natives syntax.
-    if (!success) success = () => {};
-
     failWithMessage = (msg) => eval("%AbortJS(msg)");
-    if (!fail) {
+    if (!fail)
       fail = result => failWithMessage("assertPromiseResult failed: " + result);
-    }
 
-    var test_promise =
-        promise.then(
+    eval("%IncrementWaitCount()");
+    return promise.then(
       result => {
-            try {
+        eval("%DecrementWaitCount()");
         success(result);
-            } catch (e) {
-              fail(e);
-            }
       },
       result => {
+        eval("%DecrementWaitCount()");
         fail(result);
       }
-        )
-        .then((x)=> {
-          if (--promiseTestCount == 0) testRunner.notifyDone();
-        });
-
-    if (!promiseTestChain) promiseTestChain = Promise.resolve();
-    // waitUntilDone is idempotent.
-    testRunner.waitUntilDone();
-    ++promiseTestCount;
-    return promiseTestChain.then(test_promise);
+    );
   };
 
   var OptimizationStatusImpl = undefined;

test/mjsunit/wasm/async-compile.js

@@ -20,7 +20,14 @@ function assertCompileError(buffer) {
       ex => assertTrue(ex instanceof WebAssembly.CompileError));
 }
 
-assertPromiseResult(async function basicCompile() {
+// These tests execute asynchronously. In order to avoid executing several tests
+// concurrently (which makes debugging much harder), build a promise chain to
+// start the next task only after the previous one ended.
+
+let testChain = Promise.resolve();
+let addTest = fun => testChain = testChain.then(() => fun());
+
+addTest(async function basicCompile() {
   let ok_buffer = (() => {
     var builder = new WasmModuleBuilder();
     builder.addFunction('f', kSig_i_v)
@@ -51,9 +58,9 @@ assertPromiseResult(async function basicCompile() {
   for (var i = 0; i < kNumCompiles; i++) {
     await assertCompileError(bad_buffer);
   }
-}());
+});
 
-assertPromiseResult(async function badFunctionInTheMiddle() {
+addTest(async function badFunctionInTheMiddle() {
   // We had an error where an exception was generated by a background task and
   // later thrown in a foreground task. The handle to the exception died
   // inbetween, since the HandleScope was left.
@@ -69,4 +76,4 @@ assertPromiseResult(async function badFunctionInTheMiddle() {
   }
   let buffer = builder.toBuffer();
   await assertCompileError(buffer);
-}());
+});

test/mjsunit/wasm/compilation-limits.js

@@ -106,4 +106,8 @@ async function TestAll() {
   await FailAsyncInstantiate();
 }
 
-assertPromiseResult(TestAll());
+%IncrementWaitCount();
+TestAll().then(
+  () => { %DecrementWaitCount(); },
+  () => { %DecrementWaitCount(); }
+);

test/mjsunit/wasm/instantiate-module-basic.js

@@ -70,7 +70,7 @@ function CheckInstance(instance) {
 
   print('async instantiate...');
   let instance_promise = WebAssembly.instantiate(buffer);
-  assertPromiseResult(instance_promise, pair => CheckInstance(pair.instance));
+  assertPromiseResult(instance_promise, CheckInstance);
 })();
 
 // Check that validate works correctly for a module.

test/mjsunit/wasm/wasm-api-overloading.js

@@ -30,6 +30,8 @@ assertPromiseResult(
     return WebAssembly.instantiate(wrapper);
   })(),
   pair => {
+    print(2);
+    var pair = result.pair;
     assertTrue(pair.instance instanceof WebAssembly.Instance);
     assertTrue(pair.module instanceof WebAssembly.Module);
     %ResetWasmOverloads();