Bug 1451717 - Fix scheduling of media query list events to match the spec. r=smaug

Differential Revision: https://phabricator.services.mozilla.com/D185662
This commit is contained in:
Emilio Cobos Álvarez
2023-08-16 09:07:14 +00:00
parent 3e1f759a6b
commit 3b1bd6b3ce
12 changed files with 149 additions and 144 deletions

View File

@@ -35,7 +35,9 @@ class AutoPrintEventDispatcher {
if (RefPtr<nsPresContext> presContext = doc->GetPresContext()) {
presContext->EmulateMedium(aBefore ? nsGkAtoms::print : nullptr);
// Ensure media query listeners fire.
doc->FlushPendingNotifications(FlushType::Style);
// FIXME(emilio): This is hacky, at best, but is required for compat
// with some pages, see bug 774398.
doc->EvaluateMediaQueriesAndReportChanges(/* aRecurse = */ false);
}
}
}

View File

@@ -14132,6 +14132,35 @@ void Document::SetDevToolsWatchingDOMMutations(bool aValue) {
}
}
void EvaluateMediaQueryLists(nsTArray<RefPtr<MediaQueryList>>& aListsToNotify,
Document& aDocument, bool aRecurse) {
if (nsPresContext* pc = aDocument.GetPresContext()) {
pc->FlushPendingMediaFeatureValuesChanged();
}
for (MediaQueryList* mql : aDocument.MediaQueryLists()) {
if (mql->EvaluateOnRenderingUpdate()) {
aListsToNotify.AppendElement(mql);
}
}
if (!aRecurse) {
return;
}
auto recurse = [&](Document& aSubDoc) {
EvaluateMediaQueryLists(aListsToNotify, aSubDoc, true);
return CallState::Continue;
};
aDocument.EnumerateSubDocuments(recurse);
}
void Document::EvaluateMediaQueriesAndReportChanges(bool aRecurse) {
AutoTArray<RefPtr<MediaQueryList>, 32> mqls;
EvaluateMediaQueryLists(mqls, *this, aRecurse);
for (auto& mql : mqls) {
mql->FireChangeEvent();
}
}
void Document::MaybeWarnAboutZoom() {
if (mHasWarnedAboutZoom) {
return;

View File

@@ -3581,6 +3581,9 @@ class Document : public nsINode,
void MaybeWarnAboutZoom();
// https://drafts.csswg.org/cssom-view/#evaluate-media-queries-and-report-changes
void EvaluateMediaQueriesAndReportChanges(bool aRecurse);
// ParentNode
nsIHTMLCollection* Children();
uint32_t ChildElementCount();

View File

@@ -1943,6 +1943,10 @@ void nsPresContext::MediaFeatureValuesChanged(
mPresShell->EnsureStyleFlush();
}
if (!mDocument->MediaQueryLists().isEmpty()) {
RefreshDriver()->ScheduleMediaQueryListenerUpdate();
}
if (!mPendingMediaFeatureValuesChange) {
mPendingMediaFeatureValuesChange = MakeUnique<MediaFeatureChange>(aChange);
} else {
@@ -1991,46 +1995,8 @@ bool nsPresContext::FlushPendingMediaFeatureValuesChanged() {
RebuildAllStyleData(change.mChangeHint, change.mRestyleHint);
}
if (mDocument->IsBeingUsedAsImage()) {
MOZ_ASSERT(mDocument->MediaQueryLists().isEmpty());
return changedStyle;
}
// https://drafts.csswg.org/cssom-view/#evaluate-media-queries-and-report-changes
//
// Media query list listeners should be notified from a queued task
// (in HTML5 terms), although we also want to notify them on certain
// flushes. (We're already running off an event.)
//
// TODO: This should be better integrated into the "update the rendering"
// steps: https://html.spec.whatwg.org/#update-the-rendering
//
// Note that we do this after the new style from media queries in
// style sheets has been computed.
if (mDocument->MediaQueryLists().isEmpty()) {
return changedStyle;
}
// We build a list of all the notifications we're going to send
// before we send any of them.
nsTArray<RefPtr<mozilla::dom::MediaQueryList>> listsToNotify;
for (MediaQueryList* mql = mDocument->MediaQueryLists().getFirst(); mql;
mql = static_cast<LinkedListElement<MediaQueryList>*>(mql)->getNext()) {
if (mql->MediaFeatureValuesChanged()) {
listsToNotify.AppendElement(mql);
}
}
if (!listsToNotify.IsEmpty()) {
nsContentUtils::AddScriptRunner(NS_NewRunnableFunction(
"nsPresContext::FlushPendingMediaFeatureValuesChanged",
[list = std::move(listsToNotify)] {
for (const auto& mql : list) {
nsAutoMicroTask mt;
mql->FireChangeEvent();
}
}));
for (MediaQueryList* mql : mDocument->MediaQueryLists()) {
mql->MediaFeatureValuesChanged();
}
return changedStyle;

View File

@@ -34,6 +34,7 @@
#include "mozilla/Assertions.h"
#include "mozilla/AutoRestore.h"
#include "mozilla/BasePrincipal.h"
#include "mozilla/dom/MediaQueryList.h"
#include "mozilla/CycleCollectedJSContext.h"
#include "mozilla/DebugOnly.h"
#include "mozilla/DisplayPortUtils.h"
@@ -1321,6 +1322,7 @@ nsRefreshDriver::nsRefreshDriver(nsPresContext* aPresContext)
mResizeSuppressed(false),
mNotifyDOMContentFlushed(false),
mNeedToUpdateIntersectionObservations(false),
mMightNeedMediaQueryListenerUpdate(false),
mNeedToUpdateContentRelevancy(false),
mInNormalTick(false),
mAttemptedExtraTickSinceLastVsync(false),
@@ -1513,6 +1515,18 @@ void nsRefreshDriver::DispatchVisualViewportScrollEvents() {
}
}
// https://drafts.csswg.org/cssom-view/#evaluate-media-queries-and-report-changes
void nsRefreshDriver::EvaluateMediaQueriesAndReportChanges() {
if (!mMightNeedMediaQueryListenerUpdate) {
return;
}
mMightNeedMediaQueryListenerUpdate = false;
AUTO_PROFILER_LABEL_RELEVANT_FOR_JS(
"Evaluate media queries and report changes", LAYOUT);
RefPtr<Document> doc = mPresContext->Document();
doc->EvaluateMediaQueriesAndReportChanges(/* aRecurse = */ true);
}
void nsRefreshDriver::AddPostRefreshObserver(
nsAPostRefreshObserver* aObserver) {
MOZ_ASSERT(!mPostRefreshObservers.Contains(aObserver));
@@ -1933,6 +1947,9 @@ auto nsRefreshDriver::GetReasonsToTick() const -> TickReasons {
if (mNeedToUpdateIntersectionObservations) {
reasons |= TickReasons::eNeedsToUpdateIntersectionObservations;
}
if (mMightNeedMediaQueryListenerUpdate) {
reasons |= TickReasons::eHasPendingMediaQueryListeners;
}
if (mNeedToUpdateContentRelevancy) {
reasons |= TickReasons::eNeedsToUpdateContentRelevancy;
}
@@ -1966,6 +1983,9 @@ void nsRefreshDriver::AppendTickReasonsToString(TickReasons aReasons,
if (aReasons & TickReasons::eNeedsToUpdateIntersectionObservations) {
aStr.AppendLiteral(" NeedsToUpdateIntersectionObservations");
}
if (aReasons & TickReasons::eHasPendingMediaQueryListeners) {
aStr.AppendLiteral(" HasPendingMediaQueryListeners");
}
if (aReasons & TickReasons::eNeedsToUpdateContentRelevancy) {
aStr.AppendLiteral(" NeedsToUpdateContentRelevancy");
}
@@ -2590,6 +2610,7 @@ void nsRefreshDriver::Tick(VsyncId aId, TimeStamp aNowTime,
FlushAutoFocusDocuments();
DispatchScrollEvents();
DispatchVisualViewportScrollEvents();
EvaluateMediaQueriesAndReportChanges();
DispatchAnimationEvents();
RunFullscreenSteps();
RunFrameRequestCallbacks(aNowTime);

View File

@@ -419,6 +419,11 @@ class nsRefreshDriver final : public mozilla::layers::TransactionIdAllocator,
mNeedToUpdateIntersectionObservations = true;
}
void ScheduleMediaQueryListenerUpdate() {
EnsureTimerStarted();
mMightNeedMediaQueryListenerUpdate = true;
}
void EnsureContentRelevancyUpdateHappens() {
EnsureTimerStarted();
mNeedToUpdateContentRelevancy = true;
@@ -440,6 +445,7 @@ class nsRefreshDriver final : public mozilla::layers::TransactionIdAllocator,
eHasVisualViewportResizeEvents = 1 << 4,
eHasScrollEvents = 1 << 5,
eHasVisualViewportScrollEvents = 1 << 6,
eHasPendingMediaQueryListeners = 1 << 7,
};
void AddForceNotifyContentfulPaintPresContext(nsPresContext* aPresContext);
@@ -477,7 +483,7 @@ class nsRefreshDriver final : public mozilla::layers::TransactionIdAllocator,
}
operator RefPtr<nsARefreshObserver>() { return mObserver; }
};
typedef nsTObserverArray<ObserverData> ObserverArray;
using ObserverArray = nsTObserverArray<ObserverData>;
MOZ_CAN_RUN_SCRIPT
void FlushAutoFocusDocuments();
void RunFullscreenSteps();
@@ -486,6 +492,7 @@ class nsRefreshDriver final : public mozilla::layers::TransactionIdAllocator,
void RunFrameRequestCallbacks(mozilla::TimeStamp aNowTime);
void UpdateIntersectionObservations(mozilla::TimeStamp aNowTime);
void UpdateRelevancyOfContentVisibilityAutoFrames();
void EvaluateMediaQueriesAndReportChanges();
enum class IsExtraTick {
No,
@@ -628,6 +635,10 @@ class nsRefreshDriver final : public mozilla::layers::TransactionIdAllocator,
// all our documents.
bool mNeedToUpdateIntersectionObservations : 1;
// True if we might need to report media query changes in any of our
// documents.
bool mMightNeedMediaQueryListenerUpdate : 1;
// True if we need to update the relevancy of `content-visibility: auto`
// elements in our documents.
bool mNeedToUpdateContentRelevancy : 1;

View File

@@ -14,8 +14,6 @@
#include "nsPresContext.h"
#include "mozilla/dom/Document.h"
#define ONCHANGE_STRING u"change"_ns
namespace mozilla::dom {
MediaQueryList::MediaQueryList(Document* aDocument,
@@ -24,7 +22,9 @@ MediaQueryList::MediaQueryList(Document* aDocument,
: DOMEventTargetHelper(aDocument->GetInnerWindow()),
mDocument(aDocument),
mMediaList(MediaList::Create(aMediaQueryList, aCallerType)),
mViewportDependent(mMediaList->IsViewportDependent()) {
mViewportDependent(mMediaList->IsViewportDependent()),
mMatches(mMediaList->Matches(*aDocument)),
mMatchesOnRenderingUpdate(mMatches) {
KeepAliveIfHasListenersFor(nsGkAtoms::onchange);
}
@@ -65,11 +65,6 @@ bool MediaQueryList::Matches() {
// will flush the parent document layout if appropriate.
doc->FlushPendingNotifications(FlushType::Layout);
}
if (!mMatchesValid) {
MOZ_ASSERT(!HasListeners(),
"when listeners present, must keep mMatches current");
RecomputeMatches();
}
return mMatches;
}
@@ -81,17 +76,7 @@ void MediaQueryList::AddListener(EventListener* aListener, ErrorResult& aRv) {
AddEventListenerOptionsOrBoolean options;
options.SetAsBoolean() = false;
AddEventListener(ONCHANGE_STRING, aListener, options, Nullable<bool>(), aRv);
}
void MediaQueryList::EventListenerAdded(nsAtom* aType) {
// HasListeners() might still be false if the added thing wasn't a
// listener we care about.
if (!mMatchesValid && HasListeners()) {
RecomputeMatches();
}
DOMEventTargetHelper::EventListenerAdded(aType);
AddEventListener(u"change"_ns, aListener, options, Nullable<bool>(), aRv);
}
void MediaQueryList::RemoveListener(EventListener* aListener,
@@ -103,30 +88,18 @@ void MediaQueryList::RemoveListener(EventListener* aListener,
EventListenerOptionsOrBoolean options;
options.SetAsBoolean() = false;
RemoveEventListener(ONCHANGE_STRING, aListener, options, aRv);
RemoveEventListener(u"change"_ns, aListener, options, aRv);
}
bool MediaQueryList::HasListeners() const {
return HasListenersFor(ONCHANGE_STRING);
return HasListenersFor(nsGkAtoms::onchange);
}
void MediaQueryList::Disconnect() {
DisconnectFromOwner();
IgnoreKeepAliveIfHasListenersFor(nsGkAtoms::onchange);
}
void MediaQueryList::RecomputeMatches() {
mMatches = false;
if (!mDocument) {
return;
}
mMatches = mMediaList->Matches(*mDocument);
mMatchesValid = true;
}
nsISupports* MediaQueryList::GetParentObject() const {
return ToSupports(mDocument);
}
@@ -136,17 +109,18 @@ JSObject* MediaQueryList::WrapObject(JSContext* aCx,
return MediaQueryList_Binding::Wrap(aCx, this, aGivenProto);
}
bool MediaQueryList::MediaFeatureValuesChanged() {
mMatchesValid = false;
void MediaQueryList::MediaFeatureValuesChanged() {
mMatches = mDocument && mMediaList->Matches(*mDocument);
// Note that mMatchesOnRenderingUpdate remains with the old value here.
// That gets updated in EvaluateOnRenderingUpdate().
}
if (!HasListeners()) {
return false; // No need to recompute or notify if we have no listeners.
bool MediaQueryList::EvaluateOnRenderingUpdate() {
if (mMatches == mMatchesOnRenderingUpdate) {
return false;
}
bool oldMatches = mMatches;
RecomputeMatches();
return mMatches != oldMatches;
mMatchesOnRenderingUpdate = mMatches;
return HasListeners();
}
void MediaQueryList::FireChangeEvent() {
@@ -157,9 +131,8 @@ void MediaQueryList::FireChangeEvent() {
mMediaList->GetText(init.mMedia);
RefPtr<MediaQueryListEvent> event =
MediaQueryListEvent::Constructor(this, ONCHANGE_STRING, init);
MediaQueryListEvent::Constructor(this, u"change"_ns, init);
event->SetTrusted(true);
DispatchEvent(*event);
}

View File

@@ -40,9 +40,11 @@ class MediaQueryList final : public DOMEventTargetHelper,
nsISupports* GetParentObject() const;
// Returns whether we need to notify of the change event using
// FireChangeEvent().
[[nodiscard]] bool MediaFeatureValuesChanged();
void MediaFeatureValuesChanged();
// Returns whether we need to notify of the change by dispatching a change
// event.
[[nodiscard]] bool EvaluateOnRenderingUpdate();
void FireChangeEvent();
JSObject* WrapObject(JSContext* aCx,
@@ -54,9 +56,6 @@ class MediaQueryList final : public DOMEventTargetHelper,
void AddListener(EventListener* aListener, ErrorResult& aRv);
void RemoveListener(EventListener* aListener, ErrorResult& aRv);
using DOMEventTargetHelper::EventListenerAdded;
void EventListenerAdded(nsAtom* aType) override;
IMPL_EVENT_HANDLER(change)
bool HasListeners() const;
@@ -91,11 +90,16 @@ class MediaQueryList final : public DOMEventTargetHelper,
// linked list.
RefPtr<Document> mDocument;
const RefPtr<const MediaList> mMediaList;
bool mMatches = false;
bool mMatchesValid = false;
// Whether our MediaList depends on our viewport size. Our medialist is
// immutable, so we can just compute this once and carry on with our lives.
const bool mViewportDependent;
const bool mViewportDependent : 1;
// The matches state.
// https://drafts.csswg.org/cssom-view/#mediaquerylist-matches-state
bool mMatches : 1;
// The value of the matches state on creation, or on the last rendering
// update, in order to implement:
// https://drafts.csswg.org/cssom-view/#evaluate-media-queries-and-report-changes
bool mMatchesOnRenderingUpdate : 1;
};
} // namespace mozilla::dom

View File

@@ -14,13 +14,13 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=1451199
SimpleTest.waitForExplicitFinish();
addLoadEvent(function() {
// We have to be careful to not check l.matches until the very end
// of the test.
var l = frames[0].matchMedia("(orientation: portrait)");
var iframe = document.querySelector("iframe");
iframe.width = "50";
iframe.contentDocument.documentElement.offsetWidth; // Flush layout
// We have to be careful to not check l.matches until the very end
// of the test.
var l = frames[0].matchMedia("(orientation: portrait)");
l.onchange = function() {
is(l.matches, false,
"Should not match portrait by the time we get notified");

View File

@@ -25,7 +25,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=1451199
is(l.matches, false,
"Should not match portrait by the time we get notified");
SimpleTest.finish();
});
}, { once: true });
iframe.width = "200";
});
</script>

View File

@@ -19,7 +19,13 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=542058
SimpleTest.waitForExplicitFinish();
function run() {
function tick() {
// MediaQueryList events are guaranteed to run before requestAnimationFrame
// per spec.
return new Promise(r => requestAnimationFrame(r));
}
async function run() {
var iframe = document.getElementById("subdoc");
var subdoc = iframe.contentDocument;
var subwin = iframe.contentWindow;
@@ -35,6 +41,7 @@ function run() {
iframe.style.width = w + "px";
iframe.style.height = h + "px";
subroot.offsetWidth; // flush layout
await tick();
function setup_mql(str) {
var obj = {
@@ -102,6 +109,7 @@ function run() {
var w2 = Math.floor(em_size * 10.3);
iframe.style.width = w2 + "px";
subroot.offsetWidth; // flush layout
await tick();
check_match(w_exact_w, false, "after width increase to around 10.3em");
check_notify(w_exact_w, 1, "after width increase to around 10.3em");
@@ -117,6 +125,7 @@ function run() {
var w3 = w * 2;
iframe.style.width = w3 + "px";
subroot.offsetWidth; // flush layout
await tick();
check_match(w_exact_w, false, "after width double from original");
check_notify(w_exact_w, 1, "after width double from original");
@@ -131,6 +140,7 @@ function run() {
SpecialPowers.setFullZoom(subwin, 2.0);
subroot.offsetWidth; // flush layout
await tick();
check_match(w_exact_w, true, "after zoom");
check_notify(w_exact_w, 2, "after zoom");
@@ -145,6 +155,8 @@ function run() {
SpecialPowers.setFullZoom(subwin, 1.0);
await tick();
finish_mql(w_exact_w);
finish_mql(w_min_9em);
finish_mql(w_min_10em);
@@ -152,9 +164,9 @@ function run() {
finish_mql(w_max_10em);
// Additional tests of listener mutation.
(function() {
var received = [];
var received_mql = [];
{
let received = [];
let received_mql = [];
function listener1(event) {
received.push(1);
received_mql.push(event.target);
@@ -166,8 +178,9 @@ function run() {
iframe.style.width = "200px";
subroot.offsetWidth; // flush layout
await tick();
var mql = subwin.matchMedia("(min-width: 150px)");
let mql = subwin.matchMedia("(min-width: 150px)");
mql.addListener(listener1);
mql.addListener(listener1);
mql.addListener(listener2);
@@ -175,6 +188,7 @@ function run() {
iframe.style.width = "100px";
subroot.offsetWidth; // flush layout
await tick();
is(JSON.stringify(received), "[1,2]", "duplicate listeners removed");
received = [];
@@ -182,6 +196,7 @@ function run() {
iframe.style.width = "200px";
subroot.offsetWidth; // flush layout
await tick();
is(JSON.stringify(received), "[2]", "listener removal");
received = [];
@@ -189,6 +204,7 @@ function run() {
iframe.style.width = "100px";
subroot.offsetWidth; // flush layout
await tick();
is(JSON.stringify(received), "[2,1]", "listeners notified in order");
received = [];
@@ -196,6 +212,7 @@ function run() {
iframe.style.width = "200px";
subroot.offsetWidth; // flush layout
await tick();
is(JSON.stringify(received), "[2,1]", "add of existing listener is no-op");
received = [];
@@ -203,6 +220,7 @@ function run() {
iframe.style.width = "100px";
subroot.offsetWidth; // flush layout
await tick();
is(JSON.stringify(received), "[2,1]", "add of existing listener is no-op");
mql.removeListener(listener2);
@@ -215,6 +233,7 @@ function run() {
iframe.style.width = "200px";
subroot.offsetWidth; // flush layout
await tick();
// mql (1, 2), mql2 (1)
is(JSON.stringify(received), "[1,2,1]",
@@ -241,6 +260,7 @@ function run() {
iframe.style.width = "100px";
subroot.offsetWidth; // flush layout
await tick();
// mql(1, 3)
is(JSON.stringify(received), "[1,3]",
@@ -254,6 +274,7 @@ function run() {
iframe.style.width = "200px";
subroot.offsetWidth; // flush layout
await tick();
// mql(1, 3)
is(JSON.stringify(received), "[1,3]",
@@ -262,10 +283,10 @@ function run() {
"notification order (removal tests)");
is(received_mql[1], mql,
"notification order (removal tests)");
})();
}
/* Bug 753777: test that things work in a freshly-created iframe */
(function() {
{
let newIframe = document.createElement("iframe");
document.body.appendChild(newIframe);
@@ -277,38 +298,41 @@ function run() {
"(max-width: 1px) should not match in newly-created iframe");
document.body.removeChild(newIframe);
})();
}
/* Bug 716751: listeners lost due to GC */
var gc_received = [];
(function() {
var received = [];
var listener1 = function(event) {
{
let received = [];
let listener1 = function(event) {
gc_received.push(1);
}
iframe.style.width = "200px";
subroot.offsetWidth; // flush layout
await tick();
var mql = subwin.matchMedia("(min-width: 150px)");
let mql = subwin.matchMedia("(min-width: 150px)");
mql.addListener(listener1);
is(JSON.stringify(gc_received), "[]", "GC test: before notification");
iframe.style.width = "100px";
subroot.offsetWidth; // flush layout
await tick();
is(JSON.stringify(gc_received), "[1]", "GC test: after notification 1");
// Because of conservative GC, we need to go back to the event loop
// to GC properly.
setTimeout(step2, 0);
})();
}
function step2() {
async function step2() {
SpecialPowers.DOMWindowUtils.garbageCollect();
iframe.style.width = "200px";
subroot.offsetWidth; // flush layout
await tick();
is(JSON.stringify(gc_received), "[1,1]", "GC test: after notification 2");
@@ -316,13 +340,14 @@ function run() {
}
/* Bug 1270626: listeners that throw exceptions */
function bug1270626() {
async function bug1270626() {
var throwingListener = function(event) {
throw "error";
}
iframe.style.width = "200px";
subroot.offsetWidth; // flush layout
await tick();
var mql = subwin.matchMedia("(min-width: 150px)");
mql.addListener(throwingListener);
@@ -333,6 +358,7 @@ function run() {
iframe.style.width = "100px";
subroot.offsetWidth; // flush layout
await tick();
is(SimpleTest.isExpectingUncaughtException(), false,
"should have gotten an uncaught exception");

View File

@@ -1,30 +0,0 @@
[media-query-matches-in-iframe.html]
[matchMedia('(max-width: 150px)') should not receive a change event until update the rendering step of HTML5 event loop]
expected: FAIL
[matchMedia('(width: 100px)') should not receive a change event until update the rendering step of HTML5 event loop]
expected: FAIL
[matchMedia('(orientation: portrait)') should not receive a change event until update the rendering step of HTML5 event loop]
expected: FAIL
[matchMedia('(aspect-ratio: 1/1)') should not receive a change event until update the rendering step of HTML5 event loop]
expected: FAIL
[matchMedia('(max-aspect-ratio: 4/3)') should not receive a change event until update the rendering step of HTML5 event loop]
expected: FAIL
[matchMedia('(max-width: 150px)') should receive a change event after resize event on the window but before a requestAnimationFrame callback is called]
expected: FAIL
[matchMedia('(width: 100px)') should receive a change event after resize event on the window but before a requestAnimationFrame callback is called]
expected: FAIL
[matchMedia('(orientation: portrait)') should receive a change event after resize event on the window but before a requestAnimationFrame callback is called]
expected: FAIL
[matchMedia('(aspect-ratio: 1/1)') should receive a change event after resize event on the window but before a requestAnimationFrame callback is called]
expected: FAIL
[matchMedia('(max-aspect-ratio: 4/3)') should receive a change event after resize event on the window but before a requestAnimationFrame callback is called]
expected: FAIL