WebCore:
authorbdakin@apple.com <bdakin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 Feb 2008 15:06:52 +0000 (15:06 +0000)
committerbdakin@apple.com <bdakin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 Feb 2008 15:06:52 +0000 (15:06 +0000)
        Reviewed by Geoff.

        Fix for <rdar://problem/5697882> Traffic or Street View button on
        Google Maps is sometimes not positioned correctly (17000)

        On the Mac, timers fire in the order that they are registered.
        Geoff and I discovered that this is not necessarily true on
        Windows, and that turned out to be the cause of this intermittent
        layout problem at Google Maps. This patch adds a new member
        variable to Timer to remember the timer's insertion point into the
        heap. Now when comparing timers, if two timers were registered at
        the same time, their insertion orders are compared to determine
        which should fire first. This code actually never runs on Debug
        builds on the Mac; the system clock on the Mac is accurate enough
        that it knows that the two timers were not registered at *exactly*
        the same time. This is not the case on Windows. In theory, if we
        sped up Javascript enough on the Mac, this code would run and would
        prevent misrenderings such as the one found on Google Maps.

        * platform/Timer.cpp:
        (WebCore::operator<):
        (WebCore::TimerBase::setNextFireTime):
        * platform/Timer.h:

LayoutTests:

        Test written by Geoff, reviewed by me.

        Test for <rdar://problem/5697882> Traffic or Street View button on
        Google Maps is sometimes not positioned correctly (17000)

        * fast/dom/simultaneouslyRegsiteredTimerFireOrder-expected.txt: Added.
        * fast/dom/simultaneouslyRegsiteredTimerFireOrder.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/dom/simultaneouslyRegsiteredTimerFireOrder-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/simultaneouslyRegsiteredTimerFireOrder.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/platform/Timer.cpp
WebCore/platform/Timer.h

index 0ec64a5b3103697987b21331cdbc443a017a55fc..36928b50709d1a514205ff61e83d707fbc550efd 100644 (file)
@@ -1,3 +1,13 @@
+2008-02-07  Beth Dakin  <bdakin@apple.com>
+
+        Test written by Geoff, reviewed by me.
+
+        Test for <rdar://problem/5697882> Traffic or Street View button on 
+        Google Maps is sometimes not positioned correctly (17000)
+
+        * fast/dom/simultaneouslyRegsiteredTimerFireOrder-expected.txt: Added.
+        * fast/dom/simultaneouslyRegsiteredTimerFireOrder.html: Added.
+
 2008-02-07  Nikolas Zimmermann  <zimmermann@kde.org>
 
         Rubber stamped by Eric.
diff --git a/LayoutTests/fast/dom/simultaneouslyRegsiteredTimerFireOrder-expected.txt b/LayoutTests/fast/dom/simultaneouslyRegsiteredTimerFireOrder-expected.txt
new file mode 100644 (file)
index 0000000..85de5dc
--- /dev/null
@@ -0,0 +1,103 @@
+This page verifies that timers with equivalent fire times fire in the order they were registered. If the test passes, you'll see a series of PASS messages below.
+
+PASS: firedTimers[0] should be 0 and is.\r
+PASS: firedTimers[1] should be 1 and is.\r
+PASS: firedTimers[2] should be 2 and is.\r
+PASS: firedTimers[3] should be 3 and is.\r
+PASS: firedTimers[4] should be 4 and is.\r
+PASS: firedTimers[5] should be 5 and is.\r
+PASS: firedTimers[6] should be 6 and is.\r
+PASS: firedTimers[7] should be 7 and is.\r
+PASS: firedTimers[8] should be 8 and is.\r
+PASS: firedTimers[9] should be 9 and is.\r
+PASS: firedTimers[10] should be 10 and is.\r
+PASS: firedTimers[11] should be 11 and is.\r
+PASS: firedTimers[12] should be 12 and is.\r
+PASS: firedTimers[13] should be 13 and is.\r
+PASS: firedTimers[14] should be 14 and is.\r
+PASS: firedTimers[15] should be 15 and is.\r
+PASS: firedTimers[16] should be 16 and is.\r
+PASS: firedTimers[17] should be 17 and is.\r
+PASS: firedTimers[18] should be 18 and is.\r
+PASS: firedTimers[19] should be 19 and is.\r
+PASS: firedTimers[20] should be 20 and is.\r
+PASS: firedTimers[21] should be 21 and is.\r
+PASS: firedTimers[22] should be 22 and is.\r
+PASS: firedTimers[23] should be 23 and is.\r
+PASS: firedTimers[24] should be 24 and is.\r
+PASS: firedTimers[25] should be 25 and is.\r
+PASS: firedTimers[26] should be 26 and is.\r
+PASS: firedTimers[27] should be 27 and is.\r
+PASS: firedTimers[28] should be 28 and is.\r
+PASS: firedTimers[29] should be 29 and is.\r
+PASS: firedTimers[30] should be 30 and is.\r
+PASS: firedTimers[31] should be 31 and is.\r
+PASS: firedTimers[32] should be 32 and is.\r
+PASS: firedTimers[33] should be 33 and is.\r
+PASS: firedTimers[34] should be 34 and is.\r
+PASS: firedTimers[35] should be 35 and is.\r
+PASS: firedTimers[36] should be 36 and is.\r
+PASS: firedTimers[37] should be 37 and is.\r
+PASS: firedTimers[38] should be 38 and is.\r
+PASS: firedTimers[39] should be 39 and is.\r
+PASS: firedTimers[40] should be 40 and is.\r
+PASS: firedTimers[41] should be 41 and is.\r
+PASS: firedTimers[42] should be 42 and is.\r
+PASS: firedTimers[43] should be 43 and is.\r
+PASS: firedTimers[44] should be 44 and is.\r
+PASS: firedTimers[45] should be 45 and is.\r
+PASS: firedTimers[46] should be 46 and is.\r
+PASS: firedTimers[47] should be 47 and is.\r
+PASS: firedTimers[48] should be 48 and is.\r
+PASS: firedTimers[49] should be 49 and is.\r
+PASS: firedTimers[50] should be 50 and is.\r
+PASS: firedTimers[51] should be 51 and is.\r
+PASS: firedTimers[52] should be 52 and is.\r
+PASS: firedTimers[53] should be 53 and is.\r
+PASS: firedTimers[54] should be 54 and is.\r
+PASS: firedTimers[55] should be 55 and is.\r
+PASS: firedTimers[56] should be 56 and is.\r
+PASS: firedTimers[57] should be 57 and is.\r
+PASS: firedTimers[58] should be 58 and is.\r
+PASS: firedTimers[59] should be 59 and is.\r
+PASS: firedTimers[60] should be 60 and is.\r
+PASS: firedTimers[61] should be 61 and is.\r
+PASS: firedTimers[62] should be 62 and is.\r
+PASS: firedTimers[63] should be 63 and is.\r
+PASS: firedTimers[64] should be 64 and is.\r
+PASS: firedTimers[65] should be 65 and is.\r
+PASS: firedTimers[66] should be 66 and is.\r
+PASS: firedTimers[67] should be 67 and is.\r
+PASS: firedTimers[68] should be 68 and is.\r
+PASS: firedTimers[69] should be 69 and is.\r
+PASS: firedTimers[70] should be 70 and is.\r
+PASS: firedTimers[71] should be 71 and is.\r
+PASS: firedTimers[72] should be 72 and is.\r
+PASS: firedTimers[73] should be 73 and is.\r
+PASS: firedTimers[74] should be 74 and is.\r
+PASS: firedTimers[75] should be 75 and is.\r
+PASS: firedTimers[76] should be 76 and is.\r
+PASS: firedTimers[77] should be 77 and is.\r
+PASS: firedTimers[78] should be 78 and is.\r
+PASS: firedTimers[79] should be 79 and is.\r
+PASS: firedTimers[80] should be 80 and is.\r
+PASS: firedTimers[81] should be 81 and is.\r
+PASS: firedTimers[82] should be 82 and is.\r
+PASS: firedTimers[83] should be 83 and is.\r
+PASS: firedTimers[84] should be 84 and is.\r
+PASS: firedTimers[85] should be 85 and is.\r
+PASS: firedTimers[86] should be 86 and is.\r
+PASS: firedTimers[87] should be 87 and is.\r
+PASS: firedTimers[88] should be 88 and is.\r
+PASS: firedTimers[89] should be 89 and is.\r
+PASS: firedTimers[90] should be 90 and is.\r
+PASS: firedTimers[91] should be 91 and is.\r
+PASS: firedTimers[92] should be 92 and is.\r
+PASS: firedTimers[93] should be 93 and is.\r
+PASS: firedTimers[94] should be 94 and is.\r
+PASS: firedTimers[95] should be 95 and is.\r
+PASS: firedTimers[96] should be 96 and is.\r
+PASS: firedTimers[97] should be 97 and is.\r
+PASS: firedTimers[98] should be 98 and is.\r
+PASS: firedTimers[99] should be 99 and is.\r
+
diff --git a/LayoutTests/fast/dom/simultaneouslyRegsiteredTimerFireOrder.html b/LayoutTests/fast/dom/simultaneouslyRegsiteredTimerFireOrder.html
new file mode 100644 (file)
index 0000000..247d617
--- /dev/null
@@ -0,0 +1,49 @@
+<p>
+This page verifies that timers with equivalent fire times fire in the order they
+were registered. If the test passes, you'll see a series of PASS messages below.
+</p>
+
+<pre id="pre"></pre>
+
+<script>
+function log(s)
+{
+    document.getElementById("pre").appendChild(document.createTextNode(s + "\r\n"));
+}
+
+function shouldBe(a, aDescription, b)
+{
+    if (a === b) {
+        log("PASS: " + aDescription + " should be " + b + " and is.");
+    } else {
+        log("FAIL: " + aDescription + " should be " + b + " but instead is " + a + ".");
+    }
+}
+
+var count = 100;
+var firedTimers = [];
+
+function fired(id)
+{
+    firedTimers.push(id);
+    if (id == count - 1)
+        done();
+}
+
+function done()
+{
+    for (var i = 0; i < count; ++i)
+        shouldBe(firedTimers[i], "firedTimers[" + i + "]", i);
+
+    if (window.layoutTestController)
+        layoutTestController.notifyDone();
+}
+
+if (window.layoutTestController) {
+    layoutTestController.dumpAsText();
+    layoutTestController.waitUntilDone();
+}
+
+for (var i = 0; i < count; ++i)
+    setTimeout((function() { var j = i; return function() { fired(j); }})(), 0);
+</script>
index 0d6ea65f1f6445bca24df1de44bad6ae590310dd..00aa645028ded9ea578a6bb6ae755b4b8b721a10 100644 (file)
@@ -1,3 +1,29 @@
+2008-02-07  Beth Dakin  <bdakin@apple.com>
+
+        Reviewed by Geoff.
+
+        Fix for <rdar://problem/5697882> Traffic or Street View button on 
+        Google Maps is sometimes not positioned correctly (17000)
+
+        On the Mac, timers fire in the order that they are registered. 
+        Geoff and I discovered that this is not necessarily true on 
+        Windows, and that turned out to be the cause of this intermittent 
+        layout problem at Google Maps. This patch adds a new member 
+        variable to Timer to remember the timer's insertion point into the 
+        heap. Now when comparing timers, if two timers were registered at 
+        the same time, their insertion orders are compared to determine 
+        which should fire first. This code actually never runs on Debug 
+        builds on the Mac; the system clock on the Mac is accurate enough 
+        that it knows that the two timers were not registered at *exactly* 
+        the same time. This is not the case on Windows. In theory, if we 
+        sped up Javascript enough on the Mac, this code would run and would 
+        prevent misrenderings such as the one found on Google Maps.
+
+        * platform/Timer.cpp:
+        (WebCore::operator<):
+        (WebCore::TimerBase::setNextFireTime):
+        * platform/Timer.h:
+
 2008-02-06  Justin Garcia  <justin.garcia@apple.com>
 
         Reviewed by Darin Adler.
index ee2f95970bb15eebe9f2810b6ec39ab9126cbd12..bf2eeefb6852f110ace780c962bdeaf3b67f2b2c 100644 (file)
@@ -94,9 +94,17 @@ inline TimerHeapElement& TimerHeapElement::operator=(const TimerHeapElement& o)
 
 inline bool operator<(const TimerHeapElement& a, const TimerHeapElement& b)
 {
-    // Note, this is "backwards" because the heap puts the largest element first
-    // and we want the lowest time to be the first one in the heap.
-    return b.timer()->m_nextFireTime < a.timer()->m_nextFireTime;
+    // The comparisons below are "backwards" because the heap puts the largest 
+    // element first and we want the lowest time to be the first one in the heap.
+    double aFireTime = a.timer()->m_nextFireTime;
+    double bFireTime = b.timer()->m_nextFireTime;
+    if (bFireTime != aFireTime)
+        return bFireTime < aFireTime;
+    
+    // We need to look at the difference of the insertion orders instead of comparing the two 
+    // outright in case of overflow. 
+    unsigned difference = a.timer()->m_heapInsertionOrder - b.timer()->m_heapInsertionOrder;
+    return difference < UINT_MAX / 2;
 }
 
 // ----------------
@@ -284,6 +292,8 @@ void TimerBase::setNextFireTime(double newTime)
     double oldTime = m_nextFireTime;
     if (oldTime != newTime) {
         m_nextFireTime = newTime;
+        static unsigned currentHeapInsertionOrder;
+        m_heapInsertionOrder = currentHeapInsertionOrder++;
 
         bool wasFirstTimerInHeap = m_heapIndex == 0;
 
index 2dfd4178c167e68d74f146842f37c432b01057b9..ba98ec5fb9cd012f4a64ab6a3fe4c3929d1e3217 100644 (file)
@@ -80,6 +80,7 @@ private:
     double m_nextFireTime; // 0 if inactive
     double m_repeatInterval; // 0 if not repeating
     int m_heapIndex; // -1 if not in heap
+    unsigned m_heapInsertionOrder; // Used to keep order among equal-fire-time timers
 
     friend void updateSharedTimer();
     friend void setDeferringTimers(bool);