SkNonCopyable should be used with private inheritance.

SkNonCopyable should be used with private inheritance.

This is mostly s/public SkNoncopyable/SkNoncopyable/g.

Two classes (SkDrawLooper::Context and SkPicture::OperationList) don't actually work with SkNoncopyable because they introduce a virtual destructor.  I added SkNoncopyableVirtual to make them work as intended.  Sort of questionable whether they really need to be noncopyable in the first place, but I guess it doesn't hurt to keep the behavior the same.

BUG=skia:

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

[reed1, 2014-04-07 17:35:03]

On 2014/04/07 17:30:53, mtklein wrote:

SkRefCntBase adds a virtual destructor. Is that the same construct you mention
for SkDrawLooper::Context?

[mtklein, 2014-04-07 18:00:22]

On 2014/04/07 17:35:03, reed1 wrote:
> On 2014/04/07 17:30:53, mtklein wrote:
> 
> SkRefCntBase adds a virtual destructor. Is that the same construct you mention
> for SkDrawLooper::Context?

Hmm, indeed it does.  Let me look into why that works and not the others.  It'd
be nice not to add SkNoncopyableVirtual.

[mtklein, 2014-04-07 18:16:28]

On 2014/04/07 18:00:22, mtklein wrote:
> On 2014/04/07 17:35:03, reed1 wrote:
> > On 2014/04/07 17:30:53, mtklein wrote:
> > 
> > SkRefCntBase adds a virtual destructor. Is that the same construct you
mention
> > for SkDrawLooper::Context?
> 
> Hmm, indeed it does.  Let me look into why that works and not the others. 
It'd
> be nice not to add SkNoncopyableVirtual.

Got it.  The virtual destructor wasn't the problem at all.  That fixed things
just because I added a new name.

The issue was that these two classes were declared nested inside classes which
themselves are noncopyable.

When we write code like

class Foo : SkNoncopyable {
  class Bar : SkNoncopyable {
  };
};

The "SkNoncopyable" for Foo gets looked up as "::SkNoncopyable".  The
"SkNoncopyable" for Bar gets looked up as "Foo::SkNoncopyable", and then
compilation fails because "Foo::SkNoncopyable" isn't visible to Bar (or anyone
but Foo itself, sort of the point of private inheritance).  The fix is to just
explicitly ask for ::SkNoncopyable when you derive from it in a nested class
that also derives from it.

[reed1, 2014-04-07 18:19:39]

On 2014/04/07 18:16:28, mtklein wrote:
> On 2014/04/07 18:00:22, mtklein wrote:
> > On 2014/04/07 17:35:03, reed1 wrote:
> > > On 2014/04/07 17:30:53, mtklein wrote:
> > > 
> > > SkRefCntBase adds a virtual destructor. Is that the same construct you
> mention
> > > for SkDrawLooper::Context?
> > 
> > Hmm, indeed it does.  Let me look into why that works and not the others. 
> It'd
> > be nice not to add SkNoncopyableVirtual.
> 
> Got it.  The virtual destructor wasn't the problem at all.  That fixed things
> just because I added a new name.
> 
> The issue was that these two classes were declared nested inside classes which
> themselves are noncopyable.
> 
> When we write code like
> 
> class Foo : SkNoncopyable {
>   class Bar : SkNoncopyable {
>   };
> };
> 
> The "SkNoncopyable" for Foo gets looked up as "::SkNoncopyable".  The
> "SkNoncopyable" for Bar gets looked up as "Foo::SkNoncopyable", and then
> compilation fails because "Foo::SkNoncopyable" isn't visible to Bar (or anyone
> but Foo itself, sort of the point of private inheritance).  The fix is to just
> explicitly ask for ::SkNoncopyable when you derive from it in a nested class
> that also derives from it.

Yea, we've discovered that a few times before (grep for ::SkNoncopyable)

[reed1, 2014-04-07 18:19:51]

lgtm

[mtklein, 2014-04-07 18:20:40]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2014-04-07 18:20:42]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/mtklein@chromium.org/226183018/40001

[commit-bot: I haz the power, 2014-04-07 19:35:12]

Change committed as 14081