Reinforce that ENABLE(OPENTYPE_VERTICAL) is a feature define

The reason we do not support OPENTYPE_VERTICAL on Mac yet is because no one has implemented it, not because it cannot be supported on that platform. Add this rationale to features.gypi to note that the guard should stay an ENABLE() guard.

BUG=286578
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=164443

[lgombos, 2013-12-14 19:40:13]

These changes might help with the gn transition as well..

[eseidel, 2013-12-14 20:58:23]

I can't say I have a particularly strong opinion as to the naming here.

[eseidel, 2013-12-14 20:58:42]

lgtm

[eae, 2013-12-14 23:01:08]

What is the rationale for this change? I though we where trying to move in te
opposite direction (use features.gypi instead of config.h) going forward?

Also, please explain why you are making a change in addition to what you are
changing in the change description.

[lgombos, 2013-12-15 03:51:40]

On 2013/12/14 23:01:08, eae wrote:

> What is the rationale for this change? I though we where trying to move in te
> opposite direction (use features.gypi instead of config.h) going forward?

There is a more context to this work in the referenced BUG (and some similar
commits).

Adam convinced me in an earlier discussion that we should have a different
policy for USE() and ENABLE() macros - see
https://codereview.chromium.org/22577011/#msg4 
<<Adam>> WTF_USE_* should be set in h files.  ENABLE_* should be set in
features.gypi.

My main motivation is to be consistent with the rest of the code base. Like you
hinted there are several places where these macros can be set and I'd like to
establish some consistency with this work.

> Also, please explain why you are making a change in addition to what you are
> changing in the change description.

Ok, I see a value of repeating some of the motivation from the BUG in the
ChnageLog. I'll add that the main driver is consistency. 

I'll wait a few more days for more potential feedback before landing.

[abarth-chromium, 2013-12-15 06:10:19]

On 2013/12/14 23:01:08, eae wrote:
> What is the rationale for this change? I though we where trying to move in te
> opposite direction (use features.gypi instead of config.h) going forward?

Oh, I thought the idea was to use features.gypi to control which features are
enabled.  Most of the other compile-time switches for what libraries we're using
to implement those features are handled in header files (because they're derived
from the OS defines rather than from build-time configuration).

[eae, 2013-12-15 21:56:03]

On 2013/12/15 06:10:19, abarth wrote:
> On 2013/12/14 23:01:08, eae wrote:
> > What is the rationale for this change? I though we where trying to move in
te
> > opposite direction (use features.gypi instead of config.h) going forward?
> 
> Oh, I thought the idea was to use features.gypi to control which features are
> enabled.  Most of the other compile-time switches for what libraries we're
using
> to implement those features are handled in header files (because they're
derived
> from the OS defines rather than from build-time configuration).

While I understand and support that distinction I feel it isn't applicable here
as OPENTYPE_VERTICAL is a feature and not a platform construct.

As far as I understand the only reason we do not support OPENTYPE_VERTICAL on
Mac yet is because no one has implemented it, not because it cannot be supported
on that platform. Using config.h instead of features.gypi also means we cannot
exclude the files themselves (opentype/OpenTypeVerticalData.cpp) and instead
have to wrap the entire file in an ifdef macro. 
Finally, it makes it much harder to actually do implement support for the
feature on Mac as config.h doens't allow for experimental features. I imagine
that we will want to support this on mac when we fully switch to skia and
harfbuzz on mac.

[abarth-chromium, 2013-12-15 22:23:38]

Ok, that makes sense.  I wonder if we should add a comment to features.gypi with
some information on this topic so future folks reading the file will know why
it's there.

[eae, 2013-12-15 22:31:34]

On 2013/12/15 22:23:38, abarth wrote:
> Ok, that makes sense.  I wonder if we should add a comment to features.gypi
with
> some information on this topic so future folks reading the file will know why
> it's there.

Sounds like a good idea. I can't be the only one confused about it :)

[lgombos, 2014-01-03 02:31:07]

Trivial change.

[commit-bot: I haz the power, 2014-01-03 02:53:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/l.gombos@samsung.com/112913003/20001

[commit-bot: I haz the power, 2014-01-03 04:05:53]

Change committed as 164443