Commit d182cc3f authored by Stefan Zager's avatar Stefan Zager Committed by Chromium LUCI CQ

Force lifecycle to pre-paint if IntersectionObserver is required

This is a speculative bug fix, based on the hypothesis that allowing
lifecycle updates for throttled frames that require IntersectionObserver
to proceed through compositing assignments may be the problem. During
compositing assignments phase GraphicsLayerUpdater may call
ScrollingCoordinator::UpdateCompositorScrollOffset, which may somehow
interfere with the ongoing scroll fling.

It does not appear to be necessary for lifecycle updates to proceed
through compositing assignments in this case. The justification for
going through compositing assignment was that up-to-date sticky
positioning may be required; but it appears that compositing inputs
update (pre-CAP) or pre-paint (CAP) is sufficient for that.

Bug: 1158664

Change-Id: I456421fc0886abe254462d5099e446d87290c960
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2599002Reviewed-by: default avatarXianzhu Wang <wangxianzhu@chromium.org>
Commit-Queue: Stefan Zager <szager@chromium.org>
Cr-Commit-Position: refs/heads/master@{#838931}
parent 4d6c6f41
......@@ -1138,7 +1138,7 @@ void LocalFrameView::ForceUpdateViewportIntersections() {
// update; but we can't wait for a lifecycle update to run them, because a
// hidden frame won't run lifecycle updates. Force layout and run them now.
DisallowThrottlingScope disallow_throttling(*this);
UpdateLifecycleToCompositingCleanPlusScrolling(
UpdateLifecycleToPrePaintClean(
DocumentUpdateReason::kIntersectionObservation);
UpdateViewportIntersectionsForSubtree(
IntersectionObservation::kImplicitRootObserversNeedUpdate |
......@@ -2542,7 +2542,7 @@ void LocalFrameView::UpdateLifecyclePhasesInternal(
run_more_lifecycle_phases = RunPrePaintLifecyclePhase(target_state);
DCHECK(ShouldThrottleRendering() ||
Lifecycle().GetState() >= DocumentLifecycle::kPrePaintClean);
if (!run_more_lifecycle_phases)
if (ShouldThrottleRendering() || !run_more_lifecycle_phases)
return;
run_more_lifecycle_phases =
......@@ -4544,20 +4544,10 @@ bool LocalFrameView::ShouldThrottleRendering() const {
auto* local_frame_root_view = GetFrame().LocalFrameRoot().View();
if (local_frame_root_view->IsUpdatingLifecycle() &&
intersection_observation_state_ == kRequired && !IsDisplayLocked()) {
// IntersectionObserver may rely on sticky positioning, which is not
// resolved until compositing assignments are done. If the resolution of
// sticky positioning is refactored to occur during layout, then the
// required minimum lifecycle state will depend on whether this frame
// requires occlusion tracking, since determining occlusion state requires a
// hit test, which requires the document to be pre-paint clean. So the code
// here would change to something like:
//
// auto min_state = GetFrame().NeedsOcclusionTracking() ?
// DocumentLifecycle::kPrePaintClean :
// DocumentLifecycle::kLayoutClean;
// return Lifecycle().GetState() >= min_state;
return Lifecycle().GetState() >=
DocumentLifecycle::kCompositingAssignmentsClean;
// Pre-paint guarantees that sticky positioning is finalized and hit testing
// is available (needed for HitTestForOcclusion, called from
// IntersectionGeometry).
return Lifecycle().GetState() >= DocumentLifecycle::kPrePaintClean;
}
return true;
......
......@@ -239,8 +239,8 @@ TEST_P(FrameThrottlingTest, IntersectionObservationOverridesThrottling) {
DocumentLifecycle::kPaintClean);
if (!RuntimeEnabledFeatures::CompositeAfterPaintEnabled()) {
// If IntersectionObserver is required to run, lifecycle will be updated
// through compositing.
EXPECT_EQ(kCompositingUpdateNone,
// through pre-paint, but not compositing assignment.
EXPECT_EQ(kCompositingUpdateRebuildTree,
inner_view->Compositor()->pending_update_type_);
}
EXPECT_TRUE(inner_view->Layer()->SelfNeedsRepaint());
......@@ -374,7 +374,7 @@ TEST_P(FrameThrottlingTest,
frame_document->View()->ScheduleAnimation();
frame_document->View()->GetLayoutView()->Layer()->SetNeedsRepaint();
CompositeFrame();
EXPECT_EQ(DocumentLifecycle::kCompositingAssignmentsClean,
EXPECT_EQ(DocumentLifecycle::kPrePaintClean,
frame_document->Lifecycle().GetState());
EXPECT_TRUE(frame_document->View()->ShouldThrottleRenderingForTest());
}
......
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