New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 869778 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Aug 13
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Task



Sign in to add a comment

Remove blink::V8WorkerOrWorkletEventListener

Project Member Reported by yukiy@google.com, Aug 1

Issue description

This issue for removing blink::V8WorkerOrWorkletEventListener because it is almost same with blink::V8EventListener.
 
Cc: haraken@chromium.org peria@chromium.org yukishiino@chromium.org
Components: -Enterprise Blink>Bindings
Labels: -Type-Bug -Pri-3 Pri-2 Type-Task
Owner: yukiy@google.com
Status: Started (was: Unconfirmed)
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 2

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/656101c532ce33744e7a628d21f9b2922448bef3

commit 656101c532ce33744e7a628d21f9b2922448bef3
Author: Yuki Yamada <yukiy@google.com>
Date: Thu Aug 02 05:51:37 2018

Removed V8WorkerOrWorkletEventListener::GetReceiverObject()

Removed V8WorkerOrWorkletEventListener::GetReceiverObject() because its
behavior is equivalent to V8AbstractEventListener::getReceiverObject()
as commented in FIXME.

Bug:  869778 
Change-Id: If000f1351ec1ab2d5904c853a1dddc2be9d96648
Reviewed-on: https://chromium-review.googlesource.com/1158318
Commit-Queue: Yuki Yamada <yukiy@google.com>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580083}
[modify] https://crrev.com/656101c532ce33744e7a628d21f9b2922448bef3/third_party/blink/renderer/bindings/core/v8/v8_worker_or_worklet_event_listener.cc
[modify] https://crrev.com/656101c532ce33744e7a628d21f9b2922448bef3/third_party/blink/renderer/bindings/core/v8/v8_worker_or_worklet_event_listener.h

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 6

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/19a302c3ab0d4aed140330157cf363cf15e83b7f

commit 19a302c3ab0d4aed140330157cf363cf15e83b7f
Author: Yuki Yamada <yukiy@google.com>
Date: Mon Aug 06 05:59:41 2018

Removed V8WorkerOrWorkletEventListener::HandleEvent()

Removed V8WorkerOrWorkletEventListener::HandleEvent() in order to
remove blink::V8WorkerOrWorkletEventListener.
The difference between V8AbstractEventListener::HandleEvent() and
V8WorkerOrWorkletEventListener::HandleEvent() was checking if script
controller is available or not. This should be done in not only
V8WorkerOrWorkletEventListener but also other event listeners, so this
CL added this operation by checking ScriptState::ContextIsValid().

Bug:  869778 
Change-Id: I3168f9990d2c6d076061b4d7d8f9ab85619f71d5
Reviewed-on: https://chromium-review.googlesource.com/1160069
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
Commit-Queue: Yuki Yamada <yukiy@google.com>
Cr-Commit-Position: refs/heads/master@{#580808}
[modify] https://crrev.com/19a302c3ab0d4aed140330157cf363cf15e83b7f/third_party/blink/renderer/bindings/core/v8/v8_abstract_event_listener.cc
[modify] https://crrev.com/19a302c3ab0d4aed140330157cf363cf15e83b7f/third_party/blink/renderer/bindings/core/v8/v8_worker_or_worklet_event_listener.cc
[modify] https://crrev.com/19a302c3ab0d4aed140330157cf363cf15e83b7f/third_party/blink/renderer/bindings/core/v8/v8_worker_or_worklet_event_listener.h

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3468d5a59e00bcb2c2e946a30694e6057fd9ab21

commit 3468d5a59e00bcb2c2e946a30694e6057fd9ab21
Author: Yuki Yamada <yukiy@google.com>
Date: Wed Aug 08 03:02:49 2018

Add special error handling in V8AbstractEventListener::ShouldPreventDefault()

Special error event handling in
V8AbstractEventListener::ShouldPreventDefault() is needed in accord
with HTML standard below, but was not implemented.
https://html.spec.whatwg.org/multipage/webappapis.html#the-event-handler-processing-algorithm
This CL add checking if the event is ErrorEvent and the event type is
"error". Current target should also be checked as commented in TODO.


Bug:  869778 
Change-Id: Iaebec07ab4241713e1efd40ef4dd6a6fc3cf897e
Reviewed-on: https://chromium-review.googlesource.com/1164966
Commit-Queue: Yuki Yamada <yukiy@google.com>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581454}
[delete] https://crrev.com/0403932b5ab67de9d2ba2cb8d2f8605acb1a05ed/third_party/WebKit/LayoutTests/external/wpt/html/webappapis/scripting/events/event-handler-processing-algorithm-error/window-synthetic-event-expected.txt
[modify] https://crrev.com/3468d5a59e00bcb2c2e946a30694e6057fd9ab21/third_party/blink/renderer/bindings/core/v8/v8_abstract_event_listener.cc
[modify] https://crrev.com/3468d5a59e00bcb2c2e946a30694e6057fd9ab21/third_party/blink/renderer/bindings/core/v8/v8_abstract_event_listener.h
[modify] https://crrev.com/3468d5a59e00bcb2c2e946a30694e6057fd9ab21/third_party/blink/renderer/bindings/core/v8/v8_error_handler.cc
[modify] https://crrev.com/3468d5a59e00bcb2c2e946a30694e6057fd9ab21/third_party/blink/renderer/bindings/core/v8/v8_error_handler.h

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4c5c2dbe4159228789706f9af1c15f7dd992a5be

commit 4c5c2dbe4159228789706f9af1c15f7dd992a5be
Author: Yuki Yamada <yukiy@google.com>
Date: Wed Aug 08 03:07:34 2018

Removed blink::V8WorkerOrWorkletEventListener

Removed blink::V8WorkerOrWorkletEventListener and replaced it by
blink::V8EventListener. The small differences between these were removed
in previous CLs.

Some codes for checking whether frame exists or not should have been
removed in previous CL(https://chromium-review.googlesource.com/1160069),
but it is done here.

Bug:  869778 
Change-Id: Ia6ceb8bb5481384db75265aca07e901f737873a4
Reviewed-on: https://chromium-review.googlesource.com/1160128
Commit-Queue: Yuki Yamada <yukiy@google.com>
Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581455}
[modify] https://crrev.com/4c5c2dbe4159228789706f9af1c15f7dd992a5be/third_party/blink/renderer/bindings/bindings.gni
[modify] https://crrev.com/4c5c2dbe4159228789706f9af1c15f7dd992a5be/third_party/blink/renderer/bindings/core/v8/v8_event_listener.cc
[modify] https://crrev.com/4c5c2dbe4159228789706f9af1c15f7dd992a5be/third_party/blink/renderer/bindings/core/v8/v8_event_listener_helper.cc
[delete] https://crrev.com/3468d5a59e00bcb2c2e946a30694e6057fd9ab21/third_party/blink/renderer/bindings/core/v8/v8_worker_or_worklet_event_listener.cc
[delete] https://crrev.com/3468d5a59e00bcb2c2e946a30694e6057fd9ab21/third_party/blink/renderer/bindings/core/v8/v8_worker_or_worklet_event_listener.h

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/969da3c9c6b9669fa326def221e7e818e3813367

commit 969da3c9c6b9669fa326def221e7e818e3813367
Author: Yuki Yamada <yukiy@google.com>
Date: Wed Aug 08 07:10:27 2018

Remove duplicated early return

This CL is for cleaning up of previous CL below:
https://chromium-review.googlesource.com/1160069
I added early return with checking ScriptState::ContextIsValid(), but
it should be already checked before calling
V8AbstractEventListener::HandleEvent().
https://cs.chromium.org/chromium/src/third_party/blink/renderer/bindings/core/v8/v8_abstract_event_listener.cc?l=110

Bug:  869778 
Change-Id: Ied02ee97f855fdd3c4231188e982b452b78bd06b
Reviewed-on: https://chromium-review.googlesource.com/1166607
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Commit-Queue: Yuki Yamada <yukiy@google.com>
Cr-Commit-Position: refs/heads/master@{#581491}
[modify] https://crrev.com/969da3c9c6b9669fa326def221e7e818e3813367/third_party/blink/renderer/bindings/core/v8/v8_abstract_event_listener.cc

Status: Fixed (was: Started)

Sign in to add a comment