Applying late feedback on CL https://chromiumcodereview.appspot.com/10834051.

Applying late feedback on CL https://chromiumcodereview.appspot.com/10834051.

All are stylistic changes reordering targets for better maintainability. 

BUG=None


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=149611

[noyau (Ping after 24h), 2012-07-31 15:29:57]

https://chromiumcodereview.appspot.com/10832082/diff/1/net/net.gyp
File net/net.gyp (right):

https://chromiumcodereview.appspot.com/10832082/diff/1/net/net.gyp#newcode1613
net/net.gyp:1613: ['inside_chromium_build==1 and OS != "ios"', {
wtc said:
> This change is probably worth a comment.  I can't figure out
> why iOS is excluded here.
To be honest, I don't know. This was added in the ios tree and I parroted it
here. I'm guessing that as we don't compile nor use a test server on ios there
is no point for those dependencies anyway? But that's just an educated guess.

[stuartmorgan, 2012-07-31 17:47:47]

https://chromiumcodereview.appspot.com/10832082/diff/1/net/net.gyp
File net/net.gyp (right):

https://chromiumcodereview.appspot.com/10832082/diff/1/net/net.gyp#newcode1613
net/net.gyp:1613: ['inside_chromium_build==1 and OS != "ios"', {
On 2012/07/31 15:29:57, noyau wrote:
> wtc said:
> > This change is probably worth a comment.  I can't figure out
> > why iOS is excluded here.
> To be honest, I don't know. This was added in the ios tree and I parroted it
> here. I'm guessing that as we don't compile nor use a test server on ios there
> is no point for those dependencies anyway? But that's just an educated guess.

I haven't looked up the history, but it's certainly the case that we don't have
support yet for running a test server on the host machine and having a device
talk to that server.

It's unclear to me if the cloud_policy_codegen thing is related to the test
server though.

[Ryan Sleevi, 2012-07-31 17:54:29]

https://chromiumcodereview.appspot.com/10832082/diff/1/net/net.gyp
File net/net.gyp (right):

https://chromiumcodereview.appspot.com/10832082/diff/1/net/net.gyp#newcode1613
net/net.gyp:1613: ['inside_chromium_build==1 and OS != "ios"', {
On 2012/07/31 17:47:47, stuartmorgan wrote:
> On 2012/07/31 15:29:57, noyau wrote:
> > wtc said:
> > > This change is probably worth a comment.  I can't figure out
> > > why iOS is excluded here.
> > To be honest, I don't know. This was added in the ios tree and I parroted it
> > here. I'm guessing that as we don't compile nor use a test server on ios
there
> > is no point for those dependencies anyway? But that's just an educated
guess.
> 
> I haven't looked up the history, but it's certainly the case that we don't
have
> support yet for running a test server on the host machine and having a device
> talk to that server.
> 
> It's unclear to me if the cloud_policy_codegen thing is related to the test
> server though.

It is.  http://crbug.com/119403 is about fixing this. I thought upstream had a
bug annotation here that may not have been caught in the move.

It's listed here because the Python code unconditionally imports the generated
Python protobuf-generated code.

The 'inside_chromium_build' was to ensure that these dependencies don't exist
for Chromium-in-WebKit, since as it was mentioned elsewhere,
dependencies!/dependencies/ don't work for non-existinant GYP files.

[wtc, 2012-08-01 17:38:48]

Patch set 1 LGTM.  Thanks!

It would be nice to wait for the review of either
stuartmorgan and rsleevi, since they are more familiar with
the iOS porting issues.

[Ryan Sleevi, 2012-08-01 17:46:56]

LGTM with nits that are not your fault

http://codereview.chromium.org/10832082/diff/1/net/net.gyp
File net/net.gyp (right):

http://codereview.chromium.org/10832082/diff/1/net/net.gyp#newcode1071
net/net.gyp:1071: # Android can shut down our app at any time, so we persist
session cookies.
not your fault, but...

style nit: Line wrapping
comment nit: Remove "we"

http://codereview.chromium.org/10832082/diff/1/net/net.gyp#newcode1076
net/net.gyp:1076: 'net_jni_headers',
nit: Just from looking at the other changes, '../' is sorted after the alphas
(eg: 1745 - 1749)

[noyau (Ping after 24h), 2012-08-02 09:56:38]

http://codereview.chromium.org/10832082/diff/1/net/net.gyp
File net/net.gyp (right):

http://codereview.chromium.org/10832082/diff/1/net/net.gyp#newcode1071
net/net.gyp:1071: # Android can shut down our app at any time, so we persist
session cookies.
On 2012/08/01 17:46:56, Ryan Sleevi wrote:
> not your fault, but...
> 
> style nit: Line wrapping
> comment nit: Remove "we"

Done.

http://codereview.chromium.org/10832082/diff/1/net/net.gyp#newcode1076
net/net.gyp:1076: 'net_jni_headers',
On 2012/08/01 17:46:56, Ryan Sleevi wrote:
> nit: Just from looking at the other changes, '../' is sorted after the alphas
> (eg: 1745 - 1749)
This is completely inconsistent in this file. '..' should be *before* the alpha,
it is for the 'net' target, for this one and some others. I've made it all
consistent.

[commit-bot: I haz the power, 2012-08-02 09:56:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/noyau@chromium.org/10832082/4002

[commit-bot: I haz the power, 2012-08-02 11:28:24]

Change committed as 149611