SkPDF: Add sk_sp setters; .release() becomes std::move()

SkPDF: Add sk_sp setters; .release() becomes std::move()

Note to reviewers:  Start with changes to SkPDFTypes.h

Many places that had a bare pointer owning a reference are refactored to
use a sk_sp.

There remain several places where a non-owning pointer `T*` should be
replaced with `const sk_sp<T>&` to eliminate the common pattern
`sk_sp<T>(SkRef(x))`.

Committed: https://skia.googlesource.com/skia/+/9904c9212074279380e21f96575078734dbbd308
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1775043002

Committed: https://skia.googlesource.com/skia/+/8103a34300c5de2e85793a96c4738a33fc6eb46d

[hal.canary, 2016-03-08 18:32:15]

Description was changed from

==========
SkPDF: Add sk_sp setters; .release() becomes std::move()

Note to reviewers:  Start with changes to SkPDFTypes.h

Many places that had a bare pointer owning a reference are refactored to
use a sk_sp.

There remain several places where a non-owning pointer `T*` should be
replaced with `const sk_sp<T>&` to eliminate the common pattern
`sk_sp<T>(SkRef(x))`.
==========

to

==========
SkPDF: Add sk_sp setters; .release() becomes std::move()

Note to reviewers:  Start with changes to SkPDFTypes.h

Many places that had a bare pointer owning a reference are refactored to
use a sk_sp.

There remain several places where a non-owning pointer `T*` should be
replaced with `const sk_sp<T>&` to eliminate the common pattern
`sk_sp<T>(SkRef(x))`.
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[hal.canary, 2016-03-08 18:32:27]

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

[commit-bot: I haz the power, 2016-03-08 18:32:36]

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

[hal.canary, 2016-03-08 18:32:44]

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

[hal.canary, 2016-03-08 18:32:44]

ptal

[hal.canary, 2016-03-08 18:32:59]

Description was changed from

==========
SkPDF: Add sk_sp setters; .release() becomes std::move()

Note to reviewers:  Start with changes to SkPDFTypes.h

Many places that had a bare pointer owning a reference are refactored to
use a sk_sp.

There remain several places where a non-owning pointer `T*` should be
replaced with `const sk_sp<T>&` to eliminate the common pattern
`sk_sp<T>(SkRef(x))`.
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
SkPDF: Add sk_sp setters; .release() becomes std::move()

Note to reviewers:  Start with changes to SkPDFTypes.h

Many places that had a bare pointer owning a reference are refactored to
use a sk_sp.

There remain several places where a non-owning pointer `T*` should be
replaced with `const sk_sp<T>&` to eliminate the common pattern
`sk_sp<T>(SkRef(x))`.

GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[commit-bot: I haz the power, 2016-03-08 18:51:26]

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

[commit-bot: I haz the power, 2016-03-08 18:51:27]

Dry run: This issue passed the CQ dry run.

[bungeman-skia, 2016-03-08 19:05:44]

https://codereview.chromium.org/1775043002/diff/1/src/doc/SkDocument_PDF.cpp
File src/doc/SkDocument_PDF.cpp (right):

https://codereview.chromium.org/1775043002/diff/1/src/doc/SkDocument_PDF.cpp#...
src/doc/SkDocument_PDF.cpp:127: curNodes[i]->insertObjRef("Parent", newNode); //
copy ref
I don't think the comment is necessary. newNode is a smart pointer and an
lvalue, so we know if the callee wants a reference it's going to have to set
things up so that it adds one. In other words, it logically isn't right here
that the 'copy ref' actually happens anyway (though the compiler might compile
it that way).

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

https://codereview.chromium.org/1775043002/diff/1/src/pdf/SkPDFBitmap.cpp#new...
src/pdf/SkPDFBitmap.cpp:367: pdfDict.insertObjRef("SMask", smask); // copy ref
Same here.

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

https://codereview.chromium.org/1775043002/diff/1/src/pdf/SkPDFFont.cpp#newco...
src/pdf/SkPDFFont.cpp:328: SkPDFArray* array),
nit: indentation change

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

https://codereview.chromium.org/1775043002/diff/1/src/pdf/SkPDFShader.cpp#new...
src/pdf/SkPDFShader.cpp:528: SkPDFObject* gState = nullptr) {
Skia doesn't generally like default parameters if they can be avoided.

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

https://codereview.chromium.org/1775043002/diff/1/src/doc/SkDocument_PDF.cpp
File src/doc/SkDocument_PDF.cpp (right):

https://codereview.chromium.org/1775043002/diff/1/src/doc/SkDocument_PDF.cpp#...
src/doc/SkDocument_PDF.cpp:127: curNodes[i]->insertObjRef("Parent", newNode); //
copy ref
On 2016/03/08 19:05:44, bungeman1 wrote:
> I don't think the comment is necessary. newNode is a smart pointer and an
> lvalue, so we know if the callee wants a reference it's going to have to set
> things up so that it adds one. In other words, it logically isn't right here
> that the 'copy ref' actually happens anyway (though the compiler might compile
> it that way).

Done.

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

https://codereview.chromium.org/1775043002/diff/1/src/pdf/SkPDFBitmap.cpp#new...
src/pdf/SkPDFBitmap.cpp:367: pdfDict.insertObjRef("SMask", smask); // copy ref
On 2016/03/08 19:05:44, bungeman1 wrote:
> Same here.

Done.

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

https://codereview.chromium.org/1775043002/diff/1/src/pdf/SkPDFFont.cpp#newco...
src/pdf/SkPDFFont.cpp:328: SkPDFArray* array),
On 2016/03/08 19:05:44, bungeman1 wrote:
> nit: indentation change

Done.

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

https://codereview.chromium.org/1775043002/diff/1/src/pdf/SkPDFShader.cpp#new...
src/pdf/SkPDFShader.cpp:528: SkPDFObject* gState = nullptr) {
On 2016/03/08 19:05:44, bungeman1 wrote:
> Skia doesn't generally like default parameters if they can be avoided.

Done.

[bungeman-skia, 2016-03-08 19:25:54]

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

[commit-bot: I haz the power, 2016-03-08 19:25:58]

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

[bungeman-skia, 2016-03-08 19:26:03]

Description was changed from

==========
SkPDF: Add sk_sp setters; .release() becomes std::move()

Note to reviewers:  Start with changes to SkPDFTypes.h

Many places that had a bare pointer owning a reference are refactored to
use a sk_sp.

There remain several places where a non-owning pointer `T*` should be
replaced with `const sk_sp<T>&` to eliminate the common pattern
`sk_sp<T>(SkRef(x))`.

GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
SkPDF: Add sk_sp setters; .release() becomes std::move()

Note to reviewers:  Start with changes to SkPDFTypes.h

Many places that had a bare pointer owning a reference are refactored to
use a sk_sp.

There remain several places where a non-owning pointer `T*` should be
replaced with `const sk_sp<T>&` to eliminate the common pattern
`sk_sp<T>(SkRef(x))`.
==========

[bungeman-skia, 2016-03-08 19:26:16]

lgtm

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

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

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

Dry run: This issue passed the CQ dry run.

[hal.canary, 2016-03-08 20:37:17]

The CQ bit was checked by halcanary@google.com

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

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

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

Description was changed from

==========
SkPDF: Add sk_sp setters; .release() becomes std::move()

Note to reviewers:  Start with changes to SkPDFTypes.h

Many places that had a bare pointer owning a reference are refactored to
use a sk_sp.

There remain several places where a non-owning pointer `T*` should be
replaced with `const sk_sp<T>&` to eliminate the common pattern
`sk_sp<T>(SkRef(x))`.
==========

to

==========
SkPDF: Add sk_sp setters; .release() becomes std::move()

Note to reviewers:  Start with changes to SkPDFTypes.h

Many places that had a bare pointer owning a reference are refactored to
use a sk_sp.

There remain several places where a non-owning pointer `T*` should be
replaced with `const sk_sp<T>&` to eliminate the common pattern
`sk_sp<T>(SkRef(x))`.

Committed:
https://skia.googlesource.com/skia/+/9904c9212074279380e21f96575078734dbbd308
==========

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

Committed patchset #2 (id:20001) as
https://skia.googlesource.com/skia/+/9904c9212074279380e21f96575078734dbbd308

[hal.canary, 2016-03-08 21:03:44]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/1775143002/ by halcanary@google.com.

The reason for reverting is:
https://build.chromium.org/p/client.skia/builders/Linux%20Builder/builds/6405....

[hal.canary, 2016-03-08 22:47:14]

Description was changed from

==========
SkPDF: Add sk_sp setters; .release() becomes std::move()

Note to reviewers:  Start with changes to SkPDFTypes.h

Many places that had a bare pointer owning a reference are refactored to
use a sk_sp.

There remain several places where a non-owning pointer `T*` should be
replaced with `const sk_sp<T>&` to eliminate the common pattern
`sk_sp<T>(SkRef(x))`.

Committed:
https://skia.googlesource.com/skia/+/9904c9212074279380e21f96575078734dbbd308
==========

to

==========
SkPDF: Add sk_sp setters; .release() becomes std::move()

Note to reviewers:  Start with changes to SkPDFTypes.h

Many places that had a bare pointer owning a reference are refactored to
use a sk_sp.

There remain several places where a non-owning pointer `T*` should be
replaced with `const sk_sp<T>&` to eliminate the common pattern
`sk_sp<T>(SkRef(x))`.

Committed:
https://skia.googlesource.com/skia/+/9904c9212074279380e21f96575078734dbbd308
==========

[hal.canary, 2016-03-08 22:49:10]

Description was changed from

==========
SkPDF: Add sk_sp setters; .release() becomes std::move()

Note to reviewers:  Start with changes to SkPDFTypes.h

Many places that had a bare pointer owning a reference are refactored to
use a sk_sp.

There remain several places where a non-owning pointer `T*` should be
replaced with `const sk_sp<T>&` to eliminate the common pattern
`sk_sp<T>(SkRef(x))`.

Committed:
https://skia.googlesource.com/skia/+/9904c9212074279380e21f96575078734dbbd308
==========

to

==========
SkPDF: Add sk_sp setters; .release() becomes std::move()

Note to reviewers:  Start with changes to SkPDFTypes.h

Many places that had a bare pointer owning a reference are refactored to
use a sk_sp.

There remain several places where a non-owning pointer `T*` should be
replaced with `const sk_sp<T>&` to eliminate the common pattern
`sk_sp<T>(SkRef(x))`.

Committed:
https://skia.googlesource.com/skia/+/9904c9212074279380e21f96575078734dbbd308
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[hal.canary, 2016-03-08 22:52:21]

The CQ bit was checked by halcanary@google.com

[hal.canary, 2016-03-08 22:52:22]

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/1775043002/#ps40001
(title: "fix -Wpessimizing-move")

[commit-bot: I haz the power, 2016-03-08 22:52:33]

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

[commit-bot: I haz the power, 2016-03-08 23:10:19]

Description was changed from

==========
SkPDF: Add sk_sp setters; .release() becomes std::move()

Note to reviewers:  Start with changes to SkPDFTypes.h

Many places that had a bare pointer owning a reference are refactored to
use a sk_sp.

There remain several places where a non-owning pointer `T*` should be
replaced with `const sk_sp<T>&` to eliminate the common pattern
`sk_sp<T>(SkRef(x))`.

Committed:
https://skia.googlesource.com/skia/+/9904c9212074279380e21f96575078734dbbd308
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
SkPDF: Add sk_sp setters; .release() becomes std::move()

Note to reviewers:  Start with changes to SkPDFTypes.h

Many places that had a bare pointer owning a reference are refactored to
use a sk_sp.

There remain several places where a non-owning pointer `T*` should be
replaced with `const sk_sp<T>&` to eliminate the common pattern
`sk_sp<T>(SkRef(x))`.

Committed:
https://skia.googlesource.com/skia/+/9904c9212074279380e21f96575078734dbbd308
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/8103a34300c5de2e85793a96c4738a33fc6eb46d
==========

[commit-bot: I haz the power, 2016-03-08 23:10:22]

Committed patchset #3 (id:40001) as
https://skia.googlesource.com/skia/+/8103a34300c5de2e85793a96c4738a33fc6eb46d