PDF accessibility tweaks

pdf/out_of_process_instance.cc

Index: pdf/out_of_process_instance.cc
diff --git a/pdf/out_of_process_instance.cc b/pdf/out_of_process_instance.cc
index cb75b6bb07ef36fa0795f0b19b6a310d2f7e55b5..a16e5dad73e1f3bae04101bcc8f54553a1503fa4 100644
--- a/pdf/out_of_process_instance.cc
+++ b/pdf/out_of_process_instance.cc
@@ -303,7 +303,8 @@ OutOfProcessInstance::OutOfProcessInstance(PP_Instance instance)
       stop_scrolling_(false),
       background_color_(0),
       top_toolbar_height_(0),
-      accessibility_enabled_(false) {
+      accessibility_enabled_(false),
+      accessibility_loaded_(false) {
   loader_factory_.Initialize(this);
   timer_factory_.Initialize(this);
   form_factory_.Initialize(this);
@@ -630,17 +631,21 @@ void OutOfProcessInstance::GetPrintPresetOptionsFromDocument(
 }
 
 void OutOfProcessInstance::EnableAccessibility() {
-  if (accessibility_enabled_)
+  accessibility_enabled_ = true;
+  if (document_load_state_ == LOAD_STATE_LOADING)
     return;
 
-  accessibility_enabled_ = true;
+  if (accessibility_loaded_ || document_load_state_ != LOAD_STATE_COMPLETE)
+    return;
 
+  accessibility_loaded_ = true;
   PP_PrivateAccessibilityDocInfo doc_info;
   doc_info.page_count = engine_->GetNumberOfPages();
   doc_info.text_accessible = PP_FromBool(
       engine_->HasPermission(PDFEngine::PERMISSION_COPY_ACCESSIBLE));
   doc_info.text_copyable = PP_FromBool(
       engine_->HasPermission(PDFEngine::PERMISSION_COPY));
+
   pp::PDF::SetAccessibilityDocInfo(GetPluginInstance(), &doc_info);
 
   // If the document contents isn't accessible, don't send anything more.
@@ -677,7 +682,6 @@ void OutOfProcessInstance::SendNextAccessibilityPage(int32_t page_index) {
   std::vector<PP_PrivateAccessibilityCharInfo> chars(page_info.char_count);
   for (uint32_t i = 0; i < page_info.char_count; ++i) {
     chars[i].unicode_character = engine_->GetCharUnicode(page_index, i);
-    chars[i].char_width = engine_->GetCharWidth(page_index, i);
   }
 
   std::vector<PP_PrivateAccessibilityTextRunInfo> text_runs;
@@ -690,6 +694,16 @@ void OutOfProcessInstance::SendNextAccessibilityPage(int32_t page_index) {
     text_run_info.direction = PP_PRIVATEDIRECTION_LTR;
     text_run_info.bounds = bounds;
     text_runs.push_back(text_run_info);
+
+    pp::FloatRect char_bounds = engine_->GetCharBounds(page_index, char_index);

[Lei Zhang, 2016/06/09 22:19:05]

Can you add a brief description of what this new block of code is doing?

[dmazzoni, 2016/06/10 17:44:52]

Done.

+    for (uint32_t i = 0; i < text_run_info.len - 1; i++) {
+      pp::FloatRect next_char_bounds = engine_->GetCharBounds(
+          page_index, char_index + i + 1);

[Lei Zhang, 2016/06/09 22:19:05]

Can you double check and make sure char_index + i + 1 doesn't go out of bounds?

[dmazzoni, 2016/06/10 17:44:52]

Added a DCHECK.

+      chars[char_index + i].char_width = next_char_bounds.x() - char_bounds.x();
+      char_bounds = next_char_bounds;
+    }
+    chars[char_index + text_run_info.len - 1].char_width = char_bounds.width();
+
     char_index += text_run_info.len;
   }
 
@@ -1254,6 +1268,9 @@ void OutOfProcessInstance::DocumentLoadComplete(int page_count) {
   pp::PDF::SetContentRestriction(this, content_restrictions);
 
   uma_.HistogramCustomCounts("PDF.PageCount", page_count, 1, 1000000, 50);
+
+  if (accessibility_enabled_)

[Lei Zhang, 2016/06/09 22:19:05]

This is a bit confusing, since EnableAccessibility() is the place that sets
accessibility_enabled_ to true.

[dmazzoni, 2016/06/10 17:44:52]

Split into EnableAccessibility() and LoadAccessibility().

+    EnableAccessibility();
 }
 
 void OutOfProcessInstance::RotateClockwise() {