remove non-static uses of SkOncePtr

remove non-static uses of SkOncePtr

Still slowly working through all the SK_DECLARE_STATIC_FOO macros.

SkOncePtr is complicating things by having SkOncePtr delete its pointer
and SkBaseOncePtr not.  Simplify things by removing SkOncePtr, leaving
only the leaky SkBaseOncePtr.

We replace SkOncePtr<T> instead with SkOnce and T.  In most cases this
did not need to be a pointer, and in some cases here we're even saving
a few bytes by replacing SkOncePtr<T> with SkOnce and a T.

The dependency map of SK_DECLARE_STATIC_FOO is:
  SkBaseMutex -> SkBaseSemaphore -> SkBaseOncePtr

They're intertwined enough that I think I've got to do all three in one
next CL.

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

Committed: https://skia.googlesource.com/skia/+/59c12e3f0071acfa685346487e54a2a311066dac

[mtklein_C, 2016-04-29 21:28:55]

Description was changed from

==========
remove non-static uses of SkOncePtr

Still slowly working through all the SK_DECLARE_STATIC_FOO macros.

SkOncePtr is complicating things by having SkOncePtr delete its pointer
and SkBaseOncePtr not.  Simplify things by removing SkOncePtr, leaving
only the leaky SkBaseOncePtr.

We replace SkOncePtr instead with SkOnce and its T.  In most cases this
did not need to be a pointer, and in some cases here we're even saving
a few bytes by replacing SkOncePtr<T> with SkOnce and a T.

The dependency map of SK_DECLARE_STATIC_FOO is:
  SkBaseMutex -> SkBaseSemaphore -> SkBaseOncePtr

They're intertwined enough that I think I've got to do all three in one
next CL.

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

to

==========
remove non-static uses of SkOncePtr

Still slowly working through all the SK_DECLARE_STATIC_FOO macros.

SkOncePtr is complicating things by having SkOncePtr delete its pointer
and SkBaseOncePtr not.  Simplify things by removing SkOncePtr, leaving
only the leaky SkBaseOncePtr.

We replace SkOncePtr instead with SkOnce and its T.  In most cases this
did not need to be a pointer, and in some cases here we're even saving
a few bytes by replacing SkOncePtr<T> with SkOnce and a T.

The dependency map of SK_DECLARE_STATIC_FOO is:
  SkBaseMutex -> SkBaseSemaphore -> SkBaseOncePtr

They're intertwined enough that I think I've got to do all three in one
next CL.

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

[mtklein, 2016-04-29 21:40:08]

Description was changed from

==========
remove non-static uses of SkOncePtr

Still slowly working through all the SK_DECLARE_STATIC_FOO macros.

SkOncePtr is complicating things by having SkOncePtr delete its pointer
and SkBaseOncePtr not.  Simplify things by removing SkOncePtr, leaving
only the leaky SkBaseOncePtr.

We replace SkOncePtr instead with SkOnce and its T.  In most cases this
did not need to be a pointer, and in some cases here we're even saving
a few bytes by replacing SkOncePtr<T> with SkOnce and a T.

The dependency map of SK_DECLARE_STATIC_FOO is:
  SkBaseMutex -> SkBaseSemaphore -> SkBaseOncePtr

They're intertwined enough that I think I've got to do all three in one
next CL.

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

to

==========
remove non-static uses of SkOncePtr

Still slowly working through all the SK_DECLARE_STATIC_FOO macros.

SkOncePtr is complicating things by having SkOncePtr delete its pointer
and SkBaseOncePtr not.  Simplify things by removing SkOncePtr, leaving
only the leaky SkBaseOncePtr.

We replace SkOncePtr<T> instead with SkOnce and T.  In most cases this
did not need to be a pointer, and in some cases here we're even saving
a few bytes by replacing SkOncePtr<T> with SkOnce and a T.

The dependency map of SK_DECLARE_STATIC_FOO is:
  SkBaseMutex -> SkBaseSemaphore -> SkBaseOncePtr

They're intertwined enough that I think I've got to do all three in one
next CL.

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

[mtklein, 2016-04-29 21:47:55]

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

[commit-bot: I haz the power, 2016-04-29 21:48:00]

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

[commit-bot: I haz the power, 2016-04-29 21:59:42]

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

[commit-bot: I haz the power, 2016-04-29 21:59:43]

Dry run: 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...)

[mtklein, 2016-05-01 13:18:59]

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

[mtklein, 2016-05-01 13:51:12]

The CQ bit was unchecked by mtklein@google.com

[mtklein_C, 2016-05-01 13:51:24]

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

[commit-bot: I haz the power, 2016-05-01 21:49:08]

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

[commit-bot: I haz the power, 2016-05-01 22:03:21]

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

[commit-bot: I haz the power, 2016-05-01 22:03:22]

Dry run: This issue passed the CQ dry run.

[mtklein_C, 2016-05-02 13:17:29]

mtklein@chromium.org changed reviewers:
+ herb@google.com
- mtklein@google.com

[reed1, 2016-05-02 13:26:58]

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

[reed1, 2016-05-02 13:26:59]

Is there value in encapsulating this non-ptr pairing in a different helper
class?

struct {
    T fValue;
    bool fOnce;
};

Just so the places we have a non-static usage, we can simplify/canonicalize
things?

[mtklein, 2016-05-02 13:35:42]

On 2016/05/02 at 13:26:59, reed wrote:
> Is there value in encapsulating this non-ptr pairing in a different helper
class?
> 
> struct {
>     T fValue;
>     bool fOnce;
> };
> 
> Just so the places we have a non-static usage, we can simplify/canonicalize
things?

No I don't think so.  I'd rather get people used to just using SkOnce directly
than to have variants with slightly different names and APIs as appropriate for
different situations.

Keeping the T and SkOnce separate also gives us more flexibility about how they
pack into structs.  Grouped as a struct we'll always burn sizeof(T)-1 bytes to
pad that struct out to T alignment.

(SkOnce is one byte, but SkOnce is not a bool.)

[reed1, 2016-05-02 13:45:06]

lgtm

I'm not disagreeing, mostly because I don't really understand all the details,
but we we are losing the previous "safety" that the client cannot access the
field accidentally before it had been "onced". I also see that, so far, we have
not bothered to take advantage of different packing, as we always store
(possibly correctly for readability) the byte next to the value.

Perhaps I'm just wondering if we consider a prebuilt helper for safety/clarity
reasons, even if we don't require it. Thus perhaps for these sites, it would
make things worse if we used a prebuilt, so I'm not pushing for it per-se.

[mtklein, 2016-05-02 13:58:14]

On 2016/05/02 at 13:45:06, reed wrote:
> lgtm
> 
> I also see that, so far, we have not bothered to take advantage of different
packing, as we always store (possibly correctly for readability) the byte next
to the value.

This _is_ taking advantage of different packing.  If you wrap these fields in a
struct, they are packed

    T
    Once
    sizeof(T) - 1 bytes of padding

As separate fields they are packed independently.

[mtklein, 2016-05-02 14:18:33]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2016-05-02 14:18:43]

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

[commit-bot: I haz the power, 2016-05-02 14:19:43]

Description was changed from

==========
remove non-static uses of SkOncePtr

Still slowly working through all the SK_DECLARE_STATIC_FOO macros.

SkOncePtr is complicating things by having SkOncePtr delete its pointer
and SkBaseOncePtr not.  Simplify things by removing SkOncePtr, leaving
only the leaky SkBaseOncePtr.

We replace SkOncePtr<T> instead with SkOnce and T.  In most cases this
did not need to be a pointer, and in some cases here we're even saving
a few bytes by replacing SkOncePtr<T> with SkOnce and a T.

The dependency map of SK_DECLARE_STATIC_FOO is:
  SkBaseMutex -> SkBaseSemaphore -> SkBaseOncePtr

They're intertwined enough that I think I've got to do all three in one
next CL.

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

to

==========
remove non-static uses of SkOncePtr

Still slowly working through all the SK_DECLARE_STATIC_FOO macros.

SkOncePtr is complicating things by having SkOncePtr delete its pointer
and SkBaseOncePtr not.  Simplify things by removing SkOncePtr, leaving
only the leaky SkBaseOncePtr.

We replace SkOncePtr<T> instead with SkOnce and T.  In most cases this
did not need to be a pointer, and in some cases here we're even saving
a few bytes by replacing SkOncePtr<T> with SkOnce and a T.

The dependency map of SK_DECLARE_STATIC_FOO is:
  SkBaseMutex -> SkBaseSemaphore -> SkBaseOncePtr

They're intertwined enough that I think I've got to do all three in one
next CL.

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

Committed:
https://skia.googlesource.com/skia/+/59c12e3f0071acfa685346487e54a2a311066dac
==========

[commit-bot: I haz the power, 2016-05-02 14:19:44]

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