Remove V8Initializer::MessageHandlerInWorker() reentrant check

When an exception is thrown in workers,
ExecutionContext::DispatchErrorEvent() is called

[A] From V8Initializer::MessageHandlerInWorker() in some cases and
[B] Directly (without MessageHandlerInWorker()) in other cases,

which leads calling WorkerGlobalScope.onerror.

When an exception is thrown from the
WorkerGlobalScope.onerror handler,
MessageHandlerInWorker() is called

[A] reentrantly from MessageHandlerInWorker(), and
[B] not reentrantly.

Because MessageHandlerInWorker() contains a reentrant check,
a part of unhandled exception handling is skipped only in cases [A],
but not in [B].

This CL removes the reentrant check to always perform
the unhandled exception handling, thus fixing the
behavior of crbug/1112228.

Skipping the reentrant check leads calling
ExecutionContext::DispatchErrorEvent()
from outer ExecutionContext::DispatchErrorEvent(),
but DispatchErrorEvent() is already reentrantly called
in cases [B], and DispatchErrorEvent() contains
its own reentrant handling code.

Bug: 1112228, 1111134, 1111750
Change-Id: I87b12ddad3fe082bea3429f8801563d08fc0cd6e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2332091
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Dominic Farolino <dom@chromium.org>
Cr-Commit-Position: refs/heads/master@{#795353}

third_party/blink/renderer/bindings/core/v8/v8_initializer.cc

@@ -190,21 +190,12 @@ void V8Initializer::MessageHandlerInMainThread(v8::Local<v8::Message> message,
 void V8Initializer::MessageHandlerInWorker(v8::Local<v8::Message> message,
                                            v8::Local<v8::Value> data) {
   v8::Isolate* isolate = v8::Isolate::GetCurrent();
-  V8PerIsolateData* per_isolate_data = V8PerIsolateData::From(isolate);
 
   // During the frame teardown, there may not be a valid context.
   ScriptState* script_state = ScriptState::Current(isolate);
   if (!script_state->ContextIsValid())
     return;
 
-  // Exceptions that occur in error handler should be ignored since in that case
-  // WorkerGlobalScope::dispatchErrorEvent will send the exception to the worker
-  // object.
-  if (per_isolate_data->IsReportingException())
-    return;
-
-  per_isolate_data->SetReportingException(true);
-
   ExecutionContext* context = ExecutionContext::From(script_state);
   std::unique_ptr<SourceLocation> location =
       SourceLocation::FromMessage(isolate, message, context);
@@ -231,8 +222,6 @@ void V8Initializer::MessageHandlerInWorker(v8::Local<v8::Message> message,
     ExecutionContext::From(script_state)
         ->DispatchErrorEvent(event, sanitize_script_errors);
   }
-
-  per_isolate_data->SetReportingException(false);
 }
 
 namespace {

third_party/blink/renderer/platform/bindings/v8_per_isolate_data.cc

@@ -79,7 +79,6 @@ V8PerIsolateData::V8PerIsolateData(
       constructor_mode_(ConstructorMode::kCreateNewObject),
       use_counter_disabled_(false),
       is_handling_recursion_level_error_(false),
-      is_reporting_exception_(false),
       runtime_call_stats_(base::DefaultTickClock::GetInstance()) {
   // FIXME: Remove once all v8::Isolate::GetCurrent() calls are gone.
   GetIsolate()->Enter();
@@ -103,7 +102,6 @@ V8PerIsolateData::V8PerIsolateData()
       constructor_mode_(ConstructorMode::kCreateNewObject),
       use_counter_disabled_(false),
       is_handling_recursion_level_error_(false),
-      is_reporting_exception_(false),
       runtime_call_stats_(base::DefaultTickClock::GetInstance()) {
   CHECK(IsMainThread());
 

third_party/blink/renderer/platform/bindings/v8_per_isolate_data.h

@@ -140,9 +140,6 @@ class PLATFORM_EXPORT V8PerIsolateData {
     is_handling_recursion_level_error_ = value;
   }
 
-  bool IsReportingException() const { return is_reporting_exception_; }
-  void SetReportingException(bool value) { is_reporting_exception_ = value; }
-
   bool IsUseCounterDisabled() const { return use_counter_disabled_; }
 
   V8PrivateProperty* PrivateProperty() { return private_property_.get(); }
@@ -277,7 +274,6 @@ class PLATFORM_EXPORT V8PerIsolateData {
   friend class UseCounterDisabledScope;
 
   bool is_handling_recursion_level_error_;
-  bool is_reporting_exception_;
 
   Vector<base::OnceClosure> end_of_scope_tasks_;
   std::unique_ptr<Data> thread_debugger_;

third_party/blink/web_tests/external/wpt/workers/interfaces/WorkerGlobalScope/onerror/exception-in-onerror-expected.txt

-This is a testharness.js-based test.
-PASS Throw in worker initialization: classic: listener
-PASS Throw in worker initialization: classic: handler
-FAIL Throw in setTimeout(function): classic: listener assert_unreached: Worker.onerror not fired for: Throw in error handler Reached unreachable code
-FAIL Throw in setTimeout(function): classic: handler assert_unreached: Worker.onerror not fired for: Throw in error handler Reached unreachable code
-PASS Throw in setTimeout(string): classic: listener
-PASS Throw in setTimeout(string): classic: handler
-FAIL Throw in message handler: classic: listener assert_unreached: Worker.onerror not fired for: Throw in error handler Reached unreachable code
-FAIL Throw in message handler: classic: handler assert_unreached: Worker.onerror not fired for: Throw in error handler Reached unreachable code
-FAIL Throw in worker initialization: module: listener assert_unreached: Worker.onerror not fired for: Throw in error handler Reached unreachable code
-FAIL Throw in worker initialization: module: handler assert_unreached: Worker.onerror not fired for: Throw in error handler Reached unreachable code
-FAIL Throw in setTimeout(function): module: listener assert_unreached: Worker.onerror not fired for: Throw in error handler Reached unreachable code
-FAIL Throw in setTimeout(function): module: handler assert_unreached: Worker.onerror not fired for: Throw in error handler Reached unreachable code
-PASS Throw in setTimeout(string): module: listener
-PASS Throw in setTimeout(string): module: handler
-FAIL Throw in message handler: module: listener assert_unreached: Worker.onerror not fired for: Throw in error handler Reached unreachable code
-FAIL Throw in message handler: module: handler assert_unreached: Worker.onerror not fired for: Throw in error handler Reached unreachable code
-Harness: the test ran to completion.
-