Base SkAutoTUnref on skstd::unique_ptr.

Base SkAutoTUnref on skstd::unique_ptr.

To further consolidate the various unique owning classes, this bases
SkAutoTUnref on skstd::unique_ptr. Users are updated because of two
breaking changes, swap now takes a reference and reset no longer
returns its argument.

Committed: https://skia.googlesource.com/skia/+/77a53de20d723ca21cc824fd97e68aaa60e022ea

[bungeman-skia, 2015-09-25 20:16:38]

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

[commit-bot: I haz the power, 2015-09-25 20:16:55]

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

[commit-bot: I haz the power, 2015-09-25 20:21:42]

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

[commit-bot: I haz the power, 2015-09-25 20:21:42]

Dry run: This issue passed the CQ dry run.

[bungeman-skia, 2015-09-25 20:43:09]

bungeman@google.com changed reviewers:
+ halcanary@google.com, mtklein@google.com, reed@google.com

[bungeman-skia, 2015-09-25 20:43:09]

By inspection this shouldn't affect Android, and I'm currently building Chromium
locally to see.

[hal.canary, 2015-09-25 20:46:56]

lgtm

[reed1, 2015-09-25 22:49:24]

I'm fine with the api/semantics, but it reads funny (to me) to see a shared-ptr
type inherit from unique-ptr.

[hal.canary, 2015-09-26 00:38:15]

While we are at it, we can also define a copy-constructor and copy-assignment
operator that refs the pointer.

[bungeman-skia, 2015-09-26 16:11:37]

On 2015/09/25 22:49:24, reed1 wrote:
> I'm fine with the api/semantics, but it reads funny (to me) to see a
shared-ptr
> type inherit from unique-ptr.

Well, that's the point of SkAutoTUnref, it owns a reference to a shared-ptr type
and binds that reference to its own automatic storage lifetime. It isn't a
shared-ptr itself, so it's not like a shared-ptr type is actually inheriting
from unique_ptr.

[bungeman-skia, 2015-09-26 16:13:30]

On 2015/09/26 16:11:37, bungeman1 wrote:
> On 2015/09/25 22:49:24, reed1 wrote:
> > I'm fine with the api/semantics, but it reads funny (to me) to see a
> shared-ptr
> > type inherit from unique-ptr.
> 
> Well, that's the point of SkAutoTUnref, it owns a reference to a shared-ptr
type
> and binds that reference to its own automatic storage lifetime. It isn't a
> shared-ptr itself, so it's not like a shared-ptr type is actually inheriting
> from unique_ptr.

I would be very much against this without very good motivating reason,
especially with the current automatic conversion to T*. It's already movable.

[bungeman-skia, 2015-09-26 16:14:15]

On 2015/09/26 00:38:15, Hal Canary wrote:
> While we are at it, we can also define a copy-constructor and copy-assignment
> operator that refs the pointer.

(Hitting reply on the right comment this time)

I would be very much against this without very good motivating reason,
especially with the current automatic conversion to T*. It's already movable.

[mtklein, 2015-09-27 07:16:58]

On 2015/09/26 16:14:15, bungeman1 wrote:
> On 2015/09/26 00:38:15, Hal Canary wrote:
> > While we are at it, we can also define a copy-constructor and
copy-assignment
> > operator that refs the pointer.
> 
> (Hitting reply on the right comment this time)
> 
> I would be very much against this without very good motivating reason,
> especially with the current automatic conversion to T*. It's already movable.

Out of curiosity, has the movabilty been added here by inheriting from
unique_ptr?. I wouldn't think it was safely movable before.

[hal.canary, 2015-09-27 17:56:16]

> Out of curiosity, has the movabilty been added here by inheriting from
> unique_ptr?. I wouldn't think it was safely movable before.

Right.

By the way, the motivation was to use c++11ish moves inside SkTHash.  I'd like
to have

    SkTHashMap<int, SkAutoTUnref<SkRefCnt>>

behave the way you would like it to behave.

[mtklein, 2015-09-27 21:57:49]

On 2015/09/27 17:56:16, Hal Canary wrote:
> > Out of curiosity, has the movabilty been added here by inheriting from
> > unique_ptr?. I wouldn't think it was safely movable before.
> 
> Right.
> 
> By the way, the motivation was to use c++11ish moves inside SkTHash.  I'd like
> to have
> 
>     SkTHashMap<int, SkAutoTUnref<SkRefCnt>>
> 
> behave the way you would like it to behave.

This seems fine, and that seems...maybe fine, maybe complicating a simple world.
 I think our current idiom is map<int, *> with a foreach(unref) in the
destructor of the thing owning the map.  Is that unclear?  Somehow error prone?

Are you planning on adding add(&&) and use move inside skthash?  Those do seem
like good ideas to me.  Just not quite with you there yet on the next step of
using it with SkAutoTUnref.

[mtklein, 2015-09-27 22:00:35]

(This CL totally lgtm.)

[hal.canary, 2015-09-27 22:01:28]

On 2015/09/27 21:57:49, mtklein wrote:
> Just not quite with you there yet on the next step of
> using it with SkAutoTUnref.

Let's talk when we are both are in the office.

[mtklein_C, 2015-10-01 17:13:52]

mtklein@chromium.org changed reviewers:
+ mtklein@chromium.org

[mtklein_C, 2015-10-01 17:13:53]

Anything holding this up?

[reed1, 2015-10-01 18:18:13]

api lgtm

[mtklein, 2015-10-01 18:57:08]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2015-10-01 18:58:16]

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

[commit-bot: I haz the power, 2015-10-01 18:59:09]

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

[commit-bot: I haz the power, 2015-10-01 18:59:10]

Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug-Trybot on client.skia (JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Shared-Trybot on client.skia
(JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)
  Build-Ubuntu-Clang-x86_64-Debug-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-Clang-x...)
  Build-Ubuntu-GCC-Arm64-Debug-Android-Trybot on client.skia.compile
(JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Arm...)
  Build-Ubuntu-GCC-Arm7-Debug-Android-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Arm...)
  Build-Ubuntu-GCC-Mips-Debug-Android-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Mip...)
  Build-Ubuntu-GCC-x86_64-Release-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-x86...)

[bungeman-skia, 2015-10-01 19:21:33]

The CQ bit was checked by bungeman@google.com

[bungeman-skia, 2015-10-01 19:21:34]

The patchset sent to the CQ was uploaded after l-g-t-m from
halcanary@google.com, mtklein@google.com, reed@google.com
Link to the patchset: https://codereview.chromium.org/1370803002/#ps20001
(title: "Rebase")

[commit-bot: I haz the power, 2015-10-01 19:21:41]

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

[commit-bot: I haz the power, 2015-10-01 19:28:53]

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

[hal.canary, 2015-10-01 20:27:36]

On 2015/09/26 00:38:15, Hal Canary wrote:
> While we are at it, we can also define a copy-constructor and copy-assignment
> operator that refs the pointer.

This is a potentially useful little function for "copying" an AutoUnref.

    template <class T> inline SkAutoTUnref<T> Copy(const SkAutoTUnref<T>& r) {
        return skstd::move(SkAutoTUnref<T>(SkSafeRef(r.get())));
    }

This allows

    SkAutoTUnref<MyClass> mycopy;
    mycopy = Copy(original);

or

    SkAutoTUnref<MyClass> mycopy(Copy(original));