Skip to content

Commit

Permalink
Use BinaryType for RTCDataChannel
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=260790
rdar://114559008

Reviewed by Youenn Fablet.

Aligns RTCDataChannel with its specification (and WebSocket). Also
address some nits in our WebSocket setBinaryType() implementation.

This largely matches Gecko. (Chromium does not appear to implement
this.) Gecko and the specification still have a different default
value.

Updated tests are upstreamed via
web-platform-tests/wpt#41663.

* LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCDataChannel-binaryType.window-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCDataChannel-binaryType.window.js:
(test):
(const.binaryType.of.invalidBinaryTypes.test):
* Source/WebCore/Modules/mediastream/RTCDataChannel.cpp:
(WebCore::RTCDataChannel::setBinaryType):
(WebCore::RTCDataChannel::didReceiveRawData):
(WebCore::blobKeyword): Deleted.
(WebCore::arraybufferKeyword): Deleted.
(WebCore::RTCDataChannel::binaryType const): Deleted.
* Source/WebCore/Modules/mediastream/RTCDataChannel.h:
* Source/WebCore/Modules/mediastream/RTCDataChannel.idl:
* Source/WebCore/Modules/websockets/WebSocket.cpp:
(WebCore::WebSocket::setBinaryType):
* Source/WebCore/Modules/websockets/WebSocket.h:

Canonical link: https://commits.webkit.org/267353@main
  • Loading branch information
annevk committed Aug 28, 2023
1 parent 3b7c510 commit 6cabcd4
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 54 deletions.
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@

FAIL Default binaryType value assert_equals: dc.binaryType should be 'blob' expected "blob" but got "arraybuffer"
PASS Setting binaryType to 'blob' should succeed
PASS Setting binaryType to 'arraybuffer' should succeed
PASS Setting invalid binaryType 'jellyfish' should throw SyntaxError
PASS Setting invalid binaryType 'arraybuffer ' should throw SyntaxError
PASS Setting invalid binaryType '' should throw SyntaxError
PASS Setting invalid binaryType 'null' should throw SyntaxError
PASS Setting invalid binaryType 'undefined' should throw SyntaxError
PASS Setting binaryType to 'jellyfish' should be ignored
PASS Setting binaryType to 'arraybuffer ' should be ignored
PASS Setting binaryType to '' should be ignored
PASS Setting binaryType to 'null' should be ignored
PASS Setting binaryType to 'undefined' should be ignored
PASS Setting binaryType to '234' should be ignored
PASS Setting binaryType to '54' should be ignored

Original file line number Diff line number Diff line change
@@ -1,7 +1,15 @@
'use strict';

const validBinaryTypes = ['blob', 'arraybuffer'];
const invalidBinaryTypes = ['jellyfish', 'arraybuffer ', '', null, undefined];
const invalidBinaryTypes = ['jellyfish', 'arraybuffer ', '', null, undefined, 234, 54n];

test((t) => {
const pc = new RTCPeerConnection();
t.add_cleanup(() => pc.close());
const dc = pc.createDataChannel('test-binary-type');

assert_equals(dc.binaryType, "blob", `dc.binaryType should be 'blob'`);
}, `Default binaryType value`);

for (const binaryType of validBinaryTypes) {
test((t) => {
Expand All @@ -20,8 +28,8 @@ for (const binaryType of invalidBinaryTypes) {
t.add_cleanup(() => pc.close());
const dc = pc.createDataChannel('test-binary-type');

assert_throws_dom('SyntaxError', () => {
dc.binaryType = binaryType;
});
}, `Setting invalid binaryType '${binaryType}' should throw SyntaxError`);
dc.binaryType = "arraybuffer";
dc.binaryType = binaryType;
assert_equals(dc.binaryType, "arraybuffer");
}, `Setting binaryType to '${binaryType}' should be ignored`);
}
39 changes: 3 additions & 36 deletions Source/WebCore/Modules/mediastream/RTCDataChannel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,18 +47,6 @@ namespace WebCore {

WTF_MAKE_ISO_ALLOCATED_IMPL(RTCDataChannel);

static const AtomString& blobKeyword()
{
static MainThreadNeverDestroyed<const AtomString> blob("blob"_s);
return blob;
}

static const AtomString& arraybufferKeyword()
{
static MainThreadNeverDestroyed<const AtomString> arraybuffer("arraybuffer"_s);
return arraybuffer;
}

Ref<RTCDataChannel> RTCDataChannel::create(ScriptExecutionContext& context, std::unique_ptr<RTCDataChannelHandler>&& handler, String&& label, RTCDataChannelInit&& options, RTCDataChannelState state)
{
ASSERT(handler);
Expand Down Expand Up @@ -113,30 +101,9 @@ std::optional<unsigned short> RTCDataChannel::id() const
return m_options.id;
}

const AtomString& RTCDataChannel::binaryType() const
{
switch (m_binaryType) {
case BinaryType::Blob:
return blobKeyword();
case BinaryType::ArrayBuffer:
return arraybufferKeyword();
}

ASSERT_NOT_REACHED();
return emptyAtom();
}

ExceptionOr<void> RTCDataChannel::setBinaryType(const AtomString& binaryType)
void RTCDataChannel::setBinaryType(BinaryType binaryType)
{
if (binaryType == blobKeyword()) {
m_binaryType = BinaryType::Blob;
return { };
}
if (binaryType == arraybufferKeyword()) {
m_binaryType = BinaryType::ArrayBuffer;
return { };
}
return Exception { SyntaxError };
m_binaryType = binaryType;
}

ExceptionOr<void> RTCDataChannel::send(const String& data)
Expand Down Expand Up @@ -245,7 +212,7 @@ void RTCDataChannel::didReceiveRawData(const uint8_t* data, size_t dataLength)
case BinaryType::Blob:
scheduleDispatchEvent(MessageEvent::create(Blob::create(scriptExecutionContext(), Vector { data, dataLength }, emptyString()), { }));
return;
case BinaryType::ArrayBuffer:
case BinaryType::Arraybuffer:
scheduleDispatchEvent(MessageEvent::create(ArrayBuffer::create(data, dataLength)));
return;
}
Expand Down
8 changes: 4 additions & 4 deletions Source/WebCore/Modules/mediastream/RTCDataChannel.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,9 @@ class RTCDataChannel final : public RefCounted<RTCDataChannel>, public ActiveDOM
size_t bufferedAmountLowThreshold() const { return m_bufferedAmountLowThreshold; }
void setBufferedAmountLowThreshold(size_t value) { m_bufferedAmountLowThreshold = value; }

const AtomString& binaryType() const;
ExceptionOr<void> setBinaryType(const AtomString&);
enum class BinaryType : bool { Blob, Arraybuffer };
BinaryType binaryType() const { return m_binaryType; }
void setBinaryType(BinaryType);

ExceptionOr<void> send(const String&);
ExceptionOr<void> send(JSC::ArrayBuffer&);
Expand Down Expand Up @@ -124,8 +125,7 @@ class RTCDataChannel final : public RefCounted<RTCDataChannel>, public ActiveDOM
bool m_stopped { false };
RTCDataChannelState m_readyState { RTCDataChannelState::Connecting };

enum class BinaryType { Blob, ArrayBuffer };
BinaryType m_binaryType { BinaryType::ArrayBuffer };
BinaryType m_binaryType { BinaryType::Arraybuffer };

String m_label;
RTCDataChannelInit m_options;
Expand Down
4 changes: 3 additions & 1 deletion Source/WebCore/Modules/mediastream/RTCDataChannel.idl
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/

enum BinaryType { "blob", "arraybuffer" };

[
ActiveDOMObject,
Conditional=WEB_RTC,
Expand All @@ -43,7 +45,7 @@
readonly attribute RTCPriorityType priority;
attribute unsigned long bufferedAmountLowThreshold;

attribute [AtomString] DOMString binaryType;
attribute BinaryType binaryType;

undefined send(ArrayBuffer data);
undefined send(ArrayBufferView data);
Expand Down
3 changes: 1 addition & 2 deletions Source/WebCore/Modules/websockets/WebSocket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -476,10 +476,9 @@ String WebSocket::extensions() const
return m_extensions;
}

ExceptionOr<void> WebSocket::setBinaryType(BinaryType binaryType)
void WebSocket::setBinaryType(BinaryType binaryType)
{
m_binaryType = binaryType;
return { };
}

EventTargetInterface WebSocket::eventTargetInterface() const
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/Modules/websockets/WebSocket.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ class WebSocket final : public RefCounted<WebSocket>, public EventTarget, public

enum class BinaryType : bool { Blob, Arraybuffer };
BinaryType binaryType() const { return m_binaryType; }
ExceptionOr<void> setBinaryType(BinaryType);
void setBinaryType(BinaryType);

ScriptExecutionContext* scriptExecutionContext() const final;

Expand Down

0 comments on commit 6cabcd4

Please sign in to comment.