[Cocoa] [GTK] RunLoop::Timer::isActive() is incorrect for timers while they are firing
authorggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 1 Jul 2020 02:17:25 +0000 (02:17 +0000)
committerggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 1 Jul 2020 02:17:25 +0000 (02:17 +0000)
https://bugs.webkit.org/show_bug.cgi?id=213771

Reviewed by Darin Adler.

Source/WTF:

I noticed this because it triggered an assertion failure in
BackgroundProcessResponsivenessTimer::scheduleNextResponsivenessCheck().

In WebKit timer parlance "isActive()" means "will fire again", so a
one-shot timer should report inactive right when it fires. Otherwise,
there's no way to check if it needs to be rescheduled.

* wtf/cf/RunLoopCF.cpp:
(WTF::RunLoop::TimerBase::timerFired): For one-shot timers, stop our
timer before it fires, so that we know it is not active.

* wtf/glib/RunLoopGLib.cpp:
(WTF::RunLoop::TimerBase::TimerBase): For repeating timers, reschedule
our timer before it fires, so that we know it is active.

Tools:

* TestWebKitAPI/Tests/WTF/RunLoop.cpp:
(TestWebKitAPI::DerivedOneShotTimer::fired):
(TestWebKitAPI::DerivedRepeatingTimer::fired):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@263783 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WTF/ChangeLog
Source/WTF/wtf/cf/RunLoopCF.cpp
Source/WTF/wtf/glib/RunLoopGLib.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WTF/RunLoop.cpp

index 820ada2..891ce31 100644 (file)
@@ -1,3 +1,25 @@
+2020-06-30  Geoffrey Garen  <ggaren@apple.com>
+
+        [Cocoa] [GTK] RunLoop::Timer::isActive() is incorrect for timers while they are firing
+        https://bugs.webkit.org/show_bug.cgi?id=213771
+
+        Reviewed by Darin Adler.
+
+        I noticed this because it triggered an assertion failure in
+        BackgroundProcessResponsivenessTimer::scheduleNextResponsivenessCheck().
+
+        In WebKit timer parlance "isActive()" means "will fire again", so a
+        one-shot timer should report inactive right when it fires. Otherwise,
+        there's no way to check if it needs to be rescheduled.
+
+        * wtf/cf/RunLoopCF.cpp:
+        (WTF::RunLoop::TimerBase::timerFired): For one-shot timers, stop our
+        timer before it fires, so that we know it is not active.
+
+        * wtf/glib/RunLoopGLib.cpp:
+        (WTF::RunLoop::TimerBase::TimerBase): For repeating timers, reschedule
+        our timer before it fires, so that we know it is active.
+
 2020-06-30  Mark Lam  <mark.lam@apple.com>
 
         Add handling for a case of OOME in CSSTokenizer and CSSParser.
index a57f7a2..52eef75 100644 (file)
@@ -83,11 +83,15 @@ void RunLoop::stop()
 
 // RunLoop::Timer
 
-void RunLoop::TimerBase::timerFired(CFRunLoopTimerRef, void* context)
+void RunLoop::TimerBase::timerFired(CFRunLoopTimerRef cfTimer, void* context)
 {
     TimerBase* timer = static_cast<TimerBase*>(context);
 
     AutodrainedPool pool;
+
+    if (!CFRunLoopTimerDoesRepeat(cfTimer))
+        timer->stop();
+
     timer->fired();
 }
 
index ceb38eb..212e91f 100644 (file)
@@ -174,11 +174,11 @@ RunLoop::TimerBase::TimerBase(RunLoop& runLoop)
         // before it is safe to dereference timer again.
         RunLoop::TimerBase* timer = static_cast<RunLoop::TimerBase*>(userData);
         GSource* source = timer->m_source.get();
+        if (timer->m_isRepeating)
+            timer->updateReadyTime();
         timer->fired();
         if (g_source_is_destroyed(source))
             return G_SOURCE_REMOVE;
-        if (timer->m_isRepeating)
-            timer->updateReadyTime();
         return G_SOURCE_CONTINUE;
     }, this, nullptr);
     g_source_attach(m_source.get(), m_runLoop->m_mainContext.get());
index 7060fc6..a6e1868 100644 (file)
@@ -1,3 +1,14 @@
+2020-06-30  Geoffrey Garen  <ggaren@apple.com>
+
+        [Cocoa] [GTK] RunLoop::Timer::isActive() is incorrect for timers while they are firing
+        https://bugs.webkit.org/show_bug.cgi?id=213771
+
+        Reviewed by Darin Adler.
+
+        * TestWebKitAPI/Tests/WTF/RunLoop.cpp:
+        (TestWebKitAPI::DerivedOneShotTimer::fired):
+        (TestWebKitAPI::DerivedRepeatingTimer::fired):
+
 2020-06-30  Brady Eidson  <beidson@apple.com>
 
         App-bound JavaScript and Navigation failures should have specific error codes.
index 49f6e9c..a3e74f2 100644 (file)
@@ -65,6 +65,9 @@ public:
 
     void fired()
     {
+        EXPECT_FALSE(isActive());
+        EXPECT_EQ(secondsUntilFire(), Seconds(0));
+
         m_testFinished = true;
         stop();
     }
@@ -94,9 +97,14 @@ public:
 
     void fired()
     {
+        EXPECT_TRUE(isActive());
+
         if (++m_count == 10) {
             m_testFinished = true;
             stop();
+
+            EXPECT_FALSE(isActive());
+            EXPECT_EQ(secondsUntilFire(), Seconds(0));
         }
     }