Grant independence to SkBaseMutex on Windows.

Grant independence to SkBaseMutex on Windows.

Under POSIX, class SkMutex inherits from struct SkBaseMutex.
Since we can't have a POD mutex on Windows, we used to just
typedef SkMutex SkBaseMutex there. However, that makes it
impossible to forward-declare SkBaseMutex consistently across
platforms. With this CL we declare an empty struct SkBaseMutex
on Windows, which should have 0 cost but make the compiler happy.

R=bungeman, mtklein
BUG=skia:

Committed: https://skia.googlesource.com/skia/+/cad0cb2a2dd23ceee03f24954c491344aef18c2c

[tomhudson, 2014-06-30 22:21:24]

Is this what you meant? Fired off 3 win trybots to see if it's clean...

[mtklein, 2014-06-30 22:24:01]

https://codereview.chromium.org/364473002/diff/1/src/ports/SkMutex_win.h
File src/ports/SkMutex_win.h (right):

https://codereview.chromium.org/364473002/diff/1/src/ports/SkMutex_win.h#newc...
src/ports/SkMutex_win.h:37: struct SkBaseMutex {
Don't you have to do it the other way around?  SkBaseMutex has now lost its
acquire() and release() methods, among other things.

struct SkBaseMutex {
  ... real implementation ...
};
class SkMutex : public SkBaseMutex {};

ought to work?

[tomhudson, 2014-06-30 22:30:09]

On 2014/06/30 22:24:01, mtklein wrote:
...
> ought to work?

Yep, that's what we've been hacking here since about 30 seconds before you
posted, although this version left the constructor & destructor on SkMutex
because you aren't supposed to be able to construct the Base class, right?

[mtklein, 2014-06-30 22:40:11]

On 2014/06/30 22:30:09, tomhudson wrote:
> On 2014/06/30 22:24:01, mtklein wrote:
> ...
> > ought to work?
> 
> Yep, that's what we've been hacking here since about 30 seconds before you
> posted, although this version left the constructor & destructor on SkMutex
> because you aren't supposed to be able to construct the Base class, right?

Pretty much.  It's a little more subtle.  It's not that you're not supposed to
be able to construct the base class, but rather, you're supposed to be able to
link-time initialize it.  In C++98 terms, that means it must have no
constructor, destructor, copy constructors, static data, etc... must be real
C-style POD.  C++11 relaxes that a bit, but more or less, we want it to have no
constructor.

Now, that's all silly and moot on Windows, because we have no idea what to
link-time initialize that SkBaseMutex with.  Microsoft hasn't ever published
something that we could initialize the CriticalSection with like pthreads does
with PTHREAD_MUTEX_INITIALIZER.  If any sane coder wrote the CriticalSection,
it'd be zeroes, but really, who knows?  That's why we typedef them together
there: on Windows, we do have SkBaseMutex::SkBaseMutex and it is indeed called
before main to set up the mutex.

So I think in your case you will need to put the constructor and destructor into
SkBaseMutex.  Though we certainly could do some fun follow-up archaeology where
dig in and see what InitializeCriticalSection does and try to create a bootleg
static initializer that mimics it.  I'd worry it varies from Windows release to
Window release though.

[bungeman-skia, 2014-06-30 22:45:39]

On 2014/06/30 22:40:11, mtklein wrote:
> On 2014/06/30 22:30:09, tomhudson wrote:
> > On 2014/06/30 22:24:01, mtklein wrote:
> So I think in your case you will need to put the constructor and destructor
into
> SkBaseMutex.  Though we certainly could do some fun follow-up archaeology
where
> dig in and see what InitializeCriticalSection does and try to create a bootleg
> static initializer that mimics it.  I'd worry it varies from Windows release
to
> Window release though.

It's different between XP and Vista. They haven't change it since then, but who
knows?

[tomhudson, 2014-06-30 22:46:16]

On 2014/06/30 22:40:11, mtklein wrote:
> So I think in your case you will need to put the constructor and destructor
into
> SkBaseMutex.

Sigh, I'm afraid you're right. And I'd rather keep doing this - even if I've
devolved to semi-blind hacking - than revert the multithreading fix.

[mtklein, 2014-06-30 22:47:47]

> Sigh, I'm afraid you're right. And I'd rather keep doing this - even if I've
> devolved to semi-blind hacking - than revert the multithreading fix.

Yeah man, Sheriff Sunshine says roll forward!

[tomhudson, 2014-06-30 23:08:09]

On 2014/06/30 22:47:47, mtklein wrote:
> > Sigh, I'm afraid you're right. And I'd rather keep doing this - even if I've
> > devolved to semi-blind hacking - than revert the multithreading fix.
> 
> Yeah man, Sheriff Sunshine says roll forward!

Finally passed our trybots, waiting for
Canary-Chrome-Win7-Ninja-x86-SharedLib_ToT-Trybot to engage.
Assuming that's good, can I get review? I'm leaving in 10 minutes but can try to
CQ it later tonight...

[mtklein, 2014-06-30 23:14:46]

https://codereview.chromium.org/364473002/diff/100001/src/ports/SkMutex_win.h
File src/ports/SkMutex_win.h (right):

https://codereview.chromium.org/364473002/diff/100001/src/ports/SkMutex_win.h...
src/ports/SkMutex_win.h:73: class SkMutex : public SkBaseMutex {
Can't just class SkMutex : public SkBaseMutex {}; ?

If not, LGTM.

[tomhudson, 2014-07-01 13:15:20]

The CQ bit was checked by tomhudson@google.com

[commit-bot: I haz the power, 2014-07-01 13:15:46]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/tomhudson@chromium.org/364473002/120001

[commit-bot: I haz the power, 2014-07-01 13:26:53]

Change committed as cad0cb2a2dd23ceee03f24954c491344aef18c2c