Added support for enabling GCD, or not, at build time.

Added support for enabling GCD, or not, at build time.

GCD support is off by default.

BUG=470744
Committed: https://crrev.com/f5f980445cd3b66d8735647f6c3d9ce7cd73990e
Cr-Commit-Position: refs/heads/master@{#322430}

[John Williams, 2015-03-25 18:32:59]

jrw@chromium.org changed reviewers:
+ sergeyu@chromium.org

[Sergey Ulanov, 2015-03-25 18:56:43]

mostly nits. Please open a bug for GCD work and link it from this CL.

https://codereview.chromium.org/1007153003/diff/40001/remoting/app_remoting_w...
File remoting/app_remoting_webapp_build.gypi (right):

https://codereview.chromium.org/1007153003/diff/40001/remoting/app_remoting_w...
remoting/app_remoting_webapp_build.gypi:148: '--use_gcd', '<(remoting_use_gcd)',
I don't think we need this for app remoting. They don't use directory and won't
be using GCD.

https://codereview.chromium.org/1007153003/diff/40001/remoting/webapp/build-w...
File remoting/webapp/build-webapp.py (right):

https://codereview.chromium.org/1007153003/diff/40001/remoting/webapp/build-w...
remoting/webapp/build-webapp.py:68: "!!'" + placeholder + "'", 'true' if value
else 'false')
Why do you need !! here?

https://codereview.chromium.org/1007153003/diff/40001/remoting/webapp/build-w...
remoting/webapp/build-webapp.py:343: print 'args.use_gcd {!r}'.format(use_gcd)
remove this?

https://codereview.chromium.org/1007153003/diff/40001/remoting/webapp/build-w...
remoting/webapp/build-webapp.py:400: return buildWebApp(**args.__dict__)
Use dict(args) instead of args.__dict__

https://codereview.chromium.org/1007153003/diff/40001/remoting/webapp/crd/man...
File remoting/webapp/crd/manifest.json.jinja2 (right):

https://codereview.chromium.org/1007153003/diff/40001/remoting/webapp/crd/man...
remoting/webapp/crd/manifest.json.jinja2:56: {%- if USE_GCD %}
We don't use "%-" in other places in this file and I think it's better to keep
it consistent. Please either add '-' for other jinja statements in this file or
remove it here.

[John Williams, 2015-03-25 20:32:20]

https://codereview.chromium.org/1007153003/diff/40001/remoting/app_remoting_w...
File remoting/app_remoting_webapp_build.gypi (right):

https://codereview.chromium.org/1007153003/diff/40001/remoting/app_remoting_w...
remoting/app_remoting_webapp_build.gypi:148: '--use_gcd', '<(remoting_use_gcd)',
On 2015/03/25 18:56:43, Sergey Ulanov wrote:
> I don't think we need this for app remoting. They don't use directory and
won't
> be using GCD.

Done.

https://codereview.chromium.org/1007153003/diff/40001/remoting/webapp/build-w...
File remoting/webapp/build-webapp.py (right):

https://codereview.chromium.org/1007153003/diff/40001/remoting/webapp/build-w...
remoting/webapp/build-webapp.py:68: "!!'" + placeholder + "'", 'true' if value
else 'false')
On 2015/03/25 18:56:43, Sergey Ulanov wrote:
> Why do you need !! here?

The JS code is typechecked before being processed by this script, so the value
being replaced has to be boolean.

https://codereview.chromium.org/1007153003/diff/40001/remoting/webapp/build-w...
remoting/webapp/build-webapp.py:343: print 'args.use_gcd {!r}'.format(use_gcd)
On 2015/03/25 18:56:43, Sergey Ulanov wrote:
> remove this?

Oops.

https://codereview.chromium.org/1007153003/diff/40001/remoting/webapp/build-w...
remoting/webapp/build-webapp.py:400: return buildWebApp(**args.__dict__)
On 2015/03/25 18:56:43, Sergey Ulanov wrote:
> Use dict(args) instead of args.__dict__

Using |dict| doesn't work, but |vars| does.

https://codereview.chromium.org/1007153003/diff/40001/remoting/webapp/crd/man...
File remoting/webapp/crd/manifest.json.jinja2 (right):

https://codereview.chromium.org/1007153003/diff/40001/remoting/webapp/crd/man...
remoting/webapp/crd/manifest.json.jinja2:56: {%- if USE_GCD %}
On 2015/03/25 18:56:43, Sergey Ulanov wrote:
> We don't use "%-" in other places in this file and I think it's better to keep
> it consistent. Please either add '-' for other jinja statements in this file
or
> remove it here.

Done.

[Sergey Ulanov, 2015-03-25 20:50:50]

https://codereview.chromium.org/1007153003/diff/40001/remoting/webapp/build-w...
File remoting/webapp/build-webapp.py (right):

https://codereview.chromium.org/1007153003/diff/40001/remoting/webapp/build-w...
remoting/webapp/build-webapp.py:68: "!!'" + placeholder + "'", 'true' if value
else 'false')
On 2015/03/25 20:32:19, John Williams wrote:
> On 2015/03/25 18:56:43, Sergey Ulanov wrote:
> > Why do you need !! here?
> 
> The JS code is typechecked before being processed by this script, so the value
> being replaced has to be boolean.

Right, but type is explicitly specified there anyway, so I'm not sure it's
necessary. Anyway, XMPP_SERVER_USE_TLS flag is also boolean and it's replaced
differently. It's better to make them consistent, i.e. use replaceBool() for it
too.

[John Williams, 2015-03-25 22:54:51]

https://codereview.chromium.org/1007153003/diff/40001/remoting/webapp/build-w...
File remoting/webapp/build-webapp.py (right):

https://codereview.chromium.org/1007153003/diff/40001/remoting/webapp/build-w...
remoting/webapp/build-webapp.py:68: "!!'" + placeholder + "'", 'true' if value
else 'false')
On 2015/03/25 20:50:50, Sergey Ulanov wrote:
> On 2015/03/25 20:32:19, John Williams wrote:
> > On 2015/03/25 18:56:43, Sergey Ulanov wrote:
> > > Why do you need !! here?
> > 
> > The JS code is typechecked before being processed by this script, so the
value
> > being replaced has to be boolean.
> 
> Right, but type is explicitly specified there anyway, so I'm not sure it's
> necessary. Anyway, XMPP_SERVER_USE_TLS flag is also boolean and it's replaced
> differently. It's better to make them consistent, i.e. use replaceBool() for
it
> too.

Done.

[Sergey Ulanov, 2015-03-26 00:33:02]

Looks good, but can you please open a tracking bug and link it from the
description of this CL using BUG=

[Sergey Ulanov, 2015-03-26 16:52:01]

I was actually suggesting to open more generic bug for GCD overall (I though it
would be clear from my first comment in this CL). There is no need to open a
separate bug for each small CL like this one. You should still open a bug for
GCD (for your future CLs) and then mark it as blocked on 470744. LGTM.

[John Williams, 2015-03-26 17:15:24]

The CQ bit was checked by jrw@chromium.org

[commit-bot: I haz the power, 2015-03-26 17:16:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1007153003/100001

[John Williams, 2015-03-26 17:16:55]

What if I just change the existing bug to make it cover the whole project? 
Gotta conserve those 6-digit integers, you know.

[commit-bot: I haz the power, 2015-03-26 18:26:02]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-03-26 18:26:48]

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