- Revision
- 216084
- Author
- [email protected]
- Date
- 2017-05-02 12:03:58 -0700 (Tue, 02 May 2017)
Log Message
[macOS] Flaky Crash under EventTarget::fireEventListeners on imported/blink/paint/deprecatedpaintlayer/non-self-painting-layer-overrides-visibility.html
https://bugs.webkit.org/show_bug.cgi?id=171406
<rdar://problem/30945281>
Reviewed by Eric Carlson.
Source/WebCore:
I was unfortunately unable to reproduce the flaky crash locally. However, the crash trace
indicates that one of the EventTarget::scriptExecutionContext() overrides is returning a
stale ScriptExecutionContext pointer. Since a GenericEventQueue is involved, the EventTarget
is likely a media-related object. I therefore audited media classes that override
EventTarget::scriptExecutionContext() and found several that look unsafe. I am fixing those
by having them override ContextDestructionObserver, instead of having a raw
ScriptExecutionContext pointer member. This makes sure the pointer gets nulled out whenever
the scriptexecutioncontext gets destroyed, ensuring that those classes's
EventTarget::scriptExecutionContext() overrides can never return a stale pointer.
* Modules/mediasource/SourceBufferList.cpp:
(WebCore::SourceBufferList::SourceBufferList):
* Modules/mediasource/SourceBufferList.h:
* html/track/TextTrack.cpp:
(WebCore::TextTrack::TextTrack):
* html/track/TextTrack.h:
* html/track/TrackListBase.cpp:
(TrackListBase::TrackListBase):
* html/track/TrackListBase.h:
LayoutTests:
Unskip test.
* platform/mac/TestExpectations:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (216083 => 216084)
--- trunk/LayoutTests/ChangeLog 2017-05-02 19:02:43 UTC (rev 216083)
+++ trunk/LayoutTests/ChangeLog 2017-05-02 19:03:58 UTC (rev 216084)
@@ -1,3 +1,15 @@
+2017-05-02 Chris Dumez <[email protected]>
+
+ [macOS] Flaky Crash under EventTarget::fireEventListeners on imported/blink/paint/deprecatedpaintlayer/non-self-painting-layer-overrides-visibility.html
+ https://bugs.webkit.org/show_bug.cgi?id=171406
+ <rdar://problem/30945281>
+
+ Reviewed by Eric Carlson.
+
+ Unskip test.
+
+ * platform/mac/TestExpectations:
+
2017-05-02 Matt Lewis <[email protected]>
Marked test svg/animations/animated-svg-image-removed-from-document-paused.html as flaky failure.
Modified: trunk/LayoutTests/platform/mac/TestExpectations (216083 => 216084)
--- trunk/LayoutTests/platform/mac/TestExpectations 2017-05-02 19:02:43 UTC (rev 216083)
+++ trunk/LayoutTests/platform/mac/TestExpectations 2017-05-02 19:03:58 UTC (rev 216084)
@@ -1593,6 +1593,4 @@
webkit.org/b/171272 [ Yosemite ElCapitan Sierra ] fast/text/kaithi.html [ ImageOnlyFailure ]
-webkit.org/b/171406 imported/blink/paint/deprecatedpaintlayer/non-self-painting-layer-overrides-visibility.html [ Pass Crash ]
-
webkit.org/b/171465 media/track/track-in-band-style.html [ Pass Timeout ]
Modified: trunk/Source/WebCore/ChangeLog (216083 => 216084)
--- trunk/Source/WebCore/ChangeLog 2017-05-02 19:02:43 UTC (rev 216083)
+++ trunk/Source/WebCore/ChangeLog 2017-05-02 19:03:58 UTC (rev 216084)
@@ -1,3 +1,31 @@
+2017-05-02 Chris Dumez <[email protected]>
+
+ [macOS] Flaky Crash under EventTarget::fireEventListeners on imported/blink/paint/deprecatedpaintlayer/non-self-painting-layer-overrides-visibility.html
+ https://bugs.webkit.org/show_bug.cgi?id=171406
+ <rdar://problem/30945281>
+
+ Reviewed by Eric Carlson.
+
+ I was unfortunately unable to reproduce the flaky crash locally. However, the crash trace
+ indicates that one of the EventTarget::scriptExecutionContext() overrides is returning a
+ stale ScriptExecutionContext pointer. Since a GenericEventQueue is involved, the EventTarget
+ is likely a media-related object. I therefore audited media classes that override
+ EventTarget::scriptExecutionContext() and found several that look unsafe. I am fixing those
+ by having them override ContextDestructionObserver, instead of having a raw
+ ScriptExecutionContext pointer member. This makes sure the pointer gets nulled out whenever
+ the scriptexecutioncontext gets destroyed, ensuring that those classes's
+ EventTarget::scriptExecutionContext() overrides can never return a stale pointer.
+
+ * Modules/mediasource/SourceBufferList.cpp:
+ (WebCore::SourceBufferList::SourceBufferList):
+ * Modules/mediasource/SourceBufferList.h:
+ * html/track/TextTrack.cpp:
+ (WebCore::TextTrack::TextTrack):
+ * html/track/TextTrack.h:
+ * html/track/TrackListBase.cpp:
+ (TrackListBase::TrackListBase):
+ * html/track/TrackListBase.h:
+
2017-05-02 Antti Koivisto <[email protected]>
Document style resolvers should share user rulesets
Modified: trunk/Source/WebCore/Modules/mediasource/SourceBufferList.cpp (216083 => 216084)
--- trunk/Source/WebCore/Modules/mediasource/SourceBufferList.cpp 2017-05-02 19:02:43 UTC (rev 216083)
+++ trunk/Source/WebCore/Modules/mediasource/SourceBufferList.cpp 2017-05-02 19:03:58 UTC (rev 216084)
@@ -40,7 +40,7 @@
namespace WebCore {
SourceBufferList::SourceBufferList(ScriptExecutionContext* context)
- : m_scriptExecutionContext(context)
+ : ContextDestructionObserver(context)
, m_asyncEventQueue(*this)
{
}
Modified: trunk/Source/WebCore/Modules/mediasource/SourceBufferList.h (216083 => 216084)
--- trunk/Source/WebCore/Modules/mediasource/SourceBufferList.h 2017-05-02 19:02:43 UTC (rev 216083)
+++ trunk/Source/WebCore/Modules/mediasource/SourceBufferList.h 2017-05-02 19:03:58 UTC (rev 216084)
@@ -32,6 +32,7 @@
#if ENABLE(MEDIA_SOURCE)
+#include "ContextDestructionObserver.h"
#include "EventTarget.h"
#include "GenericEventQueue.h"
#include "ScriptWrappable.h"
@@ -42,7 +43,7 @@
class SourceBuffer;
-class SourceBufferList final : public RefCounted<SourceBufferList>, public EventTargetWithInlineData {
+class SourceBufferList final : public RefCounted<SourceBufferList>, public EventTargetWithInlineData, public ContextDestructionObserver {
public:
static Ref<SourceBufferList> create(ScriptExecutionContext* context)
{
@@ -63,8 +64,8 @@
Vector<RefPtr<SourceBuffer>>::iterator end() { return m_list.end(); }
// EventTarget interface
- EventTargetInterface eventTargetInterface() const override { return SourceBufferListEventTargetInterfaceType; }
- ScriptExecutionContext* scriptExecutionContext() const override { return m_scriptExecutionContext; }
+ EventTargetInterface eventTargetInterface() const final { return SourceBufferListEventTargetInterfaceType; }
+ ScriptExecutionContext* scriptExecutionContext() const final { return ContextDestructionObserver::scriptExecutionContext(); }
using RefCounted<SourceBufferList>::ref;
using RefCounted<SourceBufferList>::deref;
@@ -77,7 +78,6 @@
void refEventTarget() override { ref(); }
void derefEventTarget() override { deref(); }
- ScriptExecutionContext* m_scriptExecutionContext;
GenericEventQueue m_asyncEventQueue;
Vector<RefPtr<SourceBuffer>> m_list;
Modified: trunk/Source/WebCore/html/track/TextTrack.cpp (216083 => 216084)
--- trunk/Source/WebCore/html/track/TextTrack.cpp 2017-05-02 19:02:43 UTC (rev 216083)
+++ trunk/Source/WebCore/html/track/TextTrack.cpp 2017-05-02 19:03:58 UTC (rev 216084)
@@ -95,7 +95,7 @@
TextTrack::TextTrack(ScriptExecutionContext* context, TextTrackClient* client, const AtomicString& kind, const AtomicString& id, const AtomicString& label, const AtomicString& language, TextTrackType type)
: TrackBase(TrackBase::TextTrack, id, label, language)
- , m_scriptExecutionContext(context)
+ , ContextDestructionObserver(context)
, m_client(client)
, m_trackType(type)
{
Modified: trunk/Source/WebCore/html/track/TextTrack.h (216083 => 216084)
--- trunk/Source/WebCore/html/track/TextTrack.h 2017-05-02 19:02:43 UTC (rev 216083)
+++ trunk/Source/WebCore/html/track/TextTrack.h 2017-05-02 19:03:58 UTC (rev 216084)
@@ -28,6 +28,7 @@
#if ENABLE(VIDEO_TRACK)
+#include "ContextDestructionObserver.h"
#include "TextTrackCue.h"
#include "TrackBase.h"
@@ -50,7 +51,7 @@
virtual void textTrackRemoveCue(TextTrack&, TextTrackCue&) = 0;
};
-class TextTrack : public TrackBase, public EventTargetWithInlineData {
+class TextTrack : public TrackBase, public EventTargetWithInlineData, public ContextDestructionObserver {
public:
static Ref<TextTrack> create(ScriptExecutionContext* context, TextTrackClient* client, const AtomicString& kind, const AtomicString& id, const AtomicString& label, const AtomicString& language)
{
@@ -59,7 +60,7 @@
virtual ~TextTrack();
EventTargetInterface eventTargetInterface() const final { return TextTrackEventTargetInterfaceType; }
- ScriptExecutionContext* scriptExecutionContext() const final { return m_scriptExecutionContext; }
+ ScriptExecutionContext* scriptExecutionContext() const final { return ContextDestructionObserver::scriptExecutionContext(); }
static TextTrack* captionMenuOffItem();
static TextTrack* captionMenuAutomaticItem();
@@ -159,7 +160,6 @@
TextTrackCueList& ensureTextTrackCueList();
- ScriptExecutionContext* m_scriptExecutionContext;
Mode m_mode { Mode::Disabled };
Kind m_kind { Kind::Subtitles };
TextTrackClient* m_client;
Modified: trunk/Source/WebCore/html/track/TrackListBase.cpp (216083 => 216084)
--- trunk/Source/WebCore/html/track/TrackListBase.cpp 2017-05-02 19:02:43 UTC (rev 216083)
+++ trunk/Source/WebCore/html/track/TrackListBase.cpp 2017-05-02 19:03:58 UTC (rev 216084)
@@ -37,11 +37,11 @@
using namespace WebCore;
TrackListBase::TrackListBase(HTMLMediaElement* element, ScriptExecutionContext* context)
- : m_context(context)
+ : ContextDestructionObserver(context)
, m_element(element)
, m_asyncEventQueue(*this)
{
- ASSERT(context->isDocument());
+ ASSERT(is<Document>(context));
}
TrackListBase::~TrackListBase()
Modified: trunk/Source/WebCore/html/track/TrackListBase.h (216083 => 216084)
--- trunk/Source/WebCore/html/track/TrackListBase.h 2017-05-02 19:02:43 UTC (rev 216083)
+++ trunk/Source/WebCore/html/track/TrackListBase.h 2017-05-02 19:03:58 UTC (rev 216084)
@@ -27,6 +27,7 @@
#if ENABLE(VIDEO_TRACK)
+#include "ContextDestructionObserver.h"
#include "EventListener.h"
#include "EventTarget.h"
#include "GenericEventQueue.h"
@@ -40,7 +41,7 @@
class Element;
class TrackBase;
-class TrackListBase : public RefCounted<TrackListBase>, public EventTargetWithInlineData {
+class TrackListBase : public RefCounted<TrackListBase>, public EventTargetWithInlineData, public ContextDestructionObserver {
public:
virtual ~TrackListBase();
@@ -52,7 +53,7 @@
EventTargetInterface eventTargetInterface() const override = 0;
using RefCounted<TrackListBase>::ref;
using RefCounted<TrackListBase>::deref;
- ScriptExecutionContext* scriptExecutionContext() const final { return m_context; }
+ ScriptExecutionContext* scriptExecutionContext() const final { return ContextDestructionObserver::scriptExecutionContext(); }
virtual void clearElement();
Element* element() const;
@@ -79,7 +80,6 @@
void refEventTarget() final { ref(); }
void derefEventTarget() final { deref(); }
- ScriptExecutionContext* m_context;
HTMLMediaElement* m_element;
GenericEventQueue m_asyncEventQueue;
