SkPdf: SkPDFFormXObject de-class-ified.

SkPdf: SkPDFFormXObject de-class-ified.

We don't need an object, just a few standard fields on the
base class; the change lets us get rid of a bunch of
boilerplate code.

I think this also reduces the cognitive load of the SkPDF
internals.

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

Committed: https://skia.googlesource.com/skia/+/4b1e17edc78c0313d5cb8a415f816f654cdfa417

[hal.canary, 2016-07-27 19:08:08]

Description was changed from

==========
SkPdf: SkPDFFormXObject de-class-ified.

BUG=skia:
==========

to

==========
SkPdf: SkPDFFormXObject de-class-ified.

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

[hal.canary, 2016-07-27 19:09:01]

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

[hal.canary, 2016-07-27 19:09:02]

I had a giant CL six weeks ago.  This is a small subset of that.  PTAL.

[hal.canary, 2016-07-27 19:34:46]

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

[commit-bot: I haz the power, 2016-07-27 19:34:52]

Dry run: 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-07-27 20:22:45]

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

[commit-bot: I haz the power, 2016-07-27 20:22:46]

Dry run: This issue passed the CQ dry run.

[tomhudson, 2016-07-27 20:30:59]

I suspect this is a good change, but as always please capture some part of the
reason for making the change in your change description.

(Even "We don't need an object, just a few standard fields on the base class;
the change lets us get rid of a bunch of boilerplate code and perhaps be a
little more efficient." - which at least helps me know there wasn't another less
predictable reason behind the change.)

https://codereview.chromium.org/2185803003/diff/20001/src/pdf/SkPDFDevice.cpp
File src/pdf/SkPDFDevice.cpp (right):

https://codereview.chromium.org/2185803003/diff/20001/src/pdf/SkPDFDevice.cpp...
src/pdf/SkPDFDevice.cpp:1617: nullptr);
I don't like this because I have to read both forward and backwards to deduce
that this is returning an sk_sp<>. But I think I've lost that fight and this is
valid in our informal style guide, if not the written one?

https://codereview.chromium.org/2185803003/diff/20001/src/pdf/SkPDFDevice.cpp...
src/pdf/SkPDFDevice.cpp:1706: *dst =
this->makeFormXObjectFromDevice().release();
Note TODO

https://codereview.chromium.org/2185803003/diff/20001/src/pdf/SkPDFFormXObjec...
File src/pdf/SkPDFFormXObject.cpp (right):

https://codereview.chromium.org/2185803003/diff/20001/src/pdf/SkPDFFormXObjec...
src/pdf/SkPDFFormXObject.cpp:21: // Right now FormXObject is only used for
saveLayer, which implies
Is this comment obsolete, since we use it for alpha masks?

https://codereview.chromium.org/2185803003/diff/20001/src/pdf/SkPDFFormXObject.h
File src/pdf/SkPDFFormXObject.h (right):

https://codereview.chromium.org/2185803003/diff/20001/src/pdf/SkPDFFormXObjec...
src/pdf/SkPDFFormXObject.h:16: objects.  A form XObject is basically a page
object with slightly
Match tense: is a ... of *a* graphics object.
Cut basically.
Maybe your intended reader understands enough about pages and graphics objects
and relationships within PDF to understand your point here. I don't.

[hal.canary, 2016-07-27 20:36:20]

Description was changed from

==========
SkPdf: SkPDFFormXObject de-class-ified.

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

to

==========
SkPdf: SkPDFFormXObject de-class-ified.

We don't need an object, just a few standard fields on the
base class; the change lets us get rid of a bunch of
boilerplate code.

I think this also reduced the cognitive load of the SkPDF
internals.

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

[hal.canary, 2016-07-27 20:41:12]

> I suspect this is a good change, but as always please
> capture some part of the reason for making the change
> in your change description.

done.

https://codereview.chromium.org/2185803003/diff/20001/src/pdf/SkPDFDevice.cpp
File src/pdf/SkPDFDevice.cpp (right):

https://codereview.chromium.org/2185803003/diff/20001/src/pdf/SkPDFDevice.cpp...
src/pdf/SkPDFDevice.cpp:1617: nullptr);
On 2016/07/27 20:30:58, tomhudson wrote:
> I don't like this because I have to read both forward and backwards to deduce
> that this is returning an sk_sp<>. But I think I've lost that fight and this
is
> valid in our informal style guide, if not the written one?

Done.

https://codereview.chromium.org/2185803003/diff/20001/src/pdf/SkPDFDevice.cpp...
src/pdf/SkPDFDevice.cpp:1706: *dst =
this->makeFormXObjectFromDevice().release();
On 2016/07/27 20:30:58, tomhudson wrote:
> Note TODO

Acknowledged.

https://codereview.chromium.org/2185803003/diff/20001/src/pdf/SkPDFFormXObjec...
File src/pdf/SkPDFFormXObject.cpp (right):

https://codereview.chromium.org/2185803003/diff/20001/src/pdf/SkPDFFormXObjec...
src/pdf/SkPDFFormXObject.cpp:21: // Right now FormXObject is only used for
saveLayer, which implies
On 2016/07/27 20:30:58, tomhudson wrote:
> Is this comment obsolete, since we use it for alpha masks?

Good question.  I don't know the answer and will note that.

https://codereview.chromium.org/2185803003/diff/20001/src/pdf/SkPDFFormXObject.h
File src/pdf/SkPDFFormXObject.h (right):

https://codereview.chromium.org/2185803003/diff/20001/src/pdf/SkPDFFormXObjec...
src/pdf/SkPDFFormXObject.h:16: objects.  A form XObject is basically a page
object with slightly
On 2016/07/27 20:30:58, tomhudson wrote:
> Match tense: is a ... of *a* graphics object.
> Cut basically.
> Maybe your intended reader understands enough about pages and graphics objects
> and relationships within PDF to understand your point here. I don't.

Done.

[hal.canary, 2016-07-27 20:41:18]

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

[commit-bot: I haz the power, 2016-07-27 20:41:22]

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-07-27 20:42:11]

Description was changed from

==========
SkPdf: SkPDFFormXObject de-class-ified.

We don't need an object, just a few standard fields on the
base class; the change lets us get rid of a bunch of
boilerplate code.

I think this also reduced the cognitive load of the SkPDF
internals.

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

to

==========
SkPdf: SkPDFFormXObject de-class-ified.

We don't need an object, just a few standard fields on the
base class; the change lets us get rid of a bunch of
boilerplate code.

I think this also reduces the cognitive load of the SkPDF
internals.

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

[tomhudson, 2016-07-27 20:43:46]

Nit: add a TODO(halcanary) or something where I had you question the validity of
the comment. Or fix it now.

LGTM

[tomhudson, 2016-07-27 20:49:30]

The CQ bit was unchecked by tomhudson@google.com

[hal.canary, 2016-07-27 20:57:38]

The CQ bit was checked by halcanary@google.com

[commit-bot: I haz the power, 2016-07-27 20:57:42]

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

[hal.canary, 2016-07-27 21:02:01]

The CQ bit was checked by halcanary@google.com

[hal.canary, 2016-07-27 21:02:02]

The patchset sent to the CQ was uploaded after l-g-t-m from tomhudson@google.com
Link to the patchset: https://codereview.chromium.org/2185803003/#ps80001
(title: "2016-07-27 (Wednesday) 17:00:53 EDT")

[commit-bot: I haz the power, 2016-07-27 21:02:11]

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-07-27 21:49:49]

Description was changed from

==========
SkPdf: SkPDFFormXObject de-class-ified.

We don't need an object, just a few standard fields on the
base class; the change lets us get rid of a bunch of
boilerplate code.

I think this also reduces the cognitive load of the SkPDF
internals.

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

to

==========
SkPdf: SkPDFFormXObject de-class-ified.

We don't need an object, just a few standard fields on the
base class; the change lets us get rid of a bunch of
boilerplate code.

I think this also reduces the cognitive load of the SkPDF
internals.

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

Committed:
https://skia.googlesource.com/skia/+/4b1e17edc78c0313d5cb8a415f816f654cdfa417
==========

[commit-bot: I haz the power, 2016-07-27 21:49:50]

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