Bug 264569 – Default binaryType value in RTCDataChannel should be 'Blob'

• Status: RESOLVED
• Resolution: CONFIGURATION CHANGED
• Component: WebRTC

[Ahmad Saleem]

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!

[Ahmad Saleem]

Typo:

BinaryType m_binaryType { BinaryType::Blog };

^ Wrong, I meant:

BinaryType m_binaryType { BinaryType::Blob };

[Ahmad Saleem]

(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.

[youenn fablet]

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).

[Ahmad Saleem]

(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

[Ahmad Saleem]

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

[youenn fablet]

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

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

[Ahmad Saleem]

(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

[Ahmad Saleem]

@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?

[Ahmad Saleem]

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

[Radar WebKit Bug Importer]

<rdar://problem/118560558>

[Ahmad Saleem]

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.

[youenn fablet]

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

[youenn fablet]

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