Bug 218874 – Removed DeferrableTaskTimer

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

[Geoffrey Garen]

Removed DeferrableTaskTimer

[Geoffrey Garen]

Created attachment 414054 [details]
Patch

[Chris Dumez]

Comment on attachment 414054 [details]
Patch

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

> Source/WebCore/platform/Timer.h:112
> +    static void startOneShot(Seconds interval, WTF::Function<void()>&& function)

Maybe it's just me but I preferred the previous order: Lambda first and then the delay. BTW, seems weird to call this interval instead of delay given that it is one shot.

The reason for my preference is that it would be consistent with window.setTimeout(() => { }, 100);

Finally, I wish we could find a better name for this function than startOneShot() given that it is a static. Maybe something like Timer::doAfter() or runAfter() (similarly to dispatch_after()).

> Source/WebKit/ChangeLog:12
> +        * UIProcess/WebPageProxy.h: Use WTF::RunLoop::Timer instead of

Good catch.

> Source/WebKit/UIProcess/WebPageProxy.h:2758
> +    WTF::RunLoop::Timer<WebPageProxy> m_updateReportedMediaCaptureStateTimer;

Why WTF:: ?

[Geoffrey Garen]

> > Source/WebCore/platform/Timer.h:112
> > +    static void startOneShot(Seconds interval, WTF::Function<void()>&& function)
> 
> Maybe it's just me but I preferred the previous order: Lambda first and then
> the delay. BTW, seems weird to call this interval instead of delay given
> that it is one shot.
> 
> The reason for my preference is that it would be consistent with
> window.setTimeout(() => { }, 100);

For the order, my thinking was that Cocoa APIs have the habit of putting the lambda last, and the reasoning is that it reads better because a lambda is a disruptive amount of text that makes it hard to see what comes after.

What do you think of that?

> Finally, I wish we could find a better name for this function than
> startOneShot() given that it is a static. Maybe something like
> Timer::doAfter() or runAfter() (similarly to dispatch_after()).

How about schedule()?

> > Source/WebKit/UIProcess/WebPageProxy.h:2758
> > +    WTF::RunLoop::Timer<WebPageProxy> m_updateReportedMediaCaptureStateTimer;
> 
> Why WTF:: ?

Will fix.

[Chris Dumez]

(In reply to Geoffrey Garen from comment #3)
> > > Source/WebCore/platform/Timer.h:112
> > > +    static void startOneShot(Seconds interval, WTF::Function<void()>&& function)
> > 
> > Maybe it's just me but I preferred the previous order: Lambda first and then
> > the delay. BTW, seems weird to call this interval instead of delay given
> > that it is one shot.
> > 
> > The reason for my preference is that it would be consistent with
> > window.setTimeout(() => { }, 100);
> 
> For the order, my thinking was that Cocoa APIs have the habit of putting the
> lambda last, and the reasoning is that it reads better because a lambda is a
> disruptive amount of text that makes it hard to see what comes after.
> 
> What do you think of that?

Not a big deal either way. It is true that dispatch_after() takes delay first and lambda last.

> 
> > Finally, I wish we could find a better name for this function than
> > startOneShot() given that it is a static. Maybe something like
> > Timer::doAfter() or runAfter() (similarly to dispatch_after()).
> 
> How about schedule()?

Sure, schedule() would be better IMO.

> 
> > > Source/WebKit/UIProcess/WebPageProxy.h:2758
> > > +    WTF::RunLoop::Timer<WebPageProxy> m_updateReportedMediaCaptureStateTimer;
> > 
> > Why WTF:: ?
> 
> Will fix.

[Geoffrey Garen]

Created attachment 414087 [details]
Patch for landing

[EWS]

Committed r269802: <https://trac.webkit.org/changeset/269802>

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

[Radar WebKit Bug Importer]

<rdar://problem/71386106>