Add File::Open / Create functions to take an rtc::Pathname

Add File::Open / Create functions to take an rtc::Pathname

When implementing ISOLATED_OUTDIR feature in WebRTC, I found two issues,
1. pathutils and flags are not accessible in testsupport. But both of them are
useful for the feature. Pathname can help to combine path with filename, while
a flag is needed to handle command line parameter.
2. rtc::File cannot accept an rtc::Pathname, which is a little bit inconvenient.

After investigating bug webrtc:3806, flags, pathutils and urlencode are
removed from rtc_base_approved because of the including of common.h. So I
replaced common.h with checks.h, and ASSERT with RTC_DCHECK. flags,
pathutils and urlencode pairs now can be placed into rtc_base_approved to
unblock file.h to include pathutils.h.

Please kindly let me know if you have other concerns about this change.

BUG=webrtc:3806, webrtc:6732
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_rel_ng;master.tryserver.chromium.mac:mac_chromium_rel_ng;master.tryserver.chromium.win:win_chromium_rel_ng;master.tryserver.chromium.android:linux_android_rel_ng

Committed: https://crrev.com/dd87d580e8ffe546bd6e22cbd1522ffe76d675a7
Cr-Commit-Position: refs/heads/master@{#15451}

[Hzj_jie, 2016-11-29 23:30:29]

The CQ bit was checked by zijiehe@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-29 23:30: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, 2016-11-29 23:33:20]

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

[commit-bot: I haz the power, 2016-11-29 23:33:21]

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

[Hzj_jie, 2016-11-29 23:33:54]

Description was changed from

==========
Add Open / Create from Pathname functions.

When implementing ISOLATED_OUTDIR feature in WebRTC, I found rtc::File cannot
accept an rtc::Pathname, which is weird. So if there is no other concern, I
would prefer to add Create / Open functions to take rtc::Pathname instead of raw
std::string.

BUG=6732
==========

to

==========
Add File::Open / Create functions to take an rtc::Pathname

When implementing ISOLATED_OUTDIR feature in WebRTC, I found rtc::File cannot
accept an rtc::Pathname, which is a little bit inconvenient. So if there is no
other concern, I would prefer to add Create / Open functions to take an
rtc::Pathname instead of raw std::string.

BUG=6732
==========

[Hzj_jie, 2016-11-29 23:33:54]

zijiehe@chromium.org changed reviewers:
+ palmkvist@webrtc.org

[Hzj_jie, 2016-11-29 23:54:31]

The CQ bit was checked by zijiehe@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-29 23:54:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[Hzj_jie, 2016-11-29 23:54:48]

The CQ bit was checked by zijiehe@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-29 23:54:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[Hzj_jie, 2016-11-29 23:55:29]

Description was changed from

==========
Add File::Open / Create functions to take an rtc::Pathname

When implementing ISOLATED_OUTDIR feature in WebRTC, I found rtc::File cannot
accept an rtc::Pathname, which is a little bit inconvenient. So if there is no
other concern, I would prefer to add Create / Open functions to take an
rtc::Pathname instead of raw std::string.

BUG=6732
==========

to

==========
Add File::Open / Create functions to take an rtc::Pathname

When implementing ISOLATED_OUTDIR feature in WebRTC, I found rtc::File cannot
accept an rtc::Pathname, which is a little bit inconvenient. So if there is no
other concern, I would prefer to add Create / Open functions to take an
rtc::Pathname instead of raw std::string.

BUG=webrtc:3806
==========

[Hzj_jie, 2016-11-29 23:55:41]

Patchset #1 (id:1) has been deleted

[commit-bot: I haz the power, 2016-11-29 23:57:40]

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

[commit-bot: I haz the power, 2016-11-29 23:57:41]

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

[Hzj_jie, 2016-11-29 23:57:56]

Description was changed from

==========
Add File::Open / Create functions to take an rtc::Pathname

When implementing ISOLATED_OUTDIR feature in WebRTC, I found rtc::File cannot
accept an rtc::Pathname, which is a little bit inconvenient. So if there is no
other concern, I would prefer to add Create / Open functions to take an
rtc::Pathname instead of raw std::string.

BUG=webrtc:3806
==========

to

==========
Add File::Open / Create functions to take an rtc::Pathname

When implementing ISOLATED_OUTDIR feature in WebRTC, I found rtc::File cannot
accept an rtc::Pathname, which is a little bit inconvenient.

After investigating bug webrtc:3806, both fileutils and pathutils are removed
from rtc_base_approved because of the including of common.h. So I replaced
common.h with checks.h, and ASSERT with RTC_DCHECK. fileutils and pathutils
pairs now can be placed into rtc_base_approved to unblock file.h to include
pathutils.h.

Please kindly let me know if you have other concerns about this change.

BUG=webrtc:3806
==========

[Hzj_jie, 2016-11-30 00:01:22]

The CQ bit was checked by zijiehe@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-30 00:01:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[Hzj_jie, 2016-11-30 00:01:34]

Patchset #1 (id:20001) has been deleted

[commit-bot: I haz the power, 2016-11-30 00:03:46]

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

[commit-bot: I haz the power, 2016-11-30 00:03:47]

Dry run: Try jobs failed on following builders:
  ios32_sim_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios32_sim_dbg/builds/12926)
  ios_arm64_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_rel/builds/15158)
  ios_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_dbg/builds/20516)
  mac_baremetal on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_baremetal/builds/16654)
  mac_compile_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_compile_dbg/builds/...)
  mac_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_rel/builds/20587)

[Hzj_jie, 2016-11-30 00:03:59]

The CQ bit was checked by zijiehe@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-30 00:04:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[Hzj_jie, 2016-11-30 00:04:06]

Description was changed from

==========
Add File::Open / Create functions to take an rtc::Pathname

When implementing ISOLATED_OUTDIR feature in WebRTC, I found rtc::File cannot
accept an rtc::Pathname, which is a little bit inconvenient.

After investigating bug webrtc:3806, both fileutils and pathutils are removed
from rtc_base_approved because of the including of common.h. So I replaced
common.h with checks.h, and ASSERT with RTC_DCHECK. fileutils and pathutils
pairs now can be placed into rtc_base_approved to unblock file.h to include
pathutils.h.

Please kindly let me know if you have other concerns about this change.

BUG=webrtc:3806
==========

to

==========
Add File::Open / Create functions to take an rtc::Pathname

When implementing ISOLATED_OUTDIR feature in WebRTC, I found rtc::File cannot
accept an rtc::Pathname, which is a little bit inconvenient.

After investigating bug webrtc:3806, fileutils, pathutils and urlencode are
removed from rtc_base_approved because of the including of common.h. So I
replaced common.h with checks.h, and ASSERT with RTC_DCHECK. fileutils,
pathutils and urlencode pairs now can be placed into rtc_base_approved to
unblock file.h to include pathutils.h.

Please kindly let me know if you have other concerns about this change.

BUG=webrtc:3806
==========

[Hzj_jie, 2016-11-30 00:04:15]

Patchset #1 (id:40001) has been deleted

[commit-bot: I haz the power, 2016-11-30 00:06:14]

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

[commit-bot: I haz the power, 2016-11-30 00:06:15]

Dry run: Try jobs failed on following builders:
  mac_asan on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_asan/builds/19842)
  mac_baremetal on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_baremetal/builds/16655)
  mac_compile_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_compile_dbg/builds/...)

[Hzj_jie, 2016-11-30 00:20:49]

The CQ bit was checked by zijiehe@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-30 00:20:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[Hzj_jie, 2016-11-30 00:20:57]

Patchset #1 (id:60001) has been deleted

[commit-bot: I haz the power, 2016-11-30 00:25:24]

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

[commit-bot: I haz the power, 2016-11-30 00:25:25]

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

[Hzj_jie, 2016-11-30 00:29:53]

zijiehe@chromium.org changed reviewers:
- palmkvist@webrtc.org

[Hzj_jie, 2016-11-30 01:13:53]

The CQ bit was checked by zijiehe@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-30 01:14:01]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[Hzj_jie, 2016-11-30 01:14:04]

Patchset #1 (id:80001) has been deleted

[Hzj_jie, 2016-11-30 01:14:46]

Description was changed from

==========
Add File::Open / Create functions to take an rtc::Pathname

When implementing ISOLATED_OUTDIR feature in WebRTC, I found rtc::File cannot
accept an rtc::Pathname, which is a little bit inconvenient.

After investigating bug webrtc:3806, fileutils, pathutils and urlencode are
removed from rtc_base_approved because of the including of common.h. So I
replaced common.h with checks.h, and ASSERT with RTC_DCHECK. fileutils,
pathutils and urlencode pairs now can be placed into rtc_base_approved to
unblock file.h to include pathutils.h.

Please kindly let me know if you have other concerns about this change.

BUG=webrtc:3806
==========

to

==========
Add File::Open / Create functions to take an rtc::Pathname

When implementing ISOLATED_OUTDIR feature in WebRTC, I found rtc::File cannot
accept an rtc::Pathname, which is a little bit inconvenient.

After investigating bug webrtc:3806, flags, pathutils and urlencode are
removed from rtc_base_approved because of the including of common.h. So I
replaced common.h with checks.h, and ASSERT with RTC_DCHECK. flags,
pathutils and urlencode pairs now can be placed into rtc_base_approved to
unblock file.h to include pathutils.h.

Please kindly let me know if you have other concerns about this change.

BUG=webrtc:3806
==========

[Hzj_jie, 2016-11-30 01:15:11]

zijiehe@chromium.org changed reviewers:
+ palmkvist@webrtc.org

[commit-bot: I haz the power, 2016-11-30 01:28:13]

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

[commit-bot: I haz the power, 2016-11-30 01:28:15]

Dry run: This issue passed the CQ dry run.

[Hzj_jie, 2016-11-30 01:36:01]

Description was changed from

==========
Add File::Open / Create functions to take an rtc::Pathname

When implementing ISOLATED_OUTDIR feature in WebRTC, I found rtc::File cannot
accept an rtc::Pathname, which is a little bit inconvenient.

After investigating bug webrtc:3806, flags, pathutils and urlencode are
removed from rtc_base_approved because of the including of common.h. So I
replaced common.h with checks.h, and ASSERT with RTC_DCHECK. flags,
pathutils and urlencode pairs now can be placed into rtc_base_approved to
unblock file.h to include pathutils.h.

Please kindly let me know if you have other concerns about this change.

BUG=webrtc:3806
==========

to

==========
Add File::Open / Create functions to take an rtc::Pathname

When implementing ISOLATED_OUTDIR feature in WebRTC, I found two issues,
1. pathutils and flags are not accessible in testsupport. But both of them are
useful for the feature. Pathname can help to combine path with filename, while
it needs a flag to handle command line parameter.
2. rtc::File cannot accept an rtc::Pathname, which is a little bit inconvenient.

After investigating bug webrtc:3806, flags, pathutils and urlencode are
removed from rtc_base_approved because of the including of common.h. So I
replaced common.h with checks.h, and ASSERT with RTC_DCHECK. flags,
pathutils and urlencode pairs now can be placed into rtc_base_approved to
unblock file.h to include pathutils.h.

Please kindly let me know if you have other concerns about this change.

BUG=webrtc:3806, webrtc:6732
==========

[Hzj_jie, 2016-11-30 01:43:13]

Description was changed from

==========
Add File::Open / Create functions to take an rtc::Pathname

When implementing ISOLATED_OUTDIR feature in WebRTC, I found two issues,
1. pathutils and flags are not accessible in testsupport. But both of them are
useful for the feature. Pathname can help to combine path with filename, while
it needs a flag to handle command line parameter.
2. rtc::File cannot accept an rtc::Pathname, which is a little bit inconvenient.

After investigating bug webrtc:3806, flags, pathutils and urlencode are
removed from rtc_base_approved because of the including of common.h. So I
replaced common.h with checks.h, and ASSERT with RTC_DCHECK. flags,
pathutils and urlencode pairs now can be placed into rtc_base_approved to
unblock file.h to include pathutils.h.

Please kindly let me know if you have other concerns about this change.

BUG=webrtc:3806, webrtc:6732
==========

to

==========
Add File::Open / Create functions to take an rtc::Pathname

When implementing ISOLATED_OUTDIR feature in WebRTC, I found two issues,
1. pathutils and flags are not accessible in testsupport. But both of them are
useful for the feature. Pathname can help to combine path with filename, while
it needs a flag to handle command line parameter.
2. rtc::File cannot accept an rtc::Pathname, which is a little bit inconvenient.

After investigating bug webrtc:3806, flags, pathutils and urlencode are
removed from rtc_base_approved because of the including of common.h. So I
replaced common.h with checks.h, and ASSERT with RTC_DCHECK. flags,
pathutils and urlencode pairs now can be placed into rtc_base_approved to
unblock file.h to include pathutils.h.

Please kindly let me know if you have other concerns about this change.

BUG=webrtc:3806, webrtc:6732

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:bot1,bot2;master.tryserver.chromium.mac:bot3;master.tryserver.chromium.win:bot4
==========

[Hzj_jie, 2016-11-30 01:43:38]

Description was changed from

==========
Add File::Open / Create functions to take an rtc::Pathname

When implementing ISOLATED_OUTDIR feature in WebRTC, I found two issues,
1. pathutils and flags are not accessible in testsupport. But both of them are
useful for the feature. Pathname can help to combine path with filename, while
it needs a flag to handle command line parameter.
2. rtc::File cannot accept an rtc::Pathname, which is a little bit inconvenient.

After investigating bug webrtc:3806, flags, pathutils and urlencode are
removed from rtc_base_approved because of the including of common.h. So I
replaced common.h with checks.h, and ASSERT with RTC_DCHECK. flags,
pathutils and urlencode pairs now can be placed into rtc_base_approved to
unblock file.h to include pathutils.h.

Please kindly let me know if you have other concerns about this change.

BUG=webrtc:3806, webrtc:6732

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:bot1,bot2;master.tryserver.chromium.mac:bot3;master.tryserver.chromium.win:bot4
==========

to

==========
Add File::Open / Create functions to take an rtc::Pathname

When implementing ISOLATED_OUTDIR feature in WebRTC, I found two issues,
1. pathutils and flags are not accessible in testsupport. But both of them are
useful for the feature. Pathname can help to combine path with filename, while
a flag is needed to handle command line parameter.
2. rtc::File cannot accept an rtc::Pathname, which is a little bit inconvenient.

After investigating bug webrtc:3806, flags, pathutils and urlencode are
removed from rtc_base_approved because of the including of common.h. So I
replaced common.h with checks.h, and ASSERT with RTC_DCHECK. flags,
pathutils and urlencode pairs now can be placed into rtc_base_approved to
unblock file.h to include pathutils.h.

Please kindly let me know if you have other concerns about this change.

BUG=webrtc:3806, webrtc:6732

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:bot1,bot2;master.tryserver.chromium.mac:bot3;master.tryserver.chromium.win:bot4
==========

[Hzj_jie, 2016-11-30 01:49:37]

Description was changed from

==========
Add File::Open / Create functions to take an rtc::Pathname

When implementing ISOLATED_OUTDIR feature in WebRTC, I found two issues,
1. pathutils and flags are not accessible in testsupport. But both of them are
useful for the feature. Pathname can help to combine path with filename, while
a flag is needed to handle command line parameter.
2. rtc::File cannot accept an rtc::Pathname, which is a little bit inconvenient.

After investigating bug webrtc:3806, flags, pathutils and urlencode are
removed from rtc_base_approved because of the including of common.h. So I
replaced common.h with checks.h, and ASSERT with RTC_DCHECK. flags,
pathutils and urlencode pairs now can be placed into rtc_base_approved to
unblock file.h to include pathutils.h.

Please kindly let me know if you have other concerns about this change.

BUG=webrtc:3806, webrtc:6732

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:bot1,bot2;master.tryserver.chromium.mac:bot3;master.tryserver.chromium.win:bot4
==========

to

==========
Add File::Open / Create functions to take an rtc::Pathname

When implementing ISOLATED_OUTDIR feature in WebRTC, I found two issues,
1. pathutils and flags are not accessible in testsupport. But both of them are
useful for the feature. Pathname can help to combine path with filename, while
a flag is needed to handle command line parameter.
2. rtc::File cannot accept an rtc::Pathname, which is a little bit inconvenient.

After investigating bug webrtc:3806, flags, pathutils and urlencode are
removed from rtc_base_approved because of the including of common.h. So I
replaced common.h with checks.h, and ASSERT with RTC_DCHECK. flags,
pathutils and urlencode pairs now can be placed into rtc_base_approved to
unblock file.h to include pathutils.h.

Please kindly let me know if you have other concerns about this change.

BUG=webrtc:3806, webrtc:6732

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_rel_ng;master.tryserver.chromium.mac:mac_chromium_rel_ng;master.tryserver.chromium.win:win_chromium_rel_ng;master.tryserver.chromium.android:linux_android_rel_ng
==========

[Hzj_jie, 2016-11-30 02:22:07]

The CQ bit was checked by zijiehe@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-30 02:22:10]

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, 2016-11-30 02:34:44]

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

[commit-bot: I haz the power, 2016-11-30 02:34:45]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[palmkvist, 2016-12-01 13:31:13]

palmkvist@webrtc.org changed reviewers:
+ stefan@webrtc.org

[palmkvist, 2016-12-01 13:31:13]

lgtm with nit, but I'm adding a non-intern reviewer as well :)

https://codereview.webrtc.org/2533213005/diff/100001/webrtc/base/urlencode.cc
File webrtc/base/urlencode.cc (right):

https://codereview.webrtc.org/2533213005/diff/100001/webrtc/base/urlencode.cc...
webrtc/base/urlencode.cc:117: RTC_DCHECK(static_cast<unsigned int>(dest - start)
< max);
RTC_DCHECK_LT

[Hzj_jie, 2016-12-06 09:56:51]

On 2016/12/01 13:31:13, palmkvist wrote:
> lgtm with nit, but I'm adding a non-intern reviewer as well :)
> 
> https://codereview.webrtc.org/2533213005/diff/100001/webrtc/base/urlencode.cc
> File webrtc/base/urlencode.cc (right):
> 
>
https://codereview.webrtc.org/2533213005/diff/100001/webrtc/base/urlencode.cc...
> webrtc/base/urlencode.cc:117: RTC_DCHECK(static_cast<unsigned int>(dest -
start)
> < max);
> RTC_DCHECK_LT

Thank you for the reviewing. Stefan, do you have other comments?

[stefan-webrtc, 2016-12-06 10:12:36]

lgtm % nit

https://codereview.webrtc.org/2533213005/diff/100001/webrtc/base/fileutils.h
File webrtc/base/fileutils.h (right):

https://codereview.webrtc.org/2533213005/diff/100001/webrtc/base/fileutils.h#...
webrtc/base/fileutils.h:177: RTC_DCHECK(NULL != organization);
organization != nullptr

similar below

[Hzj_jie, 2016-12-06 21:39:50]

The CQ bit was checked by zijiehe@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-06 21:39:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[Hzj_jie, 2016-12-06 22:33:07]

https://codereview.webrtc.org/2533213005/diff/100001/webrtc/base/fileutils.h
File webrtc/base/fileutils.h (right):

https://codereview.webrtc.org/2533213005/diff/100001/webrtc/base/fileutils.h#...
webrtc/base/fileutils.h:177: RTC_DCHECK(NULL != organization);
On 2016/12/06 10:12:36, stefan-webrtc (holmer) wrote:
> organization != nullptr
> 
> similar below

Done.

https://codereview.webrtc.org/2533213005/diff/100001/webrtc/base/urlencode.cc
File webrtc/base/urlencode.cc (right):

https://codereview.webrtc.org/2533213005/diff/100001/webrtc/base/urlencode.cc...
webrtc/base/urlencode.cc:117: RTC_DCHECK(static_cast<unsigned int>(dest - start)
< max);
On 2016/12/01 13:31:13, palmkvist wrote:
> RTC_DCHECK_LT

Done.

[commit-bot: I haz the power, 2016-12-06 22:37:11]

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

[commit-bot: I haz the power, 2016-12-06 22:37:12]

Dry run: This issue passed the CQ dry run.

[Hzj_jie, 2016-12-06 23:02:21]

The CQ bit was checked by zijiehe@chromium.org

[Hzj_jie, 2016-12-06 23:02:21]

The patchset sent to the CQ was uploaded after l-g-t-m from
palmkvist@webrtc.org, stefan@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2533213005/#ps120001 (title:
"Resolve review comments")

[commit-bot: I haz the power, 2016-12-06 23:02:32]

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, 2016-12-06 23:04:02]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1481065341290540,
"parent_rev": "ddb890c4cbcf66bf95997b5e711eb62ed8ed34b9", "commit_rev":
"6f6e8b52b7e605412d054d080b3e0cb7d01dad1e"}

[commit-bot: I haz the power, 2016-12-06 23:04:06]

Description was changed from

==========
Add File::Open / Create functions to take an rtc::Pathname

When implementing ISOLATED_OUTDIR feature in WebRTC, I found two issues,
1. pathutils and flags are not accessible in testsupport. But both of them are
useful for the feature. Pathname can help to combine path with filename, while
a flag is needed to handle command line parameter.
2. rtc::File cannot accept an rtc::Pathname, which is a little bit inconvenient.

After investigating bug webrtc:3806, flags, pathutils and urlencode are
removed from rtc_base_approved because of the including of common.h. So I
replaced common.h with checks.h, and ASSERT with RTC_DCHECK. flags,
pathutils and urlencode pairs now can be placed into rtc_base_approved to
unblock file.h to include pathutils.h.

Please kindly let me know if you have other concerns about this change.

BUG=webrtc:3806, webrtc:6732

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_rel_ng;master.tryserver.chromium.mac:mac_chromium_rel_ng;master.tryserver.chromium.win:win_chromium_rel_ng;master.tryserver.chromium.android:linux_android_rel_ng
==========

to

==========
Add File::Open / Create functions to take an rtc::Pathname

When implementing ISOLATED_OUTDIR feature in WebRTC, I found two issues,
1. pathutils and flags are not accessible in testsupport. But both of them are
useful for the feature. Pathname can help to combine path with filename, while
a flag is needed to handle command line parameter.
2. rtc::File cannot accept an rtc::Pathname, which is a little bit inconvenient.

After investigating bug webrtc:3806, flags, pathutils and urlencode are
removed from rtc_base_approved because of the including of common.h. So I
replaced common.h with checks.h, and ASSERT with RTC_DCHECK. flags,
pathutils and urlencode pairs now can be placed into rtc_base_approved to
unblock file.h to include pathutils.h.

Please kindly let me know if you have other concerns about this change.

BUG=webrtc:3806, webrtc:6732

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_rel_ng;master.tryserver.chromium.mac:mac_chromium_rel_ng;master.tryserver.chromium.win:win_chromium_rel_ng;master.tryserver.chromium.android:linux_android_rel_ng
==========

[commit-bot: I haz the power, 2016-12-06 23:04:07]

Committed patchset #2 (id:120001)

[commit-bot: I haz the power, 2016-12-06 23:04:13]

Description was changed from

==========
Add File::Open / Create functions to take an rtc::Pathname

When implementing ISOLATED_OUTDIR feature in WebRTC, I found two issues,
1. pathutils and flags are not accessible in testsupport. But both of them are
useful for the feature. Pathname can help to combine path with filename, while
a flag is needed to handle command line parameter.
2. rtc::File cannot accept an rtc::Pathname, which is a little bit inconvenient.

After investigating bug webrtc:3806, flags, pathutils and urlencode are
removed from rtc_base_approved because of the including of common.h. So I
replaced common.h with checks.h, and ASSERT with RTC_DCHECK. flags,
pathutils and urlencode pairs now can be placed into rtc_base_approved to
unblock file.h to include pathutils.h.

Please kindly let me know if you have other concerns about this change.

BUG=webrtc:3806, webrtc:6732

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_rel_ng;master.tryserver.chromium.mac:mac_chromium_rel_ng;master.tryserver.chromium.win:win_chromium_rel_ng;master.tryserver.chromium.android:linux_android_rel_ng
==========

to

==========
Add File::Open / Create functions to take an rtc::Pathname

When implementing ISOLATED_OUTDIR feature in WebRTC, I found two issues,
1. pathutils and flags are not accessible in testsupport. But both of them are
useful for the feature. Pathname can help to combine path with filename, while
a flag is needed to handle command line parameter.
2. rtc::File cannot accept an rtc::Pathname, which is a little bit inconvenient.

After investigating bug webrtc:3806, flags, pathutils and urlencode are
removed from rtc_base_approved because of the including of common.h. So I
replaced common.h with checks.h, and ASSERT with RTC_DCHECK. flags,
pathutils and urlencode pairs now can be placed into rtc_base_approved to
unblock file.h to include pathutils.h.

Please kindly let me know if you have other concerns about this change.

BUG=webrtc:3806, webrtc:6732

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_rel_ng;master.tryserver.chromium.mac:mac_chromium_rel_ng;master.tryserver.chromium.win:win_chromium_rel_ng;master.tryserver.chromium.android:linux_android_rel_ng

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

[commit-bot: I haz the power, 2016-12-06 23:04:14]

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

[nisse-webrtc, 2016-12-08 08:57:38]

Hi,

I'd just like to add that I'd like to delete all of base/pathutils.*, together
with base/fileutils.*.

I think it is a bit overengineered for webrtc, and besides test code, it is used
only in a few pieces
of deprecated code, try git grep base/pathutils.h.

Nice that the deprecated helpers in pathutils.h were deleted, but maybe a bit
unfortunate to
promote pathutils.h to rtc_base_approved.
  
If some of the pathutils features are helpful for tests, it could be moved to
some test target
rather than deleted completely.

[kjellander_webrtc, 2016-12-08 09:19:47]

On 2016/12/08 08:57:38, nisse-webrtc wrote:
> Hi,
> 
> I'd just like to add that I'd like to delete all of base/pathutils.*, together
> with base/fileutils.*.
> 
> I think it is a bit overengineered for webrtc, and besides test code, it is
used
> only in a few pieces
> of deprecated code, try git grep base/pathutils.h.
> 
> Nice that the deprecated helpers in pathutils.h were deleted, but maybe a bit
> unfortunate to
> promote pathutils.h to rtc_base_approved.
>   
> If some of the pathutils features are helpful for tests, it could be moved to
> some test target
> rather than deleted completely.

I agree, these things could be moved to test_support instead. Providing utility
classes for handling files and paths isn't something we should expose as part of
our public API.

[Hzj_jie, 2016-12-08 19:45:59]

On 2016/12/08 09:19:47, kjellander_webrtc wrote:
> On 2016/12/08 08:57:38, nisse-webrtc wrote:
> > Hi,
> > 
> > I'd just like to add that I'd like to delete all of base/pathutils.*,
together
> > with base/fileutils.*.
> > 
> > I think it is a bit overengineered for webrtc, and besides test code, it is
> used
> > only in a few pieces
> > of deprecated code, try git grep base/pathutils.h.
> > 
> > Nice that the deprecated helpers in pathutils.h were deleted, but maybe a
bit
> > unfortunate to
> > promote pathutils.h to rtc_base_approved.
> >   
> > If some of the pathutils features are helpful for tests, it could be moved
to
> > some test target
> > rather than deleted completely.
> 
> I agree, these things could be moved to test_support instead. Providing
utility
> classes for handling files and paths isn't something we should expose as part
of
> our public API.

In change https://codereview.chromium.org/2558693002/, these classes (flags /
pathutils) are used by WriteIsolatedOutput() function. So feel free to move them
out of rtc_base_approved, and put them into testsupport, if you agree they
should not be public APIs exported by rtc.