SK_ONCE for SkData

SK_ONCE for SkData and SkPathRef

Adds SK_ONCE_FRIEND, to allow SK_DEF_ONCE code to be friends with a class.  This had to go in include/core to be visible to headers there.

BUG=

Committed: http://code.google.com/p/skia/source/detail?r=11914

[mtklein, 2013-10-10 21:44:24]

Will wait on both reed and bungeman this time.

[bungeman-skia, 2013-10-11 14:35:52]

There are a lot of constraints here, and many ways to try to get around them.
This isn't the worst, but I'm not yet convinced it's the best either. meh?

[mtklein, 2013-10-15 15:52:29]

On 2013/10/11 14:35:52, bungeman1 wrote:
> There are a lot of constraints here, and many ways to try to get around them.
> This isn't the worst, but I'm not yet convinced it's the best either. meh?

So, in the interest of moving forward, do you have an alternative suggestion?

[bungeman-skia, 2013-10-15 15:54:06]

On 2013/10/15 15:52:29, mtklein wrote:
> On 2013/10/11 14:35:52, bungeman1 wrote:
> > There are a lot of constraints here, and many ways to try to get around
them.
> > This isn't the worst, but I'm not yet convinced it's the best either. meh?
> 
> So, in the interest of moving forward, do you have an alternative suggestion?

No, so lgtm, but with the proviso that this is more of a 'meh'.

[mtklein, 2013-10-22 20:52:25]

Have another look?  After lots of stewing and some cribbing from std::call_once,
I've got a new approach here that needs no friends:

SK_DECLARE_STATIC_ONCE(once_name)
...
SkOnce(&once_name, &function_to_call, arg_for_function);

[reed1, 2013-10-22 21:00:18]

lgtm, but defers to ben

[bungeman-skia, 2013-10-22 21:06:11]

https://codereview.chromium.org/26491003/diff/59001/src/core/SkOnce.h
File src/core/SkOnce.h (right):

https://codereview.chromium.org/26491003/diff/59001/src/core/SkOnce.h#newcode21
src/core/SkOnce.h:21: static SkOnceFlag name = { { PTHREAD_MUTEX_INITIALIZER },
false }
This doesn't work on Windows?

[mtklein, 2013-10-22 21:08:24]

https://codereview.chromium.org/26491003/diff/59001/src/core/SkOnce.h
File src/core/SkOnce.h (right):

https://codereview.chromium.org/26491003/diff/59001/src/core/SkOnce.h#newcode21
src/core/SkOnce.h:21: static SkOnceFlag name = { { PTHREAD_MUTEX_INITIALIZER },
false }
On 2013/10/22 21:06:11, bungeman1 wrote:
> This doesn't work on Windows?

Does SK_DECLARE_STATIC_MUTEX not work on Windows either?  This just inlines it.

[mtklein, 2013-10-22 21:09:35]

On 2013/10/22 21:08:24, mtklein wrote:
> https://codereview.chromium.org/26491003/diff/59001/src/core/SkOnce.h
> File src/core/SkOnce.h (right):
> 
>
https://codereview.chromium.org/26491003/diff/59001/src/core/SkOnce.h#newcode21
> src/core/SkOnce.h:21: static SkOnceFlag name = { { PTHREAD_MUTEX_INITIALIZER
},
> false }
> On 2013/10/22 21:06:11, bungeman1 wrote:
> > This doesn't work on Windows?
> 
> Does SK_DECLARE_STATIC_MUTEX not work on Windows either?  This just inlines
it.

Oh wait, I see.

[mtklein, 2013-10-22 21:22:08]

https://codereview.chromium.org/26491003/diff/59001/src/core/SkOnce.h
File src/core/SkOnce.h (right):

https://codereview.chromium.org/26491003/diff/59001/src/core/SkOnce.h#newcode21
src/core/SkOnce.h:21: static SkOnceFlag name = { { PTHREAD_MUTEX_INITIALIZER },
false }
On 2013/10/22 21:08:24, mtklein wrote:
> On 2013/10/22 21:06:11, bungeman1 wrote:
> > This doesn't work on Windows?
> 
> Does SK_DECLARE_STATIC_MUTEX not work on Windows either?  This just inlines
it.

Figure this is the way to go now?  How does the mutex get intitialized if you
leave off its initializer?  zero?

[bungeman-skia, 2013-10-22 21:25:58]

On 2013/10/22 21:22:08, mtklein wrote:
> https://codereview.chromium.org/26491003/diff/59001/src/core/SkOnce.h
> File src/core/SkOnce.h (right):
> 
>
https://codereview.chromium.org/26491003/diff/59001/src/core/SkOnce.h#newcode21
> src/core/SkOnce.h:21: static SkOnceFlag name = { { PTHREAD_MUTEX_INITIALIZER
},
> false }
> On 2013/10/22 21:08:24, mtklein wrote:
> > On 2013/10/22 21:06:11, bungeman1 wrote:
> > > This doesn't work on Windows?
> > 
> > Does SK_DECLARE_STATIC_MUTEX not work on Windows either?  This just inlines
> it.
> 
> Figure this is the way to go now?  How does the mutex get intitialized if you
> leave off its initializer?  zero?

At the moment there is no documented or supported way to create a statically
initialized CRITICAL_SECTION object on Windows, so there is no means of
statically creating one (or brace initializing it).

[mtklein, 2013-10-22 21:28:02]

On 2013/10/22 21:25:58, bungeman1 wrote:
> On 2013/10/22 21:22:08, mtklein wrote:
> > https://codereview.chromium.org/26491003/diff/59001/src/core/SkOnce.h
> > File src/core/SkOnce.h (right):
> > 
> >
>
https://codereview.chromium.org/26491003/diff/59001/src/core/SkOnce.h#newcode21
> > src/core/SkOnce.h:21: static SkOnceFlag name = { { PTHREAD_MUTEX_INITIALIZER
> },
> > false }
> > On 2013/10/22 21:08:24, mtklein wrote:
> > > On 2013/10/22 21:06:11, bungeman1 wrote:
> > > > This doesn't work on Windows?
> > > 
> > > Does SK_DECLARE_STATIC_MUTEX not work on Windows either?  This just
inlines
> > it.
> > 
> > Figure this is the way to go now?  How does the mutex get intitialized if
you
> > leave off its initializer?  zero?
> 
> At the moment there is no documented or supported way to create a statically
> initialized CRITICAL_SECTION object on Windows, so there is no means of
> statically creating one (or brace initializing it).

Ah, so we just shrug our shoulders and say, tests pass, let's go with it?

[mtklein, 2013-10-22 21:30:44]

That is to say, this doesn't change the status quo from the current SkOnce.h
code that uses SK_DECLARE_STATIC_MUTEX now right?

[bungeman-skia, 2013-10-22 21:38:59]

https://codereview.chromium.org/26491003/diff/159001/src/core/SkOnce.h
File src/core/SkOnce.h (right):

https://codereview.chromium.org/26491003/diff/159001/src/core/SkOnce.h#newcode26
src/core/SkOnce.h:26: static SkOnceFlag name = { false }
There is no call to InitializeCriticalSection here. On Windows, the old code was
ok, because SkBaseMutex is just a typedef of SkMutex and
SK_DECLARE_STATIC_MUTEX(sk_once_##name##_mutex); means that the SkMutex
constructor will be called at static initialization phase of startup on Windows.

[bungeman-skia, 2013-10-22 22:00:41]

On 2013/10/22 21:38:59, bungeman1 wrote:
> https://codereview.chromium.org/26491003/diff/159001/src/core/SkOnce.h
> File src/core/SkOnce.h (right):
> 
>
https://codereview.chromium.org/26491003/diff/159001/src/core/SkOnce.h#newcode26
> src/core/SkOnce.h:26: static SkOnceFlag name = { false }
> There is no call to InitializeCriticalSection here. On Windows, the old code
was
> ok, because SkBaseMutex is just a typedef of SkMutex and
> SK_DECLARE_STATIC_MUTEX(sk_once_##name##_mutex); means that the SkMutex
> constructor will be called at static initialization phase of startup on
Windows.

Ok, I totally take this back. Turns out that subsequent values will be
value-initialized, which implies default constructed. On the other hand, I think
I would prefer that to be written out, like

{ false, SkBaseMutex() }

or something to make this explicit.

[mtklein, 2013-10-22 22:17:18]

On 2013/10/22 22:00:41, bungeman1 wrote:
> On 2013/10/22 21:38:59, bungeman1 wrote:
> > https://codereview.chromium.org/26491003/diff/159001/src/core/SkOnce.h
> > File src/core/SkOnce.h (right):
> > 
> >
>
https://codereview.chromium.org/26491003/diff/159001/src/core/SkOnce.h#newcode26
> > src/core/SkOnce.h:26: static SkOnceFlag name = { false }
> > There is no call to InitializeCriticalSection here. On Windows, the old code
> was
> > ok, because SkBaseMutex is just a typedef of SkMutex and
> > SK_DECLARE_STATIC_MUTEX(sk_once_##name##_mutex); means that the SkMutex
> > constructor will be called at static initialization phase of startup on
> Windows.
> 
> Ok, I totally take this back. Turns out that subsequent values will be
> value-initialized, which implies default constructed. On the other hand, I
think
> I would prefer that to be written out, like
> 
> { false, SkBaseMutex() }
> 
> or something to make this explicit.

SGTM.  Have another look?

[bungeman-skia, 2013-10-22 22:21:55]

lgtm

[commit-bot: I haz the power, 2013-10-23 14:26:58]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/mtklein@google.com/26491003/479001

[commit-bot: I haz the power, 2013-10-23 14:44:19]

Change committed as 11914