duplicate the webrtc content_browser tests and browser tests with --enable-audio-track-processing on

duplicate the webrtc content_browser tests and browser tests with --enable-audio-track-processing on


NOTRY=true

BUG=264611
TEST=bots

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=256834

[no longer working on chromium, 2014-03-11 17:30:01]

Patrik, please take a look.

Thanks,
SX

[no longer working on chromium, 2014-03-12 15:16:19]

Patrik, PTAL.

Thanks,
SX

[phoglund_chromium, 2014-03-12 17:27:18]

That's fantastic Shijing! Just want you to move the flag setting a bit and
this'll be great.

https://codereview.chromium.org/194713010/diff/50001/chrome/browser/media/chr...
File chrome/browser/media/chrome_webrtc_audio_quality_browsertest.cc (right):

https://codereview.chromium.org/194713010/diff/50001/chrome/browser/media/chr...
chrome/browser/media/chrome_webrtc_audio_quality_browsertest.cc:96: if
(GetParam()) {
bool enable_audio_track_processing = GetParam() to make it clear what the Param
actually is.

https://codereview.chromium.org/194713010/diff/50001/chrome/browser/media/chr...
chrome/browser/media/chrome_webrtc_audio_quality_browsertest.cc:97:
CommandLine::ForCurrentProcess()->AppendSwitch(
Do this in SetupCommandLine below instead and act on the command line you get as
a parameter.

https://codereview.chromium.org/194713010/diff/50001/chrome/browser/media/chr...
File chrome/browser/media/chrome_webrtc_browsertest.cc (right):

https://codereview.chromium.org/194713010/diff/50001/chrome/browser/media/chr...
chrome/browser/media/chrome_webrtc_browsertest.cc:46:
CommandLine::ForCurrentProcess()->AppendSwitch(
Same here.

https://codereview.chromium.org/194713010/diff/50001/content/browser/media/we...
File content/browser/media/webrtc_aecdump_browsertest.cc (right):

https://codereview.chromium.org/194713010/diff/50001/content/browser/media/we...
content/browser/media/webrtc_aecdump_browsertest.cc:28:
CommandLine::ForCurrentProcess()->AppendSwitch(
Same here.

https://codereview.chromium.org/194713010/diff/50001/content/browser/media/we...
File content/browser/media/webrtc_browsertest.cc (right):

https://codereview.chromium.org/194713010/diff/50001/content/browser/media/we...
content/browser/media/webrtc_browsertest.cc:29: if (GetParam()) {
And here. Be careful though, it's possible the base class also overrides the
command line flag setup function, so make sure to call the base class' version
as well.

https://codereview.chromium.org/194713010/diff/50001/content/browser/media/we...
File content/browser/media/webrtc_getusermedia_browsertest.cc (right):

https://codereview.chromium.org/194713010/diff/50001/content/browser/media/we...
content/browser/media/webrtc_getusermedia_browsertest.cc:74: if (GetParam()) {
Same here.

[no longer working on chromium, 2014-03-12 18:06:07]

Thanks Patrik, PTAL :)

https://codereview.chromium.org/194713010/diff/50001/chrome/browser/media/chr...
File chrome/browser/media/chrome_webrtc_audio_quality_browsertest.cc (right):

https://codereview.chromium.org/194713010/diff/50001/chrome/browser/media/chr...
chrome/browser/media/chrome_webrtc_audio_quality_browsertest.cc:96: if
(GetParam()) {
On 2014/03/12 17:27:18, phoglund wrote:
> bool enable_audio_track_processing = GetParam() to make it clear what the
Param
> actually is.

Done.

https://codereview.chromium.org/194713010/diff/50001/chrome/browser/media/chr...
chrome/browser/media/chrome_webrtc_audio_quality_browsertest.cc:97:
CommandLine::ForCurrentProcess()->AppendSwitch(
On 2014/03/12 17:27:18, phoglund wrote:
> Do this in SetupCommandLine below instead and act on the command line you get
as
> a parameter.

I guess you meant SetUpCommandLine(CommandLine* command_line, bool
enable_audio_track_processing)
This is not overriding, it is creating one more overload SetUpCommandLine
method, I don't think it is worthy for such a temporary CL.

I used a member as enable_audio_track_processing_ instead.

https://codereview.chromium.org/194713010/diff/50001/chrome/browser/media/chr...
File chrome/browser/media/chrome_webrtc_browsertest.cc (right):

https://codereview.chromium.org/194713010/diff/50001/chrome/browser/media/chr...
chrome/browser/media/chrome_webrtc_browsertest.cc:46:
CommandLine::ForCurrentProcess()->AppendSwitch(
On 2014/03/12 17:27:18, phoglund wrote:
> Same here.

ditto

https://codereview.chromium.org/194713010/diff/50001/content/browser/media/we...
File content/browser/media/webrtc_aecdump_browsertest.cc (right):

https://codereview.chromium.org/194713010/diff/50001/content/browser/media/we...
content/browser/media/webrtc_aecdump_browsertest.cc:28:
CommandLine::ForCurrentProcess()->AppendSwitch(
On 2014/03/12 17:27:18, phoglund wrote:
> Same here.

ditto

https://codereview.chromium.org/194713010/diff/50001/content/browser/media/we...
File content/browser/media/webrtc_browsertest.cc (right):

https://codereview.chromium.org/194713010/diff/50001/content/browser/media/we...
content/browser/media/webrtc_browsertest.cc:29: if (GetParam()) {
On 2014/03/12 17:27:18, phoglund wrote:
> And here. Be careful though, it's possible the base class also overrides the
> command line flag setup function, so make sure to call the base class' version
> as well.

Well, lets not make the test code more complicated since this CL will be going
away in a quarter or less. I changed the code to be:
bool enable_audio_track_processing = GetParam();
if (enable_audio_track_processing) {
    CommandLine::ForCurrentProcess()->AppendSwitch(
        switches::kEnableAudioTrackProcessing);
}

Hope it is fine with you.

https://codereview.chromium.org/194713010/diff/50001/content/browser/media/we...
File content/browser/media/webrtc_getusermedia_browsertest.cc (right):

https://codereview.chromium.org/194713010/diff/50001/content/browser/media/we...
content/browser/media/webrtc_getusermedia_browsertest.cc:74: if (GetParam()) {
On 2014/03/12 17:27:18, phoglund wrote:
> Same here.

ditto, I hope not add a SetUpCommandLine(CommandLine* command_line, bool) for
such a temporary thing.

[phoglund_chromium, 2014-03-13 09:04:49]

https://codereview.chromium.org/194713010/diff/70001/chrome/browser/media/chr...
File chrome/browser/media/chrome_webrtc_audio_quality_browsertest.cc (right):

https://codereview.chromium.org/194713010/diff/70001/chrome/browser/media/chr...
chrome/browser/media/chrome_webrtc_audio_quality_browsertest.cc:95:
WebRtcAudioQualityBrowserTest() : enable_audio_track_processing_(GetParam()) {
It's not necessary to store this as a member variable; just do this in
SetUpCommandLine:

bool enable_audio_track_processing = GetParam();
if (enable_audio_track_processing_)
  command_line->AppendSwitch(switches::kEnableAudioTrackProcessing);

and get rid of the constructor.

https://codereview.chromium.org/194713010/diff/70001/content/browser/media/we...
File content/browser/media/webrtc_browsertest.cc (right):

https://codereview.chromium.org/194713010/diff/70001/content/browser/media/we...
content/browser/media/webrtc_browsertest.cc:32:
switches::kEnableAudioTrackProcessing);
Do this in SetUpCommandLine instead. It's overridden in the base class though so
we need to call the base class version as well:

virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
  WebRtcContentBrowserTest::SetUpCommandLine(command_line);

  bool enable_audio_track_processing = GetParam();
  if (enable_audio_track_processing)
    command_line->AppendSwitch(switches::kEnableAudioTrackProcessing);
}

https://codereview.chromium.org/194713010/diff/70001/content/browser/media/we...
File content/browser/media/webrtc_getusermedia_browsertest.cc (right):

https://codereview.chromium.org/194713010/diff/70001/content/browser/media/we...
content/browser/media/webrtc_getusermedia_browsertest.cc:74: bool
enable_audio_track_processing = GetParam();
See comment on WebRtcBrowserTest.

[no longer working on chromium, 2014-03-13 09:58:07]

Hi Patrik, I have moved everything to SetUpCommandLine

https://codereview.chromium.org/194713010/diff/70001/chrome/browser/media/chr...
File chrome/browser/media/chrome_webrtc_audio_quality_browsertest.cc (right):

https://codereview.chromium.org/194713010/diff/70001/chrome/browser/media/chr...
chrome/browser/media/chrome_webrtc_audio_quality_browsertest.cc:95:
WebRtcAudioQualityBrowserTest() : enable_audio_track_processing_(GetParam()) {
On 2014/03/13 09:04:49, phoglund wrote:
> It's not necessary to store this as a member variable; just do this in
> SetUpCommandLine:
> 
> bool enable_audio_track_processing = GetParam();
> if (enable_audio_track_processing_)
>   command_line->AppendSwitch(switches::kEnableAudioTrackProcessing);
> 
> and get rid of the constructor.

Done.

https://codereview.chromium.org/194713010/diff/70001/content/browser/media/we...
File content/browser/media/webrtc_browsertest.cc (right):

https://codereview.chromium.org/194713010/diff/70001/content/browser/media/we...
content/browser/media/webrtc_browsertest.cc:32:
switches::kEnableAudioTrackProcessing);
On 2014/03/13 09:04:49, phoglund wrote:
> Do this in SetUpCommandLine instead. It's overridden in the base class though
so
> we need to call the base class version as well:
> 
> virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
>   WebRtcContentBrowserTest::SetUpCommandLine(command_line);
> 
>   bool enable_audio_track_processing = GetParam();
>   if (enable_audio_track_processing)
>     command_line->AppendSwitch(switches::kEnableAudioTrackProcessing);
> }

Done.

https://codereview.chromium.org/194713010/diff/70001/content/browser/media/we...
File content/browser/media/webrtc_getusermedia_browsertest.cc (right):

https://codereview.chromium.org/194713010/diff/70001/content/browser/media/we...
content/browser/media/webrtc_getusermedia_browsertest.cc:74: bool
enable_audio_track_processing = GetParam();
On 2014/03/13 09:04:49, phoglund wrote:
> See comment on WebRtcBrowserTest.

Done.

[phoglund_chromium, 2014-03-13 10:11:24]

lgtm

[no longer working on chromium, 2014-03-13 10:42:01]

The CQ bit was checked by xians@chromium.org

[commit-bot: I haz the power, 2014-03-13 10:42:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xians@chromium.org/194713010/90001

[commit-bot: I haz the power, 2014-03-13 10:47:06]

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

[commit-bot: I haz the power, 2014-03-13 10:47:07]

Try jobs failed on following builders:
  tryserver.chromium on win_chromium_compile_dbg

[no longer working on chromium, 2014-03-13 15:04:13]

The CQ bit was checked by xians@chromium.org

[commit-bot: I haz the power, 2014-03-13 15:08:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xians@chromium.org/194713010/90001

[commit-bot: I haz the power, 2014-03-13 15:15:35]

Change committed as 256834