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 <virens@microsoft.com> Reviewed-by: Ian Prest <iapres@microsoft.com> Reviewed-by: Jochen Eisinger <jochen@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org> Cr-Commit-Position: refs/heads/master@{#720834}
iridium-browser · Dec 3, 2019 · dfc6c79 · dfc6c79
1 parent 1f50033
commit dfc6c79
Show file tree

Hide file tree

Showing 13 changed files with 138 additions and 16 deletions.
diff --git 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;
+  zoom_ = viewport_info.zoom;
-  CHECK_GT(zoom_device_scale_factor_, 0);
+  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();
+  // |scroll_| represents the x offset from which PDF content starts. It is the
-  transform->Scale(scale_factor, scale_factor);
+  // width of the PDF toolbar in pixels. Size of PDF toolbar does not change
-  transform->Translate(offset_);
+  // 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<RenderThreadObserver>::Unchecked observers_;
 
   std::unique_ptr<mojom::RenderMessageFilter> 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
@@ -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<RenderViewImpl*>(view_);
   return impl->GetWidget()->GetWebWidget();

diff --git 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
@@ -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
@@ -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
@@ -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 =
+  viewport_info.scroll.y = -top_toolbar_height_in_viewport_coords_;
-      -top_toolbar_height_in_viewport_coords_ * device_scale_;
+  viewport_info.offset.x =
-  viewport_info.offset = available_area_.point();
+      available_area_.point().x() / (device_scale_ * zoom_);
-  viewport_info.zoom_device_scale_factor = zoom_ * device_scale_;
+  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
@@ -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
@@ -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)