Bug 264569

Summary:	Default binaryType value in RTCDataChannel should be 'Blob'
Product:	WebKit	Reporter:	Ahmad Saleem <ahmad.saleem792>
Component:	WebRTC	Assignee:	Nobody <webkit-unassigned>
Status:	RESOLVED CONFIGURATION CHANGED
Severity:	Normal	CC:	webkit-bug-importer, youennf
Priority:	P2	Keywords:	InRadar, WPTImpact
Version:	WebKit Nightly Build
Hardware:	Unspecified
OS:	Unspecified

Description Ahmad Saleem 2023-11-10 05:08:29 PST

Hi Team,

While looking into WebRTC WPT failures, I came across another failing test:

WPT Test Case (Live Link) - http://wpt.live/webrtc/RTCDataChannel-binaryType.window.html

Web-Spec: https://w3c.github.io/webrtc-pc/#dom-datachannel-binarytype

"When an RTCDataChannel object is created, the binaryType attribute MUST be initialized to the string "blob"."

While in above test case, we get 'ArrayBuffer'.

Could it be to just update this?

https://searchfox.org/wubkat/rev/eaadafa59570232390736829d85d65c69bd8d91d/Source/WebCore/Modules/mediastream/RTCDataChannel.h#128

From:

BinaryType m_binaryType { BinaryType::Arraybuffer };

to:

BinaryType m_binaryType { BinaryType::Blog };

____

Just wanted to raise so we can track it and fix it.

Thanks!

Comment 1 Ahmad Saleem 2023-11-10 05:10:03 PST

Typo:

BinaryType m_binaryType { BinaryType::Blog };

^ Wrong, I meant:

BinaryType m_binaryType { BinaryType::Blob };

Comment 2 Ahmad Saleem 2023-11-10 05:43:18 PST

(In reply to Ahmad Saleem from comment #1)
> Typo:
> 
> BinaryType m_binaryType { BinaryType::Blog };
> 
> ^ Wrong, I meant:
> 
> BinaryType m_binaryType { BinaryType::Blob };

this change indeed fix the failing test case.

Comment 3 youenn fablet 2023-11-10 06:51:29 PST

This change is controversial as Chrome is not supporting blob binaryType and is defaulting to arrayBuffer.
We would need to identify the potential breakage to actually align with the spec (or change the spec if we cannot).

Comment 4 Ahmad Saleem 2023-11-10 06:52:29 PST

(In reply to youenn fablet from comment #3)
> This change is controversial as Chrome is not supporting blob binaryType and
> is defaulting to arrayBuffer.
> We would need to identify the potential breakage to actually align with the
> spec (or change the spec if we cannot).

Oh! Should I close my PR? https://github.com/WebKit/WebKit/pull/20305

Comment 5 Ahmad Saleem 2023-11-10 06:56:12 PST

WebRTC bug mentioned by Chromium for future reference: https://bugs.chromium.org/p/webrtc/issues/detail?id=2276

Comment 6 youenn fablet 2023-11-10 07:15:31 PST

> Oh! Should I close my PR? https://github.com/WebKit/WebKit/pull/20305

Let's do that until we are ready for it.

Comment 7 Ahmad Saleem 2023-11-10 07:31:53 PST

(In reply to youenn fablet from comment #6)
> > Oh! Should I close my PR? https://github.com/WebKit/WebKit/pull/20305
> 
> Let's do that until we are ready for it.

Perfect! No worries.

For future reference for anyone else: https://github.com/WebKit/WebKit/pull/20305

Comment 8 Ahmad Saleem 2023-11-10 07:36:32 PST

@youenn - just one thing to clarify that we do implement 'RTCDataChannel: Blob' support (unlike Chromium / Blink) but we don't use it as 'default' value?

Why question is stemming from that Blink fails following test as well:

>> Failed to set the 'binaryType' property on 'RTCDataChannel': Blob support not implemented yet

While Safari / WebKit - we pass this. I think Chromium / Blink has issue even in supporting 'Blob' while we just have issue around setting it as 'default'. Is Chromium / Blink looking to drop 'Blob' altogether from web-spec?

Comment 9 Ahmad Saleem 2023-11-10 07:58:38 PST

NOTE - this also progress following failing WPT test cases:

> RTCDataChannel-send.html [http://wpt.live/webrtc/RTCDataChannel-send.html]

Datachannel binaryType should receive message as Blob by default
Negotiated datachannel binaryType should receive message as Blob by default

> RTCPeerConnection-createDataChannel.html [http://wpt.live/webrtc/RTCPeerConnection-createDataChannel.html]

createDataChannel attribute default values
createDataChannel with provided parameters should initialize attributes to provided values

Comment 10 Radar WebKit Bug Importer 2023-11-17 05:09:13 PST

<rdar://problem/118560558>

Comment 11 Ahmad Saleem 2023-11-27 15:55:37 PST

This got merged in Web-Spec: https://github.com/w3c/webrtc-pc/pull/2909

Just two weeks ago:

Here: https://www.w3.org/TR/webrtc/#dom-datachannel-binarytype

The change seems to be approved by Google Engineer (https://github.com/henbos) as well.

Comment 12 youenn fablet 2023-11-28 03:13:47 PST

The plan for now is to change the spec and make the default value 'arrayBuffer' which should put WebKit conformant.

Comment 13 youenn fablet 2023-12-11 06:30:31 PST

Spec has changed to align with WebKit behaviour, closing.
We should rebase the WPT tests once updated according the latest spec.