Bug 122960 – MainThreadTask class variants should use std::unique_ptr

• Status: RESOLVED
• Resolution: INVALID
• Component: New Bugs

[Zan Dobersek]

MainThreadTask class variants should use std::unique_ptr

[Zan Dobersek]

Created attachment 214451 [details]
Patch

[Anders Carlsson]

Comment on attachment 214451 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=214451&action=review

Instead of using std::unique_ptr<MainThreadTask>(new MainThreadTask(...) you can just use std::make_unique<MainThreadTask>(...).

> Source/WebCore/platform/MainThreadTask.h:487
> -    typename CrossThreadCopier<P4>::Type, MP4, typename CrossThreadCopier<P5>::Type, MP5, typename CrossThreadCopier<P6>::Type, MP6,
> -    typename CrossThreadCopier<P7>::Type, MP7, typename CrossThreadCopier<P8>::Type, MP8>::create(
> -                                                       method,
> -                                                       CrossThreadCopier<P1>::copy(parameter1), CrossThreadCopier<P2>::copy(parameter2),
> -                                                       CrossThreadCopier<P3>::copy(parameter3), CrossThreadCopier<P4>::copy(parameter4),
> -                                                       CrossThreadCopier<P5>::copy(parameter5), CrossThreadCopier<P6>::copy(parameter6),
> -                                                       CrossThreadCopier<P7>::copy(parameter7), CrossThreadCopier<P8>::copy(parameter8)).leakPtr());
> +        typename CrossThreadCopier<P4>::Type, MP4, typename CrossThreadCopier<P5>::Type, MP5, typename CrossThreadCopier<P6>::Type, MP6,
> +        typename CrossThreadCopier<P7>::Type, MP7, typename CrossThreadCopier<P8>::Type, MP8>::create(
> +            method,
> +            CrossThreadCopier<P1>::copy(parameter1), CrossThreadCopier<P2>::copy(parameter2),
> +            CrossThreadCopier<P3>::copy(parameter3), CrossThreadCopier<P4>::copy(parameter4),
> +            CrossThreadCopier<P5>::copy(parameter5), CrossThreadCopier<P6>::copy(parameter6),
> +            CrossThreadCopier<P7>::copy(parameter7), CrossThreadCopier<P8>::copy(parameter8)).release());

I don't think there's any point in re-indenting this.

[Zan Dobersek]

(In reply to comment #2)
> (From update of attachment 214451 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=214451&action=review
> 
> Instead of using std::unique_ptr<MainThreadTask>(new MainThreadTask(...) you can just use std::make_unique<MainThreadTask>(...).
> 

That would require for the constructor to be public.

If I (can?) make the constructor public, the static create() methods could be abandoned and std::make_unique<MainThreadTask>() could be used directly.

The constructor could be kept private if std::make_unique is listed as a friend in the class' definitions. I tried that already, but couldn't set up a properly templatized friend declaration for it to work.

[Darin Adler]

(In reply to comment #3)
> That would require for the constructor to be public.

Yes, I think that’s a fine thing to do.

Another option is to use “friend” for this. We have done both in the past. Either is fine depending on your preferences.

[Darin Adler]

This class has been removed, so we no longer need to do this.