Convert spellcheck to a buildflag header.

Convert spellcheck to a buildflag header.

Move the build variables from build/config/features.gni to a new header in components/spellcheck

Removes the global defines for ENABLE_SPELLCHECK and USE_BROWSER_SPELLCHECKER and move them to a generated header.

R=rouslan
TBR=mkwst@chromium.org (_messages.h file)

Committed: https://crrev.com/41446f6842fd10bb4671afb4ecd700ab6db20158
Cr-Commit-Position: refs/heads/master@{#431962}

[brettw, 2016-11-14 19:12:49]

brettw@chromium.org changed reviewers:
+ rouslan@chromium.org

[brettw, 2016-11-14 19:13:35]

Fixes

[brettw, 2016-11-14 19:13:36]

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

[commit-bot: I haz the power, 2016-11-14 19:14:15]

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

[please use gerrit instead, 2016-11-14 19:19:08]

lgtm % a couple of fixes

https://codereview.chromium.org/2495193003/diff/20001/chrome/browser/renderer...
File chrome/browser/renderer_context_menu/render_view_context_menu.cc (right):

https://codereview.chromium.org/2495193003/diff/20001/chrome/browser/renderer...
chrome/browser/renderer_context_menu/render_view_context_menu.cc:1: BUILDFLAG
Copyright 2014 The Chromium Authors. All rights reserved.
Please fix

https://codereview.chromium.org/2495193003/diff/20001/components/spellcheck/s...
File components/spellcheck/spellcheck_build_features.gni (right):

https://codereview.chromium.org/2495193003/diff/20001/components/spellcheck/s...
components/spellcheck/spellcheck_build_features.gni:1: # Copyright 2013 The
Chromium Authors. All rights reserved.
2016

[brettw, 2016-11-14 19:28:49]

The CQ bit was unchecked by brettw@chromium.org

[brettw, 2016-11-14 19:28:54]

The CQ bit was checked by brettw@chromium.org

[brettw, 2016-11-14 19:29:11]

Both done, thanks.

[commit-bot: I haz the power, 2016-11-14 19:29:56]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-14 19:46:18]

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

[commit-bot: I haz the power, 2016-11-14 19:46:19]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[groby-ooo-7-16, 2016-11-14 20:05:27]

groby@chromium.org changed reviewers:
+ groby@chromium.org

[groby-ooo-7-16, 2016-11-14 20:05:28]

Thank you for the cleanup!

A few nits and questions, but please consider these orthogonal to CL approval.

https://codereview.chromium.org/2495193003/diff/40001/components/spellcheck/s...
File components/spellcheck/spellcheck_build_features.gni (right):

https://codereview.chromium.org/2495193003/diff/40001/components/spellcheck/s...
components/spellcheck/spellcheck_build_features.gni:6: enable_spellcheck =
!is_ios
I'm somewhat unhappy that we control what component a platform includes in
individual build files. Do we have any plans to move to a more central "this
build uses these components" kind of list?

https://codereview.chromium.org/2495193003/diff/40001/components/spellcheck/s...
components/spellcheck/spellcheck_build_features.gni:8: # Use the operating
system's spellchecker rather than hunspell.
Note: It might be worth leaving a comment that we *still* use a browser-side
checker if that is turned off, namely the red underline checker. We just don't
invoke the system checker.  Or, possibly, rename this to use_system_checker.

(Yes, I'm not happy about it, either.)

[brettw, 2016-11-14 20:41:57]

Comment

[brettw, 2016-11-14 20:41:58]

The CQ bit was checked by brettw@chromium.org

[brettw, 2016-11-14 20:41:58]

The patchset sent to the CQ was uploaded after l-g-t-m from rouslan@chromium.org
Link to the patchset: https://codereview.chromium.org/2495193003/#ps60001
(title: "Comment")

[brettw, 2016-11-14 20:42:53]

https://codereview.chromium.org/2495193003/diff/40001/components/spellcheck/s...
File components/spellcheck/spellcheck_build_features.gni (right):

https://codereview.chromium.org/2495193003/diff/40001/components/spellcheck/s...
components/spellcheck/spellcheck_build_features.gni:6: enable_spellcheck =
!is_ios
On 2016/11/14 20:05:28, groby wrote:
> I'm somewhat unhappy that we control what component a platform includes in
> individual build files. Do we have any plans to move to a more central "this
> build uses these components" kind of list?

No, I'm doing the opposite. Our build files are shared between projects and
things like skia, v8, and webrtc builds (if they haven't forked the build
config) get e.g. ENABLE_SPELLCHECK defines which makes no sense.

The build is a 200K line program, and just as we don't have all C++ globals in
one file, having all build flags is not scalable.

"gn args --list out/Default" will tell you all flags you can override (currently
about 300). These particular flags are not overridable. I almost removed them
and changed the checks to ios conditions, but keeping it seems conceptually
cleaner and we may want to tweak it in the future.

https://codereview.chromium.org/2495193003/diff/40001/components/spellcheck/s...
components/spellcheck/spellcheck_build_features.gni:8: # Use the operating
system's spellchecker rather than hunspell.
Thanks, done.

[commit-bot: I haz the power, 2016-11-14 20:43:56]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-14 20:56:02]

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

[commit-bot: I haz the power, 2016-11-14 20:56:03]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[brettw, 2016-11-14 21:08:47]

Description was changed from

==========
Convert spellcheck to a buildflag header.

Move the build variables from build/config/features.gni to a new header in
components/spellcheck

Removes the global defines for ENABLE_SPELLCHECK and USE_BROWSER_SPELLCHECKER
and move them to a generated header.
==========

to

==========
Convert spellcheck to a buildflag header.

Move the build variables from build/config/features.gni to a new header in
components/spellcheck

Removes the global defines for ENABLE_SPELLCHECK and USE_BROWSER_SPELLCHECKER
and move them to a generated header.

R=rouslan
TBR=mkwst@chromium.org (_messages.h file)
==========

[brettw, 2016-11-14 21:08:47]

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

[brettw, 2016-11-14 21:09:04]

+TBR mkwst for messages.h file.

[brettw, 2016-11-14 21:09:39]

The CQ bit was checked by brettw@chromium.org

[commit-bot: I haz the power, 2016-11-14 21:12:40]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

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

Description was changed from

==========
Convert spellcheck to a buildflag header.

Move the build variables from build/config/features.gni to a new header in
components/spellcheck

Removes the global defines for ENABLE_SPELLCHECK and USE_BROWSER_SPELLCHECKER
and move them to a generated header.

R=rouslan
TBR=mkwst@chromium.org (_messages.h file)
==========

to

==========
Convert spellcheck to a buildflag header.

Move the build variables from build/config/features.gni to a new header in
components/spellcheck

Removes the global defines for ENABLE_SPELLCHECK and USE_BROWSER_SPELLCHECKER
and move them to a generated header.

R=rouslan
TBR=mkwst@chromium.org (_messages.h file)
==========

[commit-bot: I haz the power, 2016-11-14 23:23:55]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-11-14 23:33:38]

Description was changed from

==========
Convert spellcheck to a buildflag header.

Move the build variables from build/config/features.gni to a new header in
components/spellcheck

Removes the global defines for ENABLE_SPELLCHECK and USE_BROWSER_SPELLCHECKER
and move them to a generated header.

R=rouslan
TBR=mkwst@chromium.org (_messages.h file)
==========

to

==========
Convert spellcheck to a buildflag header.

Move the build variables from build/config/features.gni to a new header in
components/spellcheck

Removes the global defines for ENABLE_SPELLCHECK and USE_BROWSER_SPELLCHECKER
and move them to a generated header.

R=rouslan
TBR=mkwst@chromium.org (_messages.h file)

Committed: https://crrev.com/41446f6842fd10bb4671afb4ecd700ab6db20158
Cr-Commit-Position: refs/heads/master@{#431962}
==========

[commit-bot: I haz the power, 2016-11-14 23:33:43]

Patchset 4 (id:??) landed as
https://crrev.com/41446f6842fd10bb4671afb4ecd700ab6db20158
Cr-Commit-Position: refs/heads/master@{#431962}