Maintain viewport when focus is restored in PDF

This CL ensures that when focus is restored to PDF, we maintain the
viewport and do not scroll even if the focused annotation is out of
viewport.

Bug: 1085614
Change-Id: Ifc4015b6d393dd1678d839a6636fbd4b335528ef
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2224946
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Kevin Babbitt <kbabbitt@microsoft.com>
Commit-Queue: Ankit Kumar 🌪️ <ankk@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#775202}

pdf/pdfium/pdfium_engine.cc

@@ -947,6 +947,7 @@ void PDFiumEngine::KillFormFocus() {
 }
 
 void PDFiumEngine::UpdateFocus(bool has_focus) {
+  base::AutoReset<bool> updating_focus_guard(&updating_focus_, true);
   if (has_focus) {
     UpdateFocusItemType(last_focused_item_type_);
     if (focus_item_type_ == FocusElementType::kPage &&

pdf/pdfium/pdfium_engine.h

@@ -724,6 +724,9 @@ class PDFiumEngine : public PDFEngine,
   // Set to true when handling long touch press.
   bool handling_long_press_ = false;
 
+  // Set to true when updating plugin focus.
+  bool updating_focus_ = false;
+
   // The focus item type for the currently focused object.
   FocusElementType focus_item_type_ = FocusElementType::kNone;
 

pdf/pdfium/pdfium_engine_unittest.cc

@@ -24,9 +24,11 @@ namespace chrome_pdf {
 namespace {
 
 using ::testing::InSequence;
+using ::testing::Invoke;
 using ::testing::IsEmpty;
 using ::testing::NiceMock;
 using ::testing::Return;
+using ::testing::StrictMock;
 
 MATCHER_P2(LayoutWithSize, width, height, "") {
   return arg.size() == pp::Size(width, height);
@@ -801,4 +803,69 @@ TEST_F(PDFiumEngineTabbingTest, RetainSelectionOnFocusNotInFormTextArea) {
   EXPECT_EQ(1u, GetSelectionSize(engine.get()));
 }
 
+class ScrollingTestClient : public TestClient {
+ public:
+  ScrollingTestClient() = default;
+  ~ScrollingTestClient() override = default;
+  ScrollingTestClient(const ScrollingTestClient&) = delete;
+  ScrollingTestClient& operator=(const ScrollingTestClient&) = delete;
+
+  // Mock PDFEngine::Client methods.
+  MOCK_METHOD1(ScrollToX, void(int));
+  MOCK_METHOD2(ScrollToY, void(int, bool));
+};
+
+TEST_F(PDFiumEngineTabbingTest, MaintainViewportWhenFocusIsUpdated) {
+  StrictMock<ScrollingTestClient> client;
+  std::unique_ptr<PDFiumEngine> engine = InitializeEngine(
+      &client, FILE_PATH_LITERAL("annotation_form_fields.pdf"));
+  ASSERT_TRUE(engine);
+  ASSERT_EQ(2, engine->GetNumberOfPages());
+  engine->PluginSizeUpdated(pp::Size(60, 40));
+
+  {
+    InSequence sequence;
+    static constexpr PP_Point kScrollValue = {510, 478};
+    EXPECT_CALL(client, ScrollToY(kScrollValue.y, false))
+        .WillOnce(Invoke(
+            [&engine]() { engine->ScrolledToYPosition(kScrollValue.y); }));
+    EXPECT_CALL(client, ScrollToX(kScrollValue.x)).WillOnce(Invoke([&engine]() {
+      engine->ScrolledToXPosition(kScrollValue.x);
+    }));
+  }
+
+  EXPECT_EQ(PDFiumEngine::FocusElementType::kNone,
+            GetFocusedElementType(engine.get()));
+  EXPECT_EQ(-1, GetLastFocusedPage(engine.get()));
+
+  // Tabbing to bring the document into focus.
+  ASSERT_TRUE(HandleTabEvent(engine.get(), 0));
+  EXPECT_EQ(PDFiumEngine::FocusElementType::kDocument,
+            GetFocusedElementType(engine.get()));
+
+  // Tab to an annotation.
+  ASSERT_TRUE(HandleTabEvent(engine.get(), 0));
+  EXPECT_EQ(PDFiumEngine::FocusElementType::kPage,
+            GetFocusedElementType(engine.get()));
+
+  // Scroll focused annotation out of viewport.
+  static constexpr PP_Point kScrollPosition = {242, 746};
+  engine->ScrolledToXPosition(kScrollPosition.x);
+  engine->ScrolledToYPosition(kScrollPosition.y);
+
+  engine->UpdateFocus(/*has_focus=*/false);
+  EXPECT_EQ(PDFiumEngine::FocusElementType::kPage,
+            GetLastFocusedElementType(engine.get()));
+  EXPECT_EQ(0, GetLastFocusedPage(engine.get()));
+  EXPECT_EQ(PDFiumEngine::FocusElementType::kPage,
+            GetFocusedElementType(engine.get()));
+  EXPECT_EQ(1, GetLastFocusedAnnotationIndex(engine.get()));
+
+  // Restore focus, we shouldn't have any calls to scroll viewport.
+  engine->UpdateFocus(/*has_focus=*/true);
+  EXPECT_EQ(PDFiumEngine::FocusElementType::kPage,
+            GetFocusedElementType(engine.get()));
+  EXPECT_EQ(0, GetLastFocusedPage(engine.get()));
+}
+
 }  // namespace chrome_pdf

pdf/pdfium/pdfium_form_filler.cc

@@ -283,16 +283,19 @@ void PDFiumFormFiller::Form_OnFocusChange(FPDF_FORMFILLINFO* param,
   if (!engine->PageIndexInBounds(page_index))
     return;
 
-  FS_RECTF annot_rect;
-  if (!FPDFAnnot_GetRect(annot, &annot_rect))
-    return;
+  // Maintain viewport if we are updating focus. This is to ensure that we don't
+  // scroll the focused annotation into view when focus is regained.
+  if (!engine->updating_focus_) {
+    FS_RECTF annot_rect;
+    if (!FPDFAnnot_GetRect(annot, &annot_rect))
+      return;
 
-  pp::Rect screen_rect = engine->pages_[page_index]->PageToScreen(
-      pp::Point(), /*zoom=*/1.0, annot_rect.left, annot_rect.top,
-      annot_rect.right, annot_rect.bottom,
-      engine->layout_.options().default_page_orientation());
-
-  engine->ScrollIntoView(screen_rect);
+    pp::Rect screen_rect = engine->pages_[page_index]->PageToScreen(
+        pp::Point(), /*zoom=*/1.0, annot_rect.left, annot_rect.top,
+        annot_rect.right, annot_rect.bottom,
+        engine->layout_.options().default_page_orientation());
+    engine->ScrollIntoView(screen_rect);
+  }
 
   engine->OnFocusedAnnotationUpdated(annot, page_index);
 }