Remove the private property "ErrorEventError"

This CL removes a private property.
We do not have to have both of the private property of
"ErrorEventError" and |error_|, the
TraceWrapperV8Reference<v8::Value> in blink::ErrorEvent.

Also, the error attribute passed to listener should be initialized to
null for dedicated workers.
https://html.spec.whatwg.org/multipage/workers.html#runtime-script-errors-2
This CL modifies some tests regarding this too.

Bug: 872138
Change-Id: I497434d04c30f222ea8847239edf4e9a0ee97b19
Reviewed-on: https://chromium-review.googlesource.com/1233080
Commit-Queue: Yuki Yamada <yukiy@google.com>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593824}

third_party/WebKit/LayoutTests/fast/workers/resources/onerror-test.js

@@ -4,15 +4,26 @@ function stripURL(url) {
     return url ? url.match( /[^\/]+\/?$/ )[0] : url;
 }
 
-function checkErrorEvent(errorEvent, obj) {
+function checkWorkerLevelErrorEvent(errorEvent, obj) {
+    window.errorEvent = errorEvent;
+    shouldBeEqualToString('errorEvent.message', obj.message);
+    shouldBeEqualToString('stripURL(errorEvent.filename)', obj.filename);
+    shouldBe('errorEvent.lineno', '' + obj.lineno);
+    shouldBe('errorEvent.colno', '' + obj.colno);
+}
+
+function checkPageLevelErrorEvent(errorEvent, obj) {
     window.errorEvent = errorEvent;
     shouldBeEqualToString('errorEvent.message', obj.message);
     shouldBeEqualToString('stripURL(errorEvent.filename)', obj.filename);
     shouldBe('errorEvent.lineno', '' + obj.lineno);
     shouldBe('errorEvent.colno', '' + obj.colno);
 
-    // The spec says the error property for worker-generated errors is always initialized to null
-    // outside the worker: http://www.whatwg.org/specs/web-apps/current-work/multipage/workers.html#runtime-script-errors-0
+    // The spec says the error property for the errors generated for dedicated
+    // workers (not DedicatedWorkerGlobalScope) should be initialized to null.
+    // This is because error property cannot always be passed to outside
+    // workers, since the value of |error| attribute is not always clonable.
+    // https://html.spec.whatwg.org/multipage/workers.html#runtime-script-errors-2
     shouldBeNull('errorEvent.error');
 }
 
@@ -21,7 +32,7 @@ function checkPostMessage(obj) {
         if (e.data.done)
             return;
         debug("\nWorker-level onerror handler triggered:");
-        checkErrorEvent(e.data, obj);
+        checkWorkerLevelErrorEvent(e.data, obj);
     };
 }
 
@@ -35,7 +46,7 @@ function checkErrorEventInHandler(error, returnValue) {
         if (error.length)
             obj = error[errorsSeen++];
 
-        checkErrorEvent(e, obj);
+        checkPageLevelErrorEvent(e, obj);
 
         if (!error.length || error.length == errorsSeen)
             finishJSTest();
@@ -52,7 +63,7 @@ function checkErrorEventInListener(error, preventDefault) {
         if (error.length)
             obj = error[errorsSeen++];
 
-        checkErrorEvent(e, obj);
+        checkPageLevelErrorEvent(e, obj);
 
         if (!error.length || error.length == errorsSeen)
             finishJSTest();

third_party/WebKit/LayoutTests/fast/workers/worker-onerror-05-expected.txt

@@ -7,14 +7,13 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 Worker-level onerror handler triggered:
 PASS errorEvent.message is "Uncaught ReferenceError: foo is not defined"
 PASS stripURL(errorEvent.filename) is "[blob: URL]"
-PASS errorEvent.lineno is 13
+PASS errorEvent.lineno is 12
 PASS errorEvent.colno is 9
-PASS errorEvent.error is null
 
 Page-level worker.onerror handler triggered:
 PASS errorEvent.message is "Uncaught ReferenceError: foo is not defined"
 PASS stripURL(errorEvent.filename) is "[blob: URL]"
-PASS errorEvent.lineno is 13
+PASS errorEvent.lineno is 12
 PASS errorEvent.colno is 9
 PASS errorEvent.error is null
 PASS successfullyParsed is true

third_party/WebKit/LayoutTests/fast/workers/worker-onerror-05.html

@@ -15,8 +15,7 @@
                 message: message,
                 filename: filename,
                 lineno: lineno,
-                colno: colno,
-                error: error
+                colno: colno
             });
             return false;
         }
@@ -29,17 +28,16 @@
         checkPostMessage({
             message: "Uncaught ReferenceError: foo is not defined",
             filename: "[blob: URL]",
-            lineno: 13,
+            lineno: 12,
             colno: 9,
         });
 
         checkErrorEventInHandler({
             message: "Uncaught ReferenceError: foo is not defined",
             filename: "[blob: URL]",
-            lineno: 13,
+            lineno: 12,
             colno: 9,
         });
     </script>
 </body>
 </html>
-

third_party/WebKit/LayoutTests/fast/workers/worker-onerror-07-expected.txt

@@ -7,9 +7,8 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 Worker-level onerror handler triggered:
 PASS errorEvent.message is "Uncaught ReferenceError: foo is not defined"
 PASS stripURL(errorEvent.filename) is "[blob: URL]"
-PASS errorEvent.lineno is 14
+PASS errorEvent.lineno is 13
 PASS errorEvent.colno is 9
-PASS errorEvent.error is null
 PASS successfullyParsed is true
 
 TEST COMPLETE

third_party/WebKit/LayoutTests/fast/workers/worker-onerror-07.html

@@ -15,8 +15,7 @@
                 message: message,
                 filename: filename,
                 lineno: lineno,
-                colno: colno,
-                error: error
+                colno: colno
             });
             setTimeout(function () { postMessage({ done: true }); }, 0);
             return true;
@@ -30,7 +29,7 @@
         checkPostMessage({
             message: "Uncaught ReferenceError: foo is not defined",
             filename: "[blob: URL]",
-            lineno: 14,
+            lineno: 13,
             colno: 9,
         });
 
@@ -38,4 +37,3 @@
     </script>
 </body>
 </html>
-

third_party/blink/renderer/bindings/core/v8/custom/v8_error_event_custom.cc

@@ -31,29 +31,19 @@
 #include "third_party/blink/renderer/bindings/core/v8/v8_error_event.h"
 
 #include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_core.h"
-#include "third_party/blink/renderer/platform/bindings/v8_private_property.h"
 
 namespace blink {
 
 void V8ErrorEvent::errorAttributeGetterCustom(
     const v8::FunctionCallbackInfo<v8::Value>& info) {
   v8::Isolate* isolate = info.GetIsolate();
-
-  auto private_error = V8PrivateProperty::GetErrorEventError(isolate);
-  v8::Local<v8::Value> cached_error;
-  if (private_error.GetOrUndefined(info.Holder()).ToLocal(&cached_error) &&
-      !cached_error->IsUndefined()) {
-    V8SetReturnValue(info, cached_error);
-    return;
-  }
-
   ErrorEvent* event = V8ErrorEvent::ToImpl(info.Holder());
   ScriptState* script_state = ScriptState::From(isolate->GetCurrentContext());
+
   ScriptValue error = event->error(script_state);
   v8::Local<v8::Value> error_value =
       error.IsEmpty() ? v8::Local<v8::Value>(v8::Null(isolate))
                       : error.V8Value();
-  private_error.Set(info.Holder(), error_value);
   V8SetReturnValue(info, error_value);
 }
 

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

@@ -36,7 +36,6 @@
 #include "third_party/blink/renderer/bindings/core/v8/script_controller.h"
 #include "third_party/blink/renderer/bindings/core/v8/v8_array_buffer_view.h"
 #include "third_party/blink/renderer/bindings/core/v8/v8_element.h"
-#include "third_party/blink/renderer/bindings/core/v8/v8_error_event.h"
 #include "third_party/blink/renderer/bindings/core/v8/v8_event_target.h"
 #include "third_party/blink/renderer/bindings/core/v8/v8_html_link_element.h"
 #include "third_party/blink/renderer/bindings/core/v8/v8_window.h"
@@ -60,7 +59,6 @@
 #include "third_party/blink/renderer/platform/bindings/runtime_call_stats.h"
 #include "third_party/blink/renderer/platform/bindings/v8_binding_macros.h"
 #include "third_party/blink/renderer/platform/bindings/v8_object_constructor.h"
-#include "third_party/blink/renderer/platform/bindings/v8_private_property.h"
 #include "third_party/blink/renderer/platform/instrumentation/tracing/traced_value.h"
 #include "third_party/blink/renderer/platform/wtf/math_extras.h"
 #include "third_party/blink/renderer/platform/wtf/std_lib_extras.h"
@@ -903,40 +901,4 @@ Vector<String> GetOwnPropertyNames(v8::Isolate* isolate,
       isolate, property_names, exception_state);
 }
 
-void StoreExceptionForInspector(ScriptState* script_state,
-                                ErrorEvent* event,
-                                v8::Local<v8::Value> data,
-                                v8::Local<v8::Object> creation_context) {
-  v8::Local<v8::Value> wrapped_event =
-      ToV8(event, creation_context, script_state->GetIsolate());
-  if (wrapped_event.IsEmpty())
-    return;
-
-  DCHECK(wrapped_event->IsObject());
-  auto private_error =
-      V8PrivateProperty::GetErrorEventError(script_state->GetIsolate());
-  private_error.Set(wrapped_event.As<v8::Object>(), data);
-}
-
-v8::Local<v8::Value> LoadExceptionForInspector(
-    ScriptState* script_state,
-    ErrorEvent* event,
-    v8::Local<v8::Object> creation_context) {
-  v8::Local<v8::Value> wrapped_event =
-      ToV8(event, creation_context, script_state->GetIsolate());
-  if (wrapped_event.IsEmpty() || !wrapped_event->IsObject())
-    return v8::Local<v8::Value>();
-
-  DCHECK(wrapped_event->IsObject());
-  auto private_error =
-      V8PrivateProperty::GetErrorEventError(script_state->GetIsolate());
-  v8::Local<v8::Value> error;
-  if (!private_error.GetOrUndefined(wrapped_event.As<v8::Object>())
-           .ToLocal(&error) ||
-      error->IsUndefined()) {
-    return v8::Local<v8::Value>();
-  }
-  return error;
-}
-
 }  // namespace blink

third_party/blink/renderer/bindings/core/v8/v8_binding_for_core.h

@@ -63,7 +63,6 @@ namespace blink {
 // dependencies to core/.
 
 class DOMWindow;
-class ErrorEvent;
 class EventTarget;
 class ExceptionState;
 class ExecutionContext;
@@ -565,26 +564,6 @@ CORE_EXPORT Vector<String> GetOwnPropertyNames(v8::Isolate*,
                                                const v8::Local<v8::Object>&,
                                                ExceptionState&);
 
-// Saves |exception| into given ErrorEvent.
-// Wrapper object for given ErrorEvent can be created in association with
-// |creation_context| on processing this method, so |creation_context| should
-// be the relevant realm of event target as defined in standard.
-// See: https://dom.spec.whatwg.org/#firing-events
-void StoreExceptionForInspector(ScriptState*,
-                                ErrorEvent*,
-                                v8::Local<v8::Value> exception,
-                                v8::Local<v8::Object> creation_context);
-
-// Returns stored exception which is related to given ErrorEvent.
-// Wrapper object for given ErrorEvent can be created in association with
-// |creation_context| on processing this method, so |creation_context| should
-// be the relevant realm of event target as defined in standard.
-// See: https://dom.spec.whatwg.org/#firing-events
-v8::Local<v8::Value> LoadExceptionForInspector(
-    ScriptState*,
-    ErrorEvent*,
-    v8::Local<v8::Object> creation_context);
-
 }  // namespace blink
 
 #endif  // THIRD_PARTY_BLINK_RENDERER_BINDINGS_CORE_V8_V8_BINDING_FOR_CORE_H_

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

@@ -63,20 +63,20 @@ v8::Local<v8::Value> V8ErrorHandler::CallListenerFunction(
       v8::Local<v8::Function>::Cast(listener);
   v8::Local<v8::Object> this_value = context->Global();
 
-  v8::Local<v8::Object> event_object;
-  if (!js_event->ToObject(context).ToLocal(&event_object))
-    return v8::Null(GetIsolate());
-  auto private_error = V8PrivateProperty::GetErrorEventError(GetIsolate());
-  v8::Local<v8::Value> error;
-  if (!private_error.GetOrUndefined(event_object).ToLocal(&error) ||
-      error->IsUndefined())
-    error = v8::Null(GetIsolate());
+  // The error attribute should be initialized to null for dedicated workers.
+  // https://html.spec.whatwg.org/multipage/workers.html#runtime-script-errors-2
+  ScriptValue error = error_event->error(script_state);
+  v8::Local<v8::Value> error_value =
+      (error.IsEmpty() ||
+       error_event->target()->InterfaceName() == EventTargetNames::Worker)
+          ? v8::Local<v8::Value>(v8::Null(GetIsolate()))
+          : error.V8Value();
 
   v8::Local<v8::Value> parameters[5] = {
       V8String(GetIsolate(), error_event->message()),
       V8String(GetIsolate(), error_event->filename()),
       v8::Integer::New(GetIsolate(), error_event->lineno()),
-      v8::Integer::New(GetIsolate(), error_event->colno()), error};
+      v8::Integer::New(GetIsolate(), error_event->colno()), error_value};
   v8::TryCatch try_catch(GetIsolate());
   try_catch.SetVerbose(true);
 

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

@@ -268,16 +268,14 @@ void V8Initializer::MessageHandlerInMainThread(v8::Local<v8::Message> message,
   else if (message->IsSharedCrossOrigin())
     access_control_status = kSharableCrossOrigin;
 
-  ErrorEvent* event =
-      ErrorEvent::Create(ToCoreStringWithNullCheck(message->Get()),
-                         std::move(location), &script_state->World());
+  ErrorEvent* event = ErrorEvent::Create(
+      ToCoreStringWithNullCheck(message->Get()), std::move(location),
+      ScriptValue::From(script_state, data), &script_state->World());
 
   String message_for_console = ExtractMessageForConsole(isolate, data);
   if (!message_for_console.IsEmpty())
     event->SetUnsanitizedMessage("Uncaught " + message_for_console);
 
-  StoreExceptionForInspector(script_state, event, data,
-                             script_state->GetContext()->Global());
   context->DispatchErrorEvent(event, access_control_status);
 }
 
@@ -311,9 +309,9 @@ void V8Initializer::MessageHandlerInWorker(v8::Local<v8::Message> message,
     return;
   }
 
-  ErrorEvent* event =
-      ErrorEvent::Create(ToCoreStringWithNullCheck(message->Get()),
-                         std::move(location), &script_state->World());
+  ErrorEvent* event = ErrorEvent::Create(
+      ToCoreStringWithNullCheck(message->Get()), std::move(location),
+      ScriptValue::From(script_state, data), &script_state->World());
 
   AccessControlStatus cors_status = message->IsSharedCrossOrigin()
                                         ? kSharableCrossOrigin
@@ -322,8 +320,6 @@ void V8Initializer::MessageHandlerInWorker(v8::Local<v8::Message> message,
   // If execution termination has been triggered as part of constructing
   // the error event from the v8::Message, quietly leave.
   if (!isolate->IsExecutionTerminating()) {
-    StoreExceptionForInspector(script_state, event, data,
-                               script_state->GetContext()->Global());
     ExecutionContext::From(script_state)
         ->DispatchErrorEvent(event, cors_status);
   }

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

@@ -39,7 +39,6 @@
 #include "third_party/blink/renderer/bindings/core/v8/script_source_code.h"
 #include "third_party/blink/renderer/bindings/core/v8/script_value.h"
 #include "third_party/blink/renderer/bindings/core/v8/source_location.h"
-#include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_core.h"
 #include "third_party/blink/renderer/bindings/core/v8/v8_code_cache.h"
 #include "third_party/blink/renderer/bindings/core/v8/v8_initializer.h"
 #include "third_party/blink/renderer/bindings/core/v8/v8_script_runner.h"
@@ -346,9 +345,6 @@ bool WorkerOrWorkletScriptController::Evaluate(
         *error_event =
             ErrorEvent::Create(state.error_message, state.location_->Clone(),
                                state.exception, world_.get());
-        StoreExceptionForInspector(script_state_, *error_event,
-                                   state.exception.V8Value(),
-                                   script_state_->GetContext()->Global());
       }
     } else {
       DCHECK(!global_scope_->ShouldSanitizeScriptError(state.location_->Url(),

third_party/blink/renderer/core/inspector/main_thread_debugger.cc

@@ -196,8 +196,12 @@ void MainThreadDebugger::ExceptionThrown(ExecutionContext* context,
   const String default_message = "Uncaught";
   if (script_state && script_state->ContextIsValid()) {
     ScriptState::Scope scope(script_state);
-    v8::Local<v8::Value> exception = LoadExceptionForInspector(
-        script_state, event, script_state->GetContext()->Global());
+    ScriptValue error = event->error(script_state);
+    v8::Local<v8::Value> exception =
+        error.IsEmpty()
+            ? v8::Local<v8::Value>(v8::Null(script_state->GetIsolate()))
+            : error.V8Value();
+
     SourceLocation* location = event->Location();
     String message = event->MessageForConsole();
     String url = location->Url();

third_party/blink/renderer/core/inspector/worker_thread_debugger.cc

@@ -31,7 +31,6 @@
 #include "third_party/blink/renderer/core/inspector/worker_thread_debugger.h"
 
 #include "third_party/blink/renderer/bindings/core/v8/source_location.h"
-#include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_core.h"
 #include "third_party/blink/renderer/bindings/core/v8/v8_script_runner.h"
 #include "third_party/blink/renderer/bindings/core/v8/worker_or_worklet_script_controller.h"
 #include "third_party/blink/renderer/core/events/error_event.h"
@@ -136,8 +135,11 @@ void WorkerThreadDebugger::ExceptionThrown(WorkerThread* worker_thread,
       worker_thread->GlobalScope()->ScriptController()->GetScriptState();
   if (script_state && script_state->ContextIsValid()) {
     ScriptState::Scope scope(script_state);
-    v8::Local<v8::Value> exception = LoadExceptionForInspector(
-        script_state, event, script_state->GetContext()->Global());
+    ScriptValue error = event->error(script_state);
+    v8::Local<v8::Value> exception =
+        error.IsEmpty()
+            ? v8::Local<v8::Value>(v8::Null(script_state->GetIsolate()))
+            : error.V8Value();
     SourceLocation* location = event->Location();
     String message = event->MessageForConsole();
     String url = location->Url();

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

@@ -33,7 +33,6 @@ class ScriptWrappable;
   X(CustomElementLifecycle, CreatedCallback)          \
   X(CustomElementLifecycle, DetachedCallback)         \
   X(DOMException, Error)                              \
-  X(ErrorEvent, Error)                                \
   X(Global, Event)                                    \
   X(IDBCursor, Request)                               \
   X(IntersectionObserver, Callback)                   \