Commit 0d109b6c authored by Yi Gu's avatar Yi Gu Committed by Commit Bot

[ScrollTimeline] Fix null-deref READ when resolved scroll source is null

The resolved scroll source of ScrollTimeline could be null so we should
check before dereference it when checking whether it uses composited
scrolling.

Bug: 1088319
Change-Id: Ib20cceefcd0e1de05cf1d5932a6da6aac37ef8b5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2223995
Commit-Queue: Yi Gu <yigu@chromium.org>
Reviewed-by: default avatarMajid Valipour <majidvp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#773847}
parent 1d092bbd
...@@ -1694,7 +1694,7 @@ Animation::CheckCanStartAnimationOnCompositorInternal() const { ...@@ -1694,7 +1694,7 @@ Animation::CheckCanStartAnimationOnCompositorInternal() const {
// are in the compositor, the animation should be composited. // are in the compositor, the animation should be composited.
if (timeline_->IsScrollTimeline() && if (timeline_->IsScrollTimeline() &&
!CompositorAnimations::CheckUsesCompositedScrolling( !CompositorAnimations::CheckUsesCompositedScrolling(
*To<ScrollTimeline>(*timeline_).ResolvedScrollSource())) To<ScrollTimeline>(*timeline_).ResolvedScrollSource()))
reasons |= CompositorAnimations::kTimelineSourceHasInvalidCompositingState; reasons |= CompositorAnimations::kTimelineSourceHasInvalidCompositingState;
// An Animation without an effect cannot produce a visual, so there is no // An Animation without an effect cannot produce a visual, so there is no
......
...@@ -775,10 +775,12 @@ void CompositorAnimations::GetAnimationOnCompositor( ...@@ -775,10 +775,12 @@ void CompositorAnimations::GetAnimationOnCompositor(
DCHECK(!keyframe_models.IsEmpty()); DCHECK(!keyframe_models.IsEmpty());
} }
bool CompositorAnimations::CheckUsesCompositedScrolling(const Node& target) { bool CompositorAnimations::CheckUsesCompositedScrolling(Node* target) {
DCHECK(target.GetDocument().Lifecycle().GetState() >= if (!target)
return false;
DCHECK(target->GetDocument().Lifecycle().GetState() >=
DocumentLifecycle::kCompositingClean); DocumentLifecycle::kCompositingClean);
auto* layout_box_model_object = target.GetLayoutBoxModelObject(); auto* layout_box_model_object = target->GetLayoutBoxModelObject();
if (!layout_box_model_object) if (!layout_box_model_object)
return false; return false;
return layout_box_model_object->UsesCompositedScrolling(); return layout_box_model_object->UsesCompositedScrolling();
......
...@@ -161,7 +161,7 @@ class CORE_EXPORT CompositorAnimations { ...@@ -161,7 +161,7 @@ class CORE_EXPORT CompositorAnimations {
static CompositorElementIdNamespace CompositorElementNamespaceForProperty( static CompositorElementIdNamespace CompositorElementNamespaceForProperty(
CSSPropertyID property); CSSPropertyID property);
static bool CheckUsesCompositedScrolling(const Node& target); static bool CheckUsesCompositedScrolling(Node* target);
private: private:
static FailureReasons CheckCanStartEffectOnCompositor( static FailureReasons CheckCanStartEffectOnCompositor(
......
...@@ -757,6 +757,7 @@ TESTHARNESS-IN-OTHER-TYPE: html/semantics/interactive-elements/the-summary-eleme ...@@ -757,6 +757,7 @@ TESTHARNESS-IN-OTHER-TYPE: html/semantics/interactive-elements/the-summary-eleme
TESTHARNESS-IN-OTHER-TYPE: html/semantics/text-level-semantics/the-ruby-element/rt-without-ruby-crash.html TESTHARNESS-IN-OTHER-TYPE: html/semantics/text-level-semantics/the-ruby-element/rt-without-ruby-crash.html
TESTHARNESS-IN-OTHER-TYPE: portals/portals-no-frame-crash.html TESTHARNESS-IN-OTHER-TYPE: portals/portals-no-frame-crash.html
TESTHARNESS-IN-OTHER-TYPE: quirks/table-replaced-descendant-percentage-height-crash.html TESTHARNESS-IN-OTHER-TYPE: quirks/table-replaced-descendant-percentage-height-crash.html
TESTHARNESS-IN-OTHER-TYPE: scroll-animations/null-scroll-source-crash.html
TESTHARNESS-IN-OTHER-TYPE: svg/extensibility/foreignObject/foreign-object-circular-filter-reference-crash.html TESTHARNESS-IN-OTHER-TYPE: svg/extensibility/foreignObject/foreign-object-circular-filter-reference-crash.html
TESTHARNESS-IN-OTHER-TYPE: svg/extensibility/foreignObject/foreign-object-under-clip-path-crash.html TESTHARNESS-IN-OTHER-TYPE: svg/extensibility/foreignObject/foreign-object-under-clip-path-crash.html
TESTHARNESS-IN-OTHER-TYPE: svg/extensibility/foreignObject/foreign-object-under-defs-crash.html TESTHARNESS-IN-OTHER-TYPE: svg/extensibility/foreignObject/foreign-object-under-defs-crash.html
......
<link rel="help" href="https://bugs.chromium.org/p/chromium/issues/detail?id=1088319">
<meta name="assert" content="Playing animation with null scroll source should not crash.">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<style>
html {
overflow: scroll;
}
body {
overflow: scroll;
}
</style>
<div id="box"></div>
<script>
test(() => {
const effect = new KeyframeEffect(box, []);
const timeline = new ScrollTimeline({
timeRange: 1000
});
const animation = new Animation(effect, timeline);
assert_equals(document.scrollingElement, null,
"This test relies on scrolling element being nil");
animation.play();
}, "Playing animation with null scroll source should not crash");
</script>
\ No newline at end of file
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