[Webkit-unassigned] [Bug 189335] [RunLoopGeneric] OneShotTimer should not remain "isActive" after fired

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=189335

--- Comment #16 from Stephan Szabo <stephan.szabo at sony.com> ---
(In reply to Geoffrey Garen from comment #15)
> Comment on attachment 349292 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=349292&action=review
> 
> > Source/WTF/wtf/generic/RunLoopGeneric.cpp:59
> > +        if (isActive())
> > +            updateReadyTime();
> 
> Why did the ready time update also move? For a non-repeating timer, it seems
> irrelevant. For a repeating timer, wouldn't I want to know the *next*
> scheduled repeat time, rather than the one that just happened?

Yes, that's true, and I'm guessing that the time the function takes shouldn't be effectively added to the repeat time, so that should move back.

> > Tools/TestWebKitAPI/Tests/WTF/RunLoop.cpp:88
> > +            RunLoop::current().dispatch([this] {
> 
> Delaying the action of this test until the next runloop iteration seems to
> skip testing the interesting case. The interesting case is whether
> isActive() is true or false *inside the timer callback function*, not at
> some later time.
> 
> Can you elaborate on why this test changed to compute its result on the next
> runloop iteration?

I'm not sure why it was done originally, but it does seem strange. I'll recheck since that test file has also seemingly changed since the original patch.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20210331/3bd9de0e/attachment.htm>