diff --git a/cc/trees/layer_tree_host.cc b/cc/trees/layer_tree_host.cc index c9159060d1d0f..0050c4c28a703 100644 --- a/cc/trees/layer_tree_host.cc +++ b/cc/trees/layer_tree_host.cc @@ -1311,7 +1311,7 @@ void LayerTreeHost::SetViewportRectAndScale( // If a new viz::LocalSurfaceId has been provided, and the viewport has // changed, we need not begin new frames until it has activated. if (previous_local_surface_id != local_surface_id_from_parent && - device_viewport_rect_changed) { + device_viewport_rect_changed && features::IsSurfaceSyncThrottling()) { SetTargetLocalSurfaceId(local_surface_id_from_parent); } diff --git a/components/viz/common/features.cc b/components/viz/common/features.cc index c8815f01444d3..f93348766467f 100644 --- a/components/viz/common/features.cc +++ b/components/viz/common/features.cc @@ -155,6 +155,12 @@ const base::Feature kUseRealVideoColorSpaceForDisplay{ "UseRealVideoColorSpaceForDisplay", base::FEATURE_DISABLED_BY_DEFAULT}; #endif +// Used by CC to throttle frame production of older surfaces. Used by the +// Browser to batch SurfaceSync calls sent to the Renderer for properties can +// change in close proximity to each other. +const base::Feature kSurfaceSyncThrottling{"SurfaceSyncThrottling", + base::FEATURE_ENABLED_BY_DEFAULT}; + const base::Feature kDrawPredictedInkPoint{"DrawPredictedInkPoint", base::FEATURE_DISABLED_BY_DEFAULT}; const char kDraw1Point12Ms[] = "1-pt-12ms"; @@ -326,6 +332,10 @@ bool UseRealVideoColorSpaceForDisplay() { } #endif +bool IsSurfaceSyncThrottling() { + return base::FeatureList::IsEnabled(kSurfaceSyncThrottling); +} + // Used by Viz to determine if viz::DisplayScheduler should dynamically adjust // its frame deadline. Returns the percentile of historic draw times to base the // deadline on. Or absl::nullopt if the feature is disabled. diff --git a/components/viz/common/features.h b/components/viz/common/features.h index c5b37cda95105..d31bf0209e342 100644 --- a/components/viz/common/features.h +++ b/components/viz/common/features.h @@ -49,6 +49,7 @@ VIZ_COMMON_EXPORT extern const base::Feature kUsePlatformDelegatedInk; #if BUILDFLAG(IS_ANDROID) VIZ_COMMON_EXPORT extern const base::Feature kUseSurfaceLayerForVideoDefault; #endif +VIZ_COMMON_EXPORT extern const base::Feature kSurfaceSyncThrottling; VIZ_COMMON_EXPORT extern const base::Feature kDynamicSchedulerForDraw; VIZ_COMMON_EXPORT extern const base::Feature kDynamicSchedulerForClients; #if BUILDFLAG(IS_MAC) @@ -92,6 +93,7 @@ VIZ_COMMON_EXPORT bool ShouldUsePlatformDelegatedInk(); VIZ_COMMON_EXPORT bool UseSurfaceLayerForVideo(); VIZ_COMMON_EXPORT bool UseRealVideoColorSpaceForDisplay(); #endif +VIZ_COMMON_EXPORT bool IsSurfaceSyncThrottling(); VIZ_COMMON_EXPORT absl::optional IsDynamicSchedulerEnabledForDraw(); VIZ_COMMON_EXPORT absl::optional IsDynamicSchedulerEnabledForClients(); VIZ_COMMON_EXPORT int MaxOverlaysConsidered(); diff --git a/content/browser/renderer_host/render_widget_host_view_android.cc b/content/browser/renderer_host/render_widget_host_view_android.cc index 0bc2959cfeff3..f93ce7209feba 100644 --- a/content/browser/renderer_host/render_widget_host_view_android.cc +++ b/content/browser/renderer_host/render_widget_host_view_android.cc @@ -243,7 +243,8 @@ RenderWidgetHostViewAndroid::RenderWidgetHostViewAndroid( page_scale_(1.f), min_page_scale_(1.f), max_page_scale_(1.f), - mouse_wheel_phase_handler_(this) { + mouse_wheel_phase_handler_(this), + is_surface_sync_throttling_(features::IsSurfaceSyncThrottling()) { // Set the layer which will hold the content layer for this view. The content // layer is managed by the DelegatedFrameHost. view_.SetLayer(cc::Layer::Create()); @@ -1207,7 +1208,7 @@ bool RenderWidgetHostViewAndroid::CanSynchronizeVisualProperties() { // // We should instead wait for the full set of new visual properties to be // available, and deliver them to the Renderer in one single update. - if (in_rotation_) + if (in_rotation_ && is_surface_sync_throttling_) return false; return true; } @@ -1593,7 +1594,7 @@ void RenderWidgetHostViewAndroid::ShowInternal() { navigation_while_hidden_ = false; delegated_frame_host_->DidNavigate(); } - } else if (rotation_override) { + } else if (rotation_override && is_surface_sync_throttling_) { // If a rotation occurred while this was not visible, we need to allocate a // new viz::LocalSurfaceId and send the current visual properties to the // Renderer. Otherwise there will be no content at all to display. @@ -2784,6 +2785,10 @@ void RenderWidgetHostViewAndroid::BeginRotationBatching() { void RenderWidgetHostViewAndroid::EndRotationBatching() { in_rotation_ = false; + // Always clear when ending batching. As WebView can trigger multiple + // OnPhysicalBackingSizeChanged which would re-trigger rotation if we were + // still tracking `fullscreen_rotation_`. crbug.com/1302964 + fullscreen_rotation_ = false; DCHECK(!rotation_metrics_.empty()); const auto delta = rotation_metrics_.back().first - base::TimeTicks(); TRACE_EVENT_NESTABLE_ASYNC_END1( diff --git a/content/browser/renderer_host/render_widget_host_view_android.h b/content/browser/renderer_host/render_widget_host_view_android.h index be5217b2fa5e3..ab53db2a8fba8 100644 --- a/content/browser/renderer_host/render_widget_host_view_android.h +++ b/content/browser/renderer_host/render_widget_host_view_android.h @@ -620,6 +620,8 @@ class CONTENT_EXPORT RenderWidgetHostViewAndroid base::android::ScopedJavaGlobalRef obj_; + bool is_surface_sync_throttling_ = false; + base::WeakPtrFactory weak_ptr_factory_{this}; }; diff --git a/content/browser/renderer_host/render_widget_host_view_android_unittest.cc b/content/browser/renderer_host/render_widget_host_view_android_unittest.cc index c32969e3ea82e..9acfc0589d419 100644 --- a/content/browser/renderer_host/render_widget_host_view_android_unittest.cc +++ b/content/browser/renderer_host/render_widget_host_view_android_unittest.cc @@ -386,12 +386,12 @@ TEST_F(RenderWidgetHostViewAndroidTest, RenderFrameSubmittedBeforeNavigation) { EXPECT_TRUE(post_nav_local_surface_id.IsNewerThan(initial_local_surface_id)); } -// Tests Rotation improvements, where multiple updates to VisualProperties are -// batched. +// Tests Rotation improvements that are behind the +// features::kSurfaceSyncThrottling flag. class RenderWidgetHostViewAndroidRotationTest : public RenderWidgetHostViewAndroidTest { public: - RenderWidgetHostViewAndroidRotationTest() = default; + RenderWidgetHostViewAndroidRotationTest(); ~RenderWidgetHostViewAndroidRotationTest() override {} void OnDidUpdateVisualPropertiesComplete( @@ -402,8 +402,16 @@ class RenderWidgetHostViewAndroidRotationTest RenderWidgetHostViewAndroid* CreateRenderWidgetHostViewAndroid( RenderWidgetHostImpl* widget_host, gfx::NativeView parent_native_view) override; + + private: + base::test::ScopedFeatureList scoped_feature_list_; }; +RenderWidgetHostViewAndroidRotationTest:: + RenderWidgetHostViewAndroidRotationTest() { + scoped_feature_list_.InitAndEnableFeature(features::kSurfaceSyncThrottling); +} + void RenderWidgetHostViewAndroidRotationTest:: OnDidUpdateVisualPropertiesComplete( const cc::RenderFrameMetadata& metadata) { @@ -624,12 +632,12 @@ TEST_F(RenderWidgetHostViewAndroidRotationTest, FullscreenRotation) { } // When exiting rotation the order of visual property updates can differ. + // Though we need to always allow Surface Sync to continue, as WebView will + // send two OnPhysicalBackingSizeChanged in a row to exit fullscreen. While + // non-WebView can alternate some combination of 1-2 + // OnPhysicalBackingSizeChanged and OnSynchronizedDisplayPropertiesChanged. delegate()->set_is_fullscreen(false); rwhva->OnPhysicalBackingSizeChanged(/* deadline_override= */ absl::nullopt); - EXPECT_FALSE(rwhva->CanSynchronizeVisualProperties()); - EXPECT_EQ(post_rotation_local_surface_id, rwhva->GetLocalSurfaceId()); - - rwhva->OnSynchronizedDisplayPropertiesChanged(/* rotation= */ true); EXPECT_TRUE(rwhva->CanSynchronizeVisualProperties()); const viz::LocalSurfaceId post_fullscreen_local_surface_id = rwhva->GetLocalSurfaceId(); @@ -637,6 +645,21 @@ TEST_F(RenderWidgetHostViewAndroidRotationTest, FullscreenRotation) { EXPECT_TRUE(post_fullscreen_local_surface_id.is_valid()); EXPECT_TRUE(post_fullscreen_local_surface_id.IsNewerThan( post_rotation_local_surface_id)); + + // When rotation begins again it should block Surface Sync again. + rwhva->OnSynchronizedDisplayPropertiesChanged(/* rotation= */ true); + EXPECT_FALSE(rwhva->CanSynchronizeVisualProperties()); + EXPECT_EQ(post_fullscreen_local_surface_id, rwhva->GetLocalSurfaceId()); + + // When rotation has completed we should begin Surface Sync again. + rwhva->OnPhysicalBackingSizeChanged(/* deadline_override= */ absl::nullopt); + const viz::LocalSurfaceId post_fullscreen_rotation_local_surface_id = + rwhva->GetLocalSurfaceId(); + EXPECT_NE(post_fullscreen_local_surface_id, + post_fullscreen_rotation_local_surface_id); + EXPECT_TRUE(post_fullscreen_rotation_local_surface_id.is_valid()); + EXPECT_TRUE(post_fullscreen_rotation_local_surface_id.IsNewerThan( + post_fullscreen_local_surface_id)); EXPECT_TRUE(rwhva->CanSynchronizeVisualProperties()); {