Replace gflags usages with rtc_base/flags in all targets based on test_main

Replace gflags usages with rtc_base/flags in all targets based on test_main

BUG=webrtc:7644
Review-Url: https://codereview.webrtc.org/2995363002
Cr-Commit-Position: refs/heads/master@{#19580}
Committed: https://chromium.googlesource.com/external/webrtc/+/9b2f20c6187f29e0c5105ff436b2fc787f924976

[oprypin_webrtc, 2017-08-22 20:50:59]

Description was changed from

==========
Replace all gflags usages with rtc_base/flags
==========

to

==========
Replace all gflags usages with rtc_base/flags

BUG=webrtc:7644
==========

[oprypin_webrtc, 2017-08-22 21:45:07]

Patchset #5 (id:80001) has been deleted

[oprypin_webrtc, 2017-08-23 11:37:14]

Description was changed from

==========
Replace all gflags usages with rtc_base/flags

BUG=webrtc:7644
==========

to

==========
Replace gflags usages with rtc_base/flags in all targets based on test_main

BUG=webrtc:7644
==========

[oprypin_webrtc, 2017-08-23 11:45:09]

Patchset #7 (id:140001) has been deleted

[oprypin_webrtc, 2017-08-23 12:55:51]

Patchset #8 (id:180001) has been deleted

[oprypin_webrtc, 2017-08-23 13:23:27]

The CQ bit was checked by oprypin@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-08-23 13:23:35]

Dry run: CQ is trying da patch.

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

[commit-bot: I haz the power, 2017-08-23 13:43:46]

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

[commit-bot: I haz the power, 2017-08-23 13:43:47]

Dry run: Try jobs failed on following builders:
  ios64_sim_ios10_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_ios10_dbg/bui...)

[oprypin_webrtc, 2017-08-23 15:57:33]

Patchset #10 (id:240001) has been deleted

[oprypin_webrtc, 2017-08-23 16:32:30]

Patchset #11 (id:280001) has been deleted

[oprypin_webrtc, 2017-08-23 17:12:14]

oprypin@webrtc.org changed reviewers:
+ kjellander@webrtc.org, kwiberg@webrtc.org

[oprypin_webrtc, 2017-08-23 18:24:53]

https://codereview.webrtc.org/2995363002/diff/200001/webrtc/test/BUILD.gn
File webrtc/test/BUILD.gn (left):

https://codereview.webrtc.org/2995363002/diff/200001/webrtc/test/BUILD.gn#old...
webrtc/test/BUILD.gn:166: if (!build_with_chromium) {
I suspect that this condition should be removed now. Should I remove and dedent
the whole following block or is it more granular?

[kjellander_webrtc, 2017-08-24 07:31:35]

https://codereview.webrtc.org/2995363002/diff/200001/webrtc/test/BUILD.gn
File webrtc/test/BUILD.gn (left):

https://codereview.webrtc.org/2995363002/diff/200001/webrtc/test/BUILD.gn#old...
webrtc/test/BUILD.gn:166: if (!build_with_chromium) {
On 2017/08/23 18:24:53, oprypin_webrtc wrote:
> I suspect that this condition should be removed now. Should I remove and
dedent
> the whole following block or is it more granular?

I think it can be removed, but to be on the safe side I'd prefer doing it in a
separate CL, we we don't end up reverting all this because of breaking the FYI
bots. Is that OK?

https://codereview.webrtc.org/2995363002/diff/320001/webrtc/modules/audio_cod...
File webrtc/modules/audio_coding/neteq/tools/neteq_quality_test.cc (right):

https://codereview.webrtc.org/2995363002/diff/320001/webrtc/modules/audio_cod...
webrtc/modules/audio_coding/neteq/tools/neteq_quality_test.cc:155: << "Invalid
burst length, should be greater than "
greater than or equal (might as well correct the previous incorrect text)

https://codereview.webrtc.org/2995363002/diff/320001/webrtc/voice_engine/test...
File webrtc/voice_engine/test/auto_test/standard/rtp_rtcp_test.cc (right):

https://codereview.webrtc.org/2995363002/diff/320001/webrtc/voice_engine/test...
webrtc/voice_engine/test/auto_test/standard/rtp_rtcp_test.cc:21:
DECLARE_bool(include_timing_dependent_tests);
I see this is moved from webrtc/voice_engine/test/auto_test/voe_standard_test.h
but why is not gflags replaced with the rtc_base flags.h?

[oprypin_webrtc, 2017-08-24 08:29:56]

https://codereview.webrtc.org/2995363002/diff/200001/webrtc/test/BUILD.gn
File webrtc/test/BUILD.gn (left):

https://codereview.webrtc.org/2995363002/diff/200001/webrtc/test/BUILD.gn#old...
webrtc/test/BUILD.gn:166: if (!build_with_chromium) {
On 2017/08/24 07:31:34, kjellander_webrtc wrote:
> On 2017/08/23 18:24:53, oprypin_webrtc wrote:
> > I suspect that this condition should be removed now. Should I remove and
> dedent
> > the whole following block or is it more granular?
> 
> I think it can be removed, but to be on the safe side I'd prefer doing it in a
> separate CL, we we don't end up reverting all this because of breaking the FYI
> bots. Is that OK?

OK. Added comment with TODO.

https://codereview.webrtc.org/2995363002/diff/320001/webrtc/modules/audio_cod...
File webrtc/modules/audio_coding/neteq/tools/neteq_quality_test.cc (right):

https://codereview.webrtc.org/2995363002/diff/320001/webrtc/modules/audio_cod...
webrtc/modules/audio_coding/neteq/tools/neteq_quality_test.cc:155: << "Invalid
burst length, should be greater than "
On 2017/08/24 07:31:34, kjellander_webrtc wrote:
> greater than or equal (might as well correct the previous incorrect text)

Done.

https://codereview.webrtc.org/2995363002/diff/320001/webrtc/voice_engine/test...
File webrtc/voice_engine/test/auto_test/standard/rtp_rtcp_test.cc (right):

https://codereview.webrtc.org/2995363002/diff/320001/webrtc/voice_engine/test...
webrtc/voice_engine/test/auto_test/standard/rtp_rtcp_test.cc:21:
DECLARE_bool(include_timing_dependent_tests);
On 2017/08/24 07:31:34, kjellander_webrtc wrote:
> I see this is moved from
webrtc/voice_engine/test/auto_test/voe_standard_test.h
> but why is not gflags replaced with the rtc_base flags.h?

This is not a target based on test_main, I just wanted to fix the usage of
gflags when I ran into it. But it's irrelevant in this CL, will be changed in
the following one. I reverted these files now.

[kjellander_webrtc, 2017-08-24 08:48:58]

lgtm

[kwiberg-webrtc, 2017-08-24 09:03:44]

https://codereview.webrtc.org/2995363002/diff/380001/webrtc/modules/audio_cod...
File webrtc/modules/audio_coding/neteq/tools/neteq_quality_test.cc (right):

https://codereview.webrtc.org/2995363002/diff/380001/webrtc/modules/audio_cod...
webrtc/modules/audio_coding/neteq/tools/neteq_quality_test.cc:32: OutputPath() +
"neteq_quality_test_out.pcm";
"Variables of class type with static storage duration are forbidden: they cause
hard-to-find bugs due to indeterminate order of construction and destruction."
(https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables)

The common workaround is something like

const std::string& DefaultInFilename() {
  static const std::string df =
      ResourcePath("audio_coding/speech_mono_16kHz", "pcm");
  return df;
}

https://codereview.webrtc.org/2995363002/diff/380001/webrtc/test/testsupport/...
File webrtc/test/testsupport/test_output.cc (right):

https://codereview.webrtc.org/2995363002/diff/380001/webrtc/test/testsupport/...
webrtc/test/testsupport/test_output.cc:22: const std::string kDefaultOutputPath
= webrtc::test::OutputPath();
Another problematic global variable.

[oprypin_webrtc, 2017-08-24 09:55:08]

The CQ bit was checked by oprypin@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-08-24 09:55:13]

Dry run: CQ is trying da patch.

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

[commit-bot: I haz the power, 2017-08-24 12:04:04]

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

[commit-bot: I haz the power, 2017-08-24 12:04:05]

Dry run: Try jobs failed on following builders:
  android_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_dbg/builds/26919)

[oprypin_webrtc, 2017-08-26 07:38:52]

https://codereview.webrtc.org/2995363002/diff/380001/webrtc/modules/audio_cod...
File webrtc/modules/audio_coding/neteq/tools/neteq_quality_test.cc (right):

https://codereview.webrtc.org/2995363002/diff/380001/webrtc/modules/audio_cod...
webrtc/modules/audio_coding/neteq/tools/neteq_quality_test.cc:32: OutputPath() +
"neteq_quality_test_out.pcm";
On 2017/08/24 09:03:44, kwiberg-webrtc wrote:
> "Variables of class type with static storage duration are forbidden: they
cause
> hard-to-find bugs due to indeterminate order of construction and destruction."
>
(https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables)
> 
> The common workaround is something like
> 
> const std::string& DefaultInFilename() {
>   static const std::string df =
>       ResourcePath("audio_coding/speech_mono_16kHz", "pcm");
>   return df;
> }

Done. Thanks for also providing the solution :)

https://codereview.webrtc.org/2995363002/diff/380001/webrtc/test/testsupport/...
File webrtc/test/testsupport/test_output.cc (right):

https://codereview.webrtc.org/2995363002/diff/380001/webrtc/test/testsupport/...
webrtc/test/testsupport/test_output.cc:22: const std::string kDefaultOutputPath
= webrtc::test::OutputPath();
On 2017/08/24 09:03:44, kwiberg-webrtc wrote:
> Another problematic global variable.

Done.

[kwiberg-webrtc, 2017-08-27 22:48:50]

lgtm

[charujain, 2017-08-28 10:41:40]

The CQ bit was checked by charujain@webrtc.org

[charujain, 2017-08-28 10:41:41]

The patchset sent to the CQ was uploaded after l-g-t-m from
kjellander@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2995363002/#ps400001 (title:
"Replace class globals")

[commit-bot: I haz the power, 2017-08-28 10:41:48]

CQ is trying da patch.

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

[commit-bot: I haz the power, 2017-08-28 12:46:10]

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

[commit-bot: I haz the power, 2017-08-28 12:46:11]

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

[oprypin_webrtc, 2017-08-28 14:51:45]

Description was changed from

==========
Replace gflags usages with rtc_base/flags in all targets based on test_main

BUG=webrtc:7644
==========

to

==========
Replace gflags usages with rtc_base/flags in all targets based on test_main

This passed on bots, but one of them had an unrelated failure, so NOTRY to land
it.

NOTRY=True
BUG=webrtc:7644
==========

[oprypin_webrtc, 2017-08-28 15:11:14]

Description was changed from

==========
Replace gflags usages with rtc_base/flags in all targets based on test_main

This passed on bots, but one of them had an unrelated failure, so NOTRY to land
it.

NOTRY=True
BUG=webrtc:7644
==========

to

==========
Replace gflags usages with rtc_base/flags in all targets based on test_main

BUG=webrtc:7644
==========

[charujain, 2017-08-29 11:28:03]

The CQ bit was checked by charujain@webrtc.org

[charujain, 2017-08-29 11:28:03]

The patchset sent to the CQ was uploaded after l-g-t-m from
kjellander@webrtc.org, kwiberg@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2995363002/#ps420001 (title:
"Fix string use after free")

[commit-bot: I haz the power, 2017-08-29 11:28:11]

CQ is trying da patch.

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

[commit-bot: I haz the power, 2017-08-29 11:47:20]

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

[commit-bot: I haz the power, 2017-08-29 11:47:21]

Try jobs failed on following builders:
  android_more_configs on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_more_configs/bu...)

[charujain, 2017-08-29 12:26:37]

The CQ bit was checked by charujain@webrtc.org

[commit-bot: I haz the power, 2017-08-29 12:26:41]

CQ is trying da patch.

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

[commit-bot: I haz the power, 2017-08-29 12:51:58]

CQ is committing da patch.

Bot data: {"patchset_id": 420001, "attempt_start_ts": 1504009597857130,
"parent_rev": "a16c70b8ba37b07b6fa82b2a4314f6f608ad079e", "commit_rev":
"9b2f20c6187f29e0c5105ff436b2fc787f924976"}

[commit-bot: I haz the power, 2017-08-29 12:52:03]

Description was changed from

==========
Replace gflags usages with rtc_base/flags in all targets based on test_main

BUG=webrtc:7644
==========

to

==========
Replace gflags usages with rtc_base/flags in all targets based on test_main

BUG=webrtc:7644

Review-Url: https://codereview.webrtc.org/2995363002
Cr-Commit-Position: refs/heads/master@{#19580}
Committed:
https://chromium.googlesource.com/external/webrtc/+/9b2f20c6187f29e0c5105ff43...
==========

[commit-bot: I haz the power, 2017-08-29 12:52:05]

Committed patchset #17 (id:420001) as
https://chromium.googlesource.com/external/webrtc/+/9b2f20c6187f29e0c5105ff43...