Changeset 175655 in webkit

Timestamp:

Nov 5, 2014, 6:21:14 PM (11 years ago)

Author:

Chris Dumez

Message:

Assertion hit DOMTimer::updateTimerIntervalIfNecessary()
​https://bugs.webkit.org/show_bug.cgi?id=138440

Reviewed by Geoffrey Garen.

Source/WebCore:

We sometimes hit the ASSERT(repeatInterval() == previousInterval)
assertion in DOMTimer::updateTimerIntervalIfNecessary() when visiting
the following pages:
​http://lifehacker.com/the-healthiest-foods-for-one-handed-snacking-while-gami-1654728164
​http://longform.org/posts/like-something-the-lord-made

After debugging, the issue turned out to be that we are comparing
floating point numbers using ==, and the check sometimes fails even
though the values really close to each other. This patch updates the
DOMTimer code to use WTF::withinEpsilon() instead of operator==()
to compare the floating point intervals.

I confirmed manually that the assertion is no longer hit.

• page/DOMTimer.cpp:

(WebCore::DOMTimer::updateTimerIntervalIfNecessary):

• platform/graphics/FloatQuad.cpp:

(WebCore::FloatQuad::isRectilinear):
(WebCore::withinEpsilon): Deleted.

Source/WTF:

Move the withinEpsilon() function to WTF to avoid code duplication.

• wtf/MathExtras.h:

(WTF::withinEpsilon):

Location:

trunk/Source

Files:

• WTF/ChangeLog (modified) (1 diff)
• WTF/wtf/MathExtras.h (modified) (1 diff)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/page/DOMTimer.cpp (modified) (2 diffs)
• WebCore/platform/graphics/FloatQuad.cpp (modified) (2 diffs)

trunk/Source/WTF/ChangeLog

@@ +1 @@
+2014-11-05  Chris Dumez  <cdumez@apple.com>
+
+        Assertion hit DOMTimer::updateTimerIntervalIfNecessary()
+        https://bugs.webkit.org/show_bug.cgi?id=138440
+
+        Reviewed by Geoffrey Garen.
+
+        Move the withinEpsilon() function to WTF to avoid code duplication.
+
+        * wtf/MathExtras.h:
+        (WTF::withinEpsilon):
+
 2014-11-05  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WTF/wtf/MathExtras.h

@@ -393 +393 @@
 }
 
+template <typename T>
+inline bool withinEpsilon(T a, T b)
+{
+    return std::abs(a - b) <= std::numeric_limits<T>::epsilon();
+}
+
 } // namespace WTF
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2014-11-05  Chris Dumez  <cdumez@apple.com>
+
+        Assertion hit DOMTimer::updateTimerIntervalIfNecessary()
+        https://bugs.webkit.org/show_bug.cgi?id=138440
+
+        Reviewed by Geoffrey Garen.
+
+        We sometimes hit the ASSERT(repeatInterval() == previousInterval)
+        assertion in DOMTimer::updateTimerIntervalIfNecessary() when visiting
+        the following pages:
+        http://lifehacker.com/the-healthiest-foods-for-one-handed-snacking-while-gami-1654728164
+        http://longform.org/posts/like-something-the-lord-made
+
+        After debugging, the issue turned out to be that we are comparing
+        floating point numbers using ==, and the check sometimes fails even
+        though the values really close to each other. This patch updates the
+        DOMTimer code to use WTF::withinEpsilon() instead of operator==()
+        to compare the floating point intervals.
+
+        I confirmed manually that the assertion is no longer hit.
+
+        * page/DOMTimer.cpp:
+        (WebCore::DOMTimer::updateTimerIntervalIfNecessary):
+        * platform/graphics/FloatQuad.cpp:
+        (WebCore::FloatQuad::isRectilinear):
+        (WebCore::withinEpsilon): Deleted.
+
 2014-11-05  Chris Fleizach  <cfleizach@apple.com>
 

trunk/Source/WebCore/page/DOMTimer.cpp

@@ -36 +36 @@
 #include <wtf/CurrentTime.h>
 #include <wtf/HashSet.h>
+#include <wtf/MathExtras.h>
 #include <wtf/StdLibExtras.h>
 
@@ -318 +319 @@
     m_currentTimerInterval = intervalClampedToMinimum();
 
-    if (previousInterval == m_currentTimerInterval)
+    if (WTF::withinEpsilon(previousInterval, m_currentTimerInterval))
         return;
 
     if (repeatInterval()) {
-        ASSERT(repeatInterval() == previousInterval);
+        ASSERT(WTF::withinEpsilon(repeatInterval(), previousInterval));
         augmentRepeatInterval(m_currentTimerInterval - previousInterval);
     } else

trunk/Source/WebCore/platform/graphics/FloatQuad.cpp

@@ -34 +34 @@
 #include <algorithm>
 #include <limits>
+#include <wtf/MathExtras.h>
 
 namespace WebCore {
@@ -91 +92 @@
 }
 
-static inline bool withinEpsilon(float a, float b)
-{
-    return fabs(a - b) < std::numeric_limits<float>::epsilon();
-}
-
 bool FloatQuad::isRectilinear() const
 {
-    return (withinEpsilon(m_p1.x(), m_p2.x()) && withinEpsilon(m_p2.y(), m_p3.y()) && withinEpsilon(m_p3.x(), m_p4.x()) && withinEpsilon(m_p4.y(), m_p1.y()))
-        || (withinEpsilon(m_p1.y(), m_p2.y()) && withinEpsilon(m_p2.x(), m_p3.x()) && withinEpsilon(m_p3.y(), m_p4.y()) && withinEpsilon(m_p4.x(), m_p1.x()));
+    return (WTF::withinEpsilon(m_p1.x(), m_p2.x()) && WTF::withinEpsilon(m_p2.y(), m_p3.y()) && WTF::withinEpsilon(m_p3.x(), m_p4.x()) && WTF::withinEpsilon(m_p4.y(), m_p1.y()))
+        || (WTF::withinEpsilon(m_p1.y(), m_p2.y()) && WTF::withinEpsilon(m_p2.x(), m_p3.x()) && WTF::withinEpsilon(m_p3.y(), m_p4.y()) && WTF::withinEpsilon(m_p4.x(), m_p1.x()));
 }