From dfc6c794d40047a0619b5abd729ea2db2d5c6760 Mon Sep 17 00:00:00 2001 From: Virender Singh Date: Tue, 3 Dec 2019 05:40:03 +0000 Subject: [PATCH] Honoring DPI changes in PdfAccessibilityTree This change fixes the incorrect scale that we apply to root transform in PdfAccessibilityTree. At present, we remove the dpi factor from root transform. This results in only zoom factor being applied when calculating bounds. For Windows, device scaling is also handled by application. This is not the case with Mac. As a result, conditional scale calculation, based on OS, should be done. Fix uses content::IsUseZoomForDSFEnabled() to find out the correct scale that needs to be applied. Bug: 1007169 Change-Id: I4cda488b8acf3823178b4ff68503b081e12aeaa2 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1820697 Commit-Queue: Virender Singh Reviewed-by: Ian Prest Reviewed-by: Jochen Eisinger Reviewed-by: Daniel Cheng Reviewed-by: Dominic Mazzoni Cr-Commit-Position: refs/heads/master@{#720834} --- .../pdf/renderer/pdf_accessibility_tree.cc | 20 +++-- .../pdf/renderer/pdf_accessibility_tree.h | 13 +++- .../pdf_accessibility_tree_browsertest.cc | 76 ++++++++++++++++++- content/public/renderer/render_thread.h | 3 + content/public/test/mock_render_thread.cc | 8 ++ content/public/test/mock_render_thread.h | 3 + content/public/test/render_view_test.cc | 4 + content/public/test/render_view_test.h | 4 + content/renderer/render_thread_impl.cc | 4 + content/renderer/render_thread_impl.h | 1 + pdf/out_of_process_instance.cc | 12 ++- ppapi/c/private/ppb_pdf.h | 3 +- ppapi/proxy/ppapi_messages.h | 3 +- 13 files changed, 138 insertions(+), 16 deletions(-) diff --git a/components/pdf/renderer/pdf_accessibility_tree.cc b/components/pdf/renderer/pdf_accessibility_tree.cc index 271f14a839a6c1..7b2f57c64630a7 100644 --- a/components/pdf/renderer/pdf_accessibility_tree.cc +++ b/components/pdf/renderer/pdf_accessibility_tree.cc @@ -14,6 +14,7 @@ #include "content/public/renderer/pepper_plugin_instance.h" #include "content/public/renderer/render_accessibility.h" #include "content/public/renderer/render_frame.h" +#include "content/public/renderer/render_thread.h" #include "content/public/renderer/render_view.h" #include "content/public/renderer/renderer_ppapi_host.h" #include "pdf/pdf_features.h" @@ -308,12 +309,12 @@ bool PdfAccessibilityTree::IsDataFromPluginValid( void PdfAccessibilityTree::SetAccessibilityViewportInfo( const PP_PrivateAccessibilityViewportInfo& viewport_info) { - zoom_device_scale_factor_ = viewport_info.zoom_device_scale_factor; - CHECK_GT(zoom_device_scale_factor_, 0); + zoom_ = viewport_info.zoom; + scale_ = viewport_info.scale; + CHECK_GT(zoom_, 0); + CHECK_GT(scale_, 0); scroll_ = ToVector2dF(viewport_info.scroll); - scroll_.Scale(1.0 / zoom_device_scale_factor_); offset_ = ToVector2dF(viewport_info.offset); - offset_.Scale(1.0 / zoom_device_scale_factor_); selection_start_page_index_ = viewport_info.selection_start_page_index; selection_start_char_index_ = viewport_info.selection_start_char_index; @@ -971,11 +972,16 @@ content::RenderAccessibility* PdfAccessibilityTree::GetRenderAccessibility() { } gfx::Transform* PdfAccessibilityTree::MakeTransformFromViewInfo() { + double applicable_scale_factor = + content::RenderThread::Get()->IsUseZoomForDSF() ? scale_ : 1; gfx::Transform* transform = new gfx::Transform(); - float scale_factor = zoom_device_scale_factor_ / GetDeviceScaleFactor(); - transform->Scale(scale_factor, scale_factor); - transform->Translate(offset_); + // |scroll_| represents the x offset from which PDF content starts. It is the + // width of the PDF toolbar in pixels. Size of PDF toolbar does not change + // with zoom. + transform->Scale(applicable_scale_factor, applicable_scale_factor); transform->Translate(-scroll_); + transform->Scale(zoom_, zoom_); + transform->Translate(offset_); return transform; } diff --git a/components/pdf/renderer/pdf_accessibility_tree.h b/components/pdf/renderer/pdf_accessibility_tree.h index b9d3c7eaf4f3bb..e13b1140e1893b 100644 --- a/components/pdf/renderer/pdf_accessibility_tree.h +++ b/components/pdf/renderer/pdf_accessibility_tree.h @@ -162,7 +162,18 @@ class PdfAccessibilityTree : public content::PluginAXTreeSource { ui::AXTree tree_; content::RendererPpapiHost* host_; PP_Instance instance_; - double zoom_device_scale_factor_ = 1.0; + // |zoom_| signifies the zoom level set in for the browser content. + // |scale_| signifies the scale level set by user. Scale is applied + // by the OS while zoom is applied by the application. Higher scale + // values are usually set to increase the size of everything on screen. + // Preferred by people with blurry/low vision. |zoom_| and |scale_| + // both help us increase/descrease the size of content on screen. + // From PDF plugin we receive all the data in logical pixels. Which is + // without the zoom and scale factor applied. We apply the |zoom_| and + // |scale_| to generate the final bounding boxes of elements in accessibility + // tree. + double zoom_ = 1.0; + double scale_ = 1.0; gfx::Vector2dF scroll_; gfx::Vector2dF offset_; uint32_t selection_start_page_index_ = 0; diff --git a/components/pdf/renderer/pdf_accessibility_tree_browsertest.cc b/components/pdf/renderer/pdf_accessibility_tree_browsertest.cc index 4932bd1811930c..19bcbf67c5034f 100644 --- a/components/pdf/renderer/pdf_accessibility_tree_browsertest.cc +++ b/components/pdf/renderer/pdf_accessibility_tree_browsertest.cc @@ -48,13 +48,21 @@ const PP_PrivateAccessibilityTextRunInfo kFourthRunMultiLine = { const char kChromiumTestUrl[] = "www.cs.chromium.org"; -void CompareRect(PP_Rect expected_rect, PP_Rect actual_rect) { +void CompareRect(const PP_Rect& expected_rect, const PP_Rect& actual_rect) { EXPECT_EQ(expected_rect.point.x, actual_rect.point.x); EXPECT_EQ(expected_rect.point.y, actual_rect.point.y); EXPECT_EQ(expected_rect.size.height, actual_rect.size.height); EXPECT_EQ(expected_rect.size.width, actual_rect.size.width); } +void CompareRect(const gfx::RectF& expected_rect, + const gfx::RectF& actual_rect) { + EXPECT_FLOAT_EQ(expected_rect.x(), actual_rect.x()); + EXPECT_FLOAT_EQ(expected_rect.y(), actual_rect.y()); + EXPECT_FLOAT_EQ(expected_rect.size().height(), actual_rect.size().height()); + EXPECT_FLOAT_EQ(expected_rect.size().width(), actual_rect.size().width()); +} + // This class overrides content::FakePepperPluginInstance to record received // action data when tests make an accessibility action call. class ActionHandlingFakePepperPluginInstance @@ -155,7 +163,8 @@ class PdfAccessibilityTreeTest : public content::RenderViewTest { ui::ResourceBundle::GetSharedInstance().AddDataPackFromPath( pak_file, ui::SCALE_FACTOR_NONE); - viewport_info_.zoom_device_scale_factor = 1.0; + viewport_info_.zoom = 1.0; + viewport_info_.scale = 1.0; viewport_info_.scroll = {0, 0}; viewport_info_.offset = {0, 0}; viewport_info_.selection_start_page_index = 0; @@ -985,4 +994,67 @@ TEST_F(PdfAccessibilityTreeTest, TestEmptyPdfAxActions) { EXPECT_FALSE(pdf_action_target->ScrollToGlobalPoint(gfx::Point())); } +TEST_F(PdfAccessibilityTreeTest, TestZoomAndScaleChanges) { + text_runs_.emplace_back(kFirstTextRun); + text_runs_.emplace_back(kSecondTextRun); + chars_.insert(chars_.end(), std::begin(kDummyCharsData), + std::end(kDummyCharsData)); + + content::RenderFrame* render_frame = view_->GetMainRenderFrame(); + render_frame->SetAccessibilityModeForTest(ui::AXMode::kWebContents); + ASSERT_TRUE(render_frame->GetRenderAccessibility()); + + ActionHandlingFakePepperPluginInstance fake_pepper_instance; + FakeRendererPpapiHost host(view_->GetMainRenderFrame(), + &fake_pepper_instance); + PP_Instance instance = 0; + PdfAccessibilityTree pdf_accessibility_tree(&host, instance); + pdf_accessibility_tree.SetAccessibilityDocInfo(doc_info_); + pdf_accessibility_tree.SetAccessibilityPageInfo(page_info_, text_runs_, + chars_, page_objects_); + + viewport_info_.zoom = 1.0; + viewport_info_.scale = 1.0; + viewport_info_.scroll = {0, -56}; + viewport_info_.offset = {57, 0}; + + pdf_accessibility_tree.SetAccessibilityViewportInfo(viewport_info_); + ui::AXNode* root_node = pdf_accessibility_tree.GetRoot(); + ASSERT_TRUE(root_node); + ASSERT_EQ(1u, root_node->children().size()); + ui::AXNode* page_node = root_node->children()[0]; + ASSERT_TRUE(page_node); + ASSERT_EQ(2u, page_node->children().size()); + ui::AXNode* para_node = page_node->children()[0]; + ASSERT_TRUE(para_node); + gfx::RectF rect = para_node->data().relative_bounds.bounds; + CompareRect({{26.0f, 189.0f}, {84.0f, 13.0f}}, rect); + gfx::Transform* transform = root_node->data().relative_bounds.transform.get(); + ASSERT_TRUE(transform); + transform->TransformRect(&rect); + CompareRect({{83.0f, 245.0f}, {84.0f, 13.0f}}, rect); + + float new_device_scale = 1.5f; + float new_zoom = 1.5f; + viewport_info_.zoom = new_zoom; + viewport_info_.scale = new_device_scale; + SetUseZoomForDSFEnabled(true); + pdf_accessibility_tree.SetAccessibilityViewportInfo(viewport_info_); + + rect = para_node->data().relative_bounds.bounds; + transform = root_node->data().relative_bounds.transform.get(); + ASSERT_TRUE(transform); + transform->TransformRect(&rect); + CompareRect({{186.75f, 509.25f}, {189.00f, 29.25f}}, rect); + + SetUseZoomForDSFEnabled(false); + pdf_accessibility_tree.SetAccessibilityViewportInfo(viewport_info_); + + rect = para_node->data().relative_bounds.bounds; + transform = root_node->data().relative_bounds.transform.get(); + ASSERT_TRUE(transform); + transform->TransformRect(&rect); + CompareRect({{124.5f, 339.5f}, {126.0f, 19.5f}}, rect); +} + } // namespace pdf diff --git a/content/public/renderer/render_thread.h b/content/public/renderer/render_thread.h index bfee9a2316d2be..a27e7aaa54af8a 100644 --- a/content/public/renderer/render_thread.h +++ b/content/public/renderer/render_thread.h @@ -108,6 +108,9 @@ class CONTENT_EXPORT RenderThread : virtual public ChildThread { // Returns the user-agent string. virtual blink::WebString GetUserAgent() = 0; virtual const blink::UserAgentMetadata& GetUserAgentMetadata() = 0; + + // Returns whether or not the use-zoom-for-dsf flag is enabled. + virtual bool IsUseZoomForDSF() = 0; }; } // namespace content diff --git a/content/public/test/mock_render_thread.cc b/content/public/test/mock_render_thread.cc index b93355ff9ffe42..7ab24dc248aa3f 100644 --- a/content/public/test/mock_render_thread.cc +++ b/content/public/test/mock_render_thread.cc @@ -225,6 +225,10 @@ blink::WebString MockRenderThread::GetUserAgent() { return blink::WebString(); } +bool MockRenderThread::IsUseZoomForDSF() { + return zoom_for_dsf_; +} + const blink::UserAgentMetadata& MockRenderThread::GetUserAgentMetadata() { return kUserAgentMetadata; } @@ -240,6 +244,10 @@ void MockRenderThread::ReleaseCachedFonts() { void MockRenderThread::SetFieldTrialGroup(const std::string& trial_name, const std::string& group_name) {} +void MockRenderThread::SetUseZoomForDSFEnabled(bool zoom_for_dsf) { + zoom_for_dsf_ = zoom_for_dsf; +} + int32_t MockRenderThread::GetNextRoutingID() { return next_routing_id_++; } diff --git a/content/public/test/mock_render_thread.h b/content/public/test/mock_render_thread.h index a6de3f8cdca7f2..2de198127506eb 100644 --- a/content/public/test/mock_render_thread.h +++ b/content/public/test/mock_render_thread.h @@ -81,12 +81,14 @@ class MockRenderThread : public RenderThread { blink::scheduler::WebRendererProcessType type) override; blink::WebString GetUserAgent() override; const blink::UserAgentMetadata& GetUserAgentMetadata() override; + bool IsUseZoomForDSF() override; #if defined(OS_WIN) void PreCacheFont(const LOGFONT& log_font) override; void ReleaseCachedFonts() override; #endif void SetFieldTrialGroup(const std::string& trial_name, const std::string& group_name) override; + void SetUseZoomForDSFEnabled(bool zoom_for_dsf); // Returns a new, unique routing ID that can be assigned to the next view, // widget, or frame. @@ -162,6 +164,7 @@ class MockRenderThread : public RenderThread { base::ObserverList::Unchecked observers_; std::unique_ptr mock_render_message_filter_; + bool zoom_for_dsf_ = false; }; } // namespace content diff --git a/content/public/test/render_view_test.cc b/content/public/test/render_view_test.cc index 6175daf10228c4..edd7111dc1fecd 100644 --- a/content/public/test/render_view_test.cc +++ b/content/public/test/render_view_test.cc @@ -768,6 +768,10 @@ void RenderViewTest::OnSameDocumentNavigation(blink::WebLocalFrame* frame, false /* content_initiated */); } +void RenderViewTest::SetUseZoomForDSFEnabled(bool enabled) { + render_thread_->SetUseZoomForDSFEnabled(enabled); +} + blink::WebWidget* RenderViewTest::GetWebWidget() { RenderViewImpl* impl = static_cast(view_); return impl->GetWidget()->GetWebWidget(); diff --git a/content/public/test/render_view_test.h b/content/public/test/render_view_test.h index 41195a24aef5cf..2347276fcec538 100644 --- a/content/public/test/render_view_test.h +++ b/content/public/test/render_view_test.h @@ -178,6 +178,10 @@ class RenderViewTest : public testing::Test { // These are all methods from RenderViewImpl that we expose to testing code. void OnSameDocumentNavigation(blink::WebLocalFrame* frame, bool is_new_navigation); + + // Enables to use zoom for device scale. + void SetUseZoomForDSFEnabled(bool zoom_for_dsf); + blink::WebWidget* GetWebWidget(); // Allows a subclass to override the various content client implementations. diff --git a/content/renderer/render_thread_impl.cc b/content/renderer/render_thread_impl.cc index a759410f600ee5..58ba27ef1ea9cc 100644 --- a/content/renderer/render_thread_impl.cc +++ b/content/renderer/render_thread_impl.cc @@ -1291,6 +1291,10 @@ const blink::UserAgentMetadata& RenderThreadImpl::GetUserAgentMetadata() { return user_agent_metadata_; } +bool RenderThreadImpl::IsUseZoomForDSF() { + return IsUseZoomForDSFEnabled(); +} + void RenderThreadImpl::OnAssociatedInterfaceRequest( const std::string& name, mojo::ScopedInterfaceEndpointHandle handle) { diff --git a/content/renderer/render_thread_impl.h b/content/renderer/render_thread_impl.h index daf8aee073337c..c6e78c0dae7e57 100644 --- a/content/renderer/render_thread_impl.h +++ b/content/renderer/render_thread_impl.h @@ -185,6 +185,7 @@ class CONTENT_EXPORT RenderThreadImpl blink::scheduler::WebRendererProcessType type) override; blink::WebString GetUserAgent() override; const blink::UserAgentMetadata& GetUserAgentMetadata() override; + bool IsUseZoomForDSF() override; // IPC::Listener implementation via ChildThreadImpl: void OnAssociatedInterfaceRequest( diff --git a/pdf/out_of_process_instance.cc b/pdf/out_of_process_instance.cc index 4fc9ee9db2679d..9c3c5396a6fa22 100644 --- a/pdf/out_of_process_instance.cc +++ b/pdf/out_of_process_instance.cc @@ -1001,10 +1001,14 @@ void OutOfProcessInstance::SendNextAccessibilityPage(int32_t page_index) { void OutOfProcessInstance::SendAccessibilityViewportInfo() { PP_PrivateAccessibilityViewportInfo viewport_info; viewport_info.scroll.x = 0; - viewport_info.scroll.y = - -top_toolbar_height_in_viewport_coords_ * device_scale_; - viewport_info.offset = available_area_.point(); - viewport_info.zoom_device_scale_factor = zoom_ * device_scale_; + viewport_info.scroll.y = -top_toolbar_height_in_viewport_coords_; + viewport_info.offset.x = + available_area_.point().x() / (device_scale_ * zoom_); + viewport_info.offset.y = + available_area_.point().y() / (device_scale_ * zoom_); + + viewport_info.zoom = zoom_; + viewport_info.scale = device_scale_; engine_->GetSelection(&viewport_info.selection_start_page_index, &viewport_info.selection_start_char_index, diff --git a/ppapi/c/private/ppb_pdf.h b/ppapi/c/private/ppb_pdf.h index 99608a2c26c2b0..b2980f644947f8 100644 --- a/ppapi/c/private/ppb_pdf.h +++ b/ppapi/c/private/ppb_pdf.h @@ -34,7 +34,8 @@ struct PP_PrivateFindResult { }; struct PP_PrivateAccessibilityViewportInfo { - double zoom_device_scale_factor; + double zoom; + double scale; struct PP_Point scroll; struct PP_Point offset; uint32_t selection_start_page_index; diff --git a/ppapi/proxy/ppapi_messages.h b/ppapi/proxy/ppapi_messages.h index f11166db32ae03..45ccd8a909150e 100644 --- a/ppapi/proxy/ppapi_messages.h +++ b/ppapi/proxy/ppapi_messages.h @@ -255,7 +255,8 @@ IPC_STRUCT_TRAITS_BEGIN(PP_PdfPrintSettings_Dev) IPC_STRUCT_TRAITS_END() IPC_STRUCT_TRAITS_BEGIN(PP_PrivateAccessibilityViewportInfo) - IPC_STRUCT_TRAITS_MEMBER(zoom_device_scale_factor) + IPC_STRUCT_TRAITS_MEMBER(zoom) + IPC_STRUCT_TRAITS_MEMBER(scale) IPC_STRUCT_TRAITS_MEMBER(scroll) IPC_STRUCT_TRAITS_MEMBER(offset) IPC_STRUCT_TRAITS_MEMBER(selection_start_page_index)