Support legacy offerToReceiveAudio/offerToReceiveVideo fields in RTCOfferOptions.

Support legacy offerToReceiveAudio/offerToReceiveVideo fields in RTCOfferOptions.

BUG=619289
Committed: https://crrev.com/b0e609db70af6bab30628e7bf14ad6a8add1a40e
Cr-Commit-Position: refs/heads/master@{#400927}

[Guido Urdaneta, 2016-06-20 12:43:02]

Description was changed from

==========
Support legacy offerToReceiveAudio/offerToReceiveVideo fields in
RTCOfferOptions.


BUG=619289
==========

to

==========
Support legacy offerToReceiveAudio/offerToReceiveVideo fields in
RTCOfferOptions.


BUG=619289
==========

[Guido Urdaneta, 2016-06-20 12:43:02]

guidou@chromium.org changed reviewers:
+ foolip@chromium.org

[Guido Urdaneta, 2016-06-20 12:44:03]

Hi, PTAL

[foolip, 2016-06-20 12:54:04]

Description was changed from

==========
Support legacy offerToReceiveAudio/offerToReceiveVideo fields in
RTCOfferOptions.


BUG=619289
==========

to

==========
Support legacy offerToReceiveAudio/offerToReceiveVideo fields in
RTCOfferOptions.

BUG=619289
==========

[foolip, 2016-06-20 13:17:46]

foolip@chromium.org changed reviewers:
+ rbyers@chromium.org

[foolip, 2016-06-20 13:17:46]

Can this be tested?

Let's wait for a day or so to see what happens to
https://github.com/w3c/webrtc-pc/issues/709, but I hope that it's added back to
the spec. Adding another API owner for sanity check, since this is adding
something ostensibly non-standard on the hope that the spec situation will sort
itself out.

https://codereview.chromium.org/2077323003/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/mediastream/RTCOfferOptions.idl (right):

https://codereview.chromium.org/2077323003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/mediastream/RTCOfferOptions.idl:10: //
TODO(guidou): Deprecate the offerToReceive fields once transceiver
Link to https://github.com/w3c/webrtc-pc/issues/709

[Guido Urdaneta, 2016-06-20 13:34:46]

Added a way to test that the offerToReceive options are not ignored.

https://codereview.chromium.org/2077323003/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/mediastream/RTCOfferOptions.idl (right):

https://codereview.chromium.org/2077323003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/mediastream/RTCOfferOptions.idl:10: //
TODO(guidou): Deprecate the offerToReceive fields once transceiver
On 2016/06/20 13:17:46, Philip Jägenstedt wrote:
> Link to https://github.com/w3c/webrtc-pc/issues/709

Done.

[foolip, 2016-06-20 13:59:05]

lgtm, but I'd like to hear Rick's thoughts as well.

https://codereview.chromium.org/2077323003/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/mediastream/RTCPeerConnection-createOffer-promise.html
(right):

https://codereview.chromium.org/2077323003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/mediastream/RTCPeerConnection-createOffer-promise.html:40:
pc.createOffer({voiceActivityDetection:true, iceRestart:true,
offerToReceiveAudio:0, offerToReceiveVideo: 0}));
An extra space here if you need to make another change anyway. (Otherwise not
worth the bot time.)

[Guido Urdaneta, 2016-06-20 14:03:36]

guidou@chromium.org changed reviewers:
+ tommi@chromium.org

[Guido Urdaneta, 2016-06-20 14:03:36]

tommi@chromium.org: Please review changes in WebKit/Source/modules/mediastream

[tommi (sloooow) - chröme, 2016-06-20 15:32:09]

lgtm

[Rick Byers, 2016-06-20 17:13:31]

On 2016/06/20 13:59:05, Philip Jägenstedt wrote:
> lgtm, but I'd like to hear Rick's thoughts as well.

If I understand correctly, all we're doing here is decoupling the question of
the API form (promise-returning or not) from the argument form (options includes
no-longer-standard APIs).  It seems safest to me to decouple these concerns.

Can we add UseCounters here to measure how often these options are used in the
wild?  That's probably the key piece of data we're missing to inform whether
they should be deprecated or re-standardized, right?

> 
>
https://codereview.chromium.org/2077323003/diff/40001/third_party/WebKit/Layo...
> File
>
third_party/WebKit/LayoutTests/fast/mediastream/RTCPeerConnection-createOffer-promise.html
> (right):
> 
>
https://codereview.chromium.org/2077323003/diff/40001/third_party/WebKit/Layo...
>
third_party/WebKit/LayoutTests/fast/mediastream/RTCPeerConnection-createOffer-promise.html:40:
> pc.createOffer({voiceActivityDetection:true, iceRestart:true,
> offerToReceiveAudio:0, offerToReceiveVideo: 0}));
> An extra space here if you need to make another change anyway. (Otherwise not
> worth the bot time.)

[Rick Byers, 2016-06-20 17:17:12]

On 2016/06/20 17:13:31, Rick Byers wrote:
> On 2016/06/20 13:59:05, Philip Jägenstedt wrote:
> > lgtm, but I'd like to hear Rick's thoughts as well.
> 
> If I understand correctly, all we're doing here is decoupling the question of
> the API form (promise-returning or not) from the argument form (options
includes
> no-longer-standard APIs).  It seems safest to me to decouple these concerns.

In case that's not obvious, that's an LGTM on the API approach from me.  I don't
think this warrants a separate intent or anything - we're just preserving the
non-standard options API we already have.

> Can we add UseCounters here to measure how often these options are used in the
> wild?  That's probably the key piece of data we're missing to inform whether
> they should be deprecated or re-standardized, right?
> 
> > 
> >
>
https://codereview.chromium.org/2077323003/diff/40001/third_party/WebKit/Layo...
> > File
> >
>
third_party/WebKit/LayoutTests/fast/mediastream/RTCPeerConnection-createOffer-promise.html
> > (right):
> > 
> >
>
https://codereview.chromium.org/2077323003/diff/40001/third_party/WebKit/Layo...
> >
>
third_party/WebKit/LayoutTests/fast/mediastream/RTCPeerConnection-createOffer-promise.html:40:
> > pc.createOffer({voiceActivityDetection:true, iceRestart:true,
> > offerToReceiveAudio:0, offerToReceiveVideo: 0}));
> > An extra space here if you need to make another change anyway. (Otherwise
not
> > worth the bot time.)

[foolip, 2016-06-20 20:52:55]

On 2016/06/20 17:17:12, Rick Byers wrote:
> On 2016/06/20 17:13:31, Rick Byers wrote:
> > On 2016/06/20 13:59:05, Philip Jägenstedt wrote:
> > > lgtm, but I'd like to hear Rick's thoughts as well.
> > 
> > If I understand correctly, all we're doing here is decoupling the question
of
> > the API form (promise-returning or not) from the argument form (options
> includes
> > no-longer-standard APIs).  It seems safest to me to decouple these concerns.
> 
> In case that's not obvious, that's an LGTM on the API approach from me.  I
don't
> think this warrants a separate intent or anything - we're just preserving the
> non-standard options API we already have.

Note that the options supported in both contexts will still not be the same. In
the IDL, the promise-based createOffer takes a RTCOfferOptions, while the
callback-base createOffer takes a Dictionary with handling in C++ which supports
more options. (The spec has RTCOfferOptions in both contexts.)

For each "legacy" thing we copy over from the callback-based to the
promise-based variant, it will likely stick forever, so we're basically deciding
the outcome of https://github.com/w3c/webrtc-pc/issues/709 which is why I wanted
a sanity check.

[Guido Urdaneta, 2016-06-21 09:08:16]

The CQ bit was checked by guidou@chromium.org

[commit-bot: I haz the power, 2016-06-21 09:08:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2077323003/40001

[commit-bot: I haz the power, 2016-06-21 09:13:32]

Description was changed from

==========
Support legacy offerToReceiveAudio/offerToReceiveVideo fields in
RTCOfferOptions.

BUG=619289
==========

to

==========
Support legacy offerToReceiveAudio/offerToReceiveVideo fields in
RTCOfferOptions.

BUG=619289
==========

[commit-bot: I haz the power, 2016-06-21 09:13:33]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-21 09:14:57]

Description was changed from

==========
Support legacy offerToReceiveAudio/offerToReceiveVideo fields in
RTCOfferOptions.

BUG=619289
==========

to

==========
Support legacy offerToReceiveAudio/offerToReceiveVideo fields in
RTCOfferOptions.

BUG=619289

Committed: https://crrev.com/b0e609db70af6bab30628e7bf14ad6a8add1a40e
Cr-Commit-Position: refs/heads/master@{#400927}
==========

[commit-bot: I haz the power, 2016-06-21 09:14:59]

Patchset 3 (id:??) landed as
https://crrev.com/b0e609db70af6bab30628e7bf14ad6a8add1a40e
Cr-Commit-Position: refs/heads/master@{#400927}