Plumb separate DidScroll message from PDF plugin to embedder.

The change in r509091 added code to notify the embedder whenever the PDF was scrolled so that touch-selection handles, which are drawn by the browser and not the PDF viewer, could have their positions adjusted to match the scroll. This was done by having PDFiumEngine call OnSelectionChanged() for each scroll, which triggers re-sending the selection coordinates to the embedder. However, it seems this also triggers a DCHECK as OnSelectionChanged() should not be called when the selection is in a form area. This CL plumbs an independent pathway from the PDF plugin to the embedder so that this DCHECK can be avoided. Bug: 784932 Change-Id: I8db353f0afa0e8c8bd625735d27e00d58c9b0dac Reviewed-on: https://chromium-review.googlesource.com/782404 Reviewed-by: Ken Buchanan <[email protected]> Reviewed-by: dsinclair <[email protected]> Reviewed-by: Jochen Eisinger <[email protected]> Commit-Queue: James MacLean <[email protected]> Cr-Commit-Position: refs/heads/master@{#518936}
feiyue1206 · Nov 23, 2017 · 289948d · 289948d
1 parent 5464476
commit 289948d
Show file tree

Hide file tree

Showing 13 changed files with 48 additions and 3 deletions.
diff --git a/components/pdf/common/pdf.mojom b/components/pdf/common/pdf.mojom
@@ -36,4 +36,8 @@ interface PdfService {
   // current selection.
   SelectionChanged(gfx.mojom.PointF left, int32 left_height,
                    gfx.mojom.PointF right, int32 right_height);
+
+  // Notifies the embedder that the PDF scrolled, and thus global coordinates
+  // of current selection may require adjustment.
+  DidScroll();
 };
diff --git a/components/pdf/renderer/pepper_pdf_host.cc b/components/pdf/renderer/pepper_pdf_host.cc
@@ -99,6 +99,8 @@ int32_t PepperPDFHost::OnResourceMessageReceived(
         OnHostMsgSetAccessibilityPageInfo)
     PPAPI_DISPATCH_HOST_RESOURCE_CALL(PpapiHostMsg_PDF_SelectionChanged,
                                       OnHostMsgSelectionChanged)
+    PPAPI_DISPATCH_HOST_RESOURCE_CALL(PpapiHostMsg_PDF_DidScroll,
+                                      OnHostMsgDidScroll)
   PPAPI_END_MESSAGE_MAP()
   return PP_ERROR_FAILED;
 }
@@ -249,6 +251,16 @@ int32_t PepperPDFHost::OnHostMsgSelectionChanged(
   return PP_OK;
 }
 
+int32_t PepperPDFHost::OnHostMsgDidScroll(
+    ppapi::host::HostMessageContext* context) {
+  mojom::PdfService* service = GetRemotePdfService();
+  if (!service)
+    return PP_ERROR_FAILED;
+
+  service->DidScroll();
+  return PP_OK;
+}
+
 void PepperPDFHost::CreatePdfAccessibilityTreeIfNeeded() {
   if (!pdf_accessibility_tree_) {
     pdf_accessibility_tree_ =

diff --git a/components/pdf/renderer/pepper_pdf_host.h b/components/pdf/renderer/pepper_pdf_host.h
@@ -112,6 +112,7 @@ class PepperPDFHost : public ppapi::host::ResourceHost,
                                     int32_t left_height,
                                     const PP_FloatPoint& right,
                                     int32_t right_height);
+  int32_t OnHostMsgDidScroll(ppapi::host::HostMessageContext* context);
 
   void CreatePdfAccessibilityTreeIfNeeded();
 

diff --git a/pdf/out_of_process_instance.cc b/pdf/out_of_process_instance.cc
@@ -1197,8 +1197,10 @@ void OutOfProcessInstance::Invalidate(const pp::Rect& rect) {
 }
 
 void OutOfProcessInstance::Scroll(const pp::Point& point) {
-  if (!image_data_.is_null())
+  if (!image_data_.is_null()) {
     paint_manager_.ScrollRect(available_area_, point);
+    pp::PDF::DidScroll(GetPluginInstance());
+  }
 }
 
 void OutOfProcessInstance::ScrollToX(int x_in_screen_coords) {

diff --git a/pdf/pdfium/pdfium_engine.cc b/pdf/pdfium/pdfium_engine.cc
@@ -1042,7 +1042,6 @@ void PDFiumEngine::ScrolledToXPosition(int position) {
   position_.set_x(position);
   CalculateVisiblePages();
   client_->Scroll(pp::Point(old_x - position, 0));
-  OnSelectionChanged();
 }
 
 void PDFiumEngine::ScrolledToYPosition(int position) {
@@ -1052,7 +1051,6 @@ void PDFiumEngine::ScrolledToYPosition(int position) {
   position_.set_y(position);
   CalculateVisiblePages();
   client_->Scroll(pp::Point(0, old_y - position));
-  OnSelectionChanged();
 }
 
 void PDFiumEngine::PrePaint() {

diff --git a/ppapi/c/private/ppb_pdf.h b/ppapi/c/private/ppb_pdf.h
@@ -168,6 +168,9 @@ struct PPB_PDF {
                            int32_t left_height,
                            const struct PP_FloatPoint* right,
                            int32_t right_height);
+
+  // Notifies embedder that the PDF has scrolled.
+  void (*DidScroll)(PP_Instance instance);
 };
 
 #endif  // PPAPI_C_PRIVATE_PPB_PDF_H_
diff --git a/ppapi/cpp/private/pdf.cc b/ppapi/cpp/private/pdf.cc
@@ -205,4 +205,11 @@ void PDF::SelectionChanged(const InstanceHandle& instance,
   }
 }
 
+// static
+void PDF::DidScroll(const InstanceHandle& instance) {
+  if (has_interface<PPB_PDF>()) {
+    get_interface<PPB_PDF>()->DidScroll(instance.pp_instance());
+  }
+}
+
 }  // namespace pp
diff --git a/ppapi/cpp/private/pdf.h b/ppapi/cpp/private/pdf.h
@@ -76,6 +76,7 @@ class PDF {
                                int32_t left_height,
                                const PP_FloatPoint& right,
                                int32_t right_height);
+  static void DidScroll(const InstanceHandle& instance);
 };
 
 }  // namespace pp

diff --git a/ppapi/proxy/pdf_resource.cc b/ppapi/proxy/pdf_resource.cc
@@ -208,5 +208,9 @@ void PDFResource::SelectionChanged(const PP_FloatPoint& left,
                                                    right_height));
 }
 
+void PDFResource::DidScroll() {
+  Post(RENDERER, PpapiHostMsg_PDF_DidScroll());
+}
+
 }  // namespace proxy
 }  // namespace ppapi
diff --git a/ppapi/proxy/pdf_resource.h b/ppapi/proxy/pdf_resource.h
@@ -65,6 +65,7 @@ class PPAPI_PROXY_EXPORT PDFResource
                         int32_t left_height,
                         const PP_FloatPoint& right,
                         int32_t right_height) override;
+  void DidScroll() override;
 
  private:
   std::string locale_;

diff --git a/ppapi/proxy/ppapi_messages.h b/ppapi/proxy/ppapi_messages.h
@@ -2570,6 +2570,9 @@ IPC_MESSAGE_CONTROL4(PpapiHostMsg_PDF_SelectionChanged,
                      PP_FloatPoint /* right */,
                      int32_t /* right_height */)
 
+// Notify that the plugin has scrolled.
+IPC_MESSAGE_CONTROL0(PpapiHostMsg_PDF_DidScroll)
+
 // VideoCapture ----------------------------------------------------------------
 
 // VideoCapture_Dev, plugin -> host

diff --git a/ppapi/thunk/ppb_pdf_api.h b/ppapi/thunk/ppb_pdf_api.h
@@ -45,6 +45,7 @@ class PPB_PDF_API {
                                 int32_t left_height,
                                 const PP_FloatPoint& right,
                                 int32_t right_height) = 0;
+  virtual void DidScroll() = 0;
 
   static const SingletonResourceID kSingletonResourceID = PDF_SINGLETON_ID;
 };

diff --git a/ppapi/thunk/ppb_pdf_thunk.cc b/ppapi/thunk/ppb_pdf_thunk.cc
@@ -180,6 +180,13 @@ void SelectionChanged(PP_Instance instance,
   enter.functions()->SelectionChanged(*left, left_height, *right, right_height);
 }
 
+void DidScroll(PP_Instance instance) {
+  EnterInstanceAPI<PPB_PDF_API> enter(instance);
+  if (enter.failed())
+    return;
+  enter.functions()->DidScroll();
+}
+
 const PPB_PDF g_ppb_pdf_thunk = {
     &GetFontFileWithFallback,
     &GetFontTableForPrivateFontFile,
@@ -200,6 +207,7 @@ const PPB_PDF g_ppb_pdf_thunk = {
     &SetAccessibilityPageInfo,
     &SetCrashData,
     &SelectionChanged,
+    &DidScroll,
 };
 
 }  // namespace