Backed out 4 changesets (bug 1737918) for causing bustages in ScrollTimeline.cpp CLOSED TREE

Backed out changeset cecdd071c1aa (bug 1737918)
Backed out changeset fcc5ecd364e4 (bug 1737918)
Backed out changeset f7bf3143e4a7 (bug 1737918)
Backed out changeset a143d2e54fb9 (bug 1737918)
This commit is contained in:
Noemi Erli 2022-04-21 01:21:14 +03:00
Родитель 4216396f3e
Коммит 560b5ccdb5
10 изменённых файлов: 60 добавлений и 376 удалений

Просмотреть файл

@ -55,7 +55,6 @@ exports.CSS_PROPERTIES = {
"revert",
"revert-layer",
"running",
"scroll",
"step-end",
"step-start",
"steps",
@ -1328,7 +1327,6 @@ exports.CSS_PROPERTIES = {
"revert",
"revert-layer",
"running",
"scroll",
"step-end",
"step-start",
"steps",
@ -3279,7 +3277,6 @@ exports.CSS_PROPERTIES = {
"revert",
"revert-layer",
"running",
"scroll",
"step-end",
"step-start",
"steps",

Просмотреть файл

@ -7,7 +7,6 @@
#include "ScrollTimeline.h"
#include "mozilla/dom/Animation.h"
#include "mozilla/dom/ElementInlines.h"
#include "mozilla/AnimationTarget.h"
#include "mozilla/DisplayPortUtils.h"
#include "mozilla/PresShell.h"
@ -46,11 +45,14 @@ NS_IMPL_ISUPPORTS_CYCLE_COLLECTION_INHERITED_0(ScrollTimeline,
TimingParams ScrollTimeline::sTiming;
ScrollTimeline::ScrollTimeline(Document* aDocument, const Scroller& aScroller,
StyleScrollAxis aAxis)
StyleScrollDirection aDirection)
: AnimationTimeline(aDocument->GetParentObject()),
mDocument(aDocument),
// FIXME: Bug 1737918: We may have to udpate the constructor arguments
// because this can be nearest, root, or a specific container. For now,
// the input is a source element directly and it is the root element.
mSource(aScroller),
mAxis(aAxis) {
mDirection(aDirection) {
MOZ_ASSERT(aDocument);
// Use default values except for |mDuration| and |mFill|.
@ -61,28 +63,6 @@ ScrollTimeline::ScrollTimeline(Document* aDocument, const Scroller& aScroller,
PlaybackDirection::Alternate, FillMode::Both);
}
static StyleScrollAxis ToStyleScrollAxis(
const StyleScrollDirection aDirection) {
switch (aDirection) {
// The spec defines auto, but there is a spec issue:
// "ISSUE 5 Define these values." in this section. The DOM interface removed
// auto and use block as default value, so we treat auto as block now.
// https://drafts.csswg.org/scroll-animations-1/#descdef-scroll-timeline-orientation
case StyleScrollDirection::Auto:
case StyleScrollDirection::Block:
return StyleScrollAxis::Block;
case StyleScrollDirection::Inline:
return StyleScrollAxis::Inline;
case StyleScrollDirection::Horizontal:
return StyleScrollAxis::Horizontal;
case StyleScrollDirection::Vertical:
return StyleScrollAxis::Vertical;
}
MOZ_ASSERT_UNREACHABLE("Unsupported StyleScrollDirection");
return StyleScrollAxis::Block;
}
already_AddRefed<ScrollTimeline> ScrollTimeline::FromRule(
const RawServoScrollTimelineRule& aRule, Document* aDocument,
const NonOwningAnimationTarget& aTarget) {
@ -93,56 +73,19 @@ already_AddRefed<ScrollTimeline> ScrollTimeline::FromRule(
// dropped automatically becuase no animation owns it and its ref-count
// becomes zero.
StyleScrollAxis axis =
ToStyleScrollAxis(Servo_ScrollTimelineRule_GetOrientation(&aRule));
StyleScrollDirection direction =
Servo_ScrollTimelineRule_GetOrientation(&aRule);
// FIXME: Bug 1737918: applying new spec update, e.g. other scrollers and
// other style values.
RefPtr<ScrollTimeline> timeline;
auto autoScroller = Scroller::Root(aTarget.mElement->OwnerDoc());
auto autoScroller = Scroller::Auto(aTarget.mElement->OwnerDoc());
auto* set =
ScrollTimelineSet::GetOrCreateScrollTimelineSet(autoScroller.mElement);
auto p = set->LookupForAdd(axis);
auto p = set->LookupForAdd(direction);
if (!p) {
timeline = new ScrollTimeline(aDocument, autoScroller, axis);
set->Add(p, axis, timeline);
} else {
timeline = p->value();
}
return timeline.forget();
}
/* static */
already_AddRefed<ScrollTimeline> ScrollTimeline::FromAnonymousScroll(
Document* aDocument, const NonOwningAnimationTarget& aTarget,
StyleScrollAxis aAxis, StyleScroller aScroller) {
MOZ_ASSERT(aTarget);
Scroller scroller;
switch (aScroller) {
case StyleScroller::Root:
scroller = Scroller::Root(aTarget.mElement->OwnerDoc());
break;
case StyleScroller::Nearest: {
Element* curr = aTarget.mElement->GetFlattenedTreeParentElement();
Element* root = aTarget.mElement->OwnerDoc()->GetDocumentElement();
while (curr && curr != root) {
const ComputedStyle* style = Servo_Element_GetMaybeOutOfDateStyle(curr);
MOZ_ASSERT(style, "The ancestor should be styled.");
if (style->StyleDisplay()->IsScrollableOverflow()) {
break;
}
curr = curr->GetFlattenedTreeParentElement();
}
// If there is no scroll container, we use root.
scroller = Scroller::Nearest(curr ? curr : root);
}
}
RefPtr<ScrollTimeline> timeline;
auto* set =
ScrollTimelineSet::GetOrCreateScrollTimelineSet(scroller.mElement);
auto p = set->LookupForAdd(aAxis);
if (!p) {
timeline = new ScrollTimeline(aDocument, scroller, aAxis);
set->Add(p, aAxis, timeline);
timeline = new ScrollTimeline(aDocument, autoScroller, direction);
set->Add(p, direction, timeline);
} else {
timeline = p->value();
}
@ -191,9 +134,12 @@ layers::ScrollDirection ScrollTimeline::Axis() const {
MOZ_ASSERT(mSource && mSource.mElement->GetPrimaryFrame());
const WritingMode wm = mSource.mElement->GetPrimaryFrame()->GetWritingMode();
return mAxis == StyleScrollAxis::Horizontal ||
(!wm.IsVertical() && mAxis == StyleScrollAxis::Inline) ||
(wm.IsVertical() && mAxis == StyleScrollAxis::Block)
return mDirection == StyleScrollDirection::Horizontal ||
(!wm.IsVertical() &&
mDirection == StyleScrollDirection::Inline) ||
(wm.IsVertical() &&
(mDirection == StyleScrollDirection::Block ||
mDirection == StyleScrollDirection::Auto))
? layers::ScrollDirection::eHorizontal
: layers::ScrollDirection::eVertical;
}
@ -231,7 +177,7 @@ void ScrollTimeline::UnregisterFromScrollSource() {
if (ScrollTimelineSet* scrollTimelineSet =
ScrollTimelineSet::GetScrollTimelineSet(mSource.mElement)) {
scrollTimelineSet->Remove(mAxis);
scrollTimelineSet->Remove(mDirection);
if (scrollTimelineSet->IsEmpty()) {
ScrollTimelineSet::DestroyScrollTimelineSet(mSource.mElement);
}
@ -244,15 +190,17 @@ const nsIScrollableFrame* ScrollTimeline::GetScrollFrame() const {
}
switch (mSource.mType) {
case StyleScroller::Root:
case Scroller::Type::Auto:
if (const PresShell* presShell =
mSource.mElement->OwnerDoc()->GetPresShell()) {
return presShell->GetRootScrollFrameAsScrollable();
}
return nullptr;
case StyleScroller::Nearest:
break;
case Scroller::Type::Other:
default:
return nsLayoutUtils::FindScrollableFrameFor(mSource.mElement);
}
return nullptr;
}
// ---------------------------------

Просмотреть файл

@ -65,22 +65,25 @@ class Element;
class ScrollTimeline final : public AnimationTimeline {
public:
struct Scroller {
StyleScroller mType = StyleScroller::Root;
// FIXME: Support nearest and replace auto with root in Bug 1737918.
enum class Type : uint8_t {
// For auto. Should be scrolling element of the owner doc.
Auto,
// For any other specific elements.
Other,
};
Type mType = Type::Auto;
RefPtr<Element> mElement;
// We use the owner doc of the animation target. This may be different from
// |mDocument| after we implement ScrollTimeline interface for script.
static Scroller Root(const Document* aOwnerDoc) {
static Scroller Auto(const Document* aOwnerDoc) {
// For auto, we use scrolling element as the default scroller.
// However, it's mutable, and we would like to keep things simple, so
// we always register the ScrollTimeline to the document element (i.e.
// root element) because the content of the root scroll frame is the root
// element.
return {StyleScroller::Root, aOwnerDoc->GetDocumentElement()};
}
static Scroller Nearest(Element* aElement) {
return {StyleScroller::Nearest, aElement};
return {Type::Auto, aOwnerDoc->GetDocumentElement()};
}
explicit operator bool() const { return mElement; }
@ -89,17 +92,14 @@ class ScrollTimeline final : public AnimationTimeline {
}
};
// FIXME: Bug 1737918: Rewrite this because @scroll-timeline will be obsolete.
static already_AddRefed<ScrollTimeline> FromRule(
const RawServoScrollTimelineRule& aRule, Document* aDocument,
const NonOwningAnimationTarget& aTarget);
static already_AddRefed<ScrollTimeline> FromAnonymousScroll(
Document* aDocument, const NonOwningAnimationTarget& aTarget,
StyleScrollAxis aAxis, StyleScroller aScroller);
bool operator==(const ScrollTimeline& aOther) const {
return mDocument == aOther.mDocument && mSource == aOther.mSource &&
mAxis == aOther.mAxis;
mDirection == aOther.mDirection;
}
NS_DECL_ISUPPORTS_INHERITED
@ -154,6 +154,11 @@ class ScrollTimeline final : public AnimationTimeline {
}
// A helper to get the physical orientation of this scroll-timeline.
//
// The spec defines auto, but there is a spec issue:
// "ISSUE 5 Define these values." in this section. The DOM interface removed
// auto and use block as default value, so we treat auto as block now.
// https://drafts.csswg.org/scroll-animations-1/#descdef-scroll-timeline-orientation
layers::ScrollDirection Axis() const;
StyleOverflow SourceScrollStyle() const;
@ -170,7 +175,7 @@ class ScrollTimeline final : public AnimationTimeline {
private:
ScrollTimeline() = delete;
ScrollTimeline(Document* aDocument, const Scroller& aScroller,
StyleScrollAxis aAxis);
StyleScrollDirection aDirection);
// Note: This function is required to be idempotent, as it can be called from
// both cycleCollection::Unlink() and ~ScrollTimeline(). When modifying this
@ -184,11 +189,14 @@ class ScrollTimeline final : public AnimationTimeline {
RefPtr<Document> mDocument;
// FIXME: Bug 1765211: We may have to update the source element once the
// overflow property of the scroll-container is updated when we are using
// nearest scroller.
// FIXME: Bug 1733260: new spec proposal uses a new way to define scroller,
// and move the element-based offset into view-timeline, so here we only
// implement the default behavior of scroll timeline:
// 1. "source" is auto (use scrolling element), and
// 2. "scroll-offsets" is none (i.e. always 0% ~ 100%).
// So now we will only use the scroll direction from @scroll-timeline rule.
Scroller mSource;
StyleScrollAxis mAxis;
StyleScrollDirection mDirection;
// Note: it's unfortunate TimingParams cannot be a const variable because
// we have to use StickyTimingDuration::FromMilliseconds() in its
@ -212,15 +220,16 @@ class ScrollTimeline final : public AnimationTimeline {
*/
class ScrollTimelineSet {
public:
// Use StyleScrollAxis as the key, so we reuse the ScrollTimeline with the
// same source and the same direction.
// Use StyleScrollDirection as the key, so we reuse the ScrollTimeline with
// the same source and the same direction.
// Note: the drawback of using the direction as the key is that we have to
// update this once we support more descriptors. This implementation assumes
// scroll-offsets will be obsolute. However, I'm pretty sure @scroll-timeline
// will be obsolute, based on the spec issue. We may have to do a lot of
// updates after the spec updates, so this tentative implmentation should be
// enough for now.
using NonOwningScrollTimelineMap = HashMap<StyleScrollAxis, ScrollTimeline*>;
using NonOwningScrollTimelineMap =
HashMap<StyleScrollDirection, ScrollTimeline*>;
~ScrollTimelineSet() = default;
@ -228,14 +237,14 @@ class ScrollTimelineSet {
static ScrollTimelineSet* GetOrCreateScrollTimelineSet(Element* aElement);
static void DestroyScrollTimelineSet(Element* aElement);
NonOwningScrollTimelineMap::AddPtr LookupForAdd(StyleScrollAxis aKey) {
NonOwningScrollTimelineMap::AddPtr LookupForAdd(StyleScrollDirection aKey) {
return mScrollTimelines.lookupForAdd(aKey);
}
void Add(NonOwningScrollTimelineMap::AddPtr& aPtr, StyleScrollAxis aKey,
void Add(NonOwningScrollTimelineMap::AddPtr& aPtr, StyleScrollDirection aKey,
ScrollTimeline* aScrollTimeline) {
Unused << mScrollTimelines.add(aPtr, aKey, aScrollTimeline);
}
void Remove(StyleScrollAxis aKey) { mScrollTimelines.remove(aKey); }
void Remove(StyleScrollDirection aKey) { mScrollTimelines.remove(aKey); }
bool IsEmpty() const { return mScrollTimelines.empty(); }

Просмотреть файл

@ -205,6 +205,8 @@ static void UpdateOldAnimationPropertiesWithNew(
}
}
// FIXME: Bug 1737918: Update the syntax of animation-timeline, and null
// timeline may be obsolete.
static already_AddRefed<dom::AnimationTimeline> GetTimeline(
const StyleAnimationTimeline& aStyleTimeline, nsPresContext* aPresContext,
const NonOwningAnimationTarget& aTarget) {
@ -227,12 +229,6 @@ static already_AddRefed<dom::AnimationTimeline> GetTimeline(
}
break;
}
case StyleAnimationTimeline::Tag::Scroll: {
const auto& scroll = aStyleTimeline.AsScroll();
timeline = ScrollTimeline::FromAnonymousScroll(
aPresContext->Document(), aTarget, scroll._0, scroll._1);
break;
}
case StyleAnimationTimeline::Tag::None:
// Keep nullptr.
break;

Просмотреть файл

@ -13533,15 +13533,6 @@ if (IsCSSPropertyPrefEnabled("layout.css.scroll-linked-animations.enabled")) {
"-\\32 0bounce",
"\\2bounce",
"-\\2bounce",
"scroll()",
"scroll(block)",
"scroll(inline)",
"scroll(horizontal)",
"scroll(vertical)",
"scroll(root)",
"scroll(nearest)",
"scroll(inline nearest)",
"scroll(vertical root)",
],
invalid_values: [
"bounce, initial",

Просмотреть файл

@ -752,80 +752,6 @@ impl Parse for AnimationName {
}
}
/// A value for the <Scroller> used in scroll().
///
/// https://drafts.csswg.org/scroll-animations-1/rewrite#typedef-scroller
#[derive(
Clone,
Debug,
Eq,
Hash,
MallocSizeOf,
Parse,
PartialEq,
SpecifiedValueInfo,
ToComputedValue,
ToCss,
ToResolvedValue,
ToShmem,
)]
#[repr(u8)]
pub enum Scroller {
/// The nearest ancestor scroll container. (Default.)
Nearest,
/// The document viewport as the scroll container.
Root,
// FIXME: Bug 1764450: Once we support container-name CSS property (Bug 1744224), we may add
// <custom-ident> here, based on the result of the spec issue:
// https://github.com/w3c/csswg-drafts/issues/7046
}
impl Default for Scroller {
fn default() -> Self {
Self::Nearest
}
}
/// A value for the <Axis> used in scroll().
///
/// https://drafts.csswg.org/scroll-animations-1/rewrite#typedef-axis
#[derive(
Clone,
Debug,
Eq,
Hash,
MallocSizeOf,
Parse,
PartialEq,
SpecifiedValueInfo,
ToComputedValue,
ToCss,
ToResolvedValue,
ToShmem,
)]
#[repr(u8)]
pub enum ScrollAxis {
/// The block axis of the scroll container. (Default.)
Block,
/// The inline axis of the scroll container.
Inline,
/// The vertical block axis of the scroll container.
Vertical,
/// The horizontal axis of the scroll container.
Horizontal,
}
impl Default for ScrollAxis {
fn default() -> Self {
Self::Block
}
}
#[inline]
fn is_default<T: Default + PartialEq>(value: &T) -> bool {
*value == Default::default()
}
/// A value for the <single-animation-timeline>.
///
/// https://drafts.csswg.org/css-animations-2/#typedef-single-animation-timeline
@ -851,13 +777,6 @@ pub enum AnimationTimeline {
None,
/// The scroll-timeline name
Timeline(TimelineName),
/// The scroll() notation
/// https://drafts.csswg.org/scroll-animations-1/rewrite#scroll-notation
#[css(function)]
Scroll(
#[css(skip_if = "is_default")] ScrollAxis,
#[css(skip_if = "is_default")] Scroller,
),
}
impl AnimationTimeline {
@ -890,16 +809,6 @@ impl Parse for AnimationTimeline {
return Ok(Self::None);
}
// https://drafts.csswg.org/scroll-animations-1/rewrite#scroll-notation
if input.try_parse(|i| i.expect_function_matching("scroll")).is_ok() {
return input.parse_nested_block(|i| {
Ok(Self::Scroll(
i.try_parse(ScrollAxis::parse).unwrap_or(ScrollAxis::Block),
i.try_parse(Scroller::parse).unwrap_or(Scroller::Nearest),
))
});
}
TimelineName::parse(context, input).map(AnimationTimeline::Timeline)
}
}

Просмотреть файл

@ -1 +1 @@
prefs: [layout.css.scroll-linked-animations.enabled:true, layout.css.individual-transform.enabled:true]
prefs: [layout.css.scroll-linked-animations.enabled:true]

Просмотреть файл

@ -42,21 +42,4 @@ test(() => {
let style = document.getElementById('target').style;
assert_not_equals(style.cssText.indexOf('animation-timeline'), -1);
}, 'The animation-timeline property shows up in CSSStyleDeclaration.cssText');
// The tentative scroll() notation:
// animation-timeline: scroll(<axis>? <scroller>?);
// <axis> = block | inline | vertical | horizontal
// <scroller> = root | nearest | <container-name>
//
// https://github.com/w3c/csswg-drafts/issues/6674
test_computed_value('animation-timeline', 'scroll()');
test_computed_value('animation-timeline', 'scroll(block)', 'scroll()');
test_computed_value('animation-timeline', 'scroll(inline)');
test_computed_value('animation-timeline', 'scroll(horizontal)');
test_computed_value('animation-timeline', 'scroll(vertical)');
test_computed_value('animation-timeline', 'scroll(root)');
test_computed_value('animation-timeline', 'scroll(nearest)', 'scroll()');
test_computed_value('animation-timeline', 'scroll(inline nearest)', 'scroll(inline)');
test_computed_value('animation-timeline', 'scroll(vertical root)');
// TODO: add <container-name> cases
</script>

Просмотреть файл

@ -34,24 +34,4 @@ test_invalid_value('animation-timeline', 'foo bar');
test_invalid_value('animation-timeline', '"foo" "bar"');
test_invalid_value('animation-timeline', 'rgb(1, 2, 3)');
test_invalid_value('animation-timeline', '#fefefe');
// The tentative scroll() notation:
// animation-timeline: scroll(<axis>? <scroller>?);
// <axis> = block | inline | vertical | horizontal
// <scroller> = root | nearest | <container-name>
//
// https://github.com/w3c/csswg-drafts/issues/6674
test_valid_value('animation-timeline', 'scroll()');
test_valid_value('animation-timeline', 'scroll(block)', 'scroll()');
test_valid_value('animation-timeline', 'scroll(inline)');
test_valid_value('animation-timeline', 'scroll(horizontal)');
test_valid_value('animation-timeline', 'scroll(vertical)');
test_valid_value('animation-timeline', 'scroll(root)');
test_valid_value('animation-timeline', 'scroll(nearest)', 'scroll()');
test_valid_value('animation-timeline', 'scroll(inline nearest)', 'scroll(inline)');
test_valid_value('animation-timeline', 'scroll(vertical root)');
// TODO: add <container-name> cases
test_invalid_value('animation-timeline', 'scroll(root block)');
test_invalid_value('animation-timeline', 'scroll(abc root)');
</script>

Просмотреть файл

@ -1,129 +0,0 @@
<!DOCTYPE html>
<title>The animation-timeline: scroll() notation</title>
<meta name="viewport" content="width=device-width,initial-scale=1,minimum-scale=1">
<link rel="help" src="https://drafts.csswg.org/scroll-animations-1/rewrite#scroll-notation">
<link rel="help" src="https://github.com/w3c/csswg-drafts/issues/6674">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<style>
@keyframes anim {
from { translate: 50px; }
to { translate: 150px; }
}
html {
min-height: 100vh;
/* This makes the max scrollable ragne be 100px in root element */
padding-bottom: 100px;
}
#container {
width: 300px;
height: 300px;
overflow: scroll;
}
#target {
width: 100px;
/* This makes the max scrollable ragne be 100px in the block direction */
height: 100px;
}
/* large block content */
.block-content {
block-size: 100%;
}
/* large inline content */
.inline-content {
inline-size: 100%;
block-size: 5px;
/* This makes the max scrollable ragne be 100px in the inline direction */
padding-inline-end: 100px;
}
</style>
<body>
<div id="log"></div>
<script>
"use strict";
const root = document.scrollingElement;
const createTargetWithStuff = function(t, contentClass) {
let container = document.createElement('div');
container.id = 'container';
let target = document.createElement('div');
target.id = 'target';
let content = document.createElement('div');
content.className = contentClass;
// <div id='container'>
// <div id='target'></div>
// <div class=contentClass></div>
// </div>
document.body.appendChild(container);
container.appendChild(target);
container.appendChild(content);
if (t && typeof t.add_cleanup === 'function') {
t.add_cleanup(() => {
content.remove();
target.remove();
container.remove();
});
}
return [container, target];
};
test(t => {
let [container, div] = createTargetWithStuff(t, 'block-content');
div.style.animation = "anim 10s linear scroll(nearest)";
root.scrollTop = 50;
assert_equals(getComputedStyle(div).translate, '50px');
container.scrollTop = 50;
assert_equals(getComputedStyle(div).translate, '100px');
root.scrollTop = 0;
}, 'animation-timeline: scroll(nearest)');
test(t => {
let [container, div] = createTargetWithStuff(t, 'block-content');
div.style.animation = "anim 10s linear scroll(root)";
container.scrollTop = 50;
assert_equals(getComputedStyle(div).translate, '50px');
root.scrollTop = 50;
assert_equals(getComputedStyle(div).translate, '100px');
root.scrollTop = 0;
}, 'animation-timeline: scroll(root)');
test(t => {
let [container, div] = createTargetWithStuff(t, 'inline-content');
div.style.animation = "anim 10s linear scroll(inline)";
container.scrollLeft = 50;
assert_equals(getComputedStyle(div).translate, '100px');
}, 'animation-timeline: scroll(inline)');
test(t => {
let [container, div] = createTargetWithStuff(t, 'block-content');
container.style.writingMode = 'vertical-lr';
div.style.animation = "anim 10s linear scroll(horizontal)";
container.scrollLeft = 50;
assert_equals(getComputedStyle(div).translate, '100px');
}, 'animation-timeline: scroll(horizontal)');
test(t => {
let [container, div] = createTargetWithStuff(t, 'inline-content');
container.style.writingMode = 'vertical-lr';
div.style.animation = "anim 10s linear scroll(vertical)";
container.scrollTop = 50;
assert_equals(getComputedStyle(div).translate, '100px');
}, 'animation-timeline: scroll(vertical)');
// TODO: Add more tests which change the overflow property of the container for
// scroll(nearest)
</script>
</body>