Commit 2d0cae27 authored by Daniel Hosseinian's avatar Daniel Hosseinian Committed by Commit Bot

Reload the PDF accessibility tree on layout changes

Currently, any layout changes applied to the PDF document (i.e. page
rotation or two-up view) after the accessibility tree has loaded does
not update the page bounds.

Because the accessiblity tree has read-only nodes, the only way to
resolve the issue is to reload all the nodes of the tree on each layout
change.

Note: This CL only fixes page bounds and positions. The bounds of page
content such as text or annotations still do not rotate with the rest
of the document. Those issues will be fixed in a follow-up.

Bug: 1150665
Change-Id: I2fbf2cb26f38ee3fed076333c0b224a8ede7947e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2553217
Commit-Queue: Daniel Hosseinian <dhoss@chromium.org>
Reviewed-by: Ankit Kumar 🌪️ <ankk@microsoft.com>
Reviewed-by: default avatarRaymes Khoury <raymes@chromium.org>
Reviewed-by: default avatarDominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: default avatarTom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#830918}
parent 7a25254f
...@@ -1302,6 +1302,7 @@ void PdfAccessibilityTree::SetAccessibilityDocInfo( ...@@ -1302,6 +1302,7 @@ void PdfAccessibilityTree::SetAccessibilityDocInfo(
if (!render_accessibility) if (!render_accessibility)
return; return;
ClearAccessibilityNodes();
doc_info_ = doc_info; doc_info_ = doc_info;
doc_node_ = doc_node_ =
CreateNode(ax::mojom::Role::kDocument, ax::mojom::Restriction::kReadOnly, CreateNode(ax::mojom::Role::kDocument, ax::mojom::Restriction::kReadOnly,
...@@ -1323,6 +1324,11 @@ void PdfAccessibilityTree::SetAccessibilityPageInfo( ...@@ -1323,6 +1324,11 @@ void PdfAccessibilityTree::SetAccessibilityPageInfo(
const std::vector<ppapi::PdfAccessibilityTextRunInfo>& text_runs, const std::vector<ppapi::PdfAccessibilityTextRunInfo>& text_runs,
const std::vector<PP_PrivateAccessibilityCharInfo>& chars, const std::vector<PP_PrivateAccessibilityCharInfo>& chars,
const ppapi::PdfAccessibilityPageObjects& page_objects) { const ppapi::PdfAccessibilityPageObjects& page_objects) {
// Outdated calls are ignored.
uint32_t page_index = page_info.page_index;
if (page_index != next_page_index_)
return;
content::RenderAccessibility* render_accessibility = GetRenderAccessibility(); content::RenderAccessibility* render_accessibility = GetRenderAccessibility();
if (!render_accessibility) if (!render_accessibility)
return; return;
...@@ -1336,9 +1342,9 @@ void PdfAccessibilityTree::SetAccessibilityPageInfo( ...@@ -1336,9 +1342,9 @@ void PdfAccessibilityTree::SetAccessibilityPageInfo(
if (invalid_plugin_message_received_) if (invalid_plugin_message_received_)
return; return;
uint32_t page_index = page_info.page_index;
CHECK_GE(page_index, 0U); CHECK_GE(page_index, 0U);
CHECK_LT(page_index, doc_info_.page_count); CHECK_LT(page_index, doc_info_.page_count);
++next_page_index_;
ui::AXNodeData* page_node = ui::AXNodeData* page_node =
CreateNode(ax::mojom::Role::kRegion, ax::mojom::Restriction::kReadOnly, CreateNode(ax::mojom::Role::kRegion, ax::mojom::Restriction::kReadOnly,
...@@ -1460,6 +1466,13 @@ bool PdfAccessibilityTree::FindCharacterOffset( ...@@ -1460,6 +1466,13 @@ bool PdfAccessibilityTree::FindCharacterOffset(
return true; return true;
} }
void PdfAccessibilityTree::ClearAccessibilityNodes() {
next_page_index_ = 0;
nodes_.clear();
node_id_to_page_char_index_.clear();
node_id_to_annotation_info_.clear();
}
content::RenderAccessibility* PdfAccessibilityTree::GetRenderAccessibility() { content::RenderAccessibility* PdfAccessibilityTree::GetRenderAccessibility() {
content::RenderFrame* render_frame = content::RenderFrame* render_frame =
host_->GetRenderFrameForInstance(instance_); host_->GetRenderFrameForInstance(instance_);
......
...@@ -121,6 +121,10 @@ class PdfAccessibilityTree : public content::PluginAXTreeSource { ...@@ -121,6 +121,10 @@ class PdfAccessibilityTree : public content::PluginAXTreeSource {
const ppapi::PdfAccessibilityPageObjects& page_objects, const ppapi::PdfAccessibilityPageObjects& page_objects,
content::RenderAccessibility* render_accessibility); content::RenderAccessibility* render_accessibility);
// Clears the local cache of node data used to create the tree so that
// replacement node data can be introduced.
void ClearAccessibilityNodes();
content::RenderAccessibility* GetRenderAccessibility(); content::RenderAccessibility* GetRenderAccessibility();
std::unique_ptr<gfx::Transform> MakeTransformFromViewInfo() const; std::unique_ptr<gfx::Transform> MakeTransformFromViewInfo() const;
...@@ -163,6 +167,10 @@ class PdfAccessibilityTree : public content::PluginAXTreeSource { ...@@ -163,6 +167,10 @@ class PdfAccessibilityTree : public content::PluginAXTreeSource {
// object to which an action can be passed. // object to which an action can be passed.
std::map<int32_t, AnnotationInfo> node_id_to_annotation_info_; std::map<int32_t, AnnotationInfo> node_id_to_annotation_info_;
bool invalid_plugin_message_received_ = false; bool invalid_plugin_message_received_ = false;
// Index of the next expected PDF accessibility page info, used to ignore
// outdated calls of SetAccessibilityPageInfo().
uint32_t next_page_index_ = 0;
}; };
} // namespace pdf } // namespace pdf
......
...@@ -233,6 +233,45 @@ TEST_F(PdfAccessibilityTreeTest, TestAccessibilityDisabledDuringPDFLoad) { ...@@ -233,6 +233,45 @@ TEST_F(PdfAccessibilityTreeTest, TestAccessibilityDisabledDuringPDFLoad) {
chars_, page_objects_); chars_, page_objects_);
} }
TEST_F(PdfAccessibilityTreeTest, TestPdfAccessibilityTreeReload) {
content::RenderFrame* render_frame = view_->GetMainRenderFrame();
ASSERT_TRUE(render_frame);
render_frame->SetAccessibilityModeForTest(ui::AXMode::kWebContents);
ASSERT_TRUE(render_frame->GetRenderAccessibility());
FakeRendererPpapiHost host(view_->GetMainRenderFrame());
PP_Instance instance = 0;
pdf::PdfAccessibilityTree pdf_accessibility_tree(&host, instance);
// Make the accessibility tree with a portrait page and then remake with a
// landscape page.
gfx::RectF page_bounds = gfx::RectF(1, 2);
for (size_t i = 1; i <= 2; ++i) {
if (i == 2)
page_bounds.Transpose();
page_info_.bounds =
PP_MakeRectFromXYWH(page_bounds.x(), page_bounds.y(),
page_bounds.width(), page_bounds.height());
pdf_accessibility_tree.SetAccessibilityViewportInfo(viewport_info_);
pdf_accessibility_tree.SetAccessibilityDocInfo(doc_info_);
pdf_accessibility_tree.SetAccessibilityPageInfo(page_info_, text_runs_,
chars_, page_objects_);
ui::AXNode* root_node = pdf_accessibility_tree.GetRoot();
ASSERT_TRUE(root_node);
EXPECT_EQ(ax::mojom::Role::kDocument, root_node->data().role);
// There should only be one page node.
ASSERT_EQ(1u, root_node->children().size());
ui::AXNode* page_node = root_node->children()[0];
ASSERT_TRUE(page_node);
EXPECT_EQ(ax::mojom::Role::kRegion, page_node->data().role);
EXPECT_EQ(page_bounds, page_node->data().relative_bounds.bounds);
}
}
TEST_F(PdfAccessibilityTreeTest, TestPdfAccessibilityTreeCreation) { TEST_F(PdfAccessibilityTreeTest, TestPdfAccessibilityTreeCreation) {
static const char kTestAltText[] = "Alternate text for image"; static const char kTestAltText[] = "Alternate text for image";
......
...@@ -816,6 +816,10 @@ void OutOfProcessInstance::LoadAccessibility() { ...@@ -816,6 +816,10 @@ void OutOfProcessInstance::LoadAccessibility() {
doc_info.text_copyable = doc_info.text_copyable =
PP_FromBool(engine()->HasPermission(PDFEngine::PERMISSION_COPY)); PP_FromBool(engine()->HasPermission(PDFEngine::PERMISSION_COPY));
// A new document layout will trigger the creation of a new accessibility
// tree, so |next_accessibility_page_index_| should be reset to ignore
// outdated asynchronous calls of SendNextAccessibilityPage().
next_accessibility_page_index_ = 0;
pp::PDF::SetAccessibilityDocInfo(GetPluginInstance(), &doc_info); pp::PDF::SetAccessibilityDocInfo(GetPluginInstance(), &doc_info);
// If the document contents isn't accessible, don't send anything more. // If the document contents isn't accessible, don't send anything more.
...@@ -836,6 +840,11 @@ void OutOfProcessInstance::LoadAccessibility() { ...@@ -836,6 +840,11 @@ void OutOfProcessInstance::LoadAccessibility() {
} }
void OutOfProcessInstance::SendNextAccessibilityPage(int32_t page_index) { void OutOfProcessInstance::SendNextAccessibilityPage(int32_t page_index) {
// Outdated calls are ignored.
if (page_index != next_accessibility_page_index_)
return;
++next_accessibility_page_index_;
AccessibilityPageInfo page_info; AccessibilityPageInfo page_info;
std::vector<pp::PDF::PrivateAccessibilityTextRunInfo> text_runs; std::vector<pp::PDF::PrivateAccessibilityTextRunInfo> text_runs;
std::vector<AccessibilityCharInfo> chars; std::vector<AccessibilityCharInfo> chars;
...@@ -1236,6 +1245,11 @@ void OutOfProcessInstance::ProposeDocumentLayout(const DocumentLayout& layout) { ...@@ -1236,6 +1245,11 @@ void OutOfProcessInstance::ProposeDocumentLayout(const DocumentLayout& layout) {
} }
dimensions.Set(kJSPageDimensions, page_dimensions_array); dimensions.Set(kJSPageDimensions, page_dimensions_array);
PostMessage(dimensions); PostMessage(dimensions);
// Reload the accessibility tree on layout changes because the relative page
// bounds are no longer valid.
if (layout.dirty() && accessibility_state_ == ACCESSIBILITY_STATE_LOADED)
LoadAccessibility();
} }
void OutOfProcessInstance::Invalidate(const gfx::Rect& rect) { void OutOfProcessInstance::Invalidate(const gfx::Rect& rect) {
......
...@@ -523,6 +523,10 @@ class OutOfProcessInstance : public PdfViewPluginBase, ...@@ -523,6 +523,10 @@ class OutOfProcessInstance : public PdfViewPluginBase,
ACCESSIBILITY_STATE_LOADED ACCESSIBILITY_STATE_LOADED
} accessibility_state_ = ACCESSIBILITY_STATE_OFF; } accessibility_state_ = ACCESSIBILITY_STATE_OFF;
// The next accessibility page index, used to track interprocess calls when
// reconstructing the tree for new document layouts.
int32_t next_accessibility_page_index_ = 0;
base::WeakPtrFactory<OutOfProcessInstance> weak_factory_{this}; base::WeakPtrFactory<OutOfProcessInstance> weak_factory_{this};
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment