Supporting context menu over external links via keyboard shortcuts

When user tabs over links and invokes context menu using Shift + F10,
link related context menu items are not shown since link url under
cursor value isn't set. This CL finds out the link url from the focused
annotation and updates link under cursor value.
Added unit test to test link under cursor value whenever focused
annotation changes.

Bug: chromium:994500
Change-Id: Iedb5db7fafcaeb47d1e1e5094b5f7e0553260cbe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2173855
Commit-Queue: Badhri Ravikumar <bravi@microsoft.com>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Ankit Kumar 🌪️ <ankk@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#768912}

pdf/pdfium/pdfium_engine.cc

@@ -357,6 +357,14 @@ void SetSelectedText(pp::Instance* instance, const std::string& selected_text) {
   pp::PDF::SetSelectedText(instance, selected_text.c_str());
 }
 
+PDFiumEngine::SetLinkUnderCursorFunction
+    g_set_link_under_cursor_func_for_testing = nullptr;
+
+void SetLinkUnderCursor(pp::Instance* instance,
+                        const std::string& link_under_cursor) {
+  pp::PDF::SetLinkUnderCursor(instance, link_under_cursor.c_str());
+}
+
 }  // namespace
 
 void InitializeSDK(bool enable_v8) {
@@ -442,6 +450,12 @@ void PDFiumEngine::OverrideSetSelectedTextFunctionForTesting(
   g_set_selected_text_func_for_testing = function;
 }
 
+// static
+void PDFiumEngine::OverrideSetLinkUnderCursorFunctionForTesting(
+    SetLinkUnderCursorFunction function) {
+  g_set_link_under_cursor_func_for_testing = function;
+}
+
 bool PDFiumEngine::New(const char* url, const char* headers) {
   url_ = url;
   if (headers)
@@ -1359,11 +1373,7 @@ bool PDFiumEngine::OnMouseMove(const pp::MouseInputEvent& event) {
                        page_y);
     }
 
-    std::string url = GetLinkAtPosition(event.GetPosition());
-    if (url != link_under_cursor_) {
-      link_under_cursor_ = url;
-      pp::PDF::SetLinkUnderCursor(GetPluginInstance(), url.c_str());
-    }
+    UpdateLinkUnderCursor(GetLinkAtPosition(event.GetPosition()));
 
     // If in form text area while left mouse button is held down, check if form
     // text selection needs to be updated.
@@ -3679,7 +3689,9 @@ void PDFiumEngine::ScrollIntoView(const pp::Rect& rect) {
   }
 }
 
-void PDFiumEngine::OnFocusedAnnotationUpdated(FPDF_ANNOTATION annot) {
+void PDFiumEngine::OnFocusedAnnotationUpdated(FPDF_ANNOTATION annot,
+                                              int page_index) {
+  SetLinkUnderCursorForAnnotation(annot, page_index);
   int form_type = FPDFAnnot_GetFormFieldType(form(), annot);
   if (form_type <= FPDF_FORMFIELD_UNKNOWN) {
     SetInFormTextArea(false);
@@ -3935,6 +3947,31 @@ void PDFiumEngine::UpdatePageCount() {
 }
 #endif  // defined(PDF_ENABLE_XFA)
 
+void PDFiumEngine::UpdateLinkUnderCursor(const std::string& target_url) {
+  if (link_under_cursor_ == target_url)
+    return;
+
+  link_under_cursor_ = target_url;
+
+  SetLinkUnderCursorFunction set_link_under_cursor_func =
+      g_set_link_under_cursor_func_for_testing
+          ? g_set_link_under_cursor_func_for_testing
+          : &SetLinkUnderCursor;
+  set_link_under_cursor_func(GetPluginInstance(), link_under_cursor_);
+}
+
+void PDFiumEngine::SetLinkUnderCursorForAnnotation(FPDF_ANNOTATION annot,
+                                                   int page_index) {
+  if (!PageIndexInBounds(page_index)) {
+    UpdateLinkUnderCursor("");
+    return;
+  }
+
+  PDFiumPage::LinkTarget target;
+  pages_[page_index]->GetLinkTarget(FPDFAnnot_GetLink(annot), &target);
+  UpdateLinkUnderCursor(target.url);
+}
+
 PDFiumEngine::ProgressivePaint::ProgressivePaint(int index,
                                                  const pp::Rect& rect)
     : page_index_(index), rect_(rect) {}

pdf/pdfium/pdfium_engine.h

@@ -65,6 +65,11 @@ class PDFiumEngine : public PDFEngine,
   static void OverrideSetSelectedTextFunctionForTesting(
       SetSelectedTextFunction function);
 
+  using SetLinkUnderCursorFunction =
+      void (*)(pp::Instance* instance, const std::string& link_under_cursor);
+  static void OverrideSetLinkUnderCursorFunctionForTesting(
+      SetLinkUnderCursorFunction function);
+
   // PDFEngine implementation.
   bool New(const char* url, const char* headers) override;
   void PageOffsetUpdated(const pp::Point& page_offset) override;
@@ -579,7 +584,7 @@ class PDFiumEngine : public PDFEngine,
   // already in view.
   void ScrollIntoView(const pp::Rect& rect);
 
-  void OnFocusedAnnotationUpdated(FPDF_ANNOTATION annot);
+  void OnFocusedAnnotationUpdated(FPDF_ANNOTATION annot, int page_index);
 
   // Fetches and populates the fields of |doc_metadata_|. To be called after the
   // document is loaded.
@@ -601,6 +606,9 @@ class PDFiumEngine : public PDFEngine,
   bool HandleTabForward(uint32_t modifiers);
   bool HandleTabBackward(uint32_t modifiers);
 
+  void UpdateLinkUnderCursor(const std::string& target_url);
+  void SetLinkUnderCursorForAnnotation(FPDF_ANNOTATION annot, int page_index);
+
   PDFEngine::Client* const client_;
 
   // The current document layout.

pdf/pdfium/pdfium_engine_unittest.cc

@@ -263,11 +263,53 @@ class PDFiumEngineTabbingTest : public PDFiumTestBase {
     return engine->selection_.size();
   }
 
+  const std::string& GetLinkUnderCursor(PDFiumEngine* engine) {
+    return engine->link_under_cursor_;
+  }
+
  protected:
   base::test::TaskEnvironment task_environment_{
       base::test::TaskEnvironment::TimeSource::MOCK_TIME};
 };
 
+TEST_F(PDFiumEngineTabbingTest, LinkUnderCursorTest) {
+  /*
+   * Document structure
+   * Document
+   * ++ Page 1
+   * ++++ Widget annotation
+   * ++++ Widget annotation
+   * ++++ Highlight annotation
+   * ++++ Link annotation
+   */
+  // Enable feature flag.
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(
+      chrome_pdf::features::kTabAcrossPDFAnnotations);
+
+  TestClient client;
+  std::unique_ptr<PDFiumEngine> engine =
+      InitializeEngine(&client, FILE_PATH_LITERAL("annots.pdf"));
+  ASSERT_TRUE(engine);
+
+  // Initial value of link under cursor.
+  EXPECT_EQ("", GetLinkUnderCursor(engine.get()));
+
+  // Tab through non-link annotations and validate link under cursor.
+  for (int i = 0; i < 4; i++) {
+    ASSERT_TRUE(HandleTabEvent(engine.get(), 0));
+    EXPECT_EQ("", GetLinkUnderCursor(engine.get()));
+  }
+
+  // Tab to Link annotation.
+  ASSERT_TRUE(HandleTabEvent(engine.get(), 0));
+  EXPECT_EQ("https://www.google.com/", GetLinkUnderCursor(engine.get()));
+
+  // Tab to previous annotation.
+  ASSERT_TRUE(HandleTabEvent(engine.get(), PP_INPUTEVENT_MODIFIER_SHIFTKEY));
+  EXPECT_EQ("", GetLinkUnderCursor(engine.get()));
+}
+
 TEST_F(PDFiumEngineTabbingTest, TabbingSupportedAnnots) {
   /*
    * Document structure

pdf/pdfium/pdfium_form_filler.cc

@@ -294,7 +294,7 @@ void PDFiumFormFiller::Form_OnFocusChange(FPDF_FORMFILLINFO* param,
 
   engine->ScrollIntoView(screen_rect);
 
-  engine->OnFocusedAnnotationUpdated(annot);
+  engine->OnFocusedAnnotationUpdated(annot, page_index);
 }
 
 // static

pdf/pdfium/pdfium_page.cc

@@ -716,6 +716,35 @@ PDFiumPage::Area PDFiumPage::GetLinkTargetAtIndex(int link_index,
   return target->url.empty() ? DOCLINK_AREA : WEBLINK_AREA;
 }
 
+PDFiumPage::Area PDFiumPage::GetLinkTarget(FPDF_LINK link, LinkTarget* target) {
+  FPDF_DEST dest_link = FPDFLink_GetDest(engine_->doc(), link);
+  if (dest_link)
+    return GetDestinationTarget(dest_link, target);
+
+  FPDF_ACTION action = FPDFLink_GetAction(link);
+  if (!action)
+    return NONSELECTABLE_AREA;
+
+  switch (FPDFAction_GetType(action)) {
+    case PDFACTION_GOTO: {
+      FPDF_DEST dest_action = FPDFAction_GetDest(engine_->doc(), action);
+      if (dest_action)
+        return GetDestinationTarget(dest_action, target);
+      // TODO(crbug.com/55776): We don't fully support all types of the
+      // in-document links.
+      return NONSELECTABLE_AREA;
+    }
+    case PDFACTION_URI:
+      return GetURITarget(action, target);
+      // TODO(crbug.com/767191): Support PDFACTION_LAUNCH.
+      // TODO(crbug.com/142344): Support PDFACTION_REMOTEGOTO.
+    case PDFACTION_LAUNCH:
+    case PDFACTION_REMOTEGOTO:
+    default:
+      return NONSELECTABLE_AREA;
+  }
+}
+
 PDFiumPage::Area PDFiumPage::GetCharIndex(const pp::Point& point,
                                           PageOrientation orientation,
                                           int* char_index,
@@ -810,35 +839,6 @@ bool PDFiumPage::IsCharIndexInBounds(int index) {
   return index >= 0 && index < GetCharCount();
 }
 
-PDFiumPage::Area PDFiumPage::GetLinkTarget(FPDF_LINK link, LinkTarget* target) {
-  FPDF_DEST dest_link = FPDFLink_GetDest(engine_->doc(), link);
-  if (dest_link)
-    return GetDestinationTarget(dest_link, target);
-
-  FPDF_ACTION action = FPDFLink_GetAction(link);
-  if (!action)
-    return NONSELECTABLE_AREA;
-
-  switch (FPDFAction_GetType(action)) {
-    case PDFACTION_GOTO: {
-      FPDF_DEST dest_action = FPDFAction_GetDest(engine_->doc(), action);
-      if (dest_action)
-        return GetDestinationTarget(dest_action, target);
-      // TODO(crbug.com/55776): We don't fully support all types of the
-      // in-document links.
-      return NONSELECTABLE_AREA;
-    }
-    case PDFACTION_URI:
-      return GetURITarget(action, target);
-    // TODO(crbug.com/767191): Support PDFACTION_LAUNCH.
-    // TODO(crbug.com/142344): Support PDFACTION_REMOTEGOTO.
-    case PDFACTION_LAUNCH:
-    case PDFACTION_REMOTEGOTO:
-    default:
-      return NONSELECTABLE_AREA;
-  }
-}
-
 PDFiumPage::Area PDFiumPage::GetDestinationTarget(FPDF_DEST destination,
                                                   LinkTarget* target) {
   if (!target)

pdf/pdfium/pdfium_page.h

@@ -99,6 +99,10 @@ class PDFiumPage {
   // |link_index| is invalid.
   Area GetLinkTargetAtIndex(int link_index, LinkTarget* target);
 
+  // Returns link type and fills target associated with a link. Returns
+  // NONSELECTABLE_AREA if link detection failed.
+  Area GetLinkTarget(FPDF_LINK link, LinkTarget* target);
+
   // Fills the output params with the (x, y) position in page coordinates and
   // zoom value of a destination.
   void GetPageDestinationTarget(FPDF_DEST destination,
@@ -194,9 +198,6 @@ class PDFiumPage {
   void PopulateHighlight(FPDF_ANNOTATION annot);
   // Populate |text_fields_| with |annot|.
   void PopulateTextField(FPDF_ANNOTATION annot);
-  // Returns link type and fills target associated with a link. Returns
-  // NONSELECTABLE_AREA if link detection failed.
-  Area GetLinkTarget(FPDF_LINK link, LinkTarget* target);
   // Returns link type and fills target associated with a destination. Returns
   // NONSELECTABLE_AREA if detection failed.
   Area GetDestinationTarget(FPDF_DEST destination, LinkTarget* target);

pdf/pdfium/pdfium_test_base.cc

@@ -28,6 +28,9 @@ bool IsValidLinkForTesting(const std::string& url) {
 void SetSelectedTextForTesting(pp::Instance* instance,
                                const std::string& selected_text) {}
 
+void SetLinkUnderCursorForTesting(pp::Instance* instance,
+                                  const std::string& link_under_cursor) {}
+
 }  // namespace
 
 PDFiumTestBase::PDFiumTestBase() = default;
@@ -47,11 +50,14 @@ void PDFiumTestBase::SetUp() {
   InitializePDFium();
   PDFiumEngine::OverrideSetSelectedTextFunctionForTesting(
       &SetSelectedTextForTesting);
+  PDFiumEngine::OverrideSetLinkUnderCursorFunctionForTesting(
+      &SetLinkUnderCursorForTesting);
   PDFiumPage::SetIsValidLinkFunctionForTesting(&IsValidLinkForTesting);
 }
 
 void PDFiumTestBase::TearDown() {
   PDFiumPage::SetIsValidLinkFunctionForTesting(nullptr);
+  PDFiumEngine::OverrideSetLinkUnderCursorFunctionForTesting(nullptr);
   PDFiumEngine::OverrideSetSelectedTextFunctionForTesting(nullptr);
   FPDF_DestroyLibrary();
 }