Bug 214340 – There should be only one RunLoop Timer class

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

[Geoffrey Garen]

There should only be one RunLoop Timer class

[Geoffrey Garen]

Created attachment 404318 [details]
WIP

[Geoffrey Garen]

Created attachment 404404 [details]
WIP

[Geoffrey Garen]

Created attachment 404456 [details]
WIP

[Geoffrey Garen]

Created attachment 404463 [details]
WIP

[Geoffrey Garen]

Created attachment 404477 [details]
WIP

[Geoffrey Garen]

Created attachment 404575 [details]
WIP

[Geoffrey Garen]

Created attachment 404632 [details]
Patch

[Geoffrey Garen]

Created attachment 404633 [details]
Patch

[Darin Adler]

Comment on attachment 404633 [details]
Patch

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

Wish there was slightly more abstraction so we didn’t have to say COCOA_EVENT_LOOP all the time.

> Source/WTF/wtf/RunLoop.h:53
> +typedef HashSet<RefPtr<SchedulePair>, SchedulePairHash> SchedulePairHashSet;

In new code we should use "using" instead of typedef.

> Source/WTF/wtf/cf/RunLoopCF.cpp:94
> +        Function<void()> function(static_cast<Function<void()>::Impl*>(context));

Kind of wish this function constructor was named since it takes ownership. A sort of asymmetry with leakImpl.

> Source/WTF/wtf/cf/RunLoopCF.cpp:122
> +        TimerBase* timer = static_cast<TimerBase*>(context);

auto?

[Geoffrey Garen]

> Wish there was slightly more abstraction so we didn’t have to say
> COCOA_EVENT_LOOP all the time.

Yeah, me too. I'm going to think about this more.

> > Source/WTF/wtf/RunLoop.h:53
> > +typedef HashSet<RefPtr<SchedulePair>, SchedulePairHash> SchedulePairHashSet;
> 
> In new code we should use "using" instead of typedef.

Fixed.

> > Source/WTF/wtf/cf/RunLoopCF.cpp:94
> > +        Function<void()> function(static_cast<Function<void()>::Impl*>(context));
> 
> Kind of wish this function constructor was named since it takes ownership. A
> sort of asymmetry with leakImpl.

I'll take a look at making the constructor private and adding a friend adoptImpl function.

> > Source/WTF/wtf/cf/RunLoopCF.cpp:122
> > +        TimerBase* timer = static_cast<TimerBase*>(context);
> 
> auto?

Fixed.

[Geoffrey Garen]

Created attachment 404690 [details]
Patch for landing

[EWS]

Committed r264586: <https://trac.webkit.org/changeset/264586>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 404690 [details].

[Radar WebKit Bug Importer]

<rdar://problem/65801842>