SkPDF: SkPDFFont organization changes.

SkPDF: SkPDFFont organization changes.

SkPDFFont:
  - SkPDFType1Font::populate() encode advances correctly.
  - break out logically independent code into new files:
    * SkPDFConvertType1FontStream
    * SkPDFMakeToUnicodeCmap
    SkPDFFont.cpp is now 380 lines smaller.
    Expose `SkPDFAppendCmapSections()` for testing.

SkPDFFontImpl.h
  - Fold into SkPDFFont.

SkPDFConvertType1FontStream:
  - Now assume given a SkStreamAsset

SkPDFFont:
  - AdvanceMetric now hidden in a anonymous namespace.

No public API changes.
TBR=reed@google.com

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2221163002

Committed: https://skia.googlesource.com/skia/+/8eccc308c8adcdf26ffc7c4dd538b71f33c6f22b

[hal.canary, 2016-08-08 19:54:32]

Description was changed from

==========
SkPDF: SkPDFFont organization changes.

SkPDFFont:
  - SkPDFType1Font::populate() encode advances correctly.
  - break out logically independent code into new files:
    * SkPDFConvertType1FontStream
    * SkPDFMakeToUnicodeCmap
    SkPDFFont.cpp is now 380 lines smaller.
    Expose `SkPDFAppendCmapSections()` for testing.

SkPDFFontImpl.h
  - Fold into SkPDFFont.

SkPDFConvertType1FontStream:
  - Now assume given a SkStreamAsset

SkPDFFont:
  - AdvanceMetric now hidden in a anonymous namespace.
==========

to

==========
SkPDF: SkPDFFont organization changes.

SkPDFFont:
  - SkPDFType1Font::populate() encode advances correctly.
  - break out logically independent code into new files:
    * SkPDFConvertType1FontStream
    * SkPDFMakeToUnicodeCmap
    SkPDFFont.cpp is now 380 lines smaller.
    Expose `SkPDFAppendCmapSections()` for testing.

SkPDFFontImpl.h
  - Fold into SkPDFFont.

SkPDFConvertType1FontStream:
  - Now assume given a SkStreamAsset

SkPDFFont:
  - AdvanceMetric now hidden in a anonymous namespace.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2221163002
==========

[hal.canary, 2016-08-08 19:54:50]

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

[commit-bot: I haz the power, 2016-08-08 19:54:53]

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

[hal.canary, 2016-08-08 19:55:03]

halcanary@google.com changed reviewers:
+ bungeman@google.com

[hal.canary, 2016-08-08 19:55:03]

PTAL

[commit-bot: I haz the power, 2016-08-08 20:03:38]

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

[commit-bot: I haz the power, 2016-08-08 20:03:39]

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

[bungeman-skia, 2016-08-09 14:36:43]

The organization does seem better this way.

https://codereview.chromium.org/2221163002/diff/1/src/pdf/SkPDFFont.cpp
File src/pdf/SkPDFFont.cpp (right):

https://codereview.chromium.org/2221163002/diff/1/src/pdf/SkPDFFont.cpp#newco...
src/pdf/SkPDFFont.cpp:66: #ifdef SK_DEBUG
It's really hard to see these ifdefs. I'd be fine with just the '#' all the way
on the left. They're ugly the way xxx_cast<type>(expr) are ugly, they're
supposed to be.

https://codereview.chromium.org/2221163002/diff/1/src/pdf/SkPDFFont.cpp#newco...
src/pdf/SkPDFFont.cpp:990: std::move(rawFontData), &header, &data, &trailer));
nit: the indentation here is weird.

[hal.canary, 2016-08-09 16:13:38]

https://codereview.chromium.org/2221163002/diff/1/src/pdf/SkPDFFont.cpp
File src/pdf/SkPDFFont.cpp (right):

https://codereview.chromium.org/2221163002/diff/1/src/pdf/SkPDFFont.cpp#newco...
src/pdf/SkPDFFont.cpp:66: #ifdef SK_DEBUG
On 2016/08/09 14:36:43, bungeman-skia wrote:
> It's really hard to see these ifdefs. I'd be fine with just the '#' all the
way
> on the left. They're ugly the way xxx_cast<type>(expr) are ugly, they're
> supposed to be.

Done.

https://codereview.chromium.org/2221163002/diff/1/src/pdf/SkPDFFont.cpp#newco...
src/pdf/SkPDFFont.cpp:990: std::move(rawFontData), &header, &data, &trailer));
On 2016/08/09 14:36:43, bungeman-skia wrote:
> nit: the indentation here is weird.

Done.

[hal.canary, 2016-08-09 16:13:59]

Patchset #2 (id:20001) has been deleted

[hal.canary, 2016-08-09 16:14:07]

The CQ bit was checked by halcanary@google.com

[commit-bot: I haz the power, 2016-08-09 16:14:24]

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-08-09 16:15:45]

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

[commit-bot: I haz the power, 2016-08-09 16:15:46]

Try jobs failed on following builders:
  skia_presubmit-Trybot on master.client.skia.fyi (JOB_FAILED,
http://build.chromium.org/p/client.skia.fyi/builders/skia_presubmit-Trybot/bu...)

[bungeman-skia, 2016-08-09 16:25:30]

https://codereview.chromium.org/2221163002/diff/40001/src/pdf/SkPDFFont.cpp
File src/pdf/SkPDFFont.cpp (right):

https://codereview.chromium.org/2221163002/diff/40001/src/pdf/SkPDFFont.cpp#n...
src/pdf/SkPDFFont.cpp:918: =
SkPDFFont::GetFontMetricsWithGlyphNames(this->typeface(), glyphs, glyphsCount);
nit: I think usually the '=' goes on the previous line.

skia$ git grep -E '^\s+=' | wc -l
      76
skia$ git grep -E '=\s*$' | wc -l
    1234

https://codereview.chromium.org/2221163002/diff/40001/src/pdf/SkPDFFont.cpp#n...
src/pdf/SkPDFFont.cpp:987: &header, &data, &trailer));
One too many ')'.

[hal.canary, 2016-08-09 16:33:10]

https://codereview.chromium.org/2221163002/diff/40001/src/pdf/SkPDFFont.cpp
File src/pdf/SkPDFFont.cpp (right):

https://codereview.chromium.org/2221163002/diff/40001/src/pdf/SkPDFFont.cpp#n...
src/pdf/SkPDFFont.cpp:918: =
SkPDFFont::GetFontMetricsWithGlyphNames(this->typeface(), glyphs, glyphsCount);
On 2016/08/09 16:25:29, bungeman-skia wrote:
> nit: I think usually the '=' goes on the previous line.
> 
> skia$ git grep -E '^\s+=' | wc -l
>       76
> skia$ git grep -E '=\s*$' | wc -l
>     1234

Done.

https://codereview.chromium.org/2221163002/diff/40001/src/pdf/SkPDFFont.cpp#n...
src/pdf/SkPDFFont.cpp:987: &header, &data, &trailer));
On 2016/08/09 16:25:29, bungeman-skia wrote:
> One too many ')'.

Done.

argh!

[hal.canary, 2016-08-09 16:33:18]

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

[commit-bot: I haz the power, 2016-08-09 16:33:25]

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

[bungeman-skia, 2016-08-09 16:39:54]

If the bots like it, lgtm too.

[hal.canary, 2016-08-09 19:24:32]

The CQ bit was checked by halcanary@google.com

[hal.canary, 2016-08-09 19:24:33]

The patchset sent to the CQ was uploaded after l-g-t-m from bungeman@google.com
Link to the patchset: https://codereview.chromium.org/2221163002/#ps100001
(title: "2016-08-09 (Tuesday) 15:24:15 EDT")

[commit-bot: I haz the power, 2016-08-09 19:24:44]

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-08-09 19:26:16]

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

[commit-bot: I haz the power, 2016-08-09 19:26:17]

Try jobs failed on following builders:
  skia_presubmit-Trybot on master.client.skia.fyi (JOB_FAILED,
http://build.chromium.org/p/client.skia.fyi/builders/skia_presubmit-Trybot/bu...)

[hal.canary, 2016-08-09 20:03:28]

Description was changed from

==========
SkPDF: SkPDFFont organization changes.

SkPDFFont:
  - SkPDFType1Font::populate() encode advances correctly.
  - break out logically independent code into new files:
    * SkPDFConvertType1FontStream
    * SkPDFMakeToUnicodeCmap
    SkPDFFont.cpp is now 380 lines smaller.
    Expose `SkPDFAppendCmapSections()` for testing.

SkPDFFontImpl.h
  - Fold into SkPDFFont.

SkPDFConvertType1FontStream:
  - Now assume given a SkStreamAsset

SkPDFFont:
  - AdvanceMetric now hidden in a anonymous namespace.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2221163002
==========

to

==========
SkPDF: SkPDFFont organization changes.

SkPDFFont:
  - SkPDFType1Font::populate() encode advances correctly.
  - break out logically independent code into new files:
    * SkPDFConvertType1FontStream
    * SkPDFMakeToUnicodeCmap
    SkPDFFont.cpp is now 380 lines smaller.
    Expose `SkPDFAppendCmapSections()` for testing.

SkPDFFontImpl.h
  - Fold into SkPDFFont.

SkPDFConvertType1FontStream:
  - Now assume given a SkStreamAsset

SkPDFFont:
  - AdvanceMetric now hidden in a anonymous namespace.

No public API changes.
TBR=reed@google.com

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2221163002
==========

[hal.canary, 2016-08-09 20:03:35]

The CQ bit was checked by halcanary@google.com

[commit-bot: I haz the power, 2016-08-09 20:03:39]

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-08-09 20:04:36]

Description was changed from

==========
SkPDF: SkPDFFont organization changes.

SkPDFFont:
  - SkPDFType1Font::populate() encode advances correctly.
  - break out logically independent code into new files:
    * SkPDFConvertType1FontStream
    * SkPDFMakeToUnicodeCmap
    SkPDFFont.cpp is now 380 lines smaller.
    Expose `SkPDFAppendCmapSections()` for testing.

SkPDFFontImpl.h
  - Fold into SkPDFFont.

SkPDFConvertType1FontStream:
  - Now assume given a SkStreamAsset

SkPDFFont:
  - AdvanceMetric now hidden in a anonymous namespace.

No public API changes.
TBR=reed@google.com

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2221163002
==========

to

==========
SkPDF: SkPDFFont organization changes.

SkPDFFont:
  - SkPDFType1Font::populate() encode advances correctly.
  - break out logically independent code into new files:
    * SkPDFConvertType1FontStream
    * SkPDFMakeToUnicodeCmap
    SkPDFFont.cpp is now 380 lines smaller.
    Expose `SkPDFAppendCmapSections()` for testing.

SkPDFFontImpl.h
  - Fold into SkPDFFont.

SkPDFConvertType1FontStream:
  - Now assume given a SkStreamAsset

SkPDFFont:
  - AdvanceMetric now hidden in a anonymous namespace.

No public API changes.
TBR=reed@google.com

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2221163002

Committed:
https://skia.googlesource.com/skia/+/8eccc308c8adcdf26ffc7c4dd538b71f33c6f22b
==========

[commit-bot: I haz the power, 2016-08-09 20:04:37]

Committed patchset #5 (id:100001) as
https://skia.googlesource.com/skia/+/8eccc308c8adcdf26ffc7c4dd538b71f33c6f22b