Skip to content

GitLab

T
tangled
Commit af9dd267 authored by cbiesinger@chromium.org's avatar cbiesinger@chromium.org
Browse files
Options

Make MediaQueryList an ActiveDOMElement 

This ensures that we never GC a MediaQueryList that has listeners.
It looks like that already doesn't happen (perhaps due to how
MediaQueryListListener keeps references to the JS object), but
this seems safer.

Review URL: https://codereview.chromium.org/397403002

git-svn-id: svn://svn.chromium.org/blink/trunk@178506 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 67a52064
Hide whitespace changes
Inline Side-by-side
Showing with 69 additions and 19 deletions
third_party/WebKit/LayoutTests/fast/media/media-query-listener-gc-expected.txt 0 → 100644
View file @ af9dd267
Media query listeners should work even after gc.
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS listenersCalled is expectedResult
PASS successfullyParsed is true
TEST COMPLETE
third_party/WebKit/LayoutTests/fast/media/media-query-listener-gc.html 0 → 100644
View file @ af9dd267
<!DOCTYPE html>
<script src="../../resources/js-test.js"></script>
<div id="sandbox"></div>
<script>
description("Media query listeners should work even after gc.");
var jsTestIsAsync = true;
var sandbox = document.getElementById("sandbox");
var iframe = document.createElement("iframe");
sandbox.appendChild(iframe);
var matchMedia = iframe.contentWindow.matchMedia;
var mediaList1 = matchMedia("(max-width: 100px)");
var listenersCalled = [];
function makeListener(label) {
return function() { listenersCalled.push(label); };
}
mediaList1.addListener(makeListener("mediaList1_1"));
mediaList1.addListener(verifyResult);
mediaList1 = null;
gc();
iframe.style.width = "200px";
var expectedResult = ["mediaList1_1"];
function verifyResult() {
shouldBe("listenersCalled", "expectedResult");
finishJSTest();
}
</script>
third_party/WebKit/Source/core/css/MediaQueryList.cpp
View file @ af9dd267
...@@ -27,13 +27,16 @@ ...@@ -27,13 +27,16 @@
namespace blink { namespace blink {
PassRefPtrWillBeRawPtr<MediaQueryList> MediaQueryList::create(PassRefPtrWillBeRawPtr<MediaQueryMatcher> matcher, PassRefPtrWillBeRawPtr<MediaQuerySet> media) PassRefPtrWillBeRawPtr<MediaQueryList> MediaQueryList::create(ExecutionContext* context, PassRefPtrWillBeRawPtr<MediaQueryMatcher> matcher, PassRefPtrWillBeRawPtr<MediaQuerySet> media)
{ {
return adoptRefWillBeNoop(new MediaQueryList(matcher, media)); RefPtrWillBeRawPtr<MediaQueryList> list = adoptRefWillBeNoop(new MediaQueryList(context, matcher, media));
list->suspendIfNeeded();
return list.release();
} }
MediaQueryList::MediaQueryList(PassRefPtrWillBeRawPtr<MediaQueryMatcher> matcher, PassRefPtrWillBeRawPtr<MediaQuerySet> media) MediaQueryList::MediaQueryList(ExecutionContext* context, PassRefPtrWillBeRawPtr<MediaQueryMatcher> matcher, PassRefPtrWillBeRawPtr<MediaQuerySet> media)
: m_matcher(matcher) : ActiveDOMObject(context)
, m_matcher(matcher)
, m_media(media) , m_media(media)
, m_matchesDirty(true) , m_matchesDirty(true)
, m_matches(false) , m_matches(false)
...@@ -81,7 +84,12 @@ void MediaQueryList::removeListener(PassRefPtrWillBeRawPtr<MediaQueryListListene ...@@ -81,7 +84,12 @@ void MediaQueryList::removeListener(PassRefPtrWillBeRawPtr<MediaQueryListListene
} }
} }
void MediaQueryList::documentDetached() bool MediaQueryList::hasPendingActivity() const
{
return m_listeners.size();
}
void MediaQueryList::stop()
{ {
m_listeners.clear(); m_listeners.clear();
} }
......
third_party/WebKit/Source/core/css/MediaQueryList.h
View file @ af9dd267
...@@ -20,6 +20,7 @@ ...@@ -20,6 +20,7 @@
#ifndef MediaQueryList_h #ifndef MediaQueryList_h
#define MediaQueryList_h #define MediaQueryList_h
#include "core/dom/ActiveDOMObject.h"
#include "platform/heap/Handle.h" #include "platform/heap/Handle.h"
#include "wtf/Forward.h" #include "wtf/Forward.h"
#include "wtf/RefCounted.h" #include "wtf/RefCounted.h"
...@@ -27,6 +28,7 @@ ...@@ -27,6 +28,7 @@
namespace blink { namespace blink {
class ExecutionContext;
class MediaQueryListListener; class MediaQueryListListener;
class MediaQueryEvaluator; class MediaQueryEvaluator;
class MediaQueryMatcher; class MediaQueryMatcher;
...@@ -37,9 +39,9 @@ class MediaQuerySet; ...@@ -37,9 +39,9 @@ class MediaQuerySet;
// retrieve the current value of the given media query and to add/remove listeners that // retrieve the current value of the given media query and to add/remove listeners that
// will be called whenever the value of the query changes. // will be called whenever the value of the query changes.
class MediaQueryList FINAL : public RefCountedWillBeGarbageCollectedFinalized<MediaQueryList> { class MediaQueryList FINAL : public RefCountedWillBeGarbageCollectedFinalized<MediaQueryList>, public ActiveDOMObject {
public: public:
static PassRefPtrWillBeRawPtr<MediaQueryList> create(PassRefPtrWillBeRawPtr<MediaQueryMatcher>, PassRefPtrWillBeRawPtr<MediaQuerySet>); static PassRefPtrWillBeRawPtr<MediaQueryList> create(ExecutionContext*, PassRefPtrWillBeRawPtr<MediaQueryMatcher>, PassRefPtrWillBeRawPtr<MediaQuerySet>);
virtual ~MediaQueryList(); virtual ~MediaQueryList();
String media() const; String media() const;
...@@ -52,10 +54,12 @@ public: ...@@ -52,10 +54,12 @@ public:
void trace(Visitor*); void trace(Visitor*);
void documentDetached(); // From ActiveDOMObject
virtual bool hasPendingActivity() const OVERRIDE;
virtual void stop() OVERRIDE;
private: private:
MediaQueryList(PassRefPtrWillBeRawPtr<MediaQueryMatcher>, PassRefPtrWillBeRawPtr<MediaQuerySet>); MediaQueryList(ExecutionContext*, PassRefPtrWillBeRawPtr<MediaQueryMatcher>, PassRefPtrWillBeRawPtr<MediaQuerySet>);
bool updateMatches(); bool updateMatches();
......
third_party/WebKit/Source/core/css/MediaQueryList.idl
View file @ af9dd267
...@@ -17,6 +17,7 @@ ...@@ -17,6 +17,7 @@
* Boston, MA 02110-1301, USA. * Boston, MA 02110-1301, USA.
*/ */
[ [
ActiveDOMObject,
NoInterfaceObject, NoInterfaceObject,
WillBeGarbageCollected WillBeGarbageCollected
] interface MediaQueryList { ] interface MediaQueryList {
......
third_party/WebKit/Source/core/css/MediaQueryMatcher.cpp
View file @ af9dd267
...@@ -48,14 +48,6 @@ void MediaQueryMatcher::documentDetached() ...@@ -48,14 +48,6 @@ void MediaQueryMatcher::documentDetached()
{ {
m_document = nullptr; m_document = nullptr;
m_evaluator = nullptr; m_evaluator = nullptr;
// Take a ref to each MediaQueryList as removing the listeners in documentDetached
// could release the last ref and mutate the m_mediaLists.
WillBeHeapVector<RefPtrWillBeMember<MediaQueryList> > lists;
copyToVector(m_mediaLists, lists);
for (size_t i = 0; i < lists.size(); ++i)
lists[i]->documentDetached();
} }
PassOwnPtr<MediaQueryEvaluator> MediaQueryMatcher::createEvaluator() const PassOwnPtr<MediaQueryEvaluator> MediaQueryMatcher::createEvaluator() const
...@@ -91,7 +83,7 @@ PassRefPtrWillBeRawPtr<MediaQueryList> MediaQueryMatcher::matchMedia(const Strin ...@@ -91,7 +83,7 @@ PassRefPtrWillBeRawPtr<MediaQueryList> MediaQueryMatcher::matchMedia(const Strin
RefPtrWillBeRawPtr<MediaQuerySet> media = MediaQuerySet::create(query); RefPtrWillBeRawPtr<MediaQuerySet> media = MediaQuerySet::create(query);
// Add warning message to inspector whenever dpi/dpcm values are used for "screen" media. // Add warning message to inspector whenever dpi/dpcm values are used for "screen" media.
reportMediaQueryWarningIfNeeded(m_document, media.get()); reportMediaQueryWarningIfNeeded(m_document, media.get());
return MediaQueryList::create(this, media); return MediaQueryList::create(m_document, this, media);
} }
void MediaQueryMatcher::addMediaQueryList(MediaQueryList* query) void MediaQueryMatcher::addMediaQueryList(MediaQueryList* query)
......
third_party/WebKit/Source/core/html/HTMLSourceElement.cpp
View file @ af9dd267
...@@ -161,7 +161,7 @@ void HTMLSourceElement::parseAttribute(const QualifiedName& name, const AtomicSt ...@@ -161,7 +161,7 @@ void HTMLSourceElement::parseAttribute(const QualifiedName& name, const AtomicSt
if (m_mediaQueryList) if (m_mediaQueryList)
m_mediaQueryList->removeListener(m_listener); m_mediaQueryList->removeListener(m_listener);
RefPtrWillBeRawPtr<MediaQuerySet> set = MediaQuerySet::create(value); RefPtrWillBeRawPtr<MediaQuerySet> set = MediaQuerySet::create(value);
m_mediaQueryList = MediaQueryList::create(&document().mediaQueryMatcher(), set.release()); m_mediaQueryList = MediaQueryList::create(&document(), &document().mediaQueryMatcher(), set.release());
m_mediaQueryList->addListener(m_listener); m_mediaQueryList->addListener(m_listener);
} }
if (name == srcsetAttr || name == sizesAttr || name == mediaAttr || name == typeAttr) { if (name == srcsetAttr || name == sizesAttr || name == mediaAttr || name == typeAttr) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment