Skip to content

Commit

Permalink
WebSocketStream: Rename "connection" to "opened"
Browse files Browse the repository at this point in the history 
See ricea/websocketstream-explainer#16

Also add some unit tests for use of AbortSignal. This was already
tested by wpts, but I need to keep the coverage bot happy.

Change-Id: I559553796a8d1358506d26800108105be58a0e13
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4685590
Reviewed-by: Nidhi Jaju <nidhijaju@chromium.org>
Commit-Queue: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1175907}
  • Loading branch information
@ricea
ricea authored and Chromium LUCI CQ committed Jul 27, 2023
1 parent 955d85b commit acaac27
Show file tree
Hide file tree
Showing 20 changed files with 213 additions and 101 deletions.
4 changes: 2 additions & 2 deletions third_party/blink/renderer/bindings/generated_in_modules.gni
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1137,8 +1137,8 @@ generated_dictionary_sources_in_modules = [
"$root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_webgl_context_event_init.h",
"$root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_websocket_close_info.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_websocket_close_info.h",
"$root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_websocket_connection.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_websocket_connection.h",
"$root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_websocket_open_info.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_websocket_open_info.h",
"$root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_websocket_stream_options.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_websocket_stream_options.h",
"$root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_write_params.cc",
Expand Down
2 changes: 1 addition & 1 deletion third_party/blink/renderer/bindings/idl_in_modules.gni
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1108,7 +1108,7 @@ static_idl_files_in_modules = get_path_info(
"//third_party/blink/renderer/modules/websockets/close_event_init.idl",
"//third_party/blink/renderer/modules/websockets/websocket.idl",
"//third_party/blink/renderer/modules/websockets/websocket_close_info.idl",
"//third_party/blink/renderer/modules/websockets/websocket_connection.idl",
"//third_party/blink/renderer/modules/websockets/websocket_open_info.idl",
"//third_party/blink/renderer/modules/websockets/websocket_stream.idl",
"//third_party/blink/renderer/modules/websockets/websocket_stream_options.idl",
"//third_party/blink/renderer/modules/webtransport/web_transport.idl",
Expand Down
3 changes: 2 additions & 1 deletion ...dules/websockets/websocket_connection.idl → ...odules/websockets/websocket_open_info.idl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// https://github.com/ricea/websocketstream-explainer/blob/master/README.md
// TODO(ricea): Add standard link when there is one.

dictionary WebSocketConnection {
dictionary WebSocketOpenInfo {
ReadableStream readable;
WritableStream writable;
DOMString extensions;
Expand Down
39 changes: 19 additions & 20 deletions third_party/blink/renderer/modules/websockets/websocket_stream.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
#include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_core.h"
#include "third_party/blink/renderer/bindings/core/v8/v8_throw_dom_exception.h"
#include "third_party/blink/renderer/bindings/modules/v8/v8_websocket_close_info.h"
#include "third_party/blink/renderer/bindings/modules/v8/v8_websocket_connection.h"
#include "third_party/blink/renderer/bindings/modules/v8/v8_websocket_open_info.h"
#include "third_party/blink/renderer/bindings/modules/v8/v8_websocket_stream_options.h"
#include "third_party/blink/renderer/core/dom/abort_signal.h"
#include "third_party/blink/renderer/core/execution_context/execution_context.h"
Expand Down Expand Up @@ -439,20 +439,19 @@ WebSocketStream::WebSocketStream(ExecutionContext* execution_context,
: ActiveScriptWrappable<WebSocketStream>({}),
ExecutionContextLifecycleObserver(execution_context),
script_state_(script_state),
connection_resolver_(
opened_resolver_(
MakeGarbageCollected<ScriptPromiseResolver>(script_state)),
closed_resolver_(
MakeGarbageCollected<ScriptPromiseResolver>(script_state)),
connection_(script_state->GetIsolate(),
connection_resolver_->Promise().V8Promise()),
opened_(script_state->GetIsolate(),
opened_resolver_->Promise().V8Promise()),
closed_(script_state->GetIsolate(),
closed_resolver_->Promise().V8Promise()) {}

WebSocketStream::~WebSocketStream() = default;

ScriptPromise WebSocketStream::connection(ScriptState* script_state) const {
return ScriptPromise(script_state,
connection_.Get(script_state->GetIsolate()));
ScriptPromise WebSocketStream::opened(ScriptState* script_state) const {
return ScriptPromise(script_state, opened_.Get(script_state->GetIsolate()));
}

ScriptPromise WebSocketStream::closed(ScriptState* script_state) const {
Expand Down Expand Up @@ -486,18 +485,18 @@ void WebSocketStream::DidConnect(const String& subprotocol,
return;
common_.SetState(WebSocketCommon::kOpen);
was_ever_connected_ = true;
auto* connection = MakeGarbageCollected<WebSocketConnection>();
connection->setProtocol(subprotocol);
connection->setExtensions(extensions);
auto* open_info = MakeGarbageCollected<WebSocketOpenInfo>();
open_info->setProtocol(subprotocol);
open_info->setExtensions(extensions);
source_ = MakeGarbageCollected<UnderlyingSource>(script_state_, this);
auto* readable = ReadableStream::CreateWithCountQueueingStrategy(
script_state_, source_, 1);
sink_ = MakeGarbageCollected<UnderlyingSink>(this);
auto* writable =
WritableStream::CreateWithCountQueueingStrategy(script_state_, sink_, 1);
connection->setReadable(readable);
connection->setWritable(writable);
connection_resolver_->Resolve(connection);
open_info->setReadable(readable);
open_info->setWritable(writable);
opened_resolver_->Resolve(open_info);
abort_handle_.Clear();
}

Expand Down Expand Up @@ -555,7 +554,7 @@ void WebSocketStream::DidClose(

ScriptState::Scope scope(script_state_);
if (!was_ever_connected_) {
connection_resolver_->Reject(CreateNetworkErrorDOMException());
opened_resolver_->Reject(CreateNetworkErrorDOMException());
}
bool all_data_was_consumed = sink_ ? sink_->AllDataHasBeenConsumed() : true;
bool was_clean = common_.GetState() == WebSocketCommon::kClosing &&
Expand Down Expand Up @@ -595,9 +594,9 @@ bool WebSocketStream::HasPendingActivity() const {

void WebSocketStream::Trace(Visitor* visitor) const {
visitor->Trace(script_state_);
visitor->Trace(connection_resolver_);
visitor->Trace(opened_resolver_);
visitor->Trace(closed_resolver_);
visitor->Trace(connection_);
visitor->Trace(opened_);
visitor->Trace(closed_);
visitor->Trace(channel_);
visitor->Trace(source_);
Expand All @@ -624,7 +623,7 @@ void WebSocketStream::Connect(ScriptState* script_state,
auto exception = V8ThrowDOMException::CreateOrEmpty(
script_state->GetIsolate(), DOMExceptionCode::kAbortError,
"WebSocket handshake was aborted");
connection_resolver_->Reject(exception);
opened_resolver_->Reject(exception);
closed_resolver_->Reject(exception);
return;
}
Expand All @@ -648,7 +647,7 @@ void WebSocketStream::Connect(ScriptState* script_state,
channel_ = nullptr;
// These need to be resolved to stop ScriptPromiseResolver::Dispose() from
// DCHECKing. It's not actually visible to JavaScript.
connection_resolver_->Resolve();
opened_resolver_->Resolve();
closed_resolver_->Resolve();
return;

Expand All @@ -659,7 +658,7 @@ void WebSocketStream::Connect(ScriptState* script_state,
"An attempt was made to break through the security policy of the "
"user agent.",
"WebSocket mixed content check failed.");
connection_resolver_->Reject(exception);
opened_resolver_->Reject(exception);
closed_resolver_->Reject(exception);
return;
}
Expand Down Expand Up @@ -725,7 +724,7 @@ void WebSocketStream::OnAbort() {
auto exception = V8ThrowDOMException::CreateOrEmpty(
script_state_->GetIsolate(), DOMExceptionCode::kAbortError,
"WebSocket handshake was aborted");
connection_resolver_->Reject(exception);
opened_resolver_->Reject(exception);
closed_resolver_->Reject(exception);
abort_handle_.Clear();
}
Expand Down
8 changes: 4 additions & 4 deletions third_party/blink/renderer/modules/websockets/websocket_stream.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class MODULES_EXPORT WebSocketStream final

// IDL properties
const KURL& url() const { return common_.Url(); }
ScriptPromise connection(ScriptState*) const;
ScriptPromise opened(ScriptState*) const;
ScriptPromise closed(ScriptState*) const;

// IDL functions
Expand Down Expand Up @@ -114,13 +114,13 @@ class MODULES_EXPORT WebSocketStream final
static WebSocketCloseInfo* MakeCloseInfo(uint16_t code, const String& reason);

const Member<ScriptState> script_state_;
const Member<ScriptPromiseResolver> connection_resolver_;
const Member<ScriptPromiseResolver> opened_resolver_;
const Member<ScriptPromiseResolver> closed_resolver_;

// These need to be cached because the Promise() method on
// ScriptPromiseResolver doesn't work any more once the promise is resolved or
// rejected.
const TraceWrapperV8Reference<v8::Promise> connection_;
const TraceWrapperV8Reference<v8::Promise> opened_;
const TraceWrapperV8Reference<v8::Promise> closed_;

Member<WebSocketChannel> channel_;
Expand All @@ -133,7 +133,7 @@ class MODULES_EXPORT WebSocketStream final
WebSocketCommon common_;

// We need to distinguish between "closing during handshake" and "closing
// after handshake" in order to reject the |connection_resolver_| correctly.
// after handshake" in order to reject the |opened_resolver_| correctly.
bool was_ever_connected_ = false;
};

Expand Down
3 changes: 2 additions & 1 deletion third_party/blink/renderer/modules/websockets/websocket_stream.idl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// Explainer: https://github.com/ricea/websocketstream-explainer/blob/master/README.md
// TODO(ricea): Add standard link when there is one.

[
Expand All @@ -11,7 +12,7 @@
] interface WebSocketStream {
[CallWith=ScriptState, RaisesException, MeasureAs=WebSocketStreamConstructor] constructor(USVString url, optional WebSocketStreamOptions options = {});
readonly attribute USVString url;
[CallWith=ScriptState] readonly attribute Promise<WebSocketConnection> connection;
[CallWith=ScriptState] readonly attribute Promise<WebSocketOpenInfo> opened;
[CallWith=ScriptState] readonly attribute Promise<WebSocketCloseInfo> closed;
[RaisesException] void close(optional WebSocketCloseInfo closeInfo = {});
};
Loading

0 comments on commit acaac27

Please sign in to comment.