Fix race on creating the default typeface.

Fix race on creating the default typeface.

BUG=skia:1906
R=mtklein@google.com, reed@google.com

Committed: https://code.google.com/p/skia/source/detail?r=12993

[bungeman-skia, 2013-12-20 16:01:40]

We either need something like this, or we need to relax the the restriction that
RefDefault/GetDefaultTypeface always returns the same typeface (and remove the
tests around this in tests/Typeface.cpp). The related bug has shown up at least
twice on the bots recently and has cause some unrelated changes to be reverted.

Note that even if we relax the restriction on returning the same typeface we
will still need to annotate the race to prevent tsan from complaining about this
in the future (and also ensure that the pointer write doesn't tear, but that
shouldn't be an issue, I think).

[reed1, 2013-12-20 16:12:42]

public api lgtm -- defer to klein mike for the rest

[mtklein, 2013-12-31 00:10:20]

Looks great.  Just the 4s and 3s jumping out at me as tricky.

https://codereview.chromium.org/113543005/diff/1/src/core/SkTypeface.cpp
File src/core/SkTypeface.cpp (right):

https://codereview.chromium.org/113543005/diff/1/src/core/SkTypeface.cpp#newc...
src/core/SkTypeface.cpp:73: static const uint32_t FONT_STYLE_COUNT = 4;
Can we cut down the 4-redundancy?  Something like this?

static SkOnceFlag gDefaultTypefaceOnce[] = { SK_ONCE_INIT, SK_ONCE_INIT,
SK_ONCE_INIT, SK_ONCE_INIT };
static const unsigned FONT_STYLE_COUNT = SK_ARRAY_COUNT(gDefaultTypefaceOnce);
static SkTypeface* gDefaultTypefaces[FONT_STYLE_COUNT] = { /*NULL, NULL, ... */
};
...
SkASSERT((unsigned)style < FONT_STYLE_COUNT);
SkASSERT(SkIsPow2(FONT_STYLE_COUNT)); 
style = (Style)(style & (FONT_STYLE_COUNT-1));

[bungeman-skia, 2014-01-09 17:01:06]

Updated to take comments into account. PTAL.

[mtklein, 2014-01-09 17:08:22]

lgtm

https://codereview.chromium.org/113543005/diff/110001/src/core/SkTypeface.cpp
File src/core/SkTypeface.cpp (right):

https://codereview.chromium.org/113543005/diff/110001/src/core/SkTypeface.cpp...
src/core/SkTypeface.cpp:81:
SK_COMPILE_ASSERT(SkTIsPow2<FONT_STYLE_COUNT>::value,
FONT_STYLE_COUNT_not_power_of_2);
Since we're leaving SkTIsPow2 in this file, why not inline it?

SK_COMPILE_ASSERT(FONT_STYLE_COUNT & (FONT_STYLE_COUNT-1) == 0,
FONT_STYLE_COUNT_not_power_of_2);

[bungeman-skia, 2014-01-09 17:12:56]

https://codereview.chromium.org/113543005/diff/110001/src/core/SkTypeface.cpp
File src/core/SkTypeface.cpp (right):

https://codereview.chromium.org/113543005/diff/110001/src/core/SkTypeface.cpp...
src/core/SkTypeface.cpp:81:
SK_COMPILE_ASSERT(SkTIsPow2<FONT_STYLE_COUNT>::value,
FONT_STYLE_COUNT_not_power_of_2);
On 2014/01/09 17:08:22, mtklein wrote:
> Since we're leaving SkTIsPow2 in this file, why not inline it?
> 
> SK_COMPILE_ASSERT(FONT_STYLE_COUNT & (FONT_STYLE_COUNT-1) == 0,
> FONT_STYLE_COUNT_not_power_of_2);

Because I like naming it (it isn't immediately obvious what it's doing) and also
because it might be nice to promote it at some point (and without a name, it
isn't going to happen).

[bungeman-skia, 2014-01-09 17:13:36]

Committed patchset #3 manually as r12993 (presubmit successful).