Commit e27a07a0 authored by Stephen McGruer's avatar Stephen McGruer Committed by Commit Bot

[ScrollTimeline] Support a null scrollSource

Bug: 909711
Change-Id: I7088eac2946a4b71ece3cea2b54d3ac9b400116c
Reviewed-on: https://chromium-review.googlesource.com/c/1354060Reviewed-by: default avatarMajid Valipour <majidvp@chromium.org>
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613152}
parent d1a3f83c
...@@ -65,8 +65,10 @@ bool StringToScrollOffset(String scroll_offset, CSSPrimitiveValue** result) { ...@@ -65,8 +65,10 @@ bool StringToScrollOffset(String scroll_offset, CSSPrimitiveValue** result) {
Node* ResolveScrollSource(Element* scroll_source) { Node* ResolveScrollSource(Element* scroll_source) {
// When in quirks mode we need the style to be clean, so we don't use // When in quirks mode we need the style to be clean, so we don't use
// |ScrollingElementNoLayout|. // |ScrollingElementNoLayout|.
if (scroll_source == scroll_source->GetDocument().scrollingElement()) if (scroll_source &&
scroll_source == scroll_source->GetDocument().scrollingElement()) {
return &scroll_source->GetDocument(); return &scroll_source->GetDocument();
}
return scroll_source; return scroll_source;
} }
} // namespace } // namespace
...@@ -122,14 +124,16 @@ ScrollTimeline::ScrollTimeline(Element* scroll_source, ...@@ -122,14 +124,16 @@ ScrollTimeline::ScrollTimeline(Element* scroll_source,
start_scroll_offset_(start_scroll_offset), start_scroll_offset_(start_scroll_offset),
end_scroll_offset_(end_scroll_offset), end_scroll_offset_(end_scroll_offset),
time_range_(time_range) { time_range_(time_range) {
DCHECK(scroll_source_);
} }
double ScrollTimeline::currentTime(bool& is_null) { double ScrollTimeline::currentTime(bool& is_null) {
is_null = true; is_null = true;
// 1. If scrollSource does not currently have a CSS layout box, or if its
// layout box is not a scroll container, return an unresolved time value. // 1. If scrollSource is null, does not currently have a CSS layout box, or if
LayoutBox* layout_box = resolved_scroll_source_->GetLayoutBox(); // its layout box is not a scroll container, return an unresolved time value.
LayoutBox* layout_box = resolved_scroll_source_
? resolved_scroll_source_->GetLayoutBox()
: nullptr;
if (!layout_box || !layout_box->HasOverflowClip()) { if (!layout_box || !layout_box->HasOverflowClip()) {
return std::numeric_limits<double>::quiet_NaN(); return std::numeric_limits<double>::quiet_NaN();
} }
...@@ -291,6 +295,9 @@ void ScrollTimeline::ResolveScrollStartAndEnd( ...@@ -291,6 +295,9 @@ void ScrollTimeline::ResolveScrollStartAndEnd(
} }
void ScrollTimeline::AttachAnimation() { void ScrollTimeline::AttachAnimation() {
if (!resolved_scroll_source_)
return;
GetActiveScrollTimelineSet().insert(resolved_scroll_source_); GetActiveScrollTimelineSet().insert(resolved_scroll_source_);
if (resolved_scroll_source_->IsElementNode()) if (resolved_scroll_source_->IsElementNode())
ToElement(resolved_scroll_source_)->SetNeedsCompositingUpdate(); ToElement(resolved_scroll_source_)->SetNeedsCompositingUpdate();
...@@ -306,6 +313,9 @@ void ScrollTimeline::AttachAnimation() { ...@@ -306,6 +313,9 @@ void ScrollTimeline::AttachAnimation() {
} }
void ScrollTimeline::DetachAnimation() { void ScrollTimeline::DetachAnimation() {
if (!resolved_scroll_source_)
return;
GetActiveScrollTimelineSet().erase(resolved_scroll_source_); GetActiveScrollTimelineSet().erase(resolved_scroll_source_);
if (resolved_scroll_source_->IsElementNode()) if (resolved_scroll_source_->IsElementNode())
ToElement(resolved_scroll_source_)->SetNeedsCompositingUpdate(); ToElement(resolved_scroll_source_)->SetNeedsCompositingUpdate();
......
...@@ -58,8 +58,10 @@ class CORE_EXPORT ScrollTimeline final : public AnimationTimeline { ...@@ -58,8 +58,10 @@ class CORE_EXPORT ScrollTimeline final : public AnimationTimeline {
// Returns the Node that should actually have the ScrollableArea (if one // Returns the Node that should actually have the ScrollableArea (if one
// exists). This can differ from |scrollSource| when |scroll_source_| is the // exists). This can differ from |scrollSource| when |scroll_source_| is the
// Document's scrollingElement. // Document's scrollingElement, and it may be null if the document was removed
// before the ScrollTimeline was created.
Node* ResolvedScrollSource() const { return resolved_scroll_source_; } Node* ResolvedScrollSource() const { return resolved_scroll_source_; }
ScrollDirection GetOrientation() const { return orientation_; } ScrollDirection GetOrientation() const { return orientation_; }
void GetCurrentAndMaxOffset(const LayoutBox*, void GetCurrentAndMaxOffset(const LayoutBox*,
......
...@@ -14,7 +14,7 @@ enum ScrollDirection { ...@@ -14,7 +14,7 @@ enum ScrollDirection {
enum ScrollTimelineAutoKeyword { "auto" }; enum ScrollTimelineAutoKeyword { "auto" };
dictionary ScrollTimelineOptions { dictionary ScrollTimelineOptions {
Element scrollSource; Element? scrollSource = null;
ScrollDirection orientation = "block"; ScrollDirection orientation = "block";
DOMString startScrollOffset = "auto"; DOMString startScrollOffset = "auto";
DOMString endScrollOffset = "auto"; DOMString endScrollOffset = "auto";
......
...@@ -220,4 +220,24 @@ TEST_F(ScrollTimelineTest, ...@@ -220,4 +220,24 @@ TEST_F(ScrollTimelineTest,
EXPECT_EQ(&GetDocument(), scroll_timeline->ResolvedScrollSource()); EXPECT_EQ(&GetDocument(), scroll_timeline->ResolvedScrollSource());
} }
TEST_F(ScrollTimelineTest, AttachOrDetachAnimationWithNullScrollSource) {
// Directly call the constructor to make it easier to pass a null
// scrollSource. The alternative approach would require us to remove the
// documentElement from the document.
Element* scroll_source = nullptr;
CSSPrimitiveValue* start_scroll_offset = nullptr;
CSSPrimitiveValue* end_scroll_offset = nullptr;
ScrollTimeline* scroll_timeline =
new ScrollTimeline(scroll_source, ScrollTimeline::Block,
start_scroll_offset, end_scroll_offset, 100);
// Sanity checks.
ASSERT_EQ(scroll_timeline->scrollSource(), nullptr);
ASSERT_EQ(scroll_timeline->ResolvedScrollSource(), nullptr);
// These calls should be no-ops in this mode, and shouldn't crash.
scroll_timeline->AttachAnimation();
scroll_timeline->DetachAnimation();
}
} // namespace blink } // namespace blink
...@@ -122,11 +122,11 @@ bool CheckElementComposited(const Node& target) { ...@@ -122,11 +122,11 @@ bool CheckElementComposited(const Node& target) {
} }
base::Optional<CompositorElementId> GetCompositorScrollElementId( base::Optional<CompositorElementId> GetCompositorScrollElementId(
const Node& node) { const Node* node) {
if (!node.GetLayoutObject() || !node.GetLayoutObject()->UniqueId()) if (!node || !node->GetLayoutObject() || !node->GetLayoutObject()->UniqueId())
return base::nullopt; return base::nullopt;
return CompositorElementIdFromUniqueObjectId( return CompositorElementIdFromUniqueObjectId(
node.GetLayoutObject()->UniqueId(), node->GetLayoutObject()->UniqueId(),
CompositorElementIdNamespace::kScroll); CompositorElementIdNamespace::kScroll);
} }
...@@ -199,7 +199,7 @@ std::unique_ptr<CompositorScrollTimeline> ToCompositorScrollTimeline( ...@@ -199,7 +199,7 @@ std::unique_ptr<CompositorScrollTimeline> ToCompositorScrollTimeline(
ScrollTimeline* scroll_timeline = ToScrollTimeline(timeline); ScrollTimeline* scroll_timeline = ToScrollTimeline(timeline);
Node* scroll_source = scroll_timeline->ResolvedScrollSource(); Node* scroll_source = scroll_timeline->ResolvedScrollSource();
base::Optional<CompositorElementId> element_id = base::Optional<CompositorElementId> element_id =
GetCompositorScrollElementId(*scroll_source); GetCompositorScrollElementId(scroll_source);
DoubleOrScrollTimelineAutoKeyword time_range; DoubleOrScrollTimelineAutoKeyword time_range;
scroll_timeline->timeRange(time_range); scroll_timeline->timeRange(time_range);
...@@ -209,7 +209,7 @@ std::unique_ptr<CompositorScrollTimeline> ToCompositorScrollTimeline( ...@@ -209,7 +209,7 @@ std::unique_ptr<CompositorScrollTimeline> ToCompositorScrollTimeline(
// TODO(smcgruer): If the scroll source later gets a LayoutBox (e.g. was // TODO(smcgruer): If the scroll source later gets a LayoutBox (e.g. was
// display:none and now isn't), we need to update the compositor to have the // display:none and now isn't), we need to update the compositor to have the
// correct orientation and start/end offset information. // correct orientation and start/end offset information.
LayoutBox* box = scroll_source->GetLayoutBox(); LayoutBox* box = scroll_source ? scroll_source->GetLayoutBox() : nullptr;
CompositorScrollTimeline::ScrollDirection orientation = ConvertOrientation( CompositorScrollTimeline::ScrollDirection orientation = ConvertOrientation(
scroll_timeline->GetOrientation(), box ? box->Style() : nullptr); scroll_timeline->GetOrientation(), box ? box->Style() : nullptr);
...@@ -572,7 +572,7 @@ void WorkletAnimation::UpdateOnCompositor() { ...@@ -572,7 +572,7 @@ void WorkletAnimation::UpdateOnCompositor() {
if (timeline_->IsScrollTimeline()) { if (timeline_->IsScrollTimeline()) {
Node* scroll_source = ToScrollTimeline(timeline_)->ResolvedScrollSource(); Node* scroll_source = ToScrollTimeline(timeline_)->ResolvedScrollSource();
LayoutBox* box = scroll_source->GetLayoutBox(); LayoutBox* box = scroll_source ? scroll_source->GetLayoutBox() : nullptr;
base::Optional<double> start_scroll_offset; base::Optional<double> start_scroll_offset;
base::Optional<double> end_scroll_offset; base::Optional<double> end_scroll_offset;
...@@ -591,7 +591,7 @@ void WorkletAnimation::UpdateOnCompositor() { ...@@ -591,7 +591,7 @@ void WorkletAnimation::UpdateOnCompositor() {
end_scroll_offset = resolved_end_scroll_offset; end_scroll_offset = resolved_end_scroll_offset;
} }
compositor_animation_->UpdateScrollTimeline( compositor_animation_->UpdateScrollTimeline(
GetCompositorScrollElementId(*scroll_source), start_scroll_offset, GetCompositorScrollElementId(scroll_source), start_scroll_offset,
end_scroll_offset); end_scroll_offset);
} }
} }
......
<!DOCTYPE html>
<script src='../../../resources/testharness.js'></script>
<script src='../../../resources/testharnessreport.js'></script>
<script>
test(function() {
document.documentElement.remove();
const timeline = new ScrollTimeline({timeRange: 100});
assert_equals(timeline.scrollSource, null);
assert_equals(timeline.currentTime, null);
}, 'The scrollSource can be null, if the document.scrollingElement does not exist');
</script>
...@@ -29,6 +29,14 @@ test(function() { ...@@ -29,6 +29,14 @@ test(function() {
assert_equals(scrollTimeline.orientation, 'inline'); assert_equals(scrollTimeline.orientation, 'inline');
}, 'Basic ScrollTimeline creation should work'); }, 'Basic ScrollTimeline creation should work');
test(function() {
const scrollTimeline = new ScrollTimeline(
{ scrollSource: null, timeRange: 100 });
// Null maps to the document scrolling element.
assert_equals(scrollTimeline.scrollSource, document.scrollingElement);
}, 'ScrollTimeline can be created with a null scrollSource');
test(function() { test(function() {
const scrollTimeline = new ScrollTimeline( const scrollTimeline = new ScrollTimeline(
{ timeRange: 100, orientation: 'block' }); { timeRange: 100, orientation: 'block' });
......
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