whitelist fallback typefaces

Suppress embedding fonts when the skp's fonts match the OS fonts.

The million SKPs generated require >5T of storage. A good deal
of that are copies of system fonts.

Chrome built with
#DEFINE SK_WHITELIST_SERIALIZED_TYPEFACES
will omit the font data if the font matches a precomputed
checksum.

The captured SKP prepends sk_ to the names of fonts that
have their data omitted. The SKP consumer can either add
renamed fonts from the recording machine, or add

gDeserializeTypefaceDelegate = WhitelistDeserializeTypeface;

which strips the sk_ prefix when deserializing typefaces.

whitelist_typefaces --check
Computes the checksums of fallback
fonts and returns 0 if the checksums match the checked-in
file SkWhitelistChecksum.cpp.

whitelist_typefaces --generate
Writes an updated version of SkWhitelistChecksum.cpp.

(Added Mike since this modifies a public header)

R=bungeman@google.com,rmistry@google.com,reed@google.com

Committed: https://skia.googlesource.com/skia/+/5ef194c31ae498166bd9c468993514c5267ea077

[_cary, 2015-08-28 20:20:25]

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

[commit-bot: I haz the power, 2015-08-28 20:20:34]

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

[commit-bot: I haz the power, 2015-08-28 20:20:35]

Note for Reviewers:
The CQ is waiting for an approval.
If you believe that the CL is not ready yet, or if you would like to L-G-T-M
with comments then please uncheck the CQ checkbox.

Waiting for LGTM from valid reviewer(s) till 2015-08-29 02:20 UTC

[commit-bot: I haz the power, 2015-08-28 20:23:15]

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

[commit-bot: I haz the power, 2015-08-28 20:23:15]

Dry run: Try jobs failed on following builders:
  Build-Win-MSVC-x86_64-Debug-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86_6...)

[_cary, 2015-08-28 20:26:34]

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

[commit-bot: I haz the power, 2015-08-28 20:26:45]

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

[bungeman-skia, 2015-08-28 21:09:57]

https://codereview.chromium.org/1317913005/diff/20001/src/core/SkString.cpp
File src/core/SkString.cpp (right):

https://codereview.chromium.org/1317913005/diff/20001/src/core/SkString.cpp#n...
src/core/SkString.cpp:380: SkString tmp(text ? text : this->c_str(), len);
What is this for? If text is NULL here, shouldn't the len always be 0 or this
should fault?

[commit-bot: I haz the power, 2015-08-29 02:19:59]

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

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

Dry run: No LGTM from a valid reviewer yet. Please ask for an LGTM from a
full Skia committer from
https://skia.googlesource.com/skia/+/master/CQ_COMMITTERS

[reed1, 2015-08-31 12:20:24]

reed@google.com changed reviewers:
+ reed@google.com

[reed1, 2015-08-31 12:20:25]

reed@google.com changed required reviewers:
+ bungeman@google.com

[reed1, 2015-08-31 12:20:25]

lgtm

https://codereview.chromium.org/1317913005/diff/20001/src/core/SkString.cpp
File src/core/SkString.cpp (right):

https://codereview.chromium.org/1317913005/diff/20001/src/core/SkString.cpp#n...
src/core/SkString.cpp:380: SkString tmp(text ? text : this->c_str(), len);
On 2015/08/28 21:09:57, bungeman1 wrote:
> What is this for? If text is NULL here, shouldn't the len always be 0 or this
> should fault?

+1 -- seems like its an error (assert?) to pass NULL w/ non-zero length

[caryclark, 2015-08-31 13:11:08]

On 2015/08/31 12:20:25, reed1 wrote:
> lgtm
> 
> https://codereview.chromium.org/1317913005/diff/20001/src/core/SkString.cpp
> File src/core/SkString.cpp (right):
> 
>
https://codereview.chromium.org/1317913005/diff/20001/src/core/SkString.cpp#n...
> src/core/SkString.cpp:380: SkString tmp(text ? text : this->c_str(), len);
> On 2015/08/28 21:09:57, bungeman1 wrote:
> > What is this for? If text is NULL here, shouldn't the len always be 0 or
this
> > should fault?
> 
> +1 -- seems like its an error (assert?) to pass NULL w/ non-zero length

I added a test to StringTest.cpp so you can see when strings momentarily have
NULL and non-zero length.

[caryclark, 2015-08-31 13:11:12]

The CQ bit was checked by caryclark@google.com

[caryclark, 2015-08-31 13:11:12]

The patchset sent to the CQ was uploaded after l-g-t-m from reed@google.com
Link to the patchset: https://codereview.chromium.org/1317913005/#ps40001
(title: "add string test")

[commit-bot: I haz the power, 2015-08-31 13:11:23]

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

[commit-bot: I haz the power, 2015-08-31 13:11:24]

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

[commit-bot: I haz the power, 2015-08-31 13:11:25]

All required reviewers (with asterisk prefixes) have not yet approved this CL.

No LGTM from a valid reviewer yet. Please ask for an LGTM from a
full Skia committer from
https://skia.googlesource.com/skia/+/master/CQ_COMMITTERS

[caryclark, 2015-08-31 13:15:11]

The CQ bit was checked by caryclark@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-08-31 13:15:22]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1317913005/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1317913005/40001

[commit-bot: I haz the power, 2015-08-31 13:18:12]

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

[commit-bot: I haz the power, 2015-08-31 13:18:13]

Dry run: Try jobs failed on following builders:
  Build-Win-MSVC-x86_64-Debug-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86_6...)

[caryclark, 2015-08-31 13:23:52]

The CQ bit was checked by caryclark@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-08-31 13:24:02]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1317913005/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1317913005/60001

[commit-bot: I haz the power, 2015-08-31 13:29:03]

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

[commit-bot: I haz the power, 2015-08-31 13:29:04]

Dry run: This issue passed the CQ dry run.

[bungeman-skia, 2015-08-31 15:09:08]

https://codereview.chromium.org/1317913005/diff/60001/src/core/SkString.cpp
File src/core/SkString.cpp (right):

https://codereview.chromium.org/1317913005/diff/60001/src/core/SkString.cpp#n...
src/core/SkString.cpp:380: SkString tmp(text ? text : this->c_str(), len);
Hmmm... I see, so SkString::resize(size_t) is called, which calls
SkString::set(NULL, len), which can fall through to the bottom case and SkString
tmp(text, len); is called, with text set to NULL. The idea of resize(int) (and
hence, I'm assuming, set(NULL, int)) is that the existing string should be
resized without changing the existing content of the SkString which fits in the
new size. The above cases all work this way because they are in-place
optimizations, but this case is actually going to do a move, so the original
data will need to be copied.

So the idea seems sound, but I'm concerned that the SkString constructor will
try to copy past the end of this->c_str() when len is greater than
fRec->fLength. SkString::AllocRec(text[], size_t) appears to always memcpy 'len'
bytes from 'text'.

[bungeman-skia, 2015-08-31 15:59:32]

lgtm

Cleanup comments can be addressed later.

https://codereview.chromium.org/1317913005/diff/80001/src/utils/SkWhitelistTy...
File src/utils/SkWhitelistTypefaces.cpp (right):

https://codereview.chromium.org/1317913005/diff/80001/src/utils/SkWhitelistTy...
src/utils/SkWhitelistTypefaces.cpp:37: struct NameTable {
Why not SkOTTableName?

https://codereview.chromium.org/1317913005/diff/80001/src/utils/SkWhitelistTy...
src/utils/SkWhitelistTypefaces.cpp:91: int stringLen = name_table(nameTable, 1,
&stringLoc);
Any reason not to use SkOTTableName::Iterator or LocalizedStrings_NameTable
here? The code here doesn't seem to handle PlatformID and EncodingID. This seems
to assume all names are UCS2 or UTF-16BE, but UCS4 and MacRoman are also used.
This seems to just turn those into garbage, so they probably won't compare equal
to any names in the whitelist, but it's not obvious that this is what is going
on.

Also, it would be nice for the 1, 16, and 21 values to be given names.

[bungeman-skia, 2015-08-31 16:12:29]

The CQ bit was checked by bungeman@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-08-31 16:12:36]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1317913005/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1317913005/80001

[commit-bot: I haz the power, 2015-08-31 16:17:46]

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

[commit-bot: I haz the power, 2015-08-31 16:17:47]

Dry run: This issue passed the CQ dry run.

[caryclark, 2015-08-31 16:22:00]

The CQ bit was checked by caryclark@google.com

[caryclark, 2015-08-31 16:22:00]

The patchset sent to the CQ was uploaded after l-g-t-m from reed@google.com
Link to the patchset: https://codereview.chromium.org/1317913005/#ps80001
(title: "remove string change")

[commit-bot: I haz the power, 2015-08-31 16:22:04]

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

[commit-bot: I haz the power, 2015-08-31 16:22:41]

Committed patchset #5 (id:80001) as
https://skia.googlesource.com/skia/+/5ef194c31ae498166bd9c468993514c5267ea077

[caryclark, 2015-09-01 12:17:37]

https://codereview.chromium.org/1317913005/diff/80001/src/utils/SkWhitelistTy...
File src/utils/SkWhitelistTypefaces.cpp (right):

https://codereview.chromium.org/1317913005/diff/80001/src/utils/SkWhitelistTy...
src/utils/SkWhitelistTypefaces.cpp:91: int stringLen = name_table(nameTable, 1,
&stringLoc);
I'll switch over to LocalizedStrings_NameTable in a separate CL.