Rearrange dependency to enable adding finch experiment in webrtc

Rearrange dependency to enable adding finch experiment in webrtc

BUG=

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

[guoweis_left_chromium, 2015-10-23 19:57:45]

Patchset #3 (id:40001) has been deleted

[guoweis_left_chromium, 2015-10-23 20:08:30]

Patchset #2 (id:20001) has been deleted

[guoweis_left_chromium, 2015-10-23 20:08:36]

Patchset #1 (id:1) has been deleted

[guoweis_left_chromium, 2015-10-23 20:20:31]

Description was changed from

==========
fix

BUG=
==========

to

==========
Rearrange dependency to enable adding finch experiment in webrtc

BUG=
==========

[guoweis_left_chromium, 2015-10-23 20:20:51]

guoweis@chromium.org changed reviewers:
+ sergeyu@chromium.org, tommi@chromium.org

[guoweis_left_chromium, 2015-10-23 20:20:51]

PTAL.

[Sergey Ulanov, 2015-10-23 20:42:27]

https://codereview.chromium.org/1419253002/diff/60001/third_party/libjingle/l...
File third_party/libjingle/libjingle.gyp (right):

https://codereview.chromium.org/1419253002/diff/60001/third_party/libjingle/l...
third_party/libjingle/libjingle.gyp:485: # GN version:
//third_party/libjingle:libjingle_webrtc_common
What is this target going to be used for?
Please add comments here and for libjingle_webrtc_shared to make it clear how
they are different.

https://codereview.chromium.org/1419253002/diff/60001/third_party/libjingle/l...
third_party/libjingle/libjingle.gyp:496: },
  # target libjingle_webrtc_common

[tommi (sloooow) - chröme, 2015-10-23 20:46:31]

https://codereview.chromium.org/1419253002/diff/60001/third_party/libjingle/l...
File third_party/libjingle/libjingle.gyp (right):

https://codereview.chromium.org/1419253002/diff/60001/third_party/libjingle/l...
third_party/libjingle/libjingle.gyp:486: 'target_name':
'libjingle_webrtc_common',
There's no target in this gyp file that depends on 'libjingle_webrtc_common'
now. What about moving this target into libjingle_nacl and rename it to
something like libjingle_webrtc_field_trial?

I'd like to keep the name libjingle_webrtc_common for the target it was usually
the name of.  The distinction between 'shared' and 'common' is too vague in my
opinion.

[guoweis_left_chromium, 2015-10-23 21:13:38]

On 2015/10/23 20:46:31, tommi wrote:
>
https://codereview.chromium.org/1419253002/diff/60001/third_party/libjingle/l...
> File third_party/libjingle/libjingle.gyp (right):
> 
>
https://codereview.chromium.org/1419253002/diff/60001/third_party/libjingle/l...
> third_party/libjingle/libjingle.gyp:486: 'target_name':
> 'libjingle_webrtc_common',
> There's no target in this gyp file that depends on 'libjingle_webrtc_common'
> now. What about moving this target into libjingle_nacl and rename it to
> something like libjingle_webrtc_field_trial?
> 
> I'd like to keep the name libjingle_webrtc_common for the target it was
usually
> the name of.  The distinction between 'shared' and 'common' is too vague in my
> opinion.

Sorry, my previous fix was completely wrong. The break I saw last night was
really in the remoting. However, in my test, I probably polluted it with some
invalid settings.

This should be the real fix.

[guoweis_left_chromium, 2015-10-23 21:14:31]

On 2015/10/23 21:13:38, guoweis_chromium wrote:
> On 2015/10/23 20:46:31, tommi wrote:
> >
>
https://codereview.chromium.org/1419253002/diff/60001/third_party/libjingle/l...
> > File third_party/libjingle/libjingle.gyp (right):
> > 
> >
>
https://codereview.chromium.org/1419253002/diff/60001/third_party/libjingle/l...
> > third_party/libjingle/libjingle.gyp:486: 'target_name':
> > 'libjingle_webrtc_common',
> > There's no target in this gyp file that depends on 'libjingle_webrtc_common'
> > now. What about moving this target into libjingle_nacl and rename it to
> > something like libjingle_webrtc_field_trial?
> > 
> > I'd like to keep the name libjingle_webrtc_common for the target it was
> usually
> > the name of.  The distinction between 'shared' and 'common' is too vague in
my
> > opinion.
> 
> Sorry, my previous fix was completely wrong. The break I saw last night was
> really in the remoting. However, in my test, I probably polluted it with some
> invalid settings.
> 
> This should be the real fix.

I probably should have sent this email when all trybots finish. Please feel free
to put this on hold and I'll ping you again when everything is green.

[guoweis_left_chromium, 2015-10-23 23:25:30]

Patchset #4 (id:120001) has been deleted

[guoweis_left_chromium, 2015-10-25 21:40:11]

Patchset #3 (id:100001) has been deleted

[guoweis_left_chromium, 2015-10-25 21:40:19]

Patchset #3 (id:140001) has been deleted

[guoweis_left_chromium, 2015-10-25 21:40:29]

Patchset #3 (id:160001) has been deleted

[guoweis_left_chromium, 2015-10-26 02:31:09]

On 2015/10/23 21:14:31, guoweis_chromium wrote:
> On 2015/10/23 21:13:38, guoweis_chromium wrote:
> > On 2015/10/23 20:46:31, tommi wrote:
> > >
> >
>
https://codereview.chromium.org/1419253002/diff/60001/third_party/libjingle/l...
> > > File third_party/libjingle/libjingle.gyp (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1419253002/diff/60001/third_party/libjingle/l...
> > > third_party/libjingle/libjingle.gyp:486: 'target_name':
> > > 'libjingle_webrtc_common',
> > > There's no target in this gyp file that depends on
'libjingle_webrtc_common'
> > > now. What about moving this target into libjingle_nacl and rename it to
> > > something like libjingle_webrtc_field_trial?
> > > 
> > > I'd like to keep the name libjingle_webrtc_common for the target it was
> > usually
> > > the name of.  The distinction between 'shared' and 'common' is too vague
in
> my
> > > opinion.
> > 
> > Sorry, my previous fix was completely wrong. The break I saw last night was
> > really in the remoting. However, in my test, I probably polluted it with
some
> > invalid settings.
> > 
> > This should be the real fix.
> 
> I probably should have sent this email when all trybots finish. Please feel
free
> to put this on hold and I'll ping you again when everything is green.

PTAL.

[Sergey Ulanov, 2015-10-26 17:43:31]

https://codereview.chromium.org/1419253002/diff/230001/third_party/libjingle/...
File third_party/libjingle/libjingle.gyp (right):

https://codereview.chromium.org/1419253002/diff/230001/third_party/libjingle/...
third_party/libjingle/libjingle.gyp:247: # p2ptransportchannel.cc now depends on
field_trial. For NACL build, a
I don't think you need to mention the name of the file here - how field_trial is
used will probably change in the future.

https://codereview.chromium.org/1419253002/diff/230001/third_party/libjingle/...
third_party/libjingle/libjingle.gyp:249: 'sources': [
For consistency with other overrides please move this to the list in
libjingle_common.gypi and then exclude it in sources! in libjingle_nacl.gyp

https://codereview.chromium.org/1419253002/diff/230001/third_party/libjingle/...
File third_party/libjingle/overrides/field_trial.cc (right):

https://codereview.chromium.org/1419253002/diff/230001/third_party/libjingle/...
third_party/libjingle/overrides/field_trial.cc:8: // Define
webrtc::field_trial::FindFullName to provide webrtc with a field trial
nit: This comment looks out of place between the two namespace statements. Move
it one line above or one line below

https://codereview.chromium.org/1419253002/diff/230001/third_party/libjingle/...
third_party/libjingle/overrides/field_trial.cc:15: 
nit: remove this empty line or add one after line 7.

[guoweis_left_chromium, 2015-10-26 18:16:44]

PTAL. Running bots now to ensure no issue.

https://codereview.chromium.org/1419253002/diff/230001/third_party/libjingle/...
File third_party/libjingle/libjingle.gyp (right):

https://codereview.chromium.org/1419253002/diff/230001/third_party/libjingle/...
third_party/libjingle/libjingle.gyp:247: # p2ptransportchannel.cc now depends on
field_trial. For NACL build, a
On 2015/10/26 17:43:31, Sergey Ulanov wrote:
> I don't think you need to mention the name of the file here - how field_trial
is
> used will probably change in the future.

Done.

https://codereview.chromium.org/1419253002/diff/230001/third_party/libjingle/...
third_party/libjingle/libjingle.gyp:249: 'sources': [
On 2015/10/26 17:43:31, Sergey Ulanov wrote:
> For consistency with other overrides please move this to the list in
> libjingle_common.gypi and then exclude it in sources! in libjingle_nacl.gyp

Done.

https://codereview.chromium.org/1419253002/diff/230001/third_party/libjingle/...
File third_party/libjingle/overrides/field_trial.cc (right):

https://codereview.chromium.org/1419253002/diff/230001/third_party/libjingle/...
third_party/libjingle/overrides/field_trial.cc:8: // Define
webrtc::field_trial::FindFullName to provide webrtc with a field trial
On 2015/10/26 17:43:31, Sergey Ulanov wrote:
> nit: This comment looks out of place between the two namespace statements.
Move
> it one line above or one line below

Done.

https://codereview.chromium.org/1419253002/diff/230001/third_party/libjingle/...
third_party/libjingle/overrides/field_trial.cc:15: 
On 2015/10/26 17:43:31, Sergey Ulanov wrote:
> nit: remove this empty line or add one after line 7.

Done.

[Sergey Ulanov, 2015-10-26 20:55:59]

lgtm

[guoweis_left_chromium, 2015-10-26 21:02:54]

The CQ bit was checked by guoweis@chromium.org

[commit-bot: I haz the power, 2015-10-26 21:04:39]

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

[commit-bot: I haz the power, 2015-10-26 21:53:54]

Committed patchset #7 (id:250001)

[commit-bot: I haz the power, 2015-10-26 21:54:40]

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