Migrate the rest of PDFiumEngine to SkBitmap

Switches remaining pp::ImageData references in PDFiumEngine to SkBitmap,
removing several conversions from pp::ImageData to SkBitmap as a result.

Adds an SkBitmap field to OutOfProcessInstance that is updated whenever
the pp::ImageData is reinitialized in response to DidChangeView(). This
avoids the need to convert to SkBitmap on every OnPaint() call.

Bug: 1099020
Change-Id: I9d5802d10db1dda5a3f5153c49511308032199ac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2274965
Commit-Queue: K. Moon <kmoon@chromium.org>
Reviewed-by: Henrique Nakashima <hnakashima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#788054}

pdf/BUILD.gn

@@ -156,6 +156,7 @@ if (enable_pdf) {
       "//base",
       "//ppapi/cpp:objects",
       "//ppapi/cpp/private:internal_module",
+      "//skia",
       "//v8",
     ]
   }

pdf/out_of_process_instance.cc

@@ -31,6 +31,7 @@
 #include "pdf/document_layout.h"
 #include "pdf/document_metadata.h"
 #include "pdf/pdf_features.h"
+#include "pdf/ppapi_migration/bitmap.h"
 #include "ppapi/c/dev/ppb_cursor_control_dev.h"
 #include "ppapi/c/pp_errors.h"
 #include "ppapi/c/private/ppb_pdf.h"
@@ -39,6 +40,7 @@
 #include "ppapi/cpp/dev/memory_dev.h"
 #include "ppapi/cpp/dev/text_input_dev.h"
 #include "ppapi/cpp/dev/url_util_dev.h"
+#include "ppapi/cpp/image_data.h"
 #include "ppapi/cpp/input_event.h"
 #include "ppapi/cpp/module.h"
 #include "ppapi/cpp/point.h"
@@ -670,6 +672,8 @@ void OutOfProcessInstance::DidChangeView(const pp::View& view) {
     if (new_image_data_size != image_data_.size()) {
       image_data_ = pp::ImageData(this, PP_IMAGEDATAFORMAT_BGRA_PREMUL,
                                   new_image_data_size, false);
+      skia_image_data_ =
+          SkBitmapFromPPImageData(std::make_unique<pp::ImageData>(image_data_));
       first_paint_ = true;
     }
 
@@ -977,7 +981,7 @@ void OutOfProcessInstance::OnPaint(const std::vector<pp::Rect>& paint_rects,
 
       std::vector<pp::Rect> pdf_ready;
       std::vector<pp::Rect> pdf_pending;
-      engine_->Paint(pdf_rect, &image_data_, &pdf_ready, &pdf_pending);
+      engine_->Paint(pdf_rect, skia_image_data_, pdf_ready, pdf_pending);
       for (auto& ready_rect : pdf_ready) {
         ready_rect.Offset(available_area_.point());
         ready->push_back(

pdf/out_of_process_instance.h

@@ -27,6 +27,7 @@
 #include "ppapi/cpp/private/find_private.h"
 #include "ppapi/cpp/url_loader.h"
 #include "ppapi/utility/completion_callback_factory.h"
+#include "third_party/skia/include/core/SkBitmap.h"
 
 namespace pp {
 class TextInput_Dev;
@@ -303,6 +304,8 @@ class OutOfProcessInstance : public pp::Instance,
   void InvalidateAfterPaintDone(int32_t /*unused_but_required*/);
 
   pp::ImageData image_data_;
+  SkBitmap skia_image_data_;  // Must be kept in sync with |image_data_|.
+
   // Used when the plugin is embedded in a page and we have to create the loader
   // ourself.
   pp::URLLoader embed_loader_;

pdf/pdf_engine.h

@@ -23,7 +23,6 @@
 #include "ppapi/c/dev/ppp_printing_dev.h"
 #include "ppapi/c/ppb_input_event.h"
 #include "ppapi/cpp/completion_callback.h"
-#include "ppapi/cpp/image_data.h"
 #include "ppapi/cpp/private/pdf.h"
 #include "ppapi/cpp/rect.h"
 #include "ppapi/cpp/size.h"
@@ -44,6 +43,7 @@ typedef void (*PDFEnsureTypefaceCharactersAccessible)(const LOGFONT* font,
 
 struct PP_PdfAccessibilityActionData;
 struct PP_PdfPrintSettings_Dev;
+class SkBitmap;
 
 namespace gfx {
 class Rect;
@@ -335,9 +335,9 @@ class PDFEngine {
   // is called once. After Paint is called n times, PostPaint is called once.
   virtual void PrePaint() = 0;
   virtual void Paint(const pp::Rect& rect,
-                     pp::ImageData* image_data,
-                     std::vector<pp::Rect>* ready,
-                     std::vector<pp::Rect>* pending) = 0;
+                     SkBitmap& image_data,
+                     std::vector<pp::Rect>& ready,
+                     std::vector<pp::Rect>& pending) = 0;
   virtual void PostPaint() = 0;
   virtual bool HandleDocumentLoad(const pp::URLLoader& loader) = 0;
   virtual bool HandleEvent(const pp::InputEvent& event) = 0;

pdf/pdfium/pdfium_engine.cc

@@ -533,13 +533,9 @@ void PDFiumEngine::PrePaint() {
 }
 
 void PDFiumEngine::Paint(const pp::Rect& rect,
-                         pp::ImageData* image_data,
-                         std::vector<pp::Rect>* ready,
-                         std::vector<pp::Rect>* pending) {
-  DCHECK(image_data);
-  DCHECK(ready);
-  DCHECK(pending);
-
+                         SkBitmap& image_data,
+                         std::vector<pp::Rect>& ready,
+                         std::vector<pp::Rect>& pending) {
   pp::Rect leftover = rect;
   for (size_t i = 0; i < visible_pages_.size(); ++i) {
     int index = visible_pages_[i];
@@ -581,7 +577,7 @@ void PDFiumEngine::Paint(const pp::Rect& rect,
           // happened, but it made scrolling up on complex PDFs very slow since
           // there would be a damaged rect at the top (from scroll) and at the
           // bottom (from toolbar).
-          pending->push_back(dirty_in_screen);
+          pending.push_back(dirty_in_screen);
           continue;
         }
       }
@@ -596,13 +592,13 @@ void PDFiumEngine::Paint(const pp::Rect& rect,
       progressive_paints_[progressive].set_painted(true);
       if (ContinuePaint(progressive, image_data)) {
         FinishPaint(progressive, image_data);
-        ready->push_back(dirty_in_screen);
+        ready.push_back(dirty_in_screen);
       } else {
-        pending->push_back(dirty_in_screen);
+        pending.push_back(dirty_in_screen);
       }
     } else {
       PaintUnavailablePage(index, dirty_in_screen, image_data);
-      ready->push_back(dirty_in_screen);
+      ready.push_back(dirty_in_screen);
     }
   }
 }
@@ -2964,11 +2960,9 @@ int PDFiumEngine::StartPaint(int page_index, const pp::Rect& dirty) {
   return progressive_paints_.size() - 1;
 }
 
-bool PDFiumEngine::ContinuePaint(int progressive_index,
-                                 pp::ImageData* image_data) {
+bool PDFiumEngine::ContinuePaint(int progressive_index, SkBitmap& image_data) {
   DCHECK_GE(progressive_index, 0);
   DCHECK_LT(static_cast<size_t>(progressive_index), progressive_paints_.size());
-  DCHECK(image_data);
 
   last_progressive_start_time_ = base::Time::Now();
 #if defined(OS_LINUX)
@@ -2990,9 +2984,7 @@ bool PDFiumEngine::ContinuePaint(int progressive_index,
     pp::Rect dirty = progressive_paints_[progressive_index].rect();
     GetPDFiumRect(page_index, dirty, &start_x, &start_y, &size_x, &size_y);
 
-    SkBitmap skia_image_data =
-        SkBitmapFromPPImageData(std::make_unique<pp::ImageData>(*image_data));
-    ScopedFPDFBitmap new_bitmap = CreateBitmap(dirty, skia_image_data);
+    ScopedFPDFBitmap new_bitmap = CreateBitmap(dirty, image_data);
     FPDFBitmap_FillRect(new_bitmap.get(), start_x, start_y, size_x, size_y,
                         0xFFFFFFFF);
     rv = FPDF_RenderPageBitmap_Start(
@@ -3000,16 +2992,14 @@ bool PDFiumEngine::ContinuePaint(int progressive_index,
         ToPDFiumRotation(layout_.options().default_page_orientation()),
         GetRenderingFlags(), this);
     progressive_paints_[progressive_index].SetBitmapAndImageData(
-        std::move(new_bitmap), std::move(skia_image_data));
+        std::move(new_bitmap), image_data);
   }
   return rv != FPDF_RENDER_TOBECONTINUED;
 }
 
-void PDFiumEngine::FinishPaint(int progressive_index,
-                               pp::ImageData* image_data) {
+void PDFiumEngine::FinishPaint(int progressive_index, SkBitmap& image_data) {
   DCHECK_GE(progressive_index, 0);
   DCHECK_LT(static_cast<size_t>(progressive_index), progressive_paints_.size());
-  DCHECK(image_data);
 
   int page_index = progressive_paints_[progressive_index].page_index();
   const pp::Rect& dirty_in_screen =
@@ -3112,10 +3102,9 @@ void PDFiumEngine::FillPageSides(int progressive_index) {
 }
 
 void PDFiumEngine::PaintPageShadow(int progressive_index,
-                                   pp::ImageData* image_data) {
+                                   SkBitmap& image_data) {
   DCHECK_GE(progressive_index, 0);
   DCHECK_LT(static_cast<size_t>(progressive_index), progressive_paints_.size());
-  DCHECK(image_data);
 
   int page_index = progressive_paints_[progressive_index].page_index();
   const pp::Rect& dirty_in_screen =
@@ -3138,10 +3127,9 @@ void PDFiumEngine::PaintPageShadow(int progressive_index,
 }
 
 void PDFiumEngine::DrawSelections(int progressive_index,
-                                  pp::ImageData* image_data) const {
+                                  SkBitmap& image_data) const {
   DCHECK_GE(progressive_index, 0);
   DCHECK_LT(static_cast<size_t>(progressive_index), progressive_paints_.size());
-  DCHECK(image_data);
 
   int page_index = progressive_paints_[progressive_index].page_index();
   const pp::Rect& dirty_in_screen =
@@ -3149,9 +3137,7 @@ void PDFiumEngine::DrawSelections(int progressive_index,
 
   void* region = nullptr;
   int stride;
-  SkBitmap skia_image_data =
-      SkBitmapFromPPImageData(std::make_unique<pp::ImageData>(*image_data));
-  GetRegion(dirty_in_screen.point(), skia_image_data, region, stride);
+  GetRegion(dirty_in_screen.point(), image_data, region, stride);
 
   std::vector<pp::Rect> highlighted_rects;
   pp::Rect visible_rect = GetVisibleRect();
@@ -3188,15 +3174,13 @@ void PDFiumEngine::DrawSelections(int progressive_index,
 
 void PDFiumEngine::PaintUnavailablePage(int page_index,
                                         const pp::Rect& dirty,
-                                        pp::ImageData* image_data) {
+                                        SkBitmap& image_data) {
   int start_x;
   int start_y;
   int size_x;
   int size_y;
   GetPDFiumRect(page_index, dirty, &start_x, &start_y, &size_x, &size_y);
-  SkBitmap skia_image_data =
-      SkBitmapFromPPImageData(std::make_unique<pp::ImageData>(*image_data));
-  ScopedFPDFBitmap bitmap(CreateBitmap(dirty, skia_image_data));
+  ScopedFPDFBitmap bitmap(CreateBitmap(dirty, image_data));
   FPDFBitmap_FillRect(bitmap.get(), start_x, start_y, size_x, size_y,
                       kPendingPageColor);
 
@@ -3485,7 +3469,7 @@ void PDFiumEngine::SetCurrentPage(int index) {
 void PDFiumEngine::DrawPageShadow(const pp::Rect& page_rc,
                                   const pp::Rect& shadow_rc,
                                   const pp::Rect& clip_rc,
-                                  pp::ImageData* image_data) {
+                                  SkBitmap& image_data) {
   pp::Rect page_rect(page_rc);
   page_rect.Offset(page_offset_);
 
@@ -3509,12 +3493,8 @@ void PDFiumEngine::DrawPageShadow(const pp::Rect& page_rc,
         depth, factor, client_->GetBackgroundColor());
   }
 
-  // TODO(crbug.com/1099020): Converting to SkBitmap here may seem silly, but
-  // this is part of a migration from pp::ImageData to SkBitmap in general.
-  DCHECK(!image_data->is_null());
-  SkBitmap bitmap =
-      SkBitmapFromPPImageData(std::make_unique<pp::ImageData>(*image_data));
-  DrawShadow(bitmap, shadow_rect, page_rect, clip_rect, *page_shadow_);
+  DCHECK(!image_data.isNull());
+  DrawShadow(image_data, shadow_rect, page_rect, clip_rect, *page_shadow_);
 }
 
 void PDFiumEngine::GetRegion(const pp::Point& location,

pdf/pdfium/pdfium_engine.h

@@ -30,7 +30,6 @@
 #include "ppapi/c/private/ppp_pdf.h"
 #include "ppapi/cpp/completion_callback.h"
 #include "ppapi/cpp/dev/buffer_dev.h"
-#include "ppapi/cpp/image_data.h"
 #include "ppapi/cpp/input_event.h"
 #include "ppapi/cpp/point.h"
 #include "ppapi/cpp/rect.h"
@@ -84,9 +83,9 @@ class PDFiumEngine : public PDFEngine,
   void ScrolledToYPosition(int position) override;
   void PrePaint() override;
   void Paint(const pp::Rect& rect,
-             pp::ImageData* image_data,
-             std::vector<pp::Rect>* ready,
-             std::vector<pp::Rect>* pending) override;
+             SkBitmap& image_data,
+             std::vector<pp::Rect>& ready,
+             std::vector<pp::Rect>& pending) override;
   void PostPaint() override;
   bool HandleDocumentLoad(const pp::URLLoader& loader) override;
   bool HandleEvent(const pp::InputEvent& event) override;
@@ -433,11 +432,11 @@ class PDFiumEngine : public PDFEngine,
 
   // Continues a paint operation that was started earlier.  Returns true if the
   // paint is done, or false if it needs to be continued.
-  bool ContinuePaint(int progressive_index, pp::ImageData* image_data);
+  bool ContinuePaint(int progressive_index, SkBitmap& image_data);
 
   // Called once PDFium is finished rendering a page so that we draw our
   // borders, highlighting etc.
-  void FinishPaint(int progressive_index, pp::ImageData* image_data);
+  void FinishPaint(int progressive_index, SkBitmap& image_data);
 
   // Stops any paints that are in progress.
   void CancelPaints();
@@ -450,15 +449,15 @@ class PDFiumEngine : public PDFEngine,
   // with the page background.
   void FillPageSides(int progressive_index);
 
-  void PaintPageShadow(int progressive_index, pp::ImageData* image_data);
+  void PaintPageShadow(int progressive_index, SkBitmap& image_data);
 
   // Highlight visible find results and selections.
-  void DrawSelections(int progressive_index, pp::ImageData* image_data) const;
+  void DrawSelections(int progressive_index, SkBitmap& image_data) const;
 
   // Paints an page that hasn't finished downloading.
   void PaintUnavailablePage(int page_index,
                             const pp::Rect& dirty,
-                            pp::ImageData* image_data);
+                            SkBitmap& image_data);
 
   // Given a page index, returns the corresponding index in progressive_rects_,
   // or -1 if it doesn't exist.
@@ -516,7 +515,7 @@ class PDFiumEngine : public PDFEngine,
   void DrawPageShadow(const pp::Rect& page_rect,
                       const pp::Rect& shadow_rect,
                       const pp::Rect& clip_rect,
-                      pp::ImageData* image_data);
+                      SkBitmap& image_data);
 
   void GetRegion(const pp::Point& location,
                  SkBitmap& image_data,