Fix zoom for PdfOopif
This CL updates the zoom system to enable PdfOopif PDFs to zoom properly, i.e. to replicate the zooming behavior of PDFs rendered with MimeHandlerViewGuest. When a PDF is viewed, the viewer's UI should not be affected by the default browser zoom, nor should it change size if the user changes either the default browser zoom or the zoom of the page displaying the PDF. If the PDF is viewed in the mainframe, this involves (i) setting the zoom mode of the main frame to manual (to prevent changes to the page's zoom being persisted in the HostZoomMap under the url of the PDF's content), and (ii) setting the scheme+host zoom level for the PDF extension to be zoom factor 1.0 (zoom level 0). This second step is what allows the UI to be rendered at a consistent size regardless of the page's zoom. If the PDF is is embedded in a page, only step (ii) is necessary, as changes to the page's zoom will be persisted under the main frame's URL. In both cases, the PDF extension monitors the page's zoom level so it can be applied to the content without applying it to the viewer's UI. There are two major changes involved in this CL: 1) The PdfViewerStreamManager is modified to a) set up a separate ZoomController for the RenderFrameHost that contains the Pdf extension (this will allow the PDF viewer's frame to keep its zoom factor at a different level than the page zoom), and b) a call to HostZoomMap to set the desired (independent) zoom factor for the PDF extension frame at 1.0. 2) Modifications to WebContentsImpl::GetPendingZoomLevel() to allow it to identify a RenderWidgetHost/RenderFrameHost that has independent zoom, and which isn't a main frame. This frame hosts the PDF extension. Note that although with PdfOopif the PDF extension frame will create its own ZoomController (1), any calls it makes to the TabsAPI zoom functions still use the ZoomController for the main frame. This allows it to prevent zoom levels for full-page PDFs from being persisted. Note also that no PDF extension JS code has been modified. Bug: 372932834 Change-Id: Iaf418d03227817ec18fe1623d1f77f93be483608 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6281436 Reviewed-by: Kevin McNee <mcnee@chromium.org> Commit-Queue: James Maclean <wjmaclean@chromium.org> Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Reviewed-by: Andy Phan <andyphan@chromium.org> Cr-Commit-Position: refs/heads/main@{#1432129}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
8e08ae3414
commit
f562b92ae0
chrome/browser
content
browser
public
@ -7421,6 +7421,10 @@ IN_PROC_BROWSER_TEST_P(WebViewFencedFrameTest, ZoomFencedFrame) {
|
||||
auto* fenced_frame_rwh = fenced_frame->GetRenderWidgetHost();
|
||||
// See Javascript fcn testZoomFencedFrame for source of the 0.95 zoom factor.
|
||||
double expected_zoom_level = blink::ZoomFactorToZoomLevel(0.95);
|
||||
// Guest has `expected_zoom_level`.
|
||||
EXPECT_DOUBLE_EQ(expected_zoom_level, content::GetPendingZoomLevel(
|
||||
guest_rfh->GetRenderWidgetHost()));
|
||||
// FencedFrame has `expected_zoom_level`.
|
||||
EXPECT_DOUBLE_EQ(expected_zoom_level,
|
||||
content::GetPendingZoomLevel(fenced_frame_rwh));
|
||||
|
||||
|
@ -8962,6 +8962,14 @@ ChromeContentBrowserClient::OverrideForInternalWebUI(content::WebUI* web_ui,
|
||||
return std::make_unique<InternalDebugPagesDisabledUI>(web_ui, url.host());
|
||||
}
|
||||
|
||||
bool ChromeContentBrowserClient::ShouldEnableSubframeZoom() {
|
||||
#if BUILDFLAG(ENABLE_PDF)
|
||||
return chrome_pdf::features::IsOopifPdfEnabled();
|
||||
#else
|
||||
return false;
|
||||
#endif
|
||||
}
|
||||
|
||||
#if BUILDFLAG(ENABLE_PDF)
|
||||
std::optional<network::CrossOriginEmbedderPolicy>
|
||||
ChromeContentBrowserClient::MaybeOverrideLocalURLCrossOriginEmbedderPolicy(
|
||||
|
@ -1170,6 +1170,8 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
|
||||
void AddExtraPartForTesting(
|
||||
std::unique_ptr<ChromeContentBrowserClientParts> part);
|
||||
|
||||
bool ShouldEnableSubframeZoom() override;
|
||||
|
||||
protected:
|
||||
static bool HandleWebUI(GURL* url, content::BrowserContext* browser_context);
|
||||
static bool HandleWebUIReverse(GURL* url,
|
||||
|
@ -16,7 +16,9 @@
|
||||
#include "base/memory/weak_ptr.h"
|
||||
#include "components/pdf/browser/pdf_frame_util.h"
|
||||
#include "components/pdf/common/pdf_util.h"
|
||||
#include "components/zoom/zoom_controller.h"
|
||||
#include "content/public/browser/global_routing_id.h"
|
||||
#include "content/public/browser/host_zoom_map.h"
|
||||
#include "content/public/browser/navigation_controller.h"
|
||||
#include "content/public/browser/navigation_handle.h"
|
||||
#include "content/public/browser/render_frame_host.h"
|
||||
@ -24,6 +26,7 @@
|
||||
#include "content/public/browser/web_contents_observer.h"
|
||||
#include "extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.h"
|
||||
#include "extensions/common/api/mime_handler.mojom.h"
|
||||
#include "extensions/common/constants.h"
|
||||
#include "extensions/common/mojom/guest_view.mojom.h"
|
||||
#include "mojo/public/cpp/bindings/associated_remote.h"
|
||||
#include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
|
||||
@ -433,6 +436,24 @@ void PdfViewerStreamManager::DidFinishNavigation(
|
||||
if (stream_info->extension_host_frame_tree_node_id() ==
|
||||
navigation_handle->GetFrameTreeNodeId()) {
|
||||
stream_info->SetDidExtensionFinishNavigation();
|
||||
if (navigation_handle->HasCommitted() &&
|
||||
!navigation_handle->IsErrorPage()) {
|
||||
// Setup zoom level for the PDF extension. Zoom level 0 corresponds
|
||||
// to zoom factor of 1, or 100%. This is done so the PDF viewer UI
|
||||
// does not change if the page zoom does. This is analogous to page
|
||||
// zoom not affecting the browser UI.
|
||||
const GURL pdf_extension_url = stream_info->stream()->handler_url();
|
||||
CHECK_EQ(pdf_extension_url, navigation_handle->GetURL());
|
||||
CHECK_EQ(extensions::kExtensionScheme, pdf_extension_url.scheme());
|
||||
content::HostZoomMap::Get(
|
||||
navigation_handle->GetRenderFrameHost()->GetSiteInstance())
|
||||
->SetZoomLevelForHostAndScheme(pdf_extension_url.scheme(),
|
||||
pdf_extension_url.host(), 0);
|
||||
// Set ZoomController on the extension host.
|
||||
zoom::ZoomController::CreateForWebContentsAndRenderFrameHost(
|
||||
web_contents(),
|
||||
navigation_handle->GetRenderFrameHost()->GetGlobalId());
|
||||
}
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
@ -20,11 +20,13 @@
|
||||
#include "content/browser/web_contents/web_contents_impl.h"
|
||||
#include "content/public/browser/browser_context.h"
|
||||
#include "content/public/browser/browser_thread.h"
|
||||
#include "content/public/browser/content_browser_client.h"
|
||||
#include "content/public/browser/host_zoom_map.h"
|
||||
#include "content/public/browser/render_frame_host.h"
|
||||
#include "content/public/browser/resource_context.h"
|
||||
#include "content/public/browser/site_instance.h"
|
||||
#include "content/public/browser/storage_partition.h"
|
||||
#include "content/public/common/content_client.h"
|
||||
#include "content/public/common/url_constants.h"
|
||||
#include "net/base/url_util.h"
|
||||
#include "third_party/blink/public/common/page/page_zoom.h"
|
||||
@ -63,6 +65,20 @@ std::string GetHostFromProcessFrame(RenderFrameHostImpl* rfh) {
|
||||
return net::GetHostOrSpecFromURL(url);
|
||||
}
|
||||
|
||||
// Allows HostZoomMap to grant independent zoom to subframes.
|
||||
BASE_FEATURE(kSubframeZoom, "SubframeZoom", base::FEATURE_ENABLED_BY_DEFAULT);
|
||||
|
||||
// Returns true if local root subframes may have different zoom levels than
|
||||
// the primary main frame.
|
||||
bool IsIndependentSubframeZoomEnabled() {
|
||||
// kSubframeZoom acts as a killswitch here. It is enabled by default, but
|
||||
// only return true here if some feature that requires subframe zoom is also
|
||||
// enabled.
|
||||
return base::FeatureList::IsEnabled(kSubframeZoom) &&
|
||||
(base::FeatureList::IsEnabled(features::kGuestViewMPArch) ||
|
||||
GetContentClient()->browser()->ShouldEnableSubframeZoom());
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
// static
|
||||
@ -374,9 +390,9 @@ void HostZoomMapImpl::SetDefaultZoomLevel(double level) {
|
||||
web_contents->GetPrimaryMainFrame());
|
||||
}
|
||||
|
||||
// If kGuestViewMPArch is enabled, then update zoom levels for subframes that
|
||||
// do not have an overriding entry.
|
||||
if (!base::FeatureList::IsEnabled(features::kGuestViewMPArch)) {
|
||||
// If independent subframe zoom is enabled, then update zoom levels for
|
||||
// subframes that do not have an overriding entry.
|
||||
if (!IsIndependentSubframeZoomEnabled()) {
|
||||
return;
|
||||
}
|
||||
for (auto ftn_id : independent_zoom_frame_tree_nodes_) {
|
||||
@ -466,7 +482,7 @@ void HostZoomMapImpl::SetTemporaryZoomLevel(
|
||||
DCHECK_CURRENTLY_ON(BrowserThread::UI);
|
||||
|
||||
RenderFrameHostImpl* rfh = RenderFrameHostImpl::FromID(rfh_id);
|
||||
if (base::FeatureList::IsEnabled(features::kGuestViewMPArch)) {
|
||||
if (IsIndependentSubframeZoomEnabled()) {
|
||||
CHECK(rfh->is_local_root());
|
||||
} else {
|
||||
DCHECK(rfh == rfh->GetOutermostMainFrame());
|
||||
@ -530,9 +546,9 @@ void HostZoomMapImpl::SendZoomLevelChange(const std::string& scheme,
|
||||
}
|
||||
|
||||
// Also loop over the independently-zoomed FTNs that aren't primary
|
||||
// mainframes. If kGuestViewMPArch isn't enabled, then there will be no
|
||||
// such frames, so early-out in that case.
|
||||
if (!base::FeatureList::IsEnabled(features::kGuestViewMPArch)) {
|
||||
// mainframes. If independent subframe zoom isn't enabled, then there will be
|
||||
// no such frames, so early-out in that case.
|
||||
if (!IsIndependentSubframeZoomEnabled()) {
|
||||
return;
|
||||
}
|
||||
for (auto ftn_id : independent_zoom_frame_tree_nodes_) {
|
||||
@ -734,10 +750,13 @@ void HostZoomMapImpl::SetIndependentZoomForFrameTreeNode(
|
||||
WebContents* web_contents,
|
||||
FrameTreeNodeId ftn_id) {
|
||||
CHECK(web_contents);
|
||||
CHECK(static_cast<RenderFrameHostImpl*>(
|
||||
web_contents->UnsafeFindFrameByFrameTreeNodeId(ftn_id))
|
||||
->is_local_root());
|
||||
auto* rfh = static_cast<RenderFrameHostImpl*>(
|
||||
web_contents->UnsafeFindFrameByFrameTreeNodeId(ftn_id));
|
||||
CHECK(rfh->is_local_root());
|
||||
independent_zoom_frame_tree_nodes_.insert(ftn_id);
|
||||
|
||||
// Force an update in case the `rfh` contains content with a different zoom.
|
||||
static_cast<WebContentsImpl*>(web_contents)->UpdateZoom(rfh->GetGlobalId());
|
||||
}
|
||||
|
||||
void HostZoomMapImpl::ClearIndependentZoomForFrameTreeNode(
|
||||
@ -745,4 +764,9 @@ void HostZoomMapImpl::ClearIndependentZoomForFrameTreeNode(
|
||||
independent_zoom_frame_tree_nodes_.erase(ftn_id);
|
||||
}
|
||||
|
||||
bool HostZoomMapImpl::IsIndependentZoomFrameTreeNode(
|
||||
FrameTreeNodeId ftn_id) const {
|
||||
return independent_zoom_frame_tree_nodes_.contains(ftn_id);
|
||||
}
|
||||
|
||||
} // namespace content
|
||||
|
@ -101,6 +101,7 @@ class CONTENT_EXPORT HostZoomMapImpl : public HostZoomMap {
|
||||
void SetIndependentZoomForFrameTreeNode(WebContents* web_contents,
|
||||
FrameTreeNodeId ftn_id) override;
|
||||
void ClearIndependentZoomForFrameTreeNode(FrameTreeNodeId ftn_id) override;
|
||||
bool IsIndependentZoomFrameTreeNode(FrameTreeNodeId ftn_id) const;
|
||||
|
||||
private:
|
||||
struct ZoomLevel {
|
||||
|
@ -712,6 +712,14 @@ void RecordRendererUnresponsiveMetrics(
|
||||
rph_priority);
|
||||
}
|
||||
|
||||
// Helper function to simplify getting the correct HostZoomMapImpl for a
|
||||
// RenderFrameHost. Implemented here to be close to its only consumer,
|
||||
// GetPendingZoomLevel().
|
||||
HostZoomMapImpl* HostZoomMapForRenderFrameHost(const RenderFrameHost* rfh) {
|
||||
return static_cast<HostZoomMapImpl*>(
|
||||
HostZoomMap::Get(rfh->GetSiteInstance()));
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
// This is a small helper class created while a JavaScript dialog is showing
|
||||
@ -8727,15 +8735,35 @@ void WebContentsImpl::RunFileChooser(
|
||||
}
|
||||
|
||||
double WebContentsImpl::GetPendingZoomLevel(RenderWidgetHostImpl* rwh) {
|
||||
// TODO(372932834): This needs to be modified for the PDF-in-OOPIF case, as
|
||||
// while the PDF will have its own RenderWidgetHost, the associated
|
||||
// RenderFrameHost won't always be at the root of a FrameTree (and the
|
||||
// FrameTree won't be of type kGuest).
|
||||
// Note: this could involve (something like) detecting that the rfh for `rwh`
|
||||
// is *not* a frame-tree root, but *does* have a temporary zoom level set in
|
||||
// HostZoomMap.
|
||||
RenderFrameHostImpl* rfh =
|
||||
rwh->frame_tree()->root()->current_frame_host()->GetOutermostMainFrame();
|
||||
RenderFrameHostImpl* rfh = nullptr;
|
||||
// Find the RenderFrameHost for `rwh`.
|
||||
ForEachRenderFrameHostImplWithAction(
|
||||
[rwh, &rfh](RenderFrameHostImpl* render_frame_host) {
|
||||
if (render_frame_host->GetRenderWidgetHost() == rwh) {
|
||||
rfh = render_frame_host;
|
||||
return RenderFrameHost::FrameIterationAction::kStop;
|
||||
}
|
||||
return RenderFrameHost::FrameIterationAction::kContinue;
|
||||
});
|
||||
|
||||
if (rfh) {
|
||||
// Find nearest ancestor that has independent zoom.
|
||||
// Use GetParentOrOuterDocument() instead of GetParent() in order
|
||||
// to "step out of" any fenced frames that are encountered.
|
||||
while (!HostZoomMapForRenderFrameHost(rfh)->IsIndependentZoomFrameTreeNode(
|
||||
rfh->GetFrameTreeNodeId()) &&
|
||||
rfh->GetParentOrOuterDocument()) {
|
||||
rfh = rfh->GetParentOrOuterDocument();
|
||||
}
|
||||
} else {
|
||||
// Not finding a `rfh` suggests that `rwh` is still in the process of being
|
||||
// set up, or is for a newly created RenderWidgetHost without a
|
||||
// RenderFrameHost (e.g. for a <select> tag popup>), and that this function
|
||||
// will be called again when there is a findable `rfh` for `rwh`. For now,
|
||||
// just return the zoom level of this WebContents, as that will most often
|
||||
// be right.
|
||||
return HostZoomMap::GetZoomLevel(this);
|
||||
}
|
||||
|
||||
GURL url;
|
||||
if (rfh->IsInBackForwardCache()) {
|
||||
@ -8759,13 +8787,12 @@ double WebContentsImpl::GetPendingZoomLevel(RenderWidgetHostImpl* rwh) {
|
||||
url = pending_entry->GetURL();
|
||||
}
|
||||
#if BUILDFLAG(IS_ANDROID)
|
||||
return HostZoomMap::GetForWebContents(this)
|
||||
return HostZoomMapForRenderFrameHost(rfh)
|
||||
->GetZoomLevelForHostAndSchemeAndroid(url.scheme(),
|
||||
net::GetHostOrSpecFromURL(url));
|
||||
#else
|
||||
return HostZoomMap::Get(rfh->GetSiteInstance())
|
||||
->GetZoomLevelForHostAndScheme(url.scheme(),
|
||||
net::GetHostOrSpecFromURL(url));
|
||||
return HostZoomMapForRenderFrameHost(rfh)->GetZoomLevelForHostAndScheme(
|
||||
url.scheme(), net::GetHostOrSpecFromURL(url));
|
||||
#endif
|
||||
}
|
||||
|
||||
|
@ -1940,4 +1940,8 @@ ContentBrowserClient::MaybeOverrideLocalURLCrossOriginEmbedderPolicy(
|
||||
return std::nullopt;
|
||||
}
|
||||
|
||||
bool ContentBrowserClient::ShouldEnableSubframeZoom() {
|
||||
return false;
|
||||
}
|
||||
|
||||
} // namespace content
|
||||
|
@ -3219,6 +3219,10 @@ class CONTENT_EXPORT ContentBrowserClient {
|
||||
virtual std::optional<network::CrossOriginEmbedderPolicy>
|
||||
MaybeOverrideLocalURLCrossOriginEmbedderPolicy(
|
||||
content::NavigationHandle* navigation_handle);
|
||||
|
||||
// Returns true if subframe zoom should be enabled for one or more features in
|
||||
// a higher layer.
|
||||
virtual bool ShouldEnableSubframeZoom();
|
||||
};
|
||||
|
||||
} // namespace content
|
||||
|
Reference in New Issue
Block a user