Refactor zoom propagation

This recent patch made zoom level a property on WebFrameWidget rather
than WebView:

https://chromium-review.googlesource.com/c/chromium/src/+/5571113

However, it left the propagation routes for zoom information in a bit of
a crazy state. This CL is a pure refactor with no behavioral change,
intended only to make the propagation mechanism more consistent and
sane.

All of the inputs to the zoom factor calculation arrive via IPC to a
frame widget. Zoom level is handled directly by WebFrameWidget; all
other factors are delegated to WebView, which propagates them down to
all WebFrameWidgets contained by the WebView. The WebFrameWidget in turn
is responsible for propagating zoom factor to the root LocalFrame of the
widget, and the root frame is responsible for propagating it to
descendant non-root frames.

This also activates the path for propagating zoom level from embedder to
process-isolated iframe via VisualProperties::zoom_level. Currently,
that value is ignored by RenderWidgetHostImpl::GetVisualProperties in
favor of the WebContents-level value. After this change, the value set
by the embedder will be used instead. Currently, the two values are
always identical, so this is a pure plumbing change with no behavior
difference. A follow-up CL will allow the value set by the embedder to
deviate from the WebContents-level value based on CSS zoom.

Bug: chromium:329482480
Change-Id: I18991c9db9f1d1be8ffad7677d6a551ab70495f9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5611483
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Commit-Queue: Stefan Zager <szager@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1313827}

content/browser/devtools/protocol/input_handler.cc

@@ -1480,8 +1480,14 @@ void InputHandler::OnWidgetForDispatchDragEvent(
 
 float InputHandler::ScaleFactor() {
   DCHECK(web_contents_);
-  return blink::PageZoomLevelToZoomFactor(
-             web_contents_->GetPendingPageZoomLevel()) *
+  RenderWidgetHostImpl* widget_host =
+      host_ ? host_->GetRenderWidgetHost() : nullptr;
+  float page_zoom_level = 0.;
+  if (widget_host && widget_host->GetView()) {
+    page_zoom_level = widget_host->GetView()->GetZoomLevel();
+  }
+
+  return blink::PageZoomLevelToZoomFactor(page_zoom_level) *
          web_contents_->GetPrimaryPage().GetPageScaleFactor();
 }
 

content/browser/renderer_host/render_widget_host_impl.cc

@@ -988,7 +988,7 @@ blink::VisualProperties RenderWidgetHostImpl::GetVisualProperties() {
   } else {
     visual_properties.display_mode = blink::mojom::DisplayMode::kBrowser;
   }
-  visual_properties.zoom_level = delegate_->GetPendingPageZoomLevel();
+  visual_properties.zoom_level = view_->GetZoomLevel();
 
   RenderViewHostDelegateView* rvh_delegate_view = delegate_->GetDelegateView();
   CHECK(rvh_delegate_view);

content/browser/renderer_host/render_widget_host_view_child_frame.cc

@@ -839,6 +839,9 @@ double RenderWidgetHostViewChildFrame::GetZoomLevel() const {
   if (adjusted_child_zoom) {
     return *adjusted_child_zoom;
   }
+  if (!frame_connector_) {
+    return RenderWidgetHostViewBase::GetZoomLevel();
+  }
   return frame_connector_->zoom_level();
 }
 

content/renderer/render_frame_impl.cc

@@ -13,6 +13,7 @@
 #include <vector>
 
 #include "base/auto_reset.h"
+#include "base/check_deref.h"
 #include "base/command_line.h"
 #include "base/containers/contains.h"
 #include "base/containers/flat_map.h"
@@ -1181,11 +1182,6 @@ perfetto::protos::pbzero::FrameDeleteIntention FrameDeleteIntentionToProto(
   return ProtoLevel::FRAME_DELETE_INTENTION_NOT_MAIN_FRAME;
 }
 
-void PropagatePageZoomToNewlyAttachedFrame(blink::WebView* web_view,
-                                           float device_scale_factor) {
-  web_view->SetZoomFactorForDeviceScaleFactor(device_scale_factor);
-}
-
 void CallClientDeferMediaLoad(base::WeakPtr<RenderFrameImpl> frame,
                               bool has_played_media_before,
                               base::OnceClosure closure) {
@@ -5139,23 +5135,9 @@ void RenderFrameImpl::UpdateStateForCommit(
   }
 
   if (IsLocalRoot()) {
-    // This goes to WebViewImpl and sets the zoom factor which will be
-    // propagated down to this RenderFrameImpl's LocalFrame in blink.
-    // Non-local-roots are able to grab the value off their parents but local
-    // roots can not and this is a huge action-at-a-distance to make up for that
-    // flaw in how LocalFrame determines the zoom factor.
-    // TODO(danakj): This should not be needed if the zoom factor/device scale
-    // factor did not need to be propagated to each frame. Since they are a
-    // global that should be okay?? The test that fails without this, for
-    // child frames, is in content_browsertests:
-    //     SitePerProcessHighDPIBrowserTest.
-    //         SubframeLoadsWithCorrectDeviceScaleFactor
-    // And when UseZoomForDSF is disabled, in content_browsertests:
-    //     IFrameZoomBrowserTest.SubframesDontZoomIndependently (and the whole
-    //     suite).
-    PropagatePageZoomToNewlyAttachedFrame(
-        GetWebView(),
-        GetLocalRootWebFrameWidget()->GetScreenInfo().device_scale_factor);
+    // This forces zoom factor to be propagated to the blink core frame.
+    auto& widget = CHECK_DEREF(GetLocalRootWebFrameWidget());
+    widget.SetZoomLevel(widget.GetZoomLevel());
   }
 
   // If we are a top frame navigation to another document we should clear any

third_party/blink/renderer/core/exported/web_view_impl.cc

@@ -462,6 +462,21 @@ SkFontHinting RendererPreferencesToSkiaHinting(
 }
 #endif  // !BUILDFLAG(IS_MAC) && !BUILDFLAG(IS_WIN)
 
+void ForEachFrameWidgetControlledByView(
+    WebViewImpl& web_view,
+    base::FunctionRef<void(WebFrameWidgetImpl*)> callback) {
+  for (WebFrame* frame = web_view.MainFrame(); frame;
+       frame = frame->TraverseNext()) {
+    if (auto* frame_impl = DynamicTo<WebLocalFrameImpl>(frame)) {
+      if (frame_impl->GetFrame()->IsLocalRoot()) {
+        if (auto* widget = frame_impl->FrameWidgetImpl()) {
+          callback(widget);
+        }
+      }
+    }
+  }
+}
+
 }  // namespace
 
 // WebView ----------------------------------------------------------------
@@ -2274,31 +2289,17 @@ void WebViewImpl::AdvanceFocus(bool reverse) {
               : mojom::blink::FocusType::kForward);
 }
 
-double WebViewImpl::ClampZoomLevel(double zoom_level) {
-  if (zoom_level < minimum_zoom_level_) {
-    return minimum_zoom_level_;
-  }
-  if (zoom_level > maximum_zoom_level_) {
-    return maximum_zoom_level_;
-  }
-  return zoom_level;
+double WebViewImpl::ClampZoomLevel(double zoom_level) const {
+  return std::max(minimum_zoom_level_,
+                  std::min(maximum_zoom_level_, zoom_level));
 }
 
-double WebViewImpl::SetMainFrameZoomLevel(double zoom_level) {
-  if (zoom_factor_for_device_scale_factor_) {
-    if (compositor_device_scale_factor_override_) {
-      page_->SetInspectorDeviceScaleFactorOverride(
-          zoom_factor_for_device_scale_factor_ /
-          compositor_device_scale_factor_override_);
-    } else {
-      page_->SetInspectorDeviceScaleFactorOverride(1.0f);
-    }
+double WebViewImpl::ZoomLevelToZoomFactor(double zoom_level,
+                                          bool for_main_frame) const {
+  double zoom_factor = PageZoomLevelToZoomFactor(zoom_level);
+  if (for_main_frame && zoom_factor_override_) {
+    zoom_factor = zoom_factor_override_;
   }
-
-  float zoom_factor =
-      zoom_factor_override_
-          ? zoom_factor_override_
-          : static_cast<float>(PageZoomLevelToZoomFactor(zoom_level));
   if (zoom_factor_for_device_scale_factor_) {
     if (compositor_device_scale_factor_override_) {
       zoom_factor *= compositor_device_scale_factor_override_;
@@ -2309,10 +2310,20 @@ double WebViewImpl::SetMainFrameZoomLevel(double zoom_level) {
   return zoom_factor;
 }
 
-void WebViewImpl::RecomputeMainFrameZoomFactor() {
-  if (auto* main_frame = MainFrameImpl()) {
-    if (auto* widget = main_frame->FrameWidgetImpl()) {
-      widget->SetZoomLevel(widget->GetZoomLevel());
+void WebViewImpl::UpdateWidgetZoomFactors() {
+  ForEachFrameWidgetControlledByView(*this, [](WebFrameWidgetImpl* widget) {
+    widget->SetZoomLevel(widget->GetZoomLevel());
+  });
+}
+
+void WebViewImpl::UpdateInspectorDeviceScaleFactorOverride() {
+  if (zoom_factor_for_device_scale_factor_) {
+    if (compositor_device_scale_factor_override_) {
+      page_->SetInspectorDeviceScaleFactorOverride(
+          zoom_factor_for_device_scale_factor_ /
+          compositor_device_scale_factor_override_);
+    } else {
+      page_->SetInspectorDeviceScaleFactorOverride(1.0f);
     }
   }
 }
@@ -2366,10 +2377,12 @@ void WebViewImpl::SetPageScaleFactor(float scale_factor) {
 void WebViewImpl::SetZoomFactorForDeviceScaleFactor(
     float zoom_factor_for_device_scale_factor) {
   DCHECK(does_composite_);
-  // We can't early-return here if these are already equal, because we may
-  // need to propagate the correct zoom factor to newly navigated frames.
-  zoom_factor_for_device_scale_factor_ = zoom_factor_for_device_scale_factor;
-  RecomputeMainFrameZoomFactor();
+  if (zoom_factor_for_device_scale_factor_ !=
+      zoom_factor_for_device_scale_factor) {
+    zoom_factor_for_device_scale_factor_ = zoom_factor_for_device_scale_factor;
+    UpdateWidgetZoomFactors();
+    UpdateInspectorDeviceScaleFactorOverride();
+  }
 }
 
 void WebViewImpl::SetPageLifecycleStateFromNewPageCommit(
@@ -3213,12 +3226,12 @@ void WebViewImpl::ConfigureAutoResizeMode() {
 
 void WebViewImpl::SetCompositorDeviceScaleFactorOverride(
     float device_scale_factor) {
-  if (compositor_device_scale_factor_override_ == device_scale_factor)
-    return;
-  compositor_device_scale_factor_override_ = device_scale_factor;
-  if (zoom_factor_for_device_scale_factor_) {
-    RecomputeMainFrameZoomFactor();
-    return;
+  if (compositor_device_scale_factor_override_ != device_scale_factor) {
+    compositor_device_scale_factor_override_ = device_scale_factor;
+    if (zoom_factor_for_device_scale_factor_) {
+      UpdateWidgetZoomFactors();
+      UpdateInspectorDeviceScaleFactorOverride();
+    }
   }
 }
 
@@ -3763,7 +3776,11 @@ void WebViewImpl::SetBackgroundColorOverrideForFullscreenController(
 
 void WebViewImpl::SetZoomFactorOverride(float zoom_factor) {
   zoom_factor_override_ = zoom_factor;
-  RecomputeMainFrameZoomFactor();
+  // This only affects the local main frame, so no need to propagate to all
+  // frame widgets.
+  if (web_widget_) {
+    web_widget_->SetZoomLevel(web_widget_->GetZoomLevel());
+  }
 }
 
 Element* WebViewImpl::FocusedElement() const {

third_party/blink/renderer/core/exported/web_view_impl.h

@@ -637,8 +637,8 @@ class CORE_EXPORT WebViewImpl final : public WebView,
   // Called when draggable regions in the page change.
   void DraggableRegionsChanged();
 
-  double ClampZoomLevel(double zoom_level);
-  double SetMainFrameZoomLevel(double zoom_level);
+  double ClampZoomLevel(double zoom_level) const;
+  double ZoomLevelToZoomFactor(double zoom_level, bool for_main_frame) const;
 
  private:
   FRIEND_TEST_ALL_PREFIXES(WebFrameTest, DivScrollIntoEditableTest);
@@ -784,7 +784,10 @@ class CORE_EXPORT WebViewImpl final : public WebView,
   // Called when mojo is disconnected.
   void MojoDisconnected();
 
-  void RecomputeMainFrameZoomFactor();
+  // Called when any input to zoom factor calculation changes on the WebView, to
+  // trigger recalculation of zoom factor for all affected widgets.
+  void UpdateWidgetZoomFactors();
+  void UpdateInspectorDeviceScaleFactorOverride();
 
   // A value provided by the browser to state that all Widgets in this
   // WebView's frame tree will never be user-visible and thus never need to

third_party/blink/renderer/core/frame/web_frame_widget_impl.cc

@@ -2177,32 +2177,68 @@ double WebFrameWidgetImpl::GetZoomLevel() {
   return zoom_level_;
 }
 
+// There are four main values that go into zoom arithmetic:
+// - "zoom level", a log-based value which represents browser zoom level.
+// - kTextSizeMultiplierRatio, a hard-coded constant used as the log base for
+//   zoom level.
+// - Hardware device pixel ratio, which is stored on WebView as
+//   zoom_factor_for_device_scale_factor_.
+// - "zoom factor", which is calculated from the first three values, with
+//   override mechanisms for testing and device emulation.
+//
+// Here and elsewhere, the code tries to be consistent in its naming conventions
+// with respect to "zoom level" vs. "zoom factor".
 void WebFrameWidgetImpl::SetZoomLevel(double zoom_level) {
+  if (ForMainFrame()) {
+    zoom_level = View()->ClampZoomLevel(zoom_level);
+  }
   // Override the zoom level with the testing one if necessary.
   if (zoom_level_for_testing_ != -INFINITY)
     zoom_level = zoom_level_for_testing_;
-
-  zoom_level = View()->ClampZoomLevel(zoom_level);
-  // Set the layout shift exclusion window for the zoom level change.
-  if (zoom_level_ != zoom_level) {
-    NotifyZoomLevelChanged(LocalRootImpl()->GetFrame());
-  }
+  bool zoom_level_changed = (zoom_level != zoom_level_);
   zoom_level_ = zoom_level;
-  double zoom_factor = View()->SetMainFrameZoomLevel(zoom_level_);
+
   if (auto* local_frame = LocalRootImpl()->GetFrame()) {
     if (Document* document = local_frame->GetDocument()) {
+      double zoom_factor =
+          View()->ZoomLevelToZoomFactor(zoom_level, ForMainFrame());
+      if (zoom_level_changed) {
+        // local_frame->SetPageZoomFactor() below will propagate to the
+        // connected LocalFrame tree, but not to RemoteFrames because they need
+        // zoom level (rather than zoom factor), which is not available to the
+        // LocalFrame.
+        ForEachRemoteFrameControlledByWidget(
+            [zoom_level](RemoteFrame* remote_frame) {
+              remote_frame->ZoomLevelChanged(zoom_level);
+            });
+
+        // Set the layout shift exclusion window for the zoom level change.
+        if (LocalFrameView* view = document->View()) {
+          view->GetLayoutShiftTracker().NotifyZoomLevelChanged();
+#if BUILDFLAG(IS_ANDROID)
+          if (ForTopMostMainFrame()) {
+            // Zoom levels are the exponent in the calculation of zoom. The zoom
+            // factor is the value shown to the user (e.g. 50% to 300%).
+            // Note: On Android, when the AccessibilityPageZoom feature is
+            // disabled, this histogrm should only include samples at 100% zoom
+            // factor.
+            UMA_HISTOGRAM_CUSTOM_EXACT_LINEAR(
+                "Accessibility.Android.PageZoom.MainFrameZoomFactor",
+                zoom_factor * 100, 50, 300, 52);
+          }
+#endif
+        }
+      }
+
+      // zoom_factor may have changed even if zoom_level did not, so we
+      // unconditionally propagate to the local root frame.
       auto* plugin_document = DynamicTo<PluginDocument>(document);
       if (!plugin_document || !plugin_document->GetPluginView()) {
+        // The local root is responsible for propagating to its connected tree
+        // of LocalFrame descendants.
         local_frame->SetPageZoomFactor(zoom_factor);
       }
     }
-
-    // Part of the UpdateVisualProperties dance we send the zoom level to
-    // RemoteFrames that are below the local root for this widget.
-    ForEachRemoteFrameControlledByWidget(
-        [zoom_level](RemoteFrame* remote_frame) {
-          remote_frame->ZoomLevelChanged(zoom_level);
-        });
   }
 }
 
@@ -5102,30 +5138,6 @@ bool WebFrameWidgetImpl::ShouldAutoDetermineCompositingToLCDTextSetting() {
   return true;
 }
 
-void WebFrameWidgetImpl::NotifyZoomLevelChanged(LocalFrame* root) {
-  if (root) {
-    Document* document = root->GetDocument();
-    DCHECK(document);
-    if (LocalFrameView* view = document->View()) {
-      view->GetLayoutShiftTracker().NotifyZoomLevelChanged();
-#if BUILDFLAG(IS_ANDROID)
-      if (ForTopMostMainFrame()) {
-        // Zoom levels are the exponent in the calculation of zoom. The zoom
-        // factor is the value shown to the user (e.g. 50% to 300%).
-        // Note: On Android, when the AccessibilityPageZoom feature is disabled,
-        // this histogrm should only include samples at 100% zoom factor.
-        UMA_HISTOGRAM_CUSTOM_EXACT_LINEAR(
-            "Accessibility.Android.PageZoom.MainFrameZoomFactor",
-            PageZoomLevelToZoomFactor(
-                View()->MainFrameWidget()->GetZoomLevel()) *
-                100,
-            50, 300, 52);
-      }
-#endif
-    }
-  }
-}
-
 bool WebFrameWidgetImpl::WillBeDestroyed() const {
   return widget_base_->WillBeDestroyed();
 }

third_party/blink/renderer/core/frame/web_frame_widget_impl.h

@@ -1016,8 +1016,6 @@ class CORE_EXPORT WebFrameWidgetImpl
   bool DidChangeFullscreenState(
       const VisualProperties& visual_properties) const;
 
-  void NotifyZoomLevelChanged(LocalFrame* root);
-
   // Satisfy the render blocking condition for cross-document view transitions.
   void NotifyViewTransitionRenderingHasBegun();