Updating OTS repo from https://github.com/khaledhosny/ots.git

The original CL is https://codereview.chromium.org/658573004

Updating with to new OTS repo from https://github.com/khaledhosny/ots.git
Hash tag imported: c24a839b1c66c4de09e58fabaacb82bf3bd692a4

Some important differences between new and old version of OTS:
1. Support for MATH table
2. Implementation changes related to new WOFF 2.0
3. Bug fixes related to OTF tables (GDEF, GSUB etc) parsing
4. Accept platform 0 encoding 1 cmap subtables
5. Allow fonts with CMAP format-12 or format-13 table

A dummy version of EnableWOFF() is added to get Blink compiled.
After the following CL is landed in Blink, it'll be removed.
Blink CL: https://codereview.chromium.org/774253008

TEST: Blink layout tests: fast/css/font-face*
BUG=339857
Committed: https://crrev.com/3b6cd2d4bf9e0b4a9311343a2997ef9363530b14
Cr-Commit-Position: refs/heads/master@{#308054}

Committed: https://crrev.com/502c6e98e76359b76197be6a8683f907076fecf9
Cr-Commit-Position: refs/heads/master@{#317189}

[h.joshi, 2014-12-03 06:16:14]

h.joshi@samsung.com changed reviewers:
+ bashi@chromium.org, behdad@chromium.org, jshin@chromium.org

[h.joshi, 2014-12-03 06:16:30]

Making changes to DEPS file as suggested.

[bashi, 2014-12-03 06:43:13]

On 2014/12/03 06:16:30, h.joshi wrote:
> Making changes to DEPS file as suggested.

Thank you for working on this. I think behdad@ and jshin@ are working with our
security team on this move so please wait for it.

[h.joshi, 2014-12-03 07:15:09]

@bashi1: Thank you for quick comment.
Pls also suggest for doubt in "https://codereview.chromium.org/658573004/"

[jungshik at Google, 2014-12-11 19:11:07]

Also for the record, add the following to the CL description:

The original CL is https://codereview.chromium.org/658573004

And, take care of the following nit. 

With that, LGTM

https://codereview.chromium.org/775893002/diff/60001/third_party/ots/ots.gyp
File third_party/ots/ots.gyp (right):

https://codereview.chromium.org/775893002/diff/60001/third_party/ots/ots.gyp#...
third_party/ots/ots.gyp:37: ],
You didn't address my comment about 4800.  4800 is not covered by
http://crbug.com/167187. 
Please, add a separate comment about 4800 referring to the github bug you filed.


How about this? 

# TODO(jschuh): 4267, 4334 are 64-bit cleanness issue.  http://crbug.com/167187
# TODO(h.joshi) : Make 4800 go away. <url to git hub bug>

[h.joshi, 2014-12-12 04:11:23]

Made suggested changes to patch description.

https://codereview.chromium.org/775893002/diff/60001/third_party/ots/ots.gyp
File third_party/ots/ots.gyp (right):

https://codereview.chromium.org/775893002/diff/60001/third_party/ots/ots.gyp#...
third_party/ots/ots.gyp:37: ],
I did replied to the same in comment
(https://codereview.chromium.org/658573004/#msg40)

Will update and add suggested description with link to
github bug in patch description.

[h.joshi, 2014-12-12 04:11:32]

The CQ bit was checked by h.joshi@samsung.com

[commit-bot: I haz the power, 2014-12-12 04:12:03]

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

[commit-bot: I haz the power, 2014-12-12 05:12:44]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2014-12-12 05:13:35]

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

[jungshik at Google, 2014-12-12 08:01:46]

You've already committed without understanding what I meant in my previous
comment. 

It's not such a big deal, but it'd be nice if you pay more attention to what I'm
talking about next time. See below.

https://codereview.chromium.org/775893002/diff/60001/third_party/ots/ots.gyp
File third_party/ots/ots.gyp (right):

https://codereview.chromium.org/775893002/diff/60001/third_party/ots/ots.gyp#...
third_party/ots/ots.gyp:37: ],
On 2014/12/12 04:11:22, h.joshi wrote:
> I did replied to the same in comment
> (https://codereview.chromium.org/658573004/#msg40)
> 
> Will update and add suggested description with link to
> github bug in patch description.

I did NOT mean adding what I suggested to the CL description. I wanted you to
update the comment in THIS file because "TODO(jschuh): http://crbug.com/167187"
is NOT applicable to 4800.

[jungshik at Google, 2014-12-12 08:05:08]

https://github.com/himanshuispresent/ots/commit/02f724f758ddb1cb264cc0259ffd5...
is NOT a solution to 4800 warning. It's just suppressing the warning while the
underlying code still remains unfixed.  

You have to change the code in OTS  so that MSVC does not issue 4800 warnings
any more when 4800 is NOT listed in the warning suppression list.

[Noel Gordon, 2014-12-12 08:11:28]

Also, per https://codereview.chromium.org/797923002 various gn bots in blink are
failing to compile.  Can we revert please, and allow commits to continue?

[jungshik at Google, 2014-12-12 08:23:04]

Anyway, thank you for getting this done !

[jungshik at Google, 2014-12-12 08:33:20]

On 2014/12/12 08:11:28, noel gordon wrote:
> Also, per https://codereview.chromium.org/797923002 various gn bots in blink
are
> failing to compile.  Can we revert please, and allow commits to continue?

hmm... I don't think
http://build.chromium.org/p/tryserver.blink/builders/android_chromium_gn_comp...
has anything to do with this CL. See https://codereview.chromium.org/774253008/
The same bot (android_chromium_gn_compile_rel) is green.  

http://build.chromium.org/p/tryserver.blink/builders/linux_chromium_gn_rel/bu...
is related to this CL (gn failure as opposed to a compiler failure), but I don't
understand why (this bot apparenlty could not find third_party/ots for some
reason). linux_chromium_gn_rel passed before this CL was committed. 

Can you try checking CQ box once more? It may have been a transient failure due
to a DEPS change in this CL.

[bashi, 2014-12-12 08:36:12]

I'm sorry, but I'm going to revert this CL, as many bots are failing. For
example,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_compile...

[bashi, 2014-12-12 08:37:03]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/797183002/ by bashi@chromium.org.

The reason for reverting is: Broke bots.

[jungshik at Google, 2014-12-12 08:46:40]

On 2014/12/12 08:37:03, bashi1 wrote:
> A revert of this CL (patchset #4 id:60001) has been created in
> https://codereview.chromium.org/797183002/ by
https://mail.google.com/mail/?view=cm&fs=1&tf=1&to=bashi@chromium.org.
> 
> The reason for reverting is: Broke bots.

It may make things worse because apparently 'gclient runhooks' on some bots (not
all bots) failed once before getting back to normal due to DEPS switch. 

For instance, see 
http://build.chromium.org/p/chromium.mac/waterfall?builder=Mac10.9%20Tests. It
failed once (gclient runhooks), but the next time, it succeeded.

[bashi, 2014-12-12 08:52:12]

On 2014/12/12 08:46:40, Jungshik Shin wrote:
> On 2014/12/12 08:37:03, bashi1 wrote:
> > A revert of this CL (patchset #4 id:60001) has been created in
> > https://codereview.chromium.org/797183002/ by
> https://mail.google.com/mail/?view=cm&fs=1&tf=1&to=bashi@chromium.org.
> > 
> > The reason for reverting is: Broke bots.
> 
> It may make things worse because apparently 'gclient runhooks' on some bots
(not
> all bots) failed once before getting back to normal due to DEPS switch. 
> 
> For instance, see 
> http://build.chromium.org/p/chromium.mac/waterfall?builder=Mac10.9%20Tests. It
> failed once (gclient runhooks), but the next time, it succeeded.

Thanks for the info. Hmm, but android_compile_rel is consistently failing..
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_compile...

What can we do to get things back normal?

[h.joshi, 2014-12-12 09:04:48]

@Jungshik Shin:
1. I did NOT mean adding what I suggested to the CL description
>> Sorry I understood it wrongly, should I make new CL and correct the same.

2. You have to change the code in OTS  so that MSVC does not issue 4800 warnings
>> Yes plan is the same and I am working on that, till my changes for code are
ready
did not want OTS code to throw warning messages if someone pulls it. So add 4800
in ignore warnings list.

One more question, @bashi1 has submitted revert of this CL
(https://codereview.chromium.org/797183002/)

Please suggest next step, bots gets to green after some time. I think some time
is taken after
commit and actually updating code on real server. Maybe that is the reason bots
fails first time.

[Noel Gordon, 2014-12-12 09:11:48]

Reverted https://codereview.chromium.org/775893002  Please respond tree redness
promptly.

[Noel Gordon, 2014-12-12 09:12:46]

And thanks for doing so bashi@.

[bashi, 2014-12-12 09:13:18]

On 2014/12/12 09:04:48, h.joshi wrote:
> @Jungshik Shin:
> 1. I did NOT mean adding what I suggested to the CL description
> >> Sorry I understood it wrongly, should I make new CL and correct the same.
> 
> 2. You have to change the code in OTS  so that MSVC does not issue 4800
warnings
> >> Yes plan is the same and I am working on that, till my changes for code are
> ready
> did not want OTS code to throw warning messages if someone pulls it. So add
4800
> in ignore warnings list.
> 
> One more question, @bashi1 has submitted revert of this CL
> (https://codereview.chromium.org/797183002/)
> 
> Please suggest next step, bots gets to green after some time. I think some
time
> is taken after
> commit and actually updating code on real server. Maybe that is the reason
bots
> fails first time.

I may have been in rush, but please note that some bots kept failing.
(android_compile_rel). I'm not sure what the cause was, though...

[jungshik at Google, 2014-12-12 09:22:29]

On 2014/12/12 09:04:48, h.joshi wrote:
> @Jungshik Shin:
> 1. I did NOT mean adding what I suggested to the CL description
> >> Sorry I understood it wrongly, should I make new CL and correct the same.

All right. You can still use this CL. 

git checkout origin/master
git pull origin master   # your CL is already reverted 
git branch <new branch name> # different from your existing branch for this CL
git cl issue 775893002  # will get this CL in a new branch
# 1. address my nit 2. Modify build/whitespace_file.txt (read the comment in the
file to see why you need to change the file)
git cl upload 



> 2. You have to change the code in OTS  so that MSVC does not issue 4800
warnings
> >> Yes plan is the same and I am working on that, till my changes for code are
> ready
> did not want OTS code to throw warning messages if someone pulls it. So add
4800
> in ignore warnings list.

Ok. I know why 4800 was added to the suppression list.  

Do not include the following lines in your comment to ots.gyp. It's unnecessary.
Just referring to a github bug is more than enough. 

> Currently patch to github
(https://github.com/himanshuispresent/ots/commit/02f724f758ddb1cb264cc0259ffd5...)
is
> submitted so that taking OTS repository to Chromium should not cause issue on
Windows bots.


 
> One more question, @bashi1 has submitted revert of this CL
> (https://codereview.chromium.org/797183002/)
> 
> Please suggest next step, bots gets to green after some time. I think some
time
> is taken after
> commit and actually updating code on real server. Maybe that is the reason
bots
> fails first time.

There are two issues: 1) 'gclient runhooks' fails on some bots in the first try
before succeeding next time 2) gn builds (even though they pass in trybots for
this CL) fail because it can't find third_party/ots.  

We may have to ask infrastructure folks to look into the issue.

[Noel Gordon, 2014-12-12 09:56:32]

http://www.chromium.org/developers/how-tos/get-the-code/working-with-nested-r...


"Adding new repositories to DEPS" may be relevant to your needs, and does
require that the you talk to infrastructure folks.

Tree is green again, adios.

[h.joshi, 2014-12-12 13:07:23]

@Jungshik Shin: I have done suggested changes, nit changes and change to
"whitespace_file.txt"
also.

[h.joshi, 2014-12-12 15:01:54]

@Jungshik Shin: Should I retry landing this patch, hoping this will not 
break bots this time.
Pls suggest

[jungshik at Google, 2014-12-12 17:46:09]

On 2014/12/12 15:01:54, h.joshi wrote:
> @Jungshik Shin: Should I retry landing this patch, hoping this will not 
> break bots this time.
> Pls suggest

Thanks a lot. Let me talk to chrome-infra folks before trying again.

[jungshik at Google, 2014-12-12 23:03:32]

The CQ bit was checked by jshin@chromium.org

[jungshik at Google, 2014-12-12 23:03:46]

The CQ bit was unchecked by jshin@chromium.org

[jungshik at Google, 2014-12-12 23:03:46]

The CQ bit was checked by jshin@chromium.org

[jungshik at Google, 2014-12-12 23:04:38]

On 2014/12/12 17:46:09, Jungshik Shin wrote:
> On 2014/12/12 15:01:54, h.joshi wrote:
> > @Jungshik Shin: Should I retry landing this patch, hoping this will not 
> > break bots this time.
> > Pls suggest
> 
> Thanks a lot. Let me talk to chrome-infra folks before trying again.

Let me try once more while writing an email to chromium-dev list. With
whitetext-file.txt change included in the CL, we may have a better luck this
time.

[commit-bot: I haz the power, 2014-12-12 23:04:56]

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

[commit-bot: I haz the power, 2014-12-13 00:21:56]

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

[commit-bot: I haz the power, 2014-12-13 00:21:57]

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

[jungshik at Google, 2014-12-13 00:29:01]

On 2014/12/12 09:56:32, noel gordon wrote:
>
http://www.chromium.org/developers/how-tos/get-the-code/working-with-nested-r...
> 
> 
> "Adding new repositories to DEPS" may be relevant to your needs, and does
> require that the you talk to infrastructure folks.

No, this is the reverse. third_party/OTS used to be pulled in via DEPS. Now, we
just added a copy to the main chromium tree. 


> 
> Tree is green again, adios.

[Noel Gordon, 2014-12-13 04:44:08]

On 2014/12/13 00:29:01, Jungshik Shin wrote:
> On 2014/12/12 09:56:32, noel gordon wrote:
> >
>
http://www.chromium.org/developers/how-tos/get-the-code/working-with-nested-r...
> >

> No, this is the reverse. third_party/OTS used to be pulled in via DEPS. Now,
we
> just added a copy to the main chromium tree.

I did say "may" be relevant to you needs :_ I didn't examine exactly how you
were pulling the dependency in.

Point was that you consider talking to infrastructure too if you need help &
advice.

[h.joshi, 2014-12-13 04:44:19]

Getting following error:
** Presubmit ERRORS **
Missing LGTM from an OWNER for these files:
    build/whitespace_file.txt

[h.joshi, 2014-12-13 04:50:12]

h.joshi@samsung.com changed reviewers:
+ jam@chromium.org

[h.joshi, 2014-12-13 04:51:15]

@John Abd-El-Malek: Pls review getting following error:
   ** Presubmit ERRORS **
      Missing LGTM from an OWNER for these files:
      build/whitespace_file.txt

[jungshik at Google, 2014-12-13 06:10:07]

On 2014/12/13 04:51:15, h.joshi wrote:
> @John Abd-El-Malek: Pls review getting following error:
>    ** Presubmit ERRORS **
>       Missing LGTM from an OWNER for these files:
>       build/whitespace_file.txt

Actually, whitespace_file.txt should NOT be included in this CL. I'm sorry that
I was mistaken about the file. 

The file has to be changed in a separate CL right after this CL is landed. The
2nd CL (touching only whitespace_file.txt) should trigger buildbots to run once
more. 

1. This CL is landed.
2. All the bots will fail to finish 'gclient runhooks' because DEPS does not
have third_party/ots any more and 'gclient runhooks' believe that 
third_party/ots is not necessary any more and automatically delete it. 
3. The 2nd CL is landed (whitespace_file.txt) to trigger a new bot cycle
4. 'gclient runhooks' will succeed because it does NOT detect any more DEPS
change and does not touch third_party/ots. 

This works for regular bots. The problem is how to deal with this issue for
trybots run by CQ or manually by developers.

[jungshik at Google, 2014-12-13 06:32:12]

On 2014/12/13 04:44:08, noel gordon wrote:
> On 2014/12/13 00:29:01, Jungshik Shin wrote:
> > On 2014/12/12 09:56:32, noel gordon wrote:
> > >
> >
>
http://www.chromium.org/developers/how-tos/get-the-code/working-with-nested-r...
> > >
> 
> > No, this is the reverse. third_party/OTS used to be pulled in via DEPS. Now,
> we
> > just added a copy to the main chromium tree.
> 
> I did say "may" be relevant to you needs :_ I didn't examine exactly how you
> were pulling the dependency in.

How am I supposed to reply to your "maybe"?   I just explained why 'your maybe'
is  not the case. 

 
> Point was that you consider talking to infrastructure too if you need help &
> advice.

See
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/2lfT0wv_7RE 
and you'll see what was going on.

[jungshik at Google, 2014-12-13 07:20:31]

On 2014/12/13 06:32:12, Jungshik Shin wrote:

> See
>
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/2lfT0wv_7RE 
> and you'll see what was going on.

As I wrote in the thread, there's a CL that would take care of this issue
(moving from DEPS to the main repo), but it's reverted. We'd better wait for
that CL to get landed first. 
The CL is at https://codereview.chromium.org/743083002 and the bug is
https://code.google.com/p/chromium/issues/detail?id=431469

[h.joshi, 2014-12-13 09:29:34]

@Jungshik Shin: Thank you for explaining me guts of try bots and
related process.
 As suggested will wait for the "743083002" to land and then process
with this patch. Mean while as suggested will remove "whitespace_file.txt"
from this CL and make new CL for that.

[jam, 2014-12-15 19:18:06]

can someone give me a short explanation for why we are forking a project made by
googlers to another github made by googler? why aren't these improvements going
to the original repo?

[behdad_google, 2014-12-15 19:45:04]

On 2014/12/15 19:18:06, jam wrote:
> can someone give me a short explanation for why we are forking a project made
by
> googlers to another github made by googler? why aren't these improvements
going
> to the original repo?

Original googlers who wrote it don't have time or interest to develop it
further.  I found someone externally who actually knows OpenType, to work on it,
because I didn't personally have time myself to do.  Mozilla is already using
it.  And my recommendation to Chrome is to use it.

It's also a move from Google Code to github, which is nice.

[jam, 2014-12-15 20:10:40]

On 2014/12/15 19:45:04, behdad_google wrote:
> On 2014/12/15 19:18:06, jam wrote:
> > can someone give me a short explanation for why we are forking a project
made
> by
> > googlers to another github made by googler? why aren't these improvements
> going
> > to the original repo?
> 
> Original googlers who wrote it don't have time or interest to develop it
> further.  I found someone externally who actually knows OpenType, to work on
it,
> because I didn't personally have time myself to do.  Mozilla is already using
> it.  And my recommendation to Chrome is to use it.
> 
> It's also a move from Google Code to github, which is nice.

ok. lgtm

[jungshik at Google, 2014-12-15 22:53:21]

Can you update a new upstream version with 4800 warning fixed?

https://codereview.chromium.org/775893002/diff/100001/third_party/ots/BUILD.gn
File third_party/ots/BUILD.gn (right):

https://codereview.chromium.org/775893002/diff/100001/third_party/ots/BUILD.g...
third_party/ots/BUILD.gn:88: "/wd4800",  # 'type' : forcing value to bool 'true'
or 'false'.
The upstream fixed 4800 warning issue. 

Update the CL to get the latest upstream with the fix and drop 4800 from
BUILD.gn and ots.gyp ? 

In ots.gyp, you can drop your TODO and change TODO(jschuh) back to what it was
before.

https://codereview.chromium.org/775893002/diff/100001/third_party/ots/ots.gyp
File third_party/ots/ots.gyp (right):

https://codereview.chromium.org/775893002/diff/100001/third_party/ots/ots.gyp...
third_party/ots/ots.gyp:34: 'msvs_disabled_warnings': [
The upstream fixed 4800 issue yesterday. Can you get the version with the 4800
fix, instead? Then, you can remove your TODO while keeping TODO(jschuh) as
before.

[h.joshi, 2014-12-16 06:12:56]

Made suggested changes.

https://codereview.chromium.org/775893002/diff/100001/third_party/ots/BUILD.gn
File third_party/ots/BUILD.gn (right):

https://codereview.chromium.org/775893002/diff/100001/third_party/ots/BUILD.g...
third_party/ots/BUILD.gn:88: "/wd4800",  # 'type' : forcing value to bool 'true'
or 'false'.
On 2014/12/15 22:53:20, Jungshik Shin wrote:
> The upstream fixed 4800 warning issue. 
> 
> Update the CL to get the latest upstream with the fix and drop 4800 from
> BUILD.gn and ots.gyp ? 
> 
> In ots.gyp, you can drop your TODO and change TODO(jschuh) back to what it was
> before. 

Done.

https://codereview.chromium.org/775893002/diff/100001/third_party/ots/ots.gyp
File third_party/ots/ots.gyp (right):

https://codereview.chromium.org/775893002/diff/100001/third_party/ots/ots.gyp...
third_party/ots/ots.gyp:34: 'msvs_disabled_warnings': [
On 2014/12/15 22:53:21, Jungshik Shin wrote:
> The upstream fixed 4800 issue yesterday. Can you get the version with the 4800
> fix, instead? Then, you can remove your TODO while keeping TODO(jschuh) as
> before. 
> 

Done.

[jungshik at Google, 2014-12-31 00:01:55]

On 2014/12/16 06:12:56, h.joshi wrote:
> Made suggested changes.
> 
> https://codereview.chromium.org/775893002/diff/100001/third_party/ots/BUILD.gn
> File third_party/ots/BUILD.gn (right):
> 
>
https://codereview.chromium.org/775893002/diff/100001/third_party/ots/BUILD.g...
> third_party/ots/BUILD.gn:88: "/wd4800",  # 'type' : forcing value to bool
'true'
> or 'false'.
> On 2014/12/15 22:53:20, Jungshik Shin wrote:
> > The upstream fixed 4800 warning issue. 
> > 
> > Update the CL to get the latest upstream with the fix and drop 4800 from
> > BUILD.gn and ots.gyp ? 
> > 
> > In ots.gyp, you can drop your TODO and change TODO(jschuh) back to what it
was
> > before. 
> 
> Done.
> 
> https://codereview.chromium.org/775893002/diff/100001/third_party/ots/ots.gyp
> File third_party/ots/ots.gyp (right):
> 
>
https://codereview.chromium.org/775893002/diff/100001/third_party/ots/ots.gyp...
> third_party/ots/ots.gyp:34: 'msvs_disabled_warnings': [
> On 2014/12/15 22:53:21, Jungshik Shin wrote:
> > The upstream fixed 4800 issue yesterday. Can you get the version with the
4800
> > fix, instead? Then, you can remove your TODO while keeping TODO(jschuh) as
> > before. 
> > 
> 
> Done.

Thanks. LGTM, but we have to wait for a blocking bug to be fixed before landing
this CL.

[jungshik at Google, 2015-02-19 23:15:38]

The CQ bit was checked by jshin@chromium.org

[jungshik at Google, 2015-02-19 23:16:18]

On 2014/12/31 00:01:55, Jungshik at google wrote:

> Thanks. LGTM, but we have to wait for a blocking bug to be fixed before
landing
> this CL.

Looks like the CL to fix the blocker is staying in. Let's land this one.

[commit-bot: I haz the power, 2015-02-19 23:16:41]

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

[commit-bot: I haz the power, 2015-02-20 00:29:56]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-02-20 00:30:50]

Patchset 7 (id:??) landed as
https://crrev.com/502c6e98e76359b76197be6a8683f907076fecf9
Cr-Commit-Position: refs/heads/master@{#317189}

[h.joshi, 2015-02-20 04:03:05]

@Jungshik: Thank you