Remove the 'audioDebugRecording' media constraint and the aec_dump AudioOptions flag.

Remove the 'audioDebugRecording' media constraint and the aec_dump AudioOptions flag.

BUG=webrtc:4690
Committed: https://crrev.com/65c8fd78c627dae246ee04b7202e296f44cb0a1c
Cr-Commit-Position: refs/heads/master@{#11753}

[the sun, 2016-01-07 14:04:17]

solenberg@webrtc.org changed reviewers:
+ pthatcher@webrtc.org

[the sun, 2016-01-13 13:22:11]

Description was changed from

==========
Remove the 'audioDebugRecording' media constraint and the aec_dump AudioOptions
flag.

BUG=webrtc:4690
==========

to

==========
Remove the 'audioDebugRecording' media constraint and the aec_dump AudioOptions
flag.

BUG=webrtc:4690
==========

[the sun, 2016-01-13 13:22:11]

solenberg@webrtc.org changed reviewers:
+ henrik.lundin@webrtc.org

[tommi, 2016-01-18 09:16:14]

+hta fyi

[hta-webrtc, 2016-01-18 10:25:46]

hta@webrtc.org changed reviewers:
+ hta@webrtc.org

[hta-webrtc, 2016-01-18 10:25:46]

What's the desired result if this constraint is specified as a mandatory or
optional constraint?

I'm not sure what current behavior is, but if I remove it in the code that I
submitted last week, you'll get a TypeError if you specify it as a mandatory
constraint, while it will be ignored as an optional constraint.

[tlegrand-webrtc, 2016-01-19 11:52:51]

Fredrik, can you update the description with why this change is needed, as
opposed to how it is done?

[the sun, 2016-01-21 13:51:14]

On 2016/01/19 11:52:51, tlegrand-webrtc wrote:
> Fredrik, can you update the description with why this change is needed, as
> opposed to how it is done?

Regard this CL as *raising the question* whether the constraint can be removed
or not.

The problem as I see it is that 'audioDebugRecording' allows someone to set up a
webpage which fills up device space for the user. Yes, the user will have to
allow the mic to be enabled, but using storage seems an unlikely consequence of
that. Correct me if I'm wrong, but the stored aecdumps won't be removed using
the usual clear cache/history options either?

So given that we have another API to enable debug recordings
(PeerConnectionFactoryInterface::StartAecDump()), is there any reason to keep
the constraint?

[Henrik Grunell WebRTC, 2016-01-25 11:08:13]

On 2016/01/21 13:51:14, the sun wrote:
> On 2016/01/19 11:52:51, tlegrand-webrtc wrote:
> > Fredrik, can you update the description with why this change is needed, as
> > opposed to how it is done?
> 
> Regard this CL as *raising the question* whether the constraint can be removed
> or not.
> 
> The problem as I see it is that 'audioDebugRecording' allows someone to set up
a
> webpage which fills up device space for the user. Yes, the user will have to
> allow the mic to be enabled, but using storage seems an unlikely consequence
of
> that. Correct me if I'm wrong, but the stored aecdumps won't be removed using
> the usual clear cache/history options either?
> 
> So given that we have another API to enable debug recordings
> (PeerConnectionFactoryInterface::StartAecDump()), is there any reason to keep
> the constraint?

Fwiw, it's not possible in Chrome to enable this via a constraint.

[the sun, 2016-02-01 09:16:14]

On 2016/01/25 11:08:13, Henrik Grunell (webrtc) wrote:
> On 2016/01/21 13:51:14, the sun wrote:
> > On 2016/01/19 11:52:51, tlegrand-webrtc wrote:
> > > Fredrik, can you update the description with why this change is needed, as
> > > opposed to how it is done?
> > 
> > Regard this CL as *raising the question* whether the constraint can be
removed
> > or not.
> > 
> > The problem as I see it is that 'audioDebugRecording' allows someone to set
up
> a
> > webpage which fills up device space for the user. Yes, the user will have to
> > allow the mic to be enabled, but using storage seems an unlikely consequence
> of
> > that. Correct me if I'm wrong, but the stored aecdumps won't be removed
using
> > the usual clear cache/history options either?
> > 
> > So given that we have another API to enable debug recordings
> > (PeerConnectionFactoryInterface::StartAecDump()), is there any reason to
keep
> > the constraint?
> 
> Fwiw, it's not possible in Chrome to enable this via a constraint.

Thanks for pointing that out Henrik!

The constraint appears unused in other clients as well; Harald, is there a
reason to keep it for compliance reasons?

[Henrik Grunell WebRTC, 2016-02-02 06:52:41]

On 2016/02/01 09:16:14, the sun wrote:
> On 2016/01/25 11:08:13, Henrik Grunell (webrtc) wrote:
> > On 2016/01/21 13:51:14, the sun wrote:
> > > On 2016/01/19 11:52:51, tlegrand-webrtc wrote:
> > > > Fredrik, can you update the description with why this change is needed,
as
> > > > opposed to how it is done?
> > > 
> > > Regard this CL as *raising the question* whether the constraint can be
> removed
> > > or not.
> > > 
> > > The problem as I see it is that 'audioDebugRecording' allows someone to
set
> up
> > a
> > > webpage which fills up device space for the user. Yes, the user will have
to
> > > allow the mic to be enabled, but using storage seems an unlikely
consequence
> > of
> > > that. Correct me if I'm wrong, but the stored aecdumps won't be removed
> using
> > > the usual clear cache/history options either?
> > > 
> > > So given that we have another API to enable debug recordings
> > > (PeerConnectionFactoryInterface::StartAecDump()), is there any reason to
> keep
> > > the constraint?
> > 
> > Fwiw, it's not possible in Chrome to enable this via a constraint.
> 
> Thanks for pointing that out Henrik!
> 
> The constraint appears unused in other clients as well; Harald, is there a
> reason to keep it for compliance reasons?

There is one place in blink to remove as well, created
https://codereview.chromium.org/1659803002/ for that. To be sure, I verified
that I couldn't enable the recording via a constraint in Chrome (before the
change in the CL).

[the sun, 2016-02-17 12:29:29]

solenberg@webrtc.org changed reviewers:
+ henrikg@webrtc.org
- henrik.lundin@webrtc.org, pthatcher@webrtc.org

[the sun, 2016-02-17 12:31:51]

On 2016/02/02 06:52:41, Henrik Grunell (webrtc) wrote:
> On 2016/02/01 09:16:14, the sun wrote:
> > On 2016/01/25 11:08:13, Henrik Grunell (webrtc) wrote:
> > > On 2016/01/21 13:51:14, the sun wrote:
> > > > On 2016/01/19 11:52:51, tlegrand-webrtc wrote:
> > > > > Fredrik, can you update the description with why this change is
needed,
> as
> > > > > opposed to how it is done?
> > > > 
> > > > Regard this CL as *raising the question* whether the constraint can be
> > removed
> > > > or not.
> > > > 
> > > > The problem as I see it is that 'audioDebugRecording' allows someone to
> set
> > up
> > > a
> > > > webpage which fills up device space for the user. Yes, the user will
have
> to
> > > > allow the mic to be enabled, but using storage seems an unlikely
> consequence
> > > of
> > > > that. Correct me if I'm wrong, but the stored aecdumps won't be removed
> > using
> > > > the usual clear cache/history options either?
> > > > 
> > > > So given that we have another API to enable debug recordings
> > > > (PeerConnectionFactoryInterface::StartAecDump()), is there any reason to
> > keep
> > > > the constraint?
> > > 
> > > Fwiw, it's not possible in Chrome to enable this via a constraint.
> > 
> > Thanks for pointing that out Henrik!
> > 
> > The constraint appears unused in other clients as well; Harald, is there a
> > reason to keep it for compliance reasons?
> 
> There is one place in blink to remove as well, created
> https://codereview.chromium.org/1659803002/ for that. To be sure, I verified
> that I couldn't enable the recording via a constraint in Chrome (before the
> change in the CL).

Changed reviewers to: hta@, henrikg@

[Henrik Grunell WebRTC, 2016-02-17 13:15:30]

On 2016/02/17 12:31:51, the sun wrote:
> On 2016/02/02 06:52:41, Henrik Grunell (webrtc) wrote:
> > On 2016/02/01 09:16:14, the sun wrote:
> > > On 2016/01/25 11:08:13, Henrik Grunell (webrtc) wrote:
> > > > On 2016/01/21 13:51:14, the sun wrote:
> > > > > On 2016/01/19 11:52:51, tlegrand-webrtc wrote:
> > > > > > Fredrik, can you update the description with why this change is
> needed,
> > as
> > > > > > opposed to how it is done?
> > > > > 
> > > > > Regard this CL as *raising the question* whether the constraint can be
> > > removed
> > > > > or not.
> > > > > 
> > > > > The problem as I see it is that 'audioDebugRecording' allows someone
to
> > set
> > > up
> > > > a
> > > > > webpage which fills up device space for the user. Yes, the user will
> have
> > to
> > > > > allow the mic to be enabled, but using storage seems an unlikely
> > consequence
> > > > of
> > > > > that. Correct me if I'm wrong, but the stored aecdumps won't be
removed
> > > using
> > > > > the usual clear cache/history options either?
> > > > > 
> > > > > So given that we have another API to enable debug recordings
> > > > > (PeerConnectionFactoryInterface::StartAecDump()), is there any reason
to
> > > keep
> > > > > the constraint?
> > > > 
> > > > Fwiw, it's not possible in Chrome to enable this via a constraint.
> > > 
> > > Thanks for pointing that out Henrik!
> > > 
> > > The constraint appears unused in other clients as well; Harald, is there a
> > > reason to keep it for compliance reasons?
> > 
> > There is one place in blink to remove as well, created
> > https://codereview.chromium.org/1659803002/ for that. To be sure, I verified
> > that I couldn't enable the recording via a constraint in Chrome (before the
> > change in the CL).
> 
> Changed reviewers to: hta@, henrikg@

Just fyi, I'm not an owner for any of the files. Still want a review?

[Henrik Grunell WebRTC, 2016-02-17 13:17:23]

Eh, I reviewed it anyway. lgtm.

[the sun, 2016-02-17 13:21:00]

On 2016/02/17 13:17:23, Henrik Grunell (webrtc) wrote:
> Eh, I reviewed it anyway. lgtm.

I thought it made sense that you review it since you took care of the
corresponding change in Chromium.

[Henrik Grunell WebRTC, 2016-02-17 13:42:28]

On 2016/02/17 13:21:00, the sun wrote:
> On 2016/02/17 13:17:23, Henrik Grunell (webrtc) wrote:
> > Eh, I reviewed it anyway. lgtm.
> 
> I thought it made sense that you review it since you took care of the
> corresponding change in Chromium.

Figured that.

[the sun, 2016-02-23 20:04:27]

On 2016/02/17 13:42:28, Henrik Grunell (webrtc) wrote:
> On 2016/02/17 13:21:00, the sun wrote:
> > On 2016/02/17 13:17:23, Henrik Grunell (webrtc) wrote:
> > > Eh, I reviewed it anyway. lgtm.
> > 
> > I thought it made sense that you review it since you took care of the
> > corresponding change in Chromium.
> 
> Figured that.

Ping hta@...

[hta-webrtc, 2016-02-23 22:16:01]

lgtm

good to see it disappear!

[the sun, 2016-02-24 08:57:44]

The CQ bit was checked by solenberg@webrtc.org

[the sun, 2016-02-24 08:57:44]

The patchset sent to the CQ was uploaded after l-g-t-m from henrikg@webrtc.org,
hta@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1565133002/#ps80001 (title:
"le rebase")

[commit-bot: I haz the power, 2016-02-24 08:57:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1565133002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1565133002/80001

[commit-bot: I haz the power, 2016-02-24 09:07:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-24 09:07:11]

Try jobs failed on following builders:
  presubmit on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/3645)

[the sun, 2016-02-24 09:22:50]

solenberg@webrtc.org changed reviewers:
+ tommi@webrtc.org

[the sun, 2016-02-24 09:22:51]

Adding tommi@ for OWNERs approval of webrtc/api

[tommi, 2016-02-24 22:41:38]

The CQ bit was checked by tommi@webrtc.org

[tommi, 2016-02-24 22:41:39]

lgtm

[commit-bot: I haz the power, 2016-02-24 22:41:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1565133002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1565133002/80001

[commit-bot: I haz the power, 2016-02-24 22:43:14]

Description was changed from

==========
Remove the 'audioDebugRecording' media constraint and the aec_dump AudioOptions
flag.

BUG=webrtc:4690
==========

to

==========
Remove the 'audioDebugRecording' media constraint and the aec_dump AudioOptions
flag.

BUG=webrtc:4690
==========

[commit-bot: I haz the power, 2016-02-24 22:43:15]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-02-24 22:43:22]

Description was changed from

==========
Remove the 'audioDebugRecording' media constraint and the aec_dump AudioOptions
flag.

BUG=webrtc:4690
==========

to

==========
Remove the 'audioDebugRecording' media constraint and the aec_dump AudioOptions
flag.

BUG=webrtc:4690

Committed: https://crrev.com/65c8fd78c627dae246ee04b7202e296f44cb0a1c
Cr-Commit-Position: refs/heads/master@{#11753}
==========

[commit-bot: I haz the power, 2016-02-24 22:43:23]

Patchset 5 (id:??) landed as
https://crrev.com/65c8fd78c627dae246ee04b7202e296f44cb0a1c
Cr-Commit-Position: refs/heads/master@{#11753}