WebrtcAudioPrivateSetAudioExperimentsFunction dummy implementation

WebrtcAudioPrivateSetAudioExperimentsFunction dummy implementation

This change introduces a dummy implementation of
WebrtcAudioPrivateSetAudioExperimentsFunction::RunAsync() which is
invoked when Chrome is build without support for WebRTC
(enable_webrtc=false).

BUG=730019, 708475
Review-Url: https://codereview.chromium.org/2949563002
Cr-Commit-Position: refs/heads/master@{#481532}
Committed: https://chromium.googlesource.com/chromium/src/+/66e6a51773c74189058e55c8b7e63353b4f469f8

[hlundin-chromium, 2017-06-19 12:35:09]

Description was changed from

==========
WebrtcAudioPrivateSetAudioExperimentsFunction dummy implementation

This change introduces a dummy implementation of
WebrtcAudioPrivateSetAudioExperimentsFunction::RunAsync() which is
invoked when Chrome is build without support for WebRTC
(enable_webrtc=false).

BUG=730019,708475
==========

to

==========
WebrtcAudioPrivateSetAudioExperimentsFunction dummy implementation

This change introduces a dummy implementation of
WebrtcAudioPrivateSetAudioExperimentsFunction::RunAsync() which is
invoked when Chrome is build without support for WebRTC
(enable_webrtc=false).

BUG=730019,708475
==========

[hlundin-chromium, 2017-06-19 12:35:09]

hlundin@chromium.org changed reviewers:
+ grunell@chromium.org, rdevlin.cronin@chromium.org

[hlundin-chromium, 2017-06-19 12:38:32]

Henrik, Devlin,

PTAL at this change.

Also, the chrome/browser/extensions/api/webrtc_audio_private/ folder does not
have an OWNERS file. Many (most?) of the other extensions/api folders have it.
Would you like me to add an OWNERS file?

Thanks!

[Henrik Grunell, 2017-06-19 20:21:08]

On 2017/06/19 12:38:32, hlundin-chromium wrote:
> Henrik, Devlin,
> 
> PTAL at this change.
> 
> Also, the chrome/browser/extensions/api/webrtc_audio_private/ folder does not
> have an OWNERS file. Many (most?) of the other extensions/api folders have it.
> Would you like me to add an OWNERS file?
> 
> Thanks!

I think an OWNERS file makes sense, but it's not that big of a deal.

Interesting that no other of the webrtc_* extension API implementations need
that build flag. None of these APIs are relevant if webrtc is disabled, so maybe
long term they shouldn't be available at all? Wdyt Devlin?

Short-term: lgtm

[hlundin-chromium, 2017-06-20 06:52:32]

On 2017/06/19 20:21:08, Henrik Grunell wrote:
> On 2017/06/19 12:38:32, hlundin-chromium wrote:
> > Henrik, Devlin,
> > 
> > PTAL at this change.
> > 
> > Also, the chrome/browser/extensions/api/webrtc_audio_private/ folder does
not
> > have an OWNERS file. Many (most?) of the other extensions/api folders have
it.
> > Would you like me to add an OWNERS file?
> > 
> > Thanks!
> 
> I think an OWNERS file makes sense, but it's not that big of a deal.
> 
> Interesting that no other of the webrtc_* extension API implementations need
> that build flag. None of these APIs are relevant if webrtc is disabled, so
maybe
> long term they shouldn't be available at all? Wdyt Devlin?
> 
> Short-term: lgtm

webrtc_logging_private_api.cc switches on build level to an alternative source
file with only stubs:
https://cs.chromium.org/chromium/src/chrome/browser/extensions/BUILD.gn?type=....

I thought about this approach, but since all but one of the methods in
webrtc_audio_private_api.cc seemed to work
fine without WebRTC support, I did not go down that route.

[Henrik Grunell, 2017-06-20 06:59:17]

On 2017/06/20 06:52:32, hlundin-chromium wrote:
> On 2017/06/19 20:21:08, Henrik Grunell wrote:
> > On 2017/06/19 12:38:32, hlundin-chromium wrote:
> > > Henrik, Devlin,
> > > 
> > > PTAL at this change.
> > > 
> > > Also, the chrome/browser/extensions/api/webrtc_audio_private/ folder does
> not
> > > have an OWNERS file. Many (most?) of the other extensions/api folders have
> it.
> > > Would you like me to add an OWNERS file?
> > > 
> > > Thanks!
> > 
> > I think an OWNERS file makes sense, but it's not that big of a deal.
> > 
> > Interesting that no other of the webrtc_* extension API implementations need
> > that build flag. None of these APIs are relevant if webrtc is disabled, so
> maybe
> > long term they shouldn't be available at all? Wdyt Devlin?
> > 
> > Short-term: lgtm
> 
> webrtc_logging_private_api.cc switches on build level to an alternative source
> file with only stubs:
>
https://cs.chromium.org/chromium/src/chrome/browser/extensions/BUILD.gn?type=....
> 
> I thought about this approach, but since all but one of the methods in
> webrtc_audio_private_api.cc seemed to work
> fine without WebRTC support, I did not go down that route.

Ah, I see. I'm fine with that.

[Devlin, 2017-06-20 17:24:34]

https://codereview.chromium.org/2949563002/diff/1/chrome/browser/extensions/a...
File
chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc
(right):

https://codereview.chromium.org/2949563002/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc:325:
SendResponse(false);
Should we set an error?

[Henrik Grunell, 2017-06-21 14:02:50]

https://codereview.chromium.org/2949563002/diff/1/chrome/browser/extensions/a...
File
chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc
(right):

https://codereview.chromium.org/2949563002/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc:325:
SendResponse(false);
On 2017/06/20 17:24:34, Devlin wrote:
> Should we set an error?

Good point. Yes. Do

SetError("Not supported");

as in the stub
https://cs.chromium.org/chromium/src/chrome/browser/extensions/api/webrtc_log...

[hlundin-chromium, 2017-06-22 09:44:31]

PTAL again.

Thanks!

https://codereview.chromium.org/2949563002/diff/1/chrome/browser/extensions/a...
File
chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc
(right):

https://codereview.chromium.org/2949563002/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc:325:
SendResponse(false);
On 2017/06/21 14:02:50, Henrik Grunell OOO back Jun 26 wrote:
> On 2017/06/20 17:24:34, Devlin wrote:
> > Should we set an error?
> 
> Good point. Yes. Do
> 
> SetError("Not supported");
> 
> as in the stub
>
https://cs.chromium.org/chromium/src/chrome/browser/extensions/api/webrtc_log...

Done.

[Devlin, 2017-06-22 13:12:28]

lgtm

[hlundin-chromium, 2017-06-22 13:55:31]

The CQ bit was checked by hlundin@chromium.org

[hlundin-chromium, 2017-06-22 13:55:31]

The patchset sent to the CQ was uploaded after l-g-t-m from grunell@chromium.org
Link to the patchset: https://codereview.chromium.org/2949563002/#ps20001
(title: "Set an error")

[commit-bot: I haz the power, 2017-06-22 13:55:42]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-22 15:06:52]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1498139731201580,
"parent_rev": "e0dbd7d9c9f8280d8be5e76d7c568beacd89333a", "commit_rev":
"66e6a51773c74189058e55c8b7e63353b4f469f8"}

[commit-bot: I haz the power, 2017-06-22 15:07:05]

Description was changed from

==========
WebrtcAudioPrivateSetAudioExperimentsFunction dummy implementation

This change introduces a dummy implementation of
WebrtcAudioPrivateSetAudioExperimentsFunction::RunAsync() which is
invoked when Chrome is build without support for WebRTC
(enable_webrtc=false).

BUG=730019,708475
==========

to

==========
WebrtcAudioPrivateSetAudioExperimentsFunction dummy implementation

This change introduces a dummy implementation of
WebrtcAudioPrivateSetAudioExperimentsFunction::RunAsync() which is
invoked when Chrome is build without support for WebRTC
(enable_webrtc=false).

BUG=730019,708475

Review-Url: https://codereview.chromium.org/2949563002
Cr-Commit-Position: refs/heads/master@{#481532}
Committed:
https://chromium.googlesource.com/chromium/src/+/66e6a51773c74189058e55c8b7e6...
==========

[commit-bot: I haz the power, 2017-06-22 15:07:06]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/66e6a51773c74189058e55c8b7e6...