Make OTS passthru GDEF/GSUB/GPOS tables

Make OTS passthru GDEF/GSUB/GPOS tables

This patch changes OTS to passthru GDEF/GSUB/GPOS because recnet
versions of HarfBuzz knows how to deal with broken tables.

BUG=526101
Committed: https://crrev.com/a5519cc580799753151272d89f23b7ed29b8f0d9
git-svn-id: svn://svn.chromium.org/blink/trunk@202033 bbb929c8-8fbe-4397-9dbb-9b2b20218538

[kojii, 2015-09-03 05:31:05]

kojii@chromium.org changed reviewers:
+ bashi@chromium.org, behdad@chromium.org, drott@chromium.org, eae@chromium.org,
jshin@chromium.org, ksakamoto@chromium.org

[kojii, 2015-09-03 05:31:05]

PTAL.

Following the discussion in crbug.com/526101, I'm open to other methods.
Comments appreciated.

I'll not land until all seem to be happy (or no response for a while will be
interpreted as so ;)

[bashi, 2015-09-03 23:00:47]

Just want to express my no objection.

[eae, 2015-09-03 23:35:29]

Seems resonable.

https://codereview.chromium.org/1306343006/diff/1/Source/platform/fonts/opent...
File Source/platform/fonts/opentype/OpenTypeSanitizer.cpp (right):

https://codereview.chromium.org/1306343006/diff/1/Source/platform/fonts/opent...
Source/platform/fonts/opentype/OpenTypeSanitizer.cpp:135: #define
HB_VERSION_ATLEAST(major, minor, micro) 0
Why is this needed?

[jungshik at Google, 2015-09-04 00:09:37]

On 2015/09/03 23:35:29, eae wrote:
> Seems resonable.
> 
>
https://codereview.chromium.org/1306343006/diff/1/Source/platform/fonts/opent...
> File Source/platform/fonts/opentype/OpenTypeSanitizer.cpp (right):
> 
>
https://codereview.chromium.org/1306343006/diff/1/Source/platform/fonts/opent...
> Source/platform/fonts/opentype/OpenTypeSanitizer.cpp:135: #define
> HB_VERSION_ATLEAST(major, minor, micro) 0
> Why is this needed?

Perhaps, that macro is not available in old versions of hb?  

With that clarified, lgtm

[eae, 2015-09-04 00:26:50]

https://codereview.chromium.org/1306343006/diff/1/Source/platform/fonts/opent...
File Source/platform/fonts/opentype/OpenTypeSanitizer.cpp (right):

https://codereview.chromium.org/1306343006/diff/1/Source/platform/fonts/opent...
Source/platform/fonts/opentype/OpenTypeSanitizer.cpp:135: #define
HB_VERSION_ATLEAST(major, minor, micro) 0
On 2015/09/03 23:35:29, eae wrote:
> Why is this needed?

If so I'd prefer the check at line 157 to look something like

#if defined(HB_VERSION_ATLEAST) && HB_VERSION_ATLEAST(1, 0, 0)

[kojii, 2015-09-04 01:10:37]

On 2015/09/04 00:26:50, eae wrote:
>
https://codereview.chromium.org/1306343006/diff/1/Source/platform/fonts/opent...
> File Source/platform/fonts/opentype/OpenTypeSanitizer.cpp (right):
> 
>
https://codereview.chromium.org/1306343006/diff/1/Source/platform/fonts/opent...
> Source/platform/fonts/opentype/OpenTypeSanitizer.cpp:135: #define
> HB_VERSION_ATLEAST(major, minor, micro) 0
> On 2015/09/03 23:35:29, eae wrote:
> > Why is this needed?
> 
> If so I'd prefer the check at line 157 to look something like
> 
> #if defined(HB_VERSION_ATLEAST) && HB_VERSION_ATLEAST(1, 0, 0)

Yeah, 0.9.27, the version of Trusty, does not have this macro.

And interestingly, C++ preprocessor fails to parse if written in one line,
results in "undefined" error, so ended up in this a bit ugly way...

[eae, 2015-09-04 16:42:07]

ok, LGTM

[Kunihiko Sakamoto, 2015-09-07 00:28:21]

lgtm

[kojii, 2015-09-08 06:06:02]

Looks like all green, I'll be landing this soon. Please speak up if any.

[kojii, 2015-09-10 03:27:14]

The CQ bit was checked by kojii@chromium.org

[commit-bot: I haz the power, 2015-09-10 03:27:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1306343006/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1306343006/1

[commit-bot: I haz the power, 2015-09-10 04:48:06]

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

[commit-bot: I haz the power, 2015-09-10 04:48:06]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[kojii, 2015-09-10 05:27:55]

The CQ bit was checked by kojii@chromium.org

[commit-bot: I haz the power, 2015-09-10 05:28:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1306343006/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1306343006/1

[commit-bot: I haz the power, 2015-09-10 06:44:01]

Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=202033

[achuithb, 2015-09-10 09:11:28]

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

[achuithb, 2015-09-10 09:11:28]

https://codereview.chromium.org/1306343006/diff/1/Source/platform/fonts/opent...
File Source/platform/fonts/opentype/OpenTypeSanitizer.cpp (right):

https://codereview.chromium.org/1306343006/diff/1/Source/platform/fonts/opent...
Source/platform/fonts/opentype/OpenTypeSanitizer.cpp:146: const uint32_t gdefTag
= TABLE_TAG('G', 'D', 'E', 'F');
These need to be surrounded by
#if HB_VERSION_ATLEAST(1, 0, 0)
#endif

I'm getting an unused variable compilation error (-Wunused-variable)

[kojii, 2015-09-10 12:54:19]

achuithb@, sorry for the trouble. pneubeck@ has landed the fix so should be ok
now. I should have remembered we don't have try-bots for that configuration and
should have tried by myself.

[achuithb, 2015-09-10 12:59:55]

On 2015/09/10 12:54:19, kojii wrote:
> achuithb@, sorry for the trouble. pneubeck@ has landed the fix so should be ok
> now. I should have remembered we don't have try-bots for that configuration
and
> should have tried by myself.

No worries, thanks!

[commit-bot: I haz the power, 2015-09-23 12:08:11]

Patchset 1 (id:??) landed as
https://crrev.com/a5519cc580799753151272d89f23b7ed29b8f0d9