Skip to content

Commit

Permalink
serial: Bubble events from SerialPort to navigator.serial
Browse files Browse the repository at this point in the history
This allows the SerialConnectionEvent interface to be removed because
the port attribute is superfluous given the target attribute.

Bug: 884928
Change-Id: I1e646c03a093cc9dd09776946597ec534e86636f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2542923
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#829837}
  • Loading branch information
reillyeon authored and Commit Bot committed Nov 20, 2020
1 parent bc4443c commit 2d3c7d1
Show file tree
Hide file tree
Showing 24 changed files with 339 additions and 146 deletions.
2 changes: 1 addition & 1 deletion chrome/browser/serial/chrome_serial_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ IN_PROC_BROWSER_TEST_F(SerialTest, RemovePort) {
removedPromise = new Promise(resolve => {
navigator.serial.addEventListener(
'disconnect', e => {
resolve(e.port === ports[0]);
resolve(e.target === ports[0]);
}, { once: true });
});
return true;
Expand Down
4 changes: 0 additions & 4 deletions third_party/blink/renderer/bindings/generated_in_modules.gni
Original file line number Diff line number Diff line change
Expand Up @@ -2258,8 +2258,6 @@ if (!is_android) {
# Serial
if (!is_android) {
generated_dictionary_sources_in_modules += [
"$root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_serial_connection_event_init.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_serial_connection_event_init.h",
"$root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_serial_input_signals.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_serial_input_signals.h",
"$root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_serial_options.cc",
Expand All @@ -2282,8 +2280,6 @@ if (!is_android) {
generated_interface_sources_in_modules += [
"$root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_serial.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_serial.h",
"$root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_serial_connection_event.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_serial_connection_event.h",
"$root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_serial_port.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_serial_port.h",
]
Expand Down
2 changes: 0 additions & 2 deletions third_party/blink/renderer/bindings/idl_in_modules.gni
Original file line number Diff line number Diff line change
Expand Up @@ -1051,8 +1051,6 @@ if (!is_android) {
[
"//third_party/blink/renderer/modules/serial/navigator_serial.idl",
"//third_party/blink/renderer/modules/serial/serial.idl",
"//third_party/blink/renderer/modules/serial/serial_connection_event.idl",
"//third_party/blink/renderer/modules/serial/serial_connection_event_init.idl",
"//third_party/blink/renderer/modules/serial/serial_input_signals.idl",
"//third_party/blink/renderer/modules/serial/serial_options.idl",
"//third_party/blink/renderer/modules/serial/serial_output_signals.idl",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
"ScreenOrientation",
"Sensor",
"Serial",
"SerialPort",
"ServiceWorker",
"ServiceWorkerContainer",
"ServiceWorkerGlobalScope",
Expand Down
2 changes: 0 additions & 2 deletions third_party/blink/renderer/modules/serial/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ blink_modules_sources("serial") {
sources = [
"serial.cc",
"serial.h",
"serial_connection_event.cc",
"serial_connection_event.h",
"serial_port.cc",
"serial_port.h",
"serial_port_underlying_sink.cc",
Expand Down
2 changes: 0 additions & 2 deletions third_party/blink/renderer/modules/serial/idls.gni
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,10 @@

modules_idl_files = [
"serial.idl",
"serial_connection_event.idl",
"serial_port.idl",
]

modules_dictionary_idl_files = [
"serial_connection_event_init.idl",
"serial_input_signals.idl",
"serial_options.idl",
"serial_output_signals.idl",
Expand Down
9 changes: 4 additions & 5 deletions third_party/blink/renderer/modules/serial/serial.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
#include "third_party/blink/renderer/core/execution_context/navigator_base.h"
#include "third_party/blink/renderer/core/frame/local_dom_window.h"
#include "third_party/blink/renderer/modules/event_target_modules_names.h"
#include "third_party/blink/renderer/modules/serial/serial_connection_event.h"
#include "third_party/blink/renderer/modules/serial/serial_port.h"
#include "third_party/blink/renderer/platform/heap/heap.h"

Expand Down Expand Up @@ -81,13 +80,13 @@ void Serial::ContextDestroyed() {
}

void Serial::OnPortAdded(mojom::blink::SerialPortInfoPtr port_info) {
DispatchEvent(*SerialConnectionEvent::Create(
event_type_names::kConnect, GetOrCreatePort(std::move(port_info))));
SerialPort* port = GetOrCreatePort(std::move(port_info));
port->DispatchEvent(*Event::CreateBubble(event_type_names::kConnect));
}

void Serial::OnPortRemoved(mojom::blink::SerialPortInfoPtr port_info) {
DispatchEvent(*SerialConnectionEvent::Create(
event_type_names::kDisconnect, GetOrCreatePort(std::move(port_info))));
SerialPort* port = GetOrCreatePort(std::move(port_info));
port->DispatchEvent(*Event::CreateBubble(event_type_names::kDisconnect));
}

ScriptPromise Serial::getPorts(ScriptState* script_state,
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

40 changes: 36 additions & 4 deletions third_party/blink/renderer/modules/serial/serial_port.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@
#include "third_party/blink/renderer/bindings/modules/v8/v8_serial_output_signals.h"
#include "third_party/blink/renderer/bindings/modules/v8/v8_serial_port_info.h"
#include "third_party/blink/renderer/core/dom/dom_exception.h"
#include "third_party/blink/renderer/core/dom/events/event.h"
#include "third_party/blink/renderer/core/streams/readable_stream.h"
#include "third_party/blink/renderer/core/streams/writable_stream.h"
#include "third_party/blink/renderer/modules/event_target_modules_names.h"
#include "third_party/blink/renderer/modules/serial/serial.h"
#include "third_party/blink/renderer/modules/serial/serial_port_underlying_sink.h"
#include "third_party/blink/renderer/modules/serial/serial_port_underlying_source.h"
Expand Down Expand Up @@ -509,10 +511,6 @@ void SerialPort::Trace(Visitor* visitor) const {
ScriptWrappable::Trace(visitor);
}

ExecutionContext* SerialPort::GetExecutionContext() const {
return parent_->GetExecutionContext();
}

bool SerialPort::HasPendingActivity() const {
// There is no need to check if the execution context has been destroyed, this
// is handled by the common tracing logic.
Expand All @@ -522,6 +520,40 @@ bool SerialPort::HasPendingActivity() const {
return port_.is_bound();
}

ExecutionContext* SerialPort::GetExecutionContext() const {
return parent_->GetExecutionContext();
}

const AtomicString& SerialPort::InterfaceName() const {
return event_target_names::kSerialPort;
}

DispatchEventResult SerialPort::DispatchEventInternal(Event& event) {
event.SetTarget(this);

// Events fired on a SerialPort instance bubble to the parent Serial instance.
event.SetEventPhase(Event::kCapturingPhase);
event.SetCurrentTarget(parent_);
parent_->FireEventListeners(event);
if (event.PropagationStopped())
goto doneDispatching;

event.SetEventPhase(Event::kAtTarget);
event.SetCurrentTarget(this);
FireEventListeners(event);
if (event.PropagationStopped() || !event.bubbles())
goto doneDispatching;

event.SetEventPhase(Event::kBubblingPhase);
event.SetCurrentTarget(parent_);
parent_->FireEventListeners(event);

doneDispatching:
event.SetCurrentTarget(nullptr);
event.SetEventPhase(Event::kNone);
return EventTarget::GetDispatchEventResult(event);
}

void SerialPort::OnReadError(device::mojom::blink::SerialReceiveError error) {
if (ReceiveErrorIsFatal(error))
read_fatal_ = true;
Expand Down
11 changes: 9 additions & 2 deletions third_party/blink/renderer/modules/serial/serial_port.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "third_party/blink/public/mojom/serial/serial.mojom-blink.h"
#include "third_party/blink/renderer/bindings/core/v8/active_script_wrappable.h"
#include "third_party/blink/renderer/bindings/core/v8/script_promise.h"
#include "third_party/blink/renderer/core/dom/events/event_target.h"
#include "third_party/blink/renderer/platform/bindings/script_wrappable.h"
#include "third_party/blink/renderer/platform/heap/handle.h"
#include "third_party/blink/renderer/platform/heap/heap_allocator.h"
Expand All @@ -34,7 +35,7 @@ class SerialPortUnderlyingSink;
class SerialPortUnderlyingSource;
class WritableStream;

class SerialPort final : public ScriptWrappable,
class SerialPort final : public EventTargetWithInlineData,
public ActiveScriptWrappable<SerialPort>,
public device::mojom::blink::SerialPortClient {
DEFINE_WRAPPERTYPEINFO();
Expand All @@ -44,6 +45,8 @@ class SerialPort final : public ScriptWrappable,
~SerialPort() override;

// Web-exposed functions
DEFINE_ATTRIBUTE_EVENT_LISTENER(connect, kConnect)
DEFINE_ATTRIBUTE_EVENT_LISTENER(disconnect, kDisconnect)
SerialPortInfo* getInfo();
ScriptPromise open(ScriptState*,
const SerialOptions* options,
Expand Down Expand Up @@ -72,9 +75,13 @@ class SerialPort final : public ScriptWrappable,
void Trace(Visitor*) const override;

// ActiveScriptWrappable
ExecutionContext* GetExecutionContext() const;
bool HasPendingActivity() const override;

// EventTargetWithInlineData
ExecutionContext* GetExecutionContext() const override;
const AtomicString& InterfaceName() const override;
DispatchEventResult DispatchEventInternal(Event& event) override;

// SerialPortClient
void OnReadError(device::mojom::blink::SerialReceiveError) override;
void OnSendError(device::mojom::blink::SerialSendError) override;
Expand Down
5 changes: 4 additions & 1 deletion third_party/blink/renderer/modules/serial/serial_port.idl
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@
ActiveScriptWrappable,
Exposed(Window Serial,DedicatedWorker Serial),
SecureContext
] interface SerialPort {
] interface SerialPort : EventTarget {
attribute EventHandler onconnect;
attribute EventHandler ondisconnect;

[CallWith=ScriptState, RaisesException]
readonly attribute ReadableStream readable;
[CallWith=ScriptState, RaisesException]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
This is a testharness.js-based test.
Found 56 tests; 47 PASS, 9 FAIL, 0 TIMEOUT, 0 NOTRUN.
Found 56 tests; 45 PASS, 11 FAIL, 0 TIMEOUT, 0 NOTRUN.
PASS idl_test setup
PASS idl_test validation
PASS Partial interface Navigator: original interface defined
Expand Down Expand Up @@ -37,10 +37,10 @@ PASS Serial interface: navigator.serial must inherit property "ondisconnect" wit
PASS Serial interface: navigator.serial must inherit property "getPorts()" with the proper type
PASS Serial interface: navigator.serial must inherit property "requestPort(optional SerialOptions)" with the proper type
PASS Serial interface: calling requestPort(optional SerialOptions) on navigator.serial with too few arguments must throw TypeError
PASS SerialPort interface: existence and properties of interface object
FAIL SerialPort interface: existence and properties of interface object assert_equals: prototype of self's property "SerialPort" is not Function.prototype expected function "function () { [native code] }" but got function "function EventTarget() { [native code] }"
PASS SerialPort interface object length
PASS SerialPort interface object name
PASS SerialPort interface: existence and properties of interface prototype object
FAIL SerialPort interface: existence and properties of interface prototype object assert_equals: prototype of SerialPort.prototype is not Object.prototype expected object "[object Object]" but got object "[object EventTarget]"
PASS SerialPort interface: existence and properties of interface prototype object's "constructor" property
PASS SerialPort interface: existence and properties of interface prototype object's @@unscopables property
FAIL SerialPort interface: operation open(optional SerialOptions) assert_equals: property has wrong .length expected 0 but got 1
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
This is a testharness.js-based test.
Found 55 tests; 46 PASS, 9 FAIL, 0 TIMEOUT, 0 NOTRUN.
Found 55 tests; 44 PASS, 11 FAIL, 0 TIMEOUT, 0 NOTRUN.
PASS idl_test setup
PASS idl_test validation
PASS Partial interface Navigator: original interface defined
Expand Down Expand Up @@ -36,10 +36,10 @@ PASS Serial interface: navigator.serial must inherit property "onconnect" with t
PASS Serial interface: navigator.serial must inherit property "ondisconnect" with the proper type
PASS Serial interface: navigator.serial must inherit property "getPorts()" with the proper type
PASS Serial interface: navigator.serial must not have property "requestPort"
PASS SerialPort interface: existence and properties of interface object
FAIL SerialPort interface: existence and properties of interface object assert_equals: prototype of self's property "SerialPort" is not Function.prototype expected function "function () { [native code] }" but got function "function EventTarget() { [native code] }"
PASS SerialPort interface object length
PASS SerialPort interface object name
PASS SerialPort interface: existence and properties of interface prototype object
FAIL SerialPort interface: existence and properties of interface prototype object assert_equals: prototype of SerialPort.prototype is not Object.prototype expected object "[object Object]" but got object "[object EventTarget]"
PASS SerialPort interface: existence and properties of interface prototype object's "constructor" property
PASS SerialPort interface: existence and properties of interface prototype object's @@unscopables property
FAIL SerialPort interface: operation open(optional SerialOptions) assert_equals: property has wrong .length expected 0 but got 1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1362,12 +1362,10 @@ Starting worker: resources/global-interface-listing-worker.js
[Worker] method getPorts
[Worker] setter onconnect
[Worker] setter ondisconnect
[Worker] interface SerialConnectionEvent : Event
[Worker] attribute @@toStringTag
[Worker] getter port
[Worker] method constructor
[Worker] interface SerialPort
[Worker] interface SerialPort : EventTarget
[Worker] attribute @@toStringTag
[Worker] getter onconnect
[Worker] getter ondisconnect
[Worker] getter readable
[Worker] getter writable
[Worker] method close
Expand All @@ -1376,6 +1374,8 @@ Starting worker: resources/global-interface-listing-worker.js
[Worker] method getSignals
[Worker] method open
[Worker] method setSignals
[Worker] setter onconnect
[Worker] setter ondisconnect
[Worker] interface ServiceWorkerRegistration : EventTarget
[Worker] attribute @@toStringTag
[Worker] getter active
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7986,12 +7986,10 @@ interface Serial : EventTarget
method requestPort
setter onconnect
setter ondisconnect
interface SerialConnectionEvent : Event
attribute @@toStringTag
getter port
method constructor
interface SerialPort
interface SerialPort : EventTarget
attribute @@toStringTag
getter onconnect
getter ondisconnect
getter readable
getter writable
method close
Expand All @@ -8000,6 +7998,8 @@ interface SerialPort
method getSignals
method open
method setSignals
setter onconnect
setter ondisconnect
interface ServiceWorker : EventTarget
attribute @@toStringTag
getter onerror
Expand Down
Loading

0 comments on commit 2d3c7d1

Please sign in to comment.