Skip to content

Commit

Permalink
binding: Improve V8 value conversions at js_event_handler.cc
Browse files Browse the repository at this point in the history
This patch replaces the usage of old API (ScriptValue::From)
with the new API (ToV8Traits<T>::ToV8).

The root cause of crbug.com/1231434 is yet unknown though,
this patch could fix the issue (or help the investigation)
because this patch optimizes the conversion that crashes
and makes it almost no-op.

Even if this patch does nothing for the bug, this is a minor
improvement of the code base.

Bug: 1231434
Change-Id: I704a53bb1570f987681060c9483abff9e3fcae95
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3043802
Auto-Submit: Yuki Shiino <yukishiino@chromium.org>
Commit-Queue: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#904127}
NOKEYCHECK=True
GitOrigin-RevId: cd3e88cc0f6256bb5ef7dc967e8c6ce537971175
  • Loading branch information
yuki3 authored and copybara-github committed Jul 22, 2021
1 parent 32b399b commit 9df1e3d
Showing 1 changed file with 20 additions and 9 deletions.
29 changes: 20 additions & 9 deletions blink/renderer/bindings/core/v8/js_event_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#include "third_party/blink/renderer/bindings/core/v8/js_event_handler.h"

#include "third_party/blink/renderer/bindings/core/v8/script_value.h"
#include "third_party/blink/renderer/bindings/core/v8/to_v8_for_core.h"
#include "third_party/blink/renderer/bindings/core/v8/to_v8_traits.h"
#include "third_party/blink/renderer/bindings/core/v8/v8_string_resource.h"
#include "third_party/blink/renderer/core/dom/events/event_target.h"
#include "third_party/blink/renderer/core/events/before_unload_event.h"
Expand Down Expand Up @@ -91,6 +91,7 @@ void JSEventHandler::InvokeInternal(EventTarget& event_target,
HeapVector<ScriptValue> arguments;
ScriptState* script_state_of_listener =
event_handler_->CallbackRelevantScriptState();
v8::Isolate* isolate = script_state_of_listener->GetIsolate();

if (special_error_event_handling) {
auto* error_event = To<ErrorEvent>(&event);
Expand All @@ -100,17 +101,27 @@ void JSEventHandler::InvokeInternal(EventTarget& event_target,
ScriptValue error_attribute = error_event->error(script_state_of_listener);
if (error_attribute.IsEmpty() ||
error_event->target()->InterfaceName() == event_target_names::kWorker) {
error_attribute =
ScriptValue::CreateNull(script_state_of_listener->GetIsolate());
error_attribute = ScriptValue::CreateNull(isolate);
}
arguments = {
ScriptValue::From(script_state_of_listener, error_event->message()),
ScriptValue::From(script_state_of_listener, error_event->filename()),
ScriptValue::From(script_state_of_listener, error_event->lineno()),
ScriptValue::From(script_state_of_listener, error_event->colno()),
ScriptValue(isolate,
ToV8Traits<IDLStringV2>::ToV8(script_state_of_listener,
error_event->message())
.ToLocalChecked()),
ScriptValue(isolate,
ToV8Traits<IDLStringV2>::ToV8(script_state_of_listener,
error_event->filename())
.ToLocalChecked()),
ScriptValue(isolate,
ToV8Traits<IDLUnsignedLong>::ToV8(script_state_of_listener,
error_event->lineno())
.ToLocalChecked()),
ScriptValue(isolate, ToV8Traits<IDLUnsignedLong>::ToV8(
script_state_of_listener, error_event->colno())
.ToLocalChecked()),
error_attribute};
} else {
arguments = {ScriptValue::From(script_state_of_listener, js_event)};
arguments.push_back(ScriptValue(isolate, js_event));
}

if (!event_handler_->IsRunnableOrThrowException(
Expand All @@ -123,7 +134,7 @@ void JSEventHandler::InvokeInternal(EventTarget& event_target,
if (!event_handler_
->InvokeWithoutRunnabilityCheck(event.currentTarget(), arguments)
.To(&result) ||
GetIsolate()->IsExecutionTerminating())
isolate->IsExecutionTerminating())
return;
v8::Local<v8::Value> v8_return_value = result.V8Value();

Expand Down

0 comments on commit 9df1e3d

Please sign in to comment.