Introduce an 'ENABLE_WEBSOCKETS' define in //net.

Introduce an 'ENABLE_WEBSOCKETS' define in //net.

Following up on https://codereview.chromium.org/2079783002,
WebSockets aren't available on iOS. Rather than hard-coding that
mapping of OS to feature into the code, we should be using a
feature-specific build flag to handle the dependency. This patch
does that.

R=mmenke@chromium.org

Committed: https://crrev.com/0cb69fcbdc735ade2e29384cb962bcadae45725e
Cr-Commit-Position: refs/heads/master@{#401034}

[Mike West, 2016-06-20 13:37:55]

Description was changed from

==========
Introduce an 'ENABLE_WEBSOCKETS' define in //net.

WebSockets aren't available on iOS. Rather than 'defined'

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

to

==========
Introduce an 'ENABLE_WEBSOCKETS' define in //net.

Following up on https://codereview.chromium.org/2079783002,
WebSockets aren't available on iOS. Rather than hard-coding that
mapping of OS to feature into the code, we should be using a
feature-specific build flag to handle the dependency. This patch
does that.

R=mmenke@chromium.org
==========

[Mike West, 2016-06-20 13:37:55]

mkwst@chromium.org changed reviewers:
+ mmenke@chromium.org

[Mike West, 2016-06-20 13:38:03]

WDYT, mmenke@?

[Mike West, 2016-06-20 14:06:10]

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

[commit-bot: I haz the power, 2016-06-20 14:06:22]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2082603002/1

[mmenke, 2016-06-20 15:14:09]

Thanks for doing this!  Groups of related platform-specific ifdefs based on what
features we support on what platform make me cringe.

https://codereview.chromium.org/2082603002/diff/1/net/BUILD.gn
File net/BUILD.gn (right):

https://codereview.chromium.org/2082603002/diff/1/net/BUILD.gn#newcode72
net/BUILD.gn:72: defines += [ "ENABLE_WEBSOCKETS" ]
Hrm...We should probably go with DISABLE_WEBSOCKETS instead.  The other features
use default on, don't see why this should be different.  Admittedly, mixing that
with the enable_websockets gyp/gn define is a little confusing.  Probably worth
changing that as well at some point - feel free to do that now, in this CL, but
also feel free not to.

Doesn't look like it's used outside net, so should be a pretty simple change.

https://codereview.chromium.org/2082603002/diff/1/net/url_request/url_request...
File net/url_request/url_request_http_job_unittest.cc (right):

https://codereview.chromium.org/2082603002/diff/1/net/url_request/url_request...
net/url_request/url_request_http_job_unittest.cc:651: #if
defined(ENABLE_WEBSOCKETS)
url_request_job_manager.cc also has an defined(OS_IOS) check that should be
updated.

[commit-bot: I haz the power, 2016-06-20 15:48:04]

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

[commit-bot: I haz the power, 2016-06-20 15:48:05]

Dry run: This issue passed the CQ dry run.

[Mike West, 2016-06-21 10:31:15]

https://codereview.chromium.org/2082603002/diff/1/net/BUILD.gn
File net/BUILD.gn (right):

https://codereview.chromium.org/2082603002/diff/1/net/BUILD.gn#newcode72
net/BUILD.gn:72: defines += [ "ENABLE_WEBSOCKETS" ]
On 2016/06/20 at 15:14:09, mmenke wrote:
> Hrm...We should probably go with DISABLE_WEBSOCKETS instead.  The other
features use default on, don't see why this should be different.

It doesn't look that clearcut. We have
"DISABLE_FILE_SUPPORT"/"DISABLE_FTP_SUPPORT"/"disable_brotli_filter" one the one
hand, but
"ENABLE_WEBSOCKETS"/"ENABLE_BUILT_IN_DNS"/"USE_KERBEROS"/"use_v8_in_net" on the
other.

> Admittedly, mixing that with the enable_websockets gyp/gn define is a little
confusing.  Probably worth changing that as well at some point - feel free to do
that now, in this CL, but also feel free not to.

If aligning on one or the other would help clarify things for the build, great.
I think that can be done in a separate CL, however.

WDYT?


 
> Doesn't look like it's used outside net, so should be a pretty simple change.

https://codereview.chromium.org/2082603002/diff/1/net/url_request/url_request...
File net/url_request/url_request_http_job_unittest.cc (right):

https://codereview.chromium.org/2082603002/diff/1/net/url_request/url_request...
net/url_request/url_request_http_job_unittest.cc:651: #if
defined(ENABLE_WEBSOCKETS)
On 2016/06/20 at 15:14:09, mmenke wrote:
> url_request_job_manager.cc also has an defined(OS_IOS) check that should be
updated.

Done.

[Mike West, 2016-06-21 10:31:21]

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

[commit-bot: I haz the power, 2016-06-21 10:31:54]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2082603002/20001

[commit-bot: I haz the power, 2016-06-21 12:08:18]

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

[commit-bot: I haz the power, 2016-06-21 12:08:19]

Dry run: This issue passed the CQ dry run.

[mmenke, 2016-06-21 16:33:54]

LGTM

https://codereview.chromium.org/2082603002/diff/1/net/BUILD.gn
File net/BUILD.gn (right):

https://codereview.chromium.org/2082603002/diff/1/net/BUILD.gn#newcode72
net/BUILD.gn:72: defines += [ "ENABLE_WEBSOCKETS" ]
On 2016/06/21 10:31:15, Mike West wrote:
> On 2016/06/20 at 15:14:09, mmenke wrote:
> > Hrm...We should probably go with DISABLE_WEBSOCKETS instead.  The other
> features use default on, don't see why this should be different.
> 
> It doesn't look that clearcut. We have
> "DISABLE_FILE_SUPPORT"/"DISABLE_FTP_SUPPORT"/"disable_brotli_filter" one the
one
> hand, but
> "ENABLE_WEBSOCKETS"/"ENABLE_BUILT_IN_DNS"/"USE_KERBEROS"/"use_v8_in_net" on
the
> other.
> 
> > Admittedly, mixing that with the enable_websockets gyp/gn define is a little
> confusing.  Probably worth changing that as well at some point - feel free to
do
> that now, in this CL, but also feel free not to.
> 
> If aligning on one or the other would help clarify things for the build,
great.
> I think that can be done in a separate CL, however.
> 
> WDYT?
> 
> 
>  
> > Doesn't look like it's used outside net, so should be a pretty simple
change.
> 

SGTM.

[Mike West, 2016-06-21 17:40:31]

The CQ bit was checked by mkwst@chromium.org

[commit-bot: I haz the power, 2016-06-21 17:41:19]

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

[commit-bot: I haz the power, 2016-06-21 17:47:00]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-21 17:50:04]

Description was changed from

==========
Introduce an 'ENABLE_WEBSOCKETS' define in //net.

Following up on https://codereview.chromium.org/2079783002,
WebSockets aren't available on iOS. Rather than hard-coding that
mapping of OS to feature into the code, we should be using a
feature-specific build flag to handle the dependency. This patch
does that.

R=mmenke@chromium.org
==========

to

==========
Introduce an 'ENABLE_WEBSOCKETS' define in //net.

Following up on https://codereview.chromium.org/2079783002,
WebSockets aren't available on iOS. Rather than hard-coding that
mapping of OS to feature into the code, we should be using a
feature-specific build flag to handle the dependency. This patch
does that.

R=mmenke@chromium.org

Committed: https://crrev.com/0cb69fcbdc735ade2e29384cb962bcadae45725e
Cr-Commit-Position: refs/heads/master@{#401034}
==========

[commit-bot: I haz the power, 2016-06-21 17:50:05]

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