Remove SK_SUPPORT_LEGACY_PUBLICEFFECTCONSTRUCTORS flag.

Remove SK_SUPPORT_LEGACY_PUBLICEFFECTCONSTRUCTORS flag.

Neither Chrome nor Android uses this flag anymore.
Make sure all constructors touched that only have one parameter are marked as 'explicit'.

BUG=2187
Committed: http://code.google.com/p/skia/source/detail?r=14749

[Dominik Grewe, 2014-05-15 10:52:42]

ptal.

[scroggo, 2014-05-15 14:56:42]

lgtm

https://codereview.chromium.org/282203004/diff/40001/include/effects/SkBlurIm...
File include/effects/SkBlurImageFilter.h (right):

https://codereview.chromium.org/282203004/diff/40001/include/effects/SkBlurIm...
include/effects/SkBlurImageFilter.h:30: SkImageFilter* input = NULL,
Defaults not needed.

https://codereview.chromium.org/282203004/diff/40001/include/effects/SkColorM...
File include/effects/SkColorMatrixFilter.h (right):

https://codereview.chromium.org/282203004/diff/40001/include/effects/SkColorM...
include/effects/SkColorMatrixFilter.h:42: explicit SkColorMatrixFilter(const
SkColorMatrix&);
Not picking on SkColorMatrixFilter in particular, but are there any of these
that we do not want anyone subclassing? If so, would it make sense to make their
constructors private?

(I'm not necessarily advocating that we make constructors private, but I just
want to make sure we deliberately chose protected instead.)

https://codereview.chromium.org/282203004/diff/40001/include/effects/SkMatrix...
File include/effects/SkMatrixConvolutionImageFilter.h (right):

https://codereview.chromium.org/282203004/diff/40001/include/effects/SkMatrix...
include/effects/SkMatrixConvolutionImageFilter.h:78: SkImageFilter* input =
NULL,
While we're moving this, I think having default parameters on a protected
constructor does not make any sense. The only place we should have a default
parameter (if anywhere) is on the public facing factory.

https://codereview.chromium.org/282203004/diff/40001/include/effects/SkOffset...
File include/effects/SkOffsetImageFilter.h (right):

https://codereview.chromium.org/282203004/diff/40001/include/effects/SkOffset...
include/effects/SkOffsetImageFilter.h:26: SkOffsetImageFilter(SkScalar dx,
SkScalar dy, SkImageFilter* input = NULL,
Once again, these default params are unnecessary.

https://codereview.chromium.org/282203004/diff/40001/include/effects/SkXfermo...
File include/effects/SkXfermodeImageFilter.h (right):

https://codereview.chromium.org/282203004/diff/40001/include/effects/SkXfermo...
include/effects/SkXfermodeImageFilter.h:47: SkImageFilter* foreground = NULL,
const CropRect* cropRect = NULL);
Please remove the defaults.

[scroggo, 2014-05-15 14:57:38]

On 2014/05/15 14:56:42, scroggo wrote:
> lgtm
> 
>
https://codereview.chromium.org/282203004/diff/40001/include/effects/SkBlurIm...
> File include/effects/SkBlurImageFilter.h (right):
> 
>
https://codereview.chromium.org/282203004/diff/40001/include/effects/SkBlurIm...
> include/effects/SkBlurImageFilter.h:30: SkImageFilter* input = NULL,
> Defaults not needed.
> 
>
https://codereview.chromium.org/282203004/diff/40001/include/effects/SkColorM...
> File include/effects/SkColorMatrixFilter.h (right):
> 
>
https://codereview.chromium.org/282203004/diff/40001/include/effects/SkColorM...
> include/effects/SkColorMatrixFilter.h:42: explicit SkColorMatrixFilter(const
> SkColorMatrix&);
> Not picking on SkColorMatrixFilter in particular, but are there any of these
> that we do not want anyone subclassing? If so, would it make sense to make
their
> constructors private?
> 
> (I'm not necessarily advocating that we make constructors private, but I just
> want to make sure we deliberately chose protected instead.)
> 
>
https://codereview.chromium.org/282203004/diff/40001/include/effects/SkMatrix...
> File include/effects/SkMatrixConvolutionImageFilter.h (right):
> 
>
https://codereview.chromium.org/282203004/diff/40001/include/effects/SkMatrix...
> include/effects/SkMatrixConvolutionImageFilter.h:78: SkImageFilter* input =
> NULL,
> While we're moving this, I think having default parameters on a protected
> constructor does not make any sense. The only place we should have a default
> parameter (if anywhere) is on the public facing factory.
> 
>
https://codereview.chromium.org/282203004/diff/40001/include/effects/SkOffset...
> File include/effects/SkOffsetImageFilter.h (right):
> 
>
https://codereview.chromium.org/282203004/diff/40001/include/effects/SkOffset...
> include/effects/SkOffsetImageFilter.h:26: SkOffsetImageFilter(SkScalar dx,
> SkScalar dy, SkImageFilter* input = NULL,
> Once again, these default params are unnecessary.
> 
>
https://codereview.chromium.org/282203004/diff/40001/include/effects/SkXfermo...
> File include/effects/SkXfermodeImageFilter.h (right):
> 
>
https://codereview.chromium.org/282203004/diff/40001/include/effects/SkXfermo...
> include/effects/SkXfermodeImageFilter.h:47: SkImageFilter* foreground = NULL,
> const CropRect* cropRect = NULL);
> Please remove the defaults.

Also, minor nit: can you add the fact that you're making constructors explicit
(where logical) to the changelist description?

[Dominik Grewe, 2014-05-15 15:14:43]

On 2014/05/15 14:57:38, scroggo wrote:
> On 2014/05/15 14:56:42, scroggo wrote:
> > lgtm
> > 
> >
>
https://codereview.chromium.org/282203004/diff/40001/include/effects/SkBlurIm...
> > File include/effects/SkBlurImageFilter.h (right):
> > 
> >
>
https://codereview.chromium.org/282203004/diff/40001/include/effects/SkBlurIm...
> > include/effects/SkBlurImageFilter.h:30: SkImageFilter* input = NULL,
> > Defaults not needed.
> > 
> >
>
https://codereview.chromium.org/282203004/diff/40001/include/effects/SkColorM...
> > File include/effects/SkColorMatrixFilter.h (right):
> > 
> >
>
https://codereview.chromium.org/282203004/diff/40001/include/effects/SkColorM...
> > include/effects/SkColorMatrixFilter.h:42: explicit SkColorMatrixFilter(const
> > SkColorMatrix&);
> > Not picking on SkColorMatrixFilter in particular, but are there any of these
> > that we do not want anyone subclassing? If so, would it make sense to make
> their
> > constructors private?
> > 
> > (I'm not necessarily advocating that we make constructors private, but I
just
> > want to make sure we deliberately chose protected instead.)
> > 
> >
>
https://codereview.chromium.org/282203004/diff/40001/include/effects/SkMatrix...
> > File include/effects/SkMatrixConvolutionImageFilter.h (right):
> > 
> >
>
https://codereview.chromium.org/282203004/diff/40001/include/effects/SkMatrix...
> > include/effects/SkMatrixConvolutionImageFilter.h:78: SkImageFilter* input =
> > NULL,
> > While we're moving this, I think having default parameters on a protected
> > constructor does not make any sense. The only place we should have a default
> > parameter (if anywhere) is on the public facing factory.
> > 
> >
>
https://codereview.chromium.org/282203004/diff/40001/include/effects/SkOffset...
> > File include/effects/SkOffsetImageFilter.h (right):
> > 
> >
>
https://codereview.chromium.org/282203004/diff/40001/include/effects/SkOffset...
> > include/effects/SkOffsetImageFilter.h:26: SkOffsetImageFilter(SkScalar dx,
> > SkScalar dy, SkImageFilter* input = NULL,
> > Once again, these default params are unnecessary.
> > 
> >
>
https://codereview.chromium.org/282203004/diff/40001/include/effects/SkXfermo...
> > File include/effects/SkXfermodeImageFilter.h (right):
> > 
> >
>
https://codereview.chromium.org/282203004/diff/40001/include/effects/SkXfermo...
> > include/effects/SkXfermodeImageFilter.h:47: SkImageFilter* foreground =
NULL,
> > const CropRect* cropRect = NULL);
> > Please remove the defaults.
> 
> Also, minor nit: can you add the fact that you're making constructors explicit
> (where logical) to the changelist description?

Good point about the default parameters. I've removed them.
I've also updated the descriptions.

I leave the question about which constructors to be private and which protected
to you guys.

[scroggo, 2014-05-15 15:21:29]

On 2014/05/15 15:14:43, Dominik Grewe wrote:
> On 2014/05/15 14:57:38, scroggo wrote:
> > On 2014/05/15 14:56:42, scroggo wrote:
> > > lgtm
> > > 
> > >
> >
>
https://codereview.chromium.org/282203004/diff/40001/include/effects/SkBlurIm...
> > > File include/effects/SkBlurImageFilter.h (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/282203004/diff/40001/include/effects/SkBlurIm...
> > > include/effects/SkBlurImageFilter.h:30: SkImageFilter* input = NULL,
> > > Defaults not needed.
> > > 
> > >
> >
>
https://codereview.chromium.org/282203004/diff/40001/include/effects/SkColorM...
> > > File include/effects/SkColorMatrixFilter.h (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/282203004/diff/40001/include/effects/SkColorM...
> > > include/effects/SkColorMatrixFilter.h:42: explicit
SkColorMatrixFilter(const
> > > SkColorMatrix&);
> > > Not picking on SkColorMatrixFilter in particular, but are there any of
these
> > > that we do not want anyone subclassing? If so, would it make sense to make
> > their
> > > constructors private?
> > > 
> > > (I'm not necessarily advocating that we make constructors private, but I
> just
> > > want to make sure we deliberately chose protected instead.)
> > > 
> > >
> >
>
https://codereview.chromium.org/282203004/diff/40001/include/effects/SkMatrix...
> > > File include/effects/SkMatrixConvolutionImageFilter.h (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/282203004/diff/40001/include/effects/SkMatrix...
> > > include/effects/SkMatrixConvolutionImageFilter.h:78: SkImageFilter* input
=
> > > NULL,
> > > While we're moving this, I think having default parameters on a protected
> > > constructor does not make any sense. The only place we should have a
default
> > > parameter (if anywhere) is on the public facing factory.
> > > 
> > >
> >
>
https://codereview.chromium.org/282203004/diff/40001/include/effects/SkOffset...
> > > File include/effects/SkOffsetImageFilter.h (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/282203004/diff/40001/include/effects/SkOffset...
> > > include/effects/SkOffsetImageFilter.h:26: SkOffsetImageFilter(SkScalar dx,
> > > SkScalar dy, SkImageFilter* input = NULL,
> > > Once again, these default params are unnecessary.
> > > 
> > >
> >
>
https://codereview.chromium.org/282203004/diff/40001/include/effects/SkXfermo...
> > > File include/effects/SkXfermodeImageFilter.h (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/282203004/diff/40001/include/effects/SkXfermo...
> > > include/effects/SkXfermodeImageFilter.h:47: SkImageFilter* foreground =
> NULL,
> > > const CropRect* cropRect = NULL);
> > > Please remove the defaults.
> > 
> > Also, minor nit: can you add the fact that you're making constructors
explicit
> > (where logical) to the changelist description?
> 
> Good point about the default parameters. I've removed them.
> I've also updated the descriptions.
> 
> I leave the question about which constructors to be private and which
protected
> to you guys.

lgtm

[reed1, 2014-05-15 15:30:40]

lgtm

[Dominik Grewe, 2014-05-15 15:35:03]

The CQ bit was checked by dominikg@chromium.org

[commit-bot: I haz the power, 2014-05-15 15:35:27]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/dominikg@chromium.org/282203004/60001

[commit-bot: I haz the power, 2014-05-15 15:40:55]

Change committed as 14749