Title: [175655] trunk/Source
Revision
175655
Author
cdu...@apple.com
Date
2014-11-05 18:21:14 -0800 (Wed, 05 Nov 2014)

Log 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):

Modified Paths

Diff

Modified: trunk/Source/WTF/ChangeLog (175654 => 175655)


--- trunk/Source/WTF/ChangeLog	2014-11-06 02:16:09 UTC (rev 175654)
+++ trunk/Source/WTF/ChangeLog	2014-11-06 02:21:14 UTC (rev 175655)
@@ -1,5 +1,17 @@
 2014-11-05  Chris Dumez  <cdu...@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  <cdu...@apple.com>
+
         Allow constructing a PassRef from a Ref
         https://bugs.webkit.org/show_bug.cgi?id=138389
 

Modified: trunk/Source/WTF/wtf/MathExtras.h (175654 => 175655)


--- trunk/Source/WTF/wtf/MathExtras.h	2014-11-06 02:16:09 UTC (rev 175654)
+++ trunk/Source/WTF/wtf/MathExtras.h	2014-11-06 02:21:14 UTC (rev 175655)
@@ -392,6 +392,12 @@
     return log2;
 }
 
+template <typename T>
+inline bool withinEpsilon(T a, T b)
+{
+    return std::abs(a - b) <= std::numeric_limits<T>::epsilon();
+}
+
 } // namespace WTF
 
 #endif // #ifndef WTF_MathExtras_h

Modified: trunk/Source/WebCore/ChangeLog (175654 => 175655)


--- trunk/Source/WebCore/ChangeLog	2014-11-06 02:16:09 UTC (rev 175654)
+++ trunk/Source/WebCore/ChangeLog	2014-11-06 02:21:14 UTC (rev 175655)
@@ -1,3 +1,30 @@
+2014-11-05  Chris Dumez  <cdu...@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  <cfleiz...@apple.com>
 
         AX: add "alt" as an overriding synonym of "-webkit-alt" (now in the CSS4 spec)

Modified: trunk/Source/WebCore/page/DOMTimer.cpp (175654 => 175655)


--- trunk/Source/WebCore/page/DOMTimer.cpp	2014-11-06 02:16:09 UTC (rev 175654)
+++ trunk/Source/WebCore/page/DOMTimer.cpp	2014-11-06 02:21:14 UTC (rev 175655)
@@ -35,6 +35,7 @@
 #include "UserGestureIndicator.h"
 #include <wtf/CurrentTime.h>
 #include <wtf/HashSet.h>
+#include <wtf/MathExtras.h>
 #include <wtf/StdLibExtras.h>
 
 #if PLATFORM(IOS)
@@ -317,11 +318,11 @@
     double previousInterval = m_currentTimerInterval;
     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
         augmentFireInterval(m_currentTimerInterval - previousInterval);

Modified: trunk/Source/WebCore/platform/graphics/FloatQuad.cpp (175654 => 175655)


--- trunk/Source/WebCore/platform/graphics/FloatQuad.cpp	2014-11-06 02:16:09 UTC (rev 175654)
+++ trunk/Source/WebCore/platform/graphics/FloatQuad.cpp	2014-11-06 02:21:14 UTC (rev 175655)
@@ -33,6 +33,7 @@
 
 #include <algorithm>
 #include <limits>
+#include <wtf/MathExtras.h>
 
 namespace WebCore {
 
@@ -90,15 +91,10 @@
     return FloatRect(left, top, right - left, bottom - top);
 }
 
-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()));
 }
 
 bool FloatQuad::containsPoint(const FloatPoint& p) const
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to