Ensure propagation of PropertyChangeForcesCommit to compositor Prior to this change, a new value for PropertyChangeForcesCommit might not make it to the compositor thread right away if BeginMainFrame resulted in no visual update and returned early. If a page is idle, it could take an arbitrary amount of time for the new value to be propagated. With this CL, we force a commit to be sent to the compositor when a new value occurs. Bug: 40914013 Change-Id: I3920c0fa0d7dba9c123a1488a1546ca77b9033a0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7239030 Reviewed-by: Morten Stenshorne <mstensho@chromium.org> Commit-Queue: Stefan Zager <szager@chromium.org> Cr-Commit-Position: refs/heads/main@{#1558342}
diff --git a/cc/trees/layer_tree_host.cc b/cc/trees/layer_tree_host.cc index 3af15b9..3c183cd 100644 --- a/cc/trees/layer_tree_host.cc +++ b/cc/trees/layer_tree_host.cc
@@ -376,6 +376,7 @@ DCHECK(IsMainThread()); inside_main_frame_ = false; client_->DidBeginMainFrame(); + force_commit_for_propagation_ = false; } void LayerTreeHost::BeginMainFrameNotExpectedSoon() { @@ -813,9 +814,10 @@ } void LayerTreeHost::RequestMainFrameOnCompositorAnimation( - PropertyChangeForcesCommitCriteria property_change_forces_commit_criteria) { - pending_commit_state()->property_change_forces_commit_criteria = - property_change_forces_commit_criteria; + PropertyChangeForcesCommitCriteria criteria, + bool force_propagation) { + pending_commit_state()->property_change_forces_commit_criteria = criteria; + force_commit_for_propagation_ |= force_propagation; } void LayerTreeHost::SetDebugState(const LayerTreeDebugState& new_debug_state) {
diff --git a/cc/trees/layer_tree_host.h b/cc/trees/layer_tree_host.h index de13ed82..42b39e8e8 100644 --- a/cc/trees/layer_tree_host.h +++ b/cc/trees/layer_tree_host.h
@@ -379,6 +379,10 @@ return defer_main_frame_update_count_; } + bool force_commit_for_propagation() const { + return force_commit_for_propagation_; + } + // Synchronously performs a complete main frame update, commit and compositor // frame. Used only in single threaded mode when the compositor's internal // scheduling is disabled. @@ -396,8 +400,8 @@ // Requests a main frame if a composited animation changes a draw property. void RequestMainFrameOnCompositorAnimation( - PropertyChangeForcesCommitCriteria - property_change_forces_commit_criteria); + PropertyChangeForcesCommitCriteria criteria, + bool force_propagation); // Input Handling --------------------------------------------- @@ -1089,6 +1093,10 @@ // destroyed midway which causes a crash. crbug.com/654672 bool inside_main_frame_ = false; + // Set to force a commit during BeginMainFrame even if there are no actual + // rendering changes, to ensure the bits in CommitState are propagated. + bool force_commit_for_propagation_ = true; + // State cached until impl side is initialized. raw_ptr<TaskGraphRunner> task_graph_runner_;
diff --git a/cc/trees/proxy_main.cc b/cc/trees/proxy_main.cc index a4a83046..22a34097 100644 --- a/cc/trees/proxy_main.cc +++ b/cc/trees/proxy_main.cc
@@ -401,7 +401,8 @@ // to corresponding cc display list. An exception is for painted scrollbars, // which paint eagerly during layer update. bool updated = should_update_layers && layer_tree_host_->UpdateLayers(); - if (synchronous_composite_for_test_callback_) { + if (layer_tree_host_->force_commit_for_propagation() || + synchronous_composite_for_test_callback_) { updated = true; }
diff --git a/third_party/blink/renderer/core/frame/local_frame_view.cc b/third_party/blink/renderer/core/frame/local_frame_view.cc index 7ab0942..594335f 100644 --- a/third_party/blink/renderer/core/frame/local_frame_view.cc +++ b/third_party/blink/renderer/core/frame/local_frame_view.cc
@@ -2280,6 +2280,19 @@ return Lifecycle().GetState() == target_state; } +cc::PropertyChangeForcesCommitCriteria LocalFrameView::ForceCommitCriteria() + const { + cc::PropertyChangeForcesCommitCriteria criteria = + cc::PropertyChangeForcesCommitCriteria::kNone; + if (HasActiveIntersectionObservations() || + HasRunningAnchorTransformAnimation()) { + criteria = NeedsOcclusionTracking() && HasActiveIntersectionObservations() + ? cc::PropertyChangeForcesCommitCriteria::kAny + : cc::PropertyChangeForcesCommitCriteria::kTransform; + } + return criteria; +} + void LocalFrameView::UpdateLifecyclePhasesInternal( DocumentLifecycle::LifecycleState target_state) { // TODO(https://crbug.com/1196853): Switch to ScriptForbiddenScope once @@ -2289,6 +2302,8 @@ // RunPostLayoutSnapshotClientSteps must not run more than once. bool should_run_post_layout_snapshot_client_steps = true; + auto old_force_commit_criteria = ForceCommitCriteria(); + // Run style, layout, compositing and prepaint lifecycle phases and deliver // resize observations if required. Resize observer callbacks/delegates have // the potential to dirty layout (until loop limit is reached) and therefore @@ -2437,12 +2452,15 @@ } UpdateIntersectionObserverStatus(); - if (HasActiveIntersectionObservations() || - HasRunningAnchorTransformAnimation()) { + + auto new_force_commit_criteria = ForceCommitCriteria(); + bool force_propagation = + new_force_commit_criteria != old_force_commit_criteria; + if (force_propagation || new_force_commit_criteria != + cc::PropertyChangeForcesCommitCriteria::kNone) { + // Force a commit even if there are no updates, to propagate these bits. GetChromeClient()->RequestMainFrameOnCompositorAnimation( - *frame_, NeedsOcclusionTracking() && HasActiveIntersectionObservations() - ? cc::PropertyChangeForcesCommitCriteria::kAny - : cc::PropertyChangeForcesCommitCriteria::kTransform); + *frame_, new_force_commit_criteria, force_propagation); } // 21. For each doc of docs, mark paint timing for doc.
diff --git a/third_party/blink/renderer/core/frame/local_frame_view.h b/third_party/blink/renderer/core/frame/local_frame_view.h index dbd1b9c3..8062721 100644 --- a/third_party/blink/renderer/core/frame/local_frame_view.h +++ b/third_party/blink/renderer/core/frame/local_frame_view.h
@@ -71,6 +71,7 @@ class Layer; class PaintRecord; enum class PaintHoldingCommitTrigger; +enum class PropertyChangeForcesCommitCriteria; struct PaintBenchmarkResult; } @@ -980,6 +981,8 @@ LayoutSVGRoot* EmbeddedReplacedContent() const; + cc::PropertyChangeForcesCommitCriteria ForceCommitCriteria() const; + void PrepareForLifecycleUpdateRecursive(); // Returns whether the lifecycle was successfully updated to the
diff --git a/third_party/blink/renderer/core/frame/web_frame_widget_impl.cc b/third_party/blink/renderer/core/frame/web_frame_widget_impl.cc index f72c82d..ddcb255 100644 --- a/third_party/blink/renderer/core/frame/web_frame_widget_impl.cc +++ b/third_party/blink/renderer/core/frame/web_frame_widget_impl.cc
@@ -2133,10 +2133,10 @@ } void WebFrameWidgetImpl::RequestMainFrameOnCompositorAnimation( - cc::PropertyChangeForcesCommitCriteria - property_change_forces_commit_criteria) { + cc::PropertyChangeForcesCommitCriteria criteria, + bool force_propagation) { widget_base_->LayerTreeHost()->RequestMainFrameOnCompositorAnimation( - property_change_forces_commit_criteria); + criteria, force_propagation); } std::optional<int> WebFrameWidgetImpl::GetMaxRenderBufferBounds() const {
diff --git a/third_party/blink/renderer/core/frame/web_frame_widget_impl.h b/third_party/blink/renderer/core/frame/web_frame_widget_impl.h index 93a321d..090c8a9e4 100644 --- a/third_party/blink/renderer/core/frame/web_frame_widget_impl.h +++ b/third_party/blink/renderer/core/frame/web_frame_widget_impl.h
@@ -564,8 +564,8 @@ void SetShouldThrottleFrameRate(bool flag); void RequestMainFrameOnCompositorAnimation( - cc::PropertyChangeForcesCommitCriteria - property_change_forces_commit_criteria); + cc::PropertyChangeForcesCommitCriteria criteria, + bool force_propagation); // Pause all rendering (main and compositor thread) in the compositor. [[nodiscard]] std::unique_ptr<cc::ScopedPauseRendering> PauseRendering();
diff --git a/third_party/blink/renderer/core/loader/empty_clients.h b/third_party/blink/renderer/core/loader/empty_clients.h index 554f444..8b348b88 100644 --- a/third_party/blink/renderer/core/loader/empty_clients.h +++ b/third_party/blink/renderer/core/loader/empty_clients.h
@@ -135,8 +135,8 @@ void SetShouldThrottleFrameRate(bool flag, LocalFrame& main_frame) override {} void RequestMainFrameOnCompositorAnimation( LocalFrame&, - cc::PropertyChangeForcesCommitCriteria animation_forces_commit) override { - } + cc::PropertyChangeForcesCommitCriteria criteria, + bool force_propagation) override {} void StartDragging(LocalFrame*, const WebDragData&, DragOperationsMask,
diff --git a/third_party/blink/renderer/core/page/chrome_client.h b/third_party/blink/renderer/core/page/chrome_client.h index 4f87807..d692bd8 100644 --- a/third_party/blink/renderer/core/page/chrome_client.h +++ b/third_party/blink/renderer/core/page/chrome_client.h
@@ -236,8 +236,8 @@ LocalFrame& main_frame) = 0; virtual void RequestMainFrameOnCompositorAnimation( LocalFrame&, - cc::PropertyChangeForcesCommitCriteria - property_change_forces_commit_criteria) = 0; + cc::PropertyChangeForcesCommitCriteria criteria, + bool force_propagation) = 0; virtual std::unique_ptr<cc::ScopedPauseRendering> PauseRendering( LocalFrame& main_frame) = 0;
diff --git a/third_party/blink/renderer/core/page/chrome_client_impl.cc b/third_party/blink/renderer/core/page/chrome_client_impl.cc index 46b64bd..38564c3 100644 --- a/third_party/blink/renderer/core/page/chrome_client_impl.cc +++ b/third_party/blink/renderer/core/page/chrome_client_impl.cc
@@ -1208,12 +1208,11 @@ void ChromeClientImpl::RequestMainFrameOnCompositorAnimation( LocalFrame& frame, - cc::PropertyChangeForcesCommitCriteria - property_change_forces_commit_criteria) { + cc::PropertyChangeForcesCommitCriteria criteria, + bool force_propagation) { WebFrameWidgetImpl* widget = WebLocalFrameImpl::FromFrame(frame)->LocalRootFrameWidget(); - widget->RequestMainFrameOnCompositorAnimation( - property_change_forces_commit_criteria); + widget->RequestMainFrameOnCompositorAnimation(criteria, force_propagation); } void ChromeClientImpl::SetHasScrollEventHandlers(LocalFrame* frame,
diff --git a/third_party/blink/renderer/core/page/chrome_client_impl.h b/third_party/blink/renderer/core/page/chrome_client_impl.h index 327c5715..73b8050c 100644 --- a/third_party/blink/renderer/core/page/chrome_client_impl.h +++ b/third_party/blink/renderer/core/page/chrome_client_impl.h
@@ -95,8 +95,8 @@ void SetShouldThrottleFrameRate(bool flag, LocalFrame& main_frame) override; void RequestMainFrameOnCompositorAnimation( LocalFrame&, - cc::PropertyChangeForcesCommitCriteria - property_change_forces_commit_criteria) override; + cc::PropertyChangeForcesCommitCriteria criteria, + bool force_propagation) override; std::unique_ptr<cc::ScopedPauseRendering> PauseRendering( LocalFrame&) override; std::optional<int> GetMaxRenderBufferBounds(LocalFrame&) const override;
diff --git a/third_party/blink/web_tests/external/wpt/intersection-observer/animating.html b/third_party/blink/web_tests/external/wpt/intersection-observer/animating.html index 28e7bdb..af27d018 100644 --- a/third_party/blink/web_tests/external/wpt/intersection-observer/animating.html +++ b/third_party/blink/web_tests/external/wpt/intersection-observer/animating.html
@@ -33,20 +33,22 @@ let target = document.getElementById("target"); let observer = new IntersectionObserver(function(changes) { entries = entries.concat(changes); + assert_true(entries[0].isIntersecting); changes.forEach(entry => { if (!entry.isIntersecting) { resolve("Received not-intersecting notification before animationend."); } }); }); - observer.observe(target); - await waitForNotification(); - assert_equals(entries.length, 1); - assert_true(entries[0].isIntersecting); target.style.animation = "3s linear slideup"; setTimeout(() => { reject("Did not get a not-intersecting notification within 2 seconds."); }, 2000); + target.addEventListener("animationend", evt => { + reject("animationend event fired before not-intersecting notification."); + }); + await new Promise(resolve => setTimeout(resolve, 500)); + observer.observe(target); }); }, "IntersectionObserver generates notifications when " + "a transform animation changes intersection state");