Commit c01a0825 authored by Steve Anton's avatar Steve Anton Committed by Commit Bot

Add CloseReason to RTCIceTransport, RTCQuicTransport and RTCQuicStream

RTCQuicStream needs to know if it is being closed by
ContextDestroyed (which is only known from RTCIceTransport) to
avoid rejected promises.

Adding a CloseReason additionally moves all closing code to one
method which makes it easier to reason about.

Bug: 874296
Tbr: hbos@chromium.org
Change-Id: I58afc7b56412c836b8d178868297c4fbf76320c1
Reviewed-on: https://chromium-review.googlesource.com/c/1334935
Commit-Queue: Steve Anton <steveanton@chromium.org>
Reviewed-by: default avatarSteve Anton <steveanton@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608581}
parent de8b748a
......@@ -357,7 +357,7 @@ void RTCIceTransport::start(RTCIceParameters* remote_parameters,
proxy_->Start(ConvertIceParameters(remote_parameters), role,
initial_remote_candidates);
if (consumer_) {
consumer_->OnTransportStarted();
consumer_->OnIceTransportStarted();
}
} else {
remote_candidates_.clear();
......@@ -372,14 +372,7 @@ void RTCIceTransport::stop() {
if (IsClosed()) {
return;
}
if (HasConsumer()) {
consumer_->stop();
}
// Stopping the consumer should cause it to disconnect.
DCHECK(!HasConsumer());
state_ = RTCIceTransportState::kClosed;
selected_candidate_pair_ = nullptr;
proxy_.reset();
Close(CloseReason::kStopped);
}
void RTCIceTransport::addRemoteCandidate(RTCIceCandidate* remote_candidate,
......@@ -475,6 +468,18 @@ void RTCIceTransport::OnSelectedCandidatePairChanged(
DispatchEvent(*Event::Create(event_type_names::kSelectedcandidatepairchange));
}
void RTCIceTransport::Close(CloseReason reason) {
DCHECK_NE(state_, RTCIceTransportState::kClosed);
if (HasConsumer()) {
consumer_->OnIceTransportClosed(reason);
}
// Notifying the consumer that we're closing should cause it to disconnect.
DCHECK(!HasConsumer());
state_ = RTCIceTransportState::kClosed;
selected_candidate_pair_ = nullptr;
proxy_.reset();
}
bool RTCIceTransport::RaiseExceptionIfClosed(
ExceptionState& exception_state) const {
if (IsClosed()) {
......@@ -495,7 +500,10 @@ ExecutionContext* RTCIceTransport::GetExecutionContext() const {
}
void RTCIceTransport::ContextDestroyed(ExecutionContext*) {
stop();
if (IsClosed()) {
return;
}
Close(CloseReason::kContextDestroyed);
}
bool RTCIceTransport::HasPendingActivity() const {
......
......@@ -50,6 +50,13 @@ class MODULES_EXPORT RTCIceTransport final
USING_GARBAGE_COLLECTED_MIXIN(RTCIceTransport);
public:
enum class CloseReason {
// stop() was called.
kStopped,
// The ExecutionContext is being destroyed.
kContextDestroyed,
};
static RTCIceTransport* Create(ExecutionContext* context);
static RTCIceTransport* Create(
ExecutionContext* context,
......@@ -133,6 +140,11 @@ class MODULES_EXPORT RTCIceTransport final
// password.
void GenerateLocalParameters();
// Permenantly closes the RTCIceTransport with the given reason.
// The RTCIceTransport must not already be closed.
// This will transition the state to closed.
void Close(CloseReason reason);
bool RaiseExceptionIfClosed(ExceptionState& exception_state) const;
cricket::IceRole role_ = cricket::ICEROLE_UNKNOWN;
......
......@@ -211,19 +211,20 @@ TEST_F(RTCIceTransportTest, StopDeletesIceTransportAdapter) {
// Test that the IceTransportAdapter is deleted on ContextDestroyed.
TEST_F(RTCIceTransportTest, ContextDestroyedDeletesIceTransportAdapter) {
V8TestingScope scope;
bool mock_deleted = false;
auto mock = std::make_unique<MockIceTransportAdapter>();
EXPECT_CALL(*mock, Die()).WillOnce(Assign(&mock_deleted, true));
{
V8TestingScope scope;
Persistent<RTCIceTransport> ice_transport =
CreateIceTransport(scope, std::move(mock));
RTCIceGatherOptions* options = RTCIceGatherOptions::Create();
options->setGatherPolicy("all");
ice_transport->gather(options, ASSERT_NO_EXCEPTION);
auto mock = std::make_unique<MockIceTransportAdapter>();
EXPECT_CALL(*mock, Die()).WillOnce(Assign(&mock_deleted, true));
Persistent<RTCIceTransport> ice_transport =
CreateIceTransport(scope, std::move(mock));
RTCIceGatherOptions* options = RTCIceGatherOptions::Create();
options->setGatherPolicy("all");
ice_transport->gather(options, ASSERT_NO_EXCEPTION);
} // ContextDestroyed when V8TestingScope goes out of scope.
ice_transport->ContextDestroyed(scope.GetExecutionContext());
RunUntilIdle();
EXPECT_TRUE(mock_deleted);
......
......@@ -4,7 +4,6 @@
#include "third_party/blink/renderer/modules/peerconnection/rtc_quic_stream.h"
#include "third_party/blink/renderer/core/dom/events/event.h"
#include "third_party/blink/renderer/modules/peerconnection/rtc_quic_transport.h"
#include "third_party/blink/renderer/platform/bindings/exception_state.h"
namespace blink {
......@@ -14,10 +13,7 @@ const uint32_t RTCQuicStream::kWriteBufferSize = 4 * 1024;
RTCQuicStream::RTCQuicStream(ExecutionContext* context,
RTCQuicTransport* transport,
QuicStreamProxy* stream_proxy)
: EventTargetWithInlineData(),
ContextClient(context),
transport_(transport),
proxy_(stream_proxy) {
: ContextClient(context), transport_(transport), proxy_(stream_proxy) {
DCHECK(transport_);
DCHECK(proxy_);
}
......@@ -96,7 +92,7 @@ void RTCQuicStream::finish() {
state_ = RTCQuicStreamState::kClosing;
} else {
DCHECK_EQ(state_, RTCQuicStreamState::kClosing);
Close();
Close(CloseReason::kReadWriteFinished);
}
}
......@@ -104,27 +100,11 @@ void RTCQuicStream::reset() {
if (IsClosed()) {
return;
}
proxy_->Reset();
readable_ = false;
Close();
}
void RTCQuicStream::Stop() {
readable_ = false;
state_ = RTCQuicStreamState::kClosed;
write_buffered_amount_ = 0;
proxy_ = nullptr;
}
void RTCQuicStream::Close() {
Stop();
transport_->RemoveStream(this);
Close(CloseReason::kLocalReset);
}
void RTCQuicStream::OnRemoteReset() {
DCHECK_NE(state_, RTCQuicStreamState::kClosed);
Close();
DispatchEvent(*Event::Create(event_type_names::kStatechange));
Close(CloseReason::kRemoteReset);
}
void RTCQuicStream::OnDataReceived(Vector<uint8_t> data, bool fin) {
......@@ -139,7 +119,7 @@ void RTCQuicStream::OnDataReceived(Vector<uint8_t> data, bool fin) {
state_ = RTCQuicStreamState::kClosing;
} else {
DCHECK_EQ(state_, RTCQuicStreamState::kClosing);
Close();
Close(CloseReason::kReadWriteFinished);
}
DispatchEvent(*Event::Create(event_type_names::kStatechange));
}
......@@ -149,6 +129,58 @@ void RTCQuicStream::OnWriteDataConsumed(uint32_t amount) {
write_buffered_amount_ -= amount;
}
void RTCQuicStream::OnQuicTransportClosed(
RTCQuicTransport::CloseReason reason) {
switch (reason) {
case RTCQuicTransport::CloseReason::kContextDestroyed:
Close(CloseReason::kContextDestroyed);
break;
default:
Close(CloseReason::kQuicTransportClosed);
break;
}
}
void RTCQuicStream::Close(CloseReason reason) {
DCHECK_NE(state_, RTCQuicStreamState::kClosed);
// Tear down the QuicStreamProxy.
// If the Close is caused by a remote event or regular use of WriteData, the
// QuicStreamProxy will have already been deleted.
// If the Close is caused by the transport then the transport is responsible
// for deleting the QuicStreamProxy.
if (reason == CloseReason::kLocalReset) {
// This deletes the QuicStreamProxy.
proxy_->Reset();
}
proxy_ = nullptr;
// Remove this stream from the RTCQuicTransport unless closing from a
// transport-level event.
switch (reason) {
case CloseReason::kReadWriteFinished:
case CloseReason::kLocalReset:
case CloseReason::kRemoteReset:
transport_->RemoveStream(this);
break;
case CloseReason::kQuicTransportClosed:
case CloseReason::kContextDestroyed:
// The RTCQuicTransport will handle clearing its list of streams.
break;
}
// Clear observable state.
readable_ = false;
write_buffered_amount_ = 0;
// Change the state. Fire the statechange event only if the close is caused by
// a remote stream event.
state_ = RTCQuicStreamState::kClosed;
if (reason == CloseReason::kRemoteReset) {
DispatchEvent(*Event::Create(event_type_names::kStatechange));
}
}
const AtomicString& RTCQuicStream::InterfaceName() const {
return event_target_names::kRTCQuicStream;
}
......
......@@ -11,13 +11,15 @@
#include "third_party/blink/renderer/modules/event_target_modules.h"
#include "third_party/blink/renderer/modules/modules_export.h"
#include "third_party/blink/renderer/modules/peerconnection/adapters/quic_stream_proxy.h"
#include "third_party/blink/renderer/modules/peerconnection/rtc_quic_transport.h"
namespace blink {
class RTCQuicTransport;
enum class RTCQuicStreamState { kNew, kOpening, kOpen, kClosing, kClosed };
// The RTCQuicStream does not need to be ActiveScriptWrappable since the
// RTCQuicTransport that it is associated with holds a strong reference to it
// as long as it is not closed.
class MODULES_EXPORT RTCQuicStream final : public EventTargetWithInlineData,
public ContextClient,
public QuicStreamProxy::Delegate {
......@@ -27,14 +29,26 @@ class MODULES_EXPORT RTCQuicStream final : public EventTargetWithInlineData,
// TODO(steveanton): These maybe should be adjustable.
static const uint32_t kWriteBufferSize;
enum class CloseReason {
// Both read and write sides have been finished.
kReadWriteFinished,
// reset() was called.
kLocalReset,
// The remote stream sent a reset().
kRemoteReset,
// The RTCQuicTransport has closed.
kQuicTransportClosed,
// The ExecutionContext is being destroyed.
kContextDestroyed,
};
RTCQuicStream(ExecutionContext* context,
RTCQuicTransport* transport,
QuicStreamProxy* stream_proxy);
~RTCQuicStream() override;
// Called from the RTCQuicTransport when it is being stopped.
void Stop();
bool IsClosed() const { return state_ == RTCQuicStreamState::kClosed; }
// Called by the RTCQuicTransport when it is being closed.
void OnQuicTransportClosed(RTCQuicTransport::CloseReason reason);
// rtc_quic_stream.idl
RTCQuicTransport* transport() const;
......@@ -55,16 +69,18 @@ class MODULES_EXPORT RTCQuicStream final : public EventTargetWithInlineData,
void Trace(blink::Visitor* visitor) override;
private:
// Closes the stream. This will change the state to kClosed and deregister it
// from the RTCQuicTransport. The QuicStreamProxy can no longer be used after
// this point.
void Close();
// QuicStreamProxy::Delegate overrides.
void OnRemoteReset() override;
void OnDataReceived(Vector<uint8_t> data, bool fin) override;
void OnWriteDataConsumed(uint32_t amount) override;
// Permenantly closes the RTCQuicStream with the given reason.
// The RTCQuicStream must not already be closed.
// This will transition the state to closed.
void Close(CloseReason reason);
bool IsClosed() const { return state_ == RTCQuicStreamState::kClosed; }
Member<RTCQuicTransport> transport_;
RTCQuicStreamState state_ = RTCQuicStreamState::kOpen;
bool readable_ = true;
......
......@@ -109,7 +109,7 @@ RTCQuicTransport::RTCQuicTransport(
const HeapVector<Member<RTCCertificate>>& certificates,
ExceptionState& exception_state,
std::unique_ptr<P2PQuicTransportFactory> p2p_quic_transport_factory)
: ContextLifecycleObserver(context),
: ContextClient(context),
transport_(transport),
certificates_(certificates),
p2p_quic_transport_factory_(std::move(p2p_quic_transport_factory)) {
......@@ -120,18 +120,6 @@ RTCQuicTransport::~RTCQuicTransport() {
DCHECK(!proxy_);
}
void RTCQuicTransport::Close(RTCQuicTransportState new_state) {
DCHECK(!IsClosed());
for (RTCQuicStream* stream : streams_) {
stream->Stop();
}
streams_.clear();
transport_->DisconnectConsumer(this);
proxy_.reset();
state_ = new_state;
DCHECK(IsClosed());
}
RTCIceTransport* RTCQuicTransport::transport() const {
return transport_;
}
......@@ -251,7 +239,7 @@ void RTCQuicTransport::StartConnection() {
proxy_->Start(std::move(rtc_fingerprints));
}
void RTCQuicTransport::OnTransportStarted() {
void RTCQuicTransport::OnIceTransportStarted() {
// The RTCIceTransport has now been started.
if (remote_parameters_) {
StartConnection();
......@@ -262,10 +250,7 @@ void RTCQuicTransport::stop() {
if (IsClosed()) {
return;
}
if (proxy_) {
proxy_->Stop();
}
Close(RTCQuicTransportState::kClosed);
Close(CloseReason::kLocalStopped);
}
RTCQuicStream* RTCQuicTransport::createStream(ExceptionState& exception_state) {
......@@ -301,13 +286,11 @@ void RTCQuicTransport::OnConnected() {
void RTCQuicTransport::OnConnectionFailed(const std::string& error_details,
bool from_remote) {
Close(RTCQuicTransportState::kFailed);
DispatchEvent(*Event::Create(event_type_names::kStatechange));
Close(CloseReason::kFailed);
}
void RTCQuicTransport::OnRemoteStopped() {
Close(RTCQuicTransportState::kClosed);
DispatchEvent(*Event::Create(event_type_names::kStatechange));
Close(CloseReason::kRemoteStopped);
}
void RTCQuicTransport::OnStream(QuicStreamProxy* stream_proxy) {
......@@ -315,6 +298,57 @@ void RTCQuicTransport::OnStream(QuicStreamProxy* stream_proxy) {
DispatchEvent(*RTCQuicStreamEvent::Create(stream));
}
void RTCQuicTransport::OnIceTransportClosed(
RTCIceTransport::CloseReason reason) {
if (reason == RTCIceTransport::CloseReason::kContextDestroyed) {
Close(CloseReason::kContextDestroyed);
} else {
Close(CloseReason::kIceTransportClosed);
}
}
void RTCQuicTransport::Close(CloseReason reason) {
DCHECK(!IsClosed());
// Disconnect from the RTCIceTransport, allowing a new RTCQuicTransport to
// connect to it.
transport_->DisconnectConsumer(this);
// Notify the active streams that the transport is closing.
for (RTCQuicStream* stream : streams_) {
stream->OnQuicTransportClosed(reason);
}
streams_.clear();
// Tear down the QuicTransportProxy and change the state.
switch (reason) {
case CloseReason::kLocalStopped:
case CloseReason::kIceTransportClosed:
case CloseReason::kContextDestroyed:
// The QuicTransportProxy may be active so gracefully Stop() before
// destroying it.
if (proxy_) {
proxy_->Stop();
proxy_.reset();
}
state_ = RTCQuicTransportState::kClosed;
break;
case CloseReason::kRemoteStopped:
case CloseReason::kFailed:
// The QuicTransportProxy has already been closed by the event, so just go
// ahead and delete it.
proxy_.reset();
state_ =
(reason == CloseReason::kFailed ? RTCQuicTransportState::kFailed
: RTCQuicTransportState::kClosed);
DispatchEvent(*Event::Create(event_type_names::kStatechange));
break;
}
DCHECK(!proxy_);
DCHECK(IsClosed());
}
bool RTCQuicTransport::RaiseExceptionIfClosed(
ExceptionState& exception_state) const {
if (IsClosed()) {
......@@ -331,15 +365,7 @@ const AtomicString& RTCQuicTransport::InterfaceName() const {
}
ExecutionContext* RTCQuicTransport::GetExecutionContext() const {
return ContextLifecycleObserver::GetExecutionContext();
}
void RTCQuicTransport::ContextDestroyed(ExecutionContext*) {
stop();
}
bool RTCQuicTransport::HasPendingActivity() const {
return static_cast<bool>(proxy_);
return ContextClient::GetExecutionContext();
}
void RTCQuicTransport::Trace(blink::Visitor* visitor) {
......@@ -349,7 +375,7 @@ void RTCQuicTransport::Trace(blink::Visitor* visitor) {
visitor->Trace(remote_parameters_);
visitor->Trace(streams_);
EventTargetWithInlineData::Trace(visitor);
ContextLifecycleObserver::Trace(visitor);
ContextClient::Trace(visitor);
}
} // namespace blink
......@@ -5,10 +5,10 @@
#ifndef THIRD_PARTY_BLINK_RENDERER_MODULES_PEERCONNECTION_RTC_QUIC_TRANSPORT_H_
#define THIRD_PARTY_BLINK_RENDERER_MODULES_PEERCONNECTION_RTC_QUIC_TRANSPORT_H_
#include "third_party/blink/renderer/bindings/core/v8/active_script_wrappable.h"
#include "third_party/blink/renderer/core/dom/context_lifecycle_observer.h"
#include "third_party/blink/renderer/modules/event_target_modules.h"
#include "third_party/blink/renderer/modules/peerconnection/adapters/quic_transport_proxy.h"
#include "third_party/blink/renderer/modules/peerconnection/rtc_ice_transport.h"
#include "third_party/blink/renderer/modules/peerconnection/rtc_quic_parameters.h"
namespace blink {
......@@ -16,7 +16,6 @@ namespace blink {
class DOMArrayBuffer;
class ExceptionState;
class RTCCertificate;
class RTCIceTransport;
class RTCQuicStream;
class P2PQuicTransportFactory;
......@@ -28,15 +27,30 @@ enum class RTCQuicTransportState {
kFailed
};
// The RTCQuicTransport does not need to be ActiveScriptWrappable since the
// RTCIceTransport to which it is attached holds a strong reference to it as
// long as it is alive.
class MODULES_EXPORT RTCQuicTransport final
: public EventTargetWithInlineData,
public ActiveScriptWrappable<RTCQuicTransport>,
public ContextLifecycleObserver,
public ContextClient,
public QuicTransportProxy::Delegate {
DEFINE_WRAPPERTYPEINFO();
USING_GARBAGE_COLLECTED_MIXIN(RTCQuicTransport);
public:
enum class CloseReason {
// stop() was called.
kLocalStopped,
// The remote side closed the QUIC connection.
kRemoteStopped,
// The QUIC connection failed.
kFailed,
// The RTCIceTransport was closed.
kIceTransportClosed,
// The ExecutionContext is being destroyed.
kContextDestroyed,
};
static RTCQuicTransport* Create(
ExecutionContext* context,
RTCIceTransport* transport,
......@@ -51,6 +65,12 @@ class MODULES_EXPORT RTCQuicTransport final
~RTCQuicTransport() override;
// Called by the RTCIceTransport when it is being closed.
void OnIceTransportClosed(RTCIceTransport::CloseReason reason);
// Called by the RTCIceTransport when its start() method is called.
void OnIceTransportStarted();
RTCQuicStream* AddStream(QuicStreamProxy* stream_proxy);
void RemoveStream(RTCQuicStream* stream);
......@@ -69,19 +89,10 @@ class MODULES_EXPORT RTCQuicTransport final
DEFINE_ATTRIBUTE_EVENT_LISTENER(error, kError);
DEFINE_ATTRIBUTE_EVENT_LISTENER(quicstream, kQuicstream);
// Called by the RTCIceTransport when its start() method is called.
void OnTransportStarted();
// EventTarget overrides.
const AtomicString& InterfaceName() const override;
ExecutionContext* GetExecutionContext() const override;
// ContextLifecycleObserver overrides.
void ContextDestroyed(ExecutionContext*) override;
// ActiveScriptWrappable overrides.
bool HasPendingActivity() const override;
// For garbage collection.
void Trace(blink::Visitor* visitor) override;
......@@ -100,15 +111,17 @@ class MODULES_EXPORT RTCQuicTransport final
void OnRemoteStopped() override;
void OnStream(QuicStreamProxy* stream_proxy) override;
bool IsClosed() const { return state_ == RTCQuicTransportState::kClosed; }
bool RaiseExceptionIfClosed(ExceptionState& exception_state) const;
// Starts the underlying QUIC connection.
void StartConnection();
// Close all streams, delete the underlying QUIC transport, and transition to
// the given state, closed or failed.
void Close(RTCQuicTransportState new_state);
// Permenantly closes the RTCQuicTransport with the given reason.
// The RTCQuicTransport must not already be closed or failed.
// This will transition the state to either closed or failed according to the
// reason.
void Close(CloseReason reason);
bool IsClosed() const { return state_ == RTCQuicTransportState::kClosed; }
bool RaiseExceptionIfClosed(ExceptionState& exception_state) const;
Member<RTCIceTransport> transport_;
RTCQuicTransportState state_ = RTCQuicTransportState::kNew;
......
......@@ -13,7 +13,6 @@ enum RTCQuicTransportState {
// https://w3c.github.io/webrtc-quic/#quic-transport*
[
ActiveScriptWrappable,
Constructor(RTCIceTransport transport, sequence<RTCCertificate> certificates),
ConstructorCallWith=ExecutionContext,
RaisesException=Constructor,
......
......@@ -246,22 +246,24 @@ TEST_F(RTCQuicTransportTest, RTCIceTransportStopDeletesP2PQuicTransport) {
// is ContextDestroyed.
TEST_F(RTCQuicTransportTest,
RTCIceTransportContextDestroyedDeletesP2PQuicTransport) {
V8TestingScope scope;
bool mock_deleted = false;
{
V8TestingScope scope;
Persistent<RTCIceTransport> ice_transport = CreateIceTransport(scope);
ice_transport->start(CreateRemoteRTCIceParameters1(), "controlling",
ASSERT_NO_EXCEPTION);
Persistent<RTCIceTransport> ice_transport = CreateIceTransport(scope);
ice_transport->start(CreateRemoteRTCIceParameters1(), "controlling",
ASSERT_NO_EXCEPTION);
bool mock_deleted = false;
auto mock_transport = std::make_unique<MockP2PQuicTransport>();
EXPECT_CALL(*mock_transport, Die()).WillOnce(Assign(&mock_deleted, true));
auto mock_transport = std::make_unique<MockP2PQuicTransport>();
EXPECT_CALL(*mock_transport, Die()).WillOnce(Assign(&mock_deleted, true));
Persistent<RTCQuicTransport> quic_transport =
CreateQuicTransport(scope, ice_transport, GenerateLocalRTCCertificates(),
std::move(mock_transport));
quic_transport->start(CreateRemoteRTCQuicParameters1(), ASSERT_NO_EXCEPTION);
Persistent<RTCQuicTransport> quic_transport = CreateQuicTransport(
scope, ice_transport, GenerateLocalRTCCertificates(),
std::move(mock_transport));
quic_transport->start(CreateRemoteRTCQuicParameters1(),
ASSERT_NO_EXCEPTION);
} // ContextDestroyed when V8TestingScope goes out of scope.
ice_transport->ContextDestroyed(scope.GetExecutionContext());
RunUntilIdle();
EXPECT_TRUE(mock_deleted);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment