Attachment 241308 Details for Bug 138394 – Patch

[patch] Patch

bug-138394-20141110144450.patch (text/plain), 7.67 KB, created by Jer Noble on 2014-11-10 14:44:48 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Jer Noble

Created: 2014-11-10 14:44:48 PST

Size: 7.67 KB

patch

obsolete

>Subversion Revision: 175824
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 82e5084677ed3bfa8bf2d490ae79d9e118226d9f..31d72e60d48a64a27df95c6ab310cfdb1c49a272 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,35 @@
>+2014-11-10  Jer Noble  <jer.noble@apple.com>
>+
>+        [Mac] media/track/audio-track.html is flakey
>+        https://bugs.webkit.org/show_bug.cgi?id=138394
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The above test fails if the HTMLMediaElement's "canplaythrough" event fires before the AudioTrackList's
>+        "addtrack" event fires. This can happen because each object keeps its own GenericEventQueue, which
>+        empties when that individual queue's timer fires. So events can be enqueued in the following order:
>+        1, A, 2, B, 3, C; but fired in a different order: 1, 2, 3, A, B, C.
>+
>+        Make events enqueued in GenericEventQueues globally ordered, so that events are fired in the order
>+        which they are enqueued, regardless of which object owns the queue. Use a static queue of
>+        GenericEventQueues to manage which GenericEventQueue fires and in what order.
>+
>+        GenericEventQueues will use a WeakPtrFactory to cancel pending events. Revoking a GenericEventQueue's
>+        weak pointers means those queue's entries will be skipped the next time the meta-queue is processed.
>+
>+        * dom/GenericEventQueue.cpp:
>+        (WebCore::GenericEventQueue::GenericEventQueue): Create a WeakPtrFactory instead of a Timer.
>+        (WebCore::GenericEventQueue::enqueueEvent): Enqueue this queue with the MetaQueue.
>+        (WebCore::GenericEventQueue::sharedTimer): Lazily-initializing accessor.
>+        (WebCore::GenericEventQueue::sharedTimerFired):  Ask each queue to dispatch one event.
>+        (WebCore::GenericEventQueue::pendingQueues): Lazily-initializing accessor.
>+        (WebCore::GenericEventQueue::dispatchOneEvent): Renamed from timerFired.
>+        (WebCore::GenericEventQueue::close): Revoke all WeakPtrs.
>+        (WebCore::GenericEventQueue::cancelAllEvents): Ditto.
>+        (WebCore::GenericEventQueue::hasPendingEvents): Use !isEmpty().
>+        (WebCore::GenericEventQueue::timerFired): Deleted.
>+        * dom/GenericEventQueue.h:
>+
> 2014-11-04  Jer Noble  <jer.noble@apple.com>
> 
>         REGRESSION(r174823): Several tests fail due to canplaythrough firing before tracks are available
>diff --git a/Source/WebCore/dom/GenericEventQueue.cpp b/Source/WebCore/dom/GenericEventQueue.cpp
>index b72f7e7a1ce6e831c61e3595125ad1a2bfe12bc5..82fcdbbd5e42c311b1fc094108890ad2f610ecb9 100644
>--- a/Source/WebCore/dom/GenericEventQueue.cpp
>+++ b/Source/WebCore/dom/GenericEventQueue.cpp
>@@ -28,12 +28,15 @@
> 
> #include "Event.h"
> #include "EventTarget.h"
>+#include "Timer.h"
>+#include <wtf/NeverDestroyed.h>
>+#include <wtf/text/CString.h>
> 
> namespace WebCore {
> 
> GenericEventQueue::GenericEventQueue(EventTarget& owner)
>     : m_owner(owner)
>-    , m_timer(this, &GenericEventQueue::timerFired)
>+    , m_weakPtrFactory(this)
>     , m_isClosed(false)
> {
> }
>@@ -51,45 +54,68 @@ bool GenericEventQueue::enqueueEvent(PassRefPtr<Event> event)
>         event->setTarget(0);
> 
>     m_pendingEvents.append(event);
>-
>-    if (!m_timer.isActive())
>-        m_timer.startOneShot(0);
>+    pendingQueues().append(m_weakPtrFactory.createWeakPtr());
>+    if (!sharedTimer().isActive())
>+        sharedTimer().startOneShot(0);
> 
>     return true;
> }
> 
>-void GenericEventQueue::timerFired(Timer&)
>+Timer& GenericEventQueue::sharedTimer()
> {
>-    ASSERT(!m_timer.isActive());
>-    ASSERT(!m_pendingEvents.isEmpty());
>+    static NeverDestroyed<Timer> timer(GenericEventQueue::sharedTimerFired);
>+    return timer.get();
>+}
>+
>+void GenericEventQueue::sharedTimerFired()
>+{
>+    ASSERT(!sharedTimer().isActive());
>+    ASSERT(!pendingQueues().isEmpty());
>+
>+    while (!pendingQueues().isEmpty()) {
>+        WeakPtr<GenericEventQueue> queue = pendingQueues().takeFirst();
>+        if (!queue)
>+            continue;
>+        queue->dispatchOneEvent();
>+    }
>+
>+    if (sharedTimer().isActive())
>+        sharedTimer().stop();
>+}
> 
>-    Vector<RefPtr<Event>> pendingEvents;
>-    m_pendingEvents.swap(pendingEvents);
>+Deque<WeakPtr<GenericEventQueue>>& GenericEventQueue::pendingQueues()
>+{
>+    static NeverDestroyed<Deque<WeakPtr<GenericEventQueue>>> queues;
>+    return queues.get();
>+}
>+
>+void GenericEventQueue::dispatchOneEvent()
>+{
>+    ASSERT(!m_pendingEvents.isEmpty());
> 
>     Ref<EventTarget> protect(m_owner);
>-    for (unsigned i = 0; i < pendingEvents.size(); ++i) {
>-        EventTarget& target = pendingEvents[i]->target() ? *pendingEvents[i]->target() : m_owner;
>-        target.dispatchEvent(pendingEvents[i].release());
>-    }
>+    RefPtr<Event> event = m_pendingEvents.takeFirst();
>+    EventTarget& target = event->target() ? *event->target() : m_owner;
>+    target.dispatchEvent(event.release());
> }
> 
> void GenericEventQueue::close()
> {
>     m_isClosed = true;
> 
>-    m_timer.stop();
>+    m_weakPtrFactory.revokeAll();
>     m_pendingEvents.clear();
> }
> 
> void GenericEventQueue::cancelAllEvents()
> {
>-    m_timer.stop();
>+    m_weakPtrFactory.revokeAll();
>     m_pendingEvents.clear();
> }
> 
> bool GenericEventQueue::hasPendingEvents() const
> {
>-    return m_pendingEvents.size();
>+    return !m_pendingEvents.isEmpty();
> }
> 
> }
>diff --git a/Source/WebCore/dom/GenericEventQueue.h b/Source/WebCore/dom/GenericEventQueue.h
>index 36558902ab30ede27cfe9e4c0f7d83fd68817ea8..6ba86e2260adad811c77ea99af637b45f00dfff9 100644
>--- a/Source/WebCore/dom/GenericEventQueue.h
>+++ b/Source/WebCore/dom/GenericEventQueue.h
>@@ -26,15 +26,16 @@
> #ifndef GenericEventQueue_h
> #define GenericEventQueue_h
> 
>-#include "Timer.h"
>+#include <wtf/Deque.h>
> #include <wtf/Forward.h>
> #include <wtf/RefPtr.h>
>-#include <wtf/Vector.h>
>+#include <wtf/WeakPtr.h>
> 
> namespace WebCore {
> 
> class Event;
> class EventTarget;
>+class Timer;
> 
> class GenericEventQueue {
> public:
>@@ -48,11 +49,15 @@ public:
>     bool hasPendingEvents() const;
> 
> private:
>-    void timerFired(Timer&);
>+    static Timer& sharedTimer();
>+    static void sharedTimerFired();
>+    static Deque<WeakPtr<GenericEventQueue>>& pendingQueues();
>+
>+    void dispatchOneEvent();
> 
>     EventTarget& m_owner;
>-    Vector<RefPtr<Event>> m_pendingEvents;
>-    Timer m_timer;
>+    Deque<RefPtr<Event>> m_pendingEvents;
>+    WeakPtrFactory<GenericEventQueue> m_weakPtrFactory;
>     bool m_isClosed;
> };
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index b106fe9c758e1d586a01c84f0bb3b1ac4a135777..f3029e16cdf1dcfda1a6c1fb4157e14da7371768 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,12 @@
>+2014-11-10  Jer Noble  <jer.noble@apple.com>
>+
>+        [Mac] media/track/audio-track.html is flakey
>+        https://bugs.webkit.org/show_bug.cgi?id=138394
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * platform/mac/TestExpectations:
>+
> 2014-11-04  Jer Noble  <jer.noble@apple.com>
> 
>         REGRESSION(r174823): Several tests fail due to canplaythrough firing before tracks are available
>diff --git a/LayoutTests/platform/mac/TestExpectations b/LayoutTests/platform/mac/TestExpectations
>index 8525ccd67e380e83646539d0330cf2573ebade02..7d8ad7a2088549440dc9a1032990373c8a23d927 100644
>--- a/LayoutTests/platform/mac/TestExpectations
>+++ b/LayoutTests/platform/mac/TestExpectations
>@@ -1502,7 +1502,5 @@ webkit.org/b/136109 [ Yosemite ] fast/multicol/mixed-opacity-fixed-test.html  [
> [ Yosemite ] inspector/model/parse-script-syntax-tree.html [ Failure Crash ]
> [ Yosemite ] inspector/protocol-promise-result.html [ Failure Crash ]
> 
>-webkit.org/b/138394 media/track/audio-track.html [ Pass Failure ]
>-
> # Specific to Yosemite's font fallback
> webkit.org/b/138328 [ Mavericks MountainLion ] platform/mac/fast/text/font-cursive-italic-cjk.html [ Skip ]

Flags:

ap: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 138394: 241010 | 241308