Send PDF document layout instead of just size

Change the PDFEngine::Client::DocumentSizeUpdated() handler to take a
full DocumentLayout, instead of just the pp::Size. This will be needed
when we send additional layout information to the extension.

Eliminate PDFEngine::GetPageRect(), since we get all the same
information from the DocumentLayout now.

Rename this method to ProposeDocumentLayout(), since it gets called for
more than just updating the document size. "DocumentLayoutUpdated" might
have been a more accurate rename, but won't make sense once we switch to
asynchronous layout.

Bug: 885110
Change-Id: Ic883ed219fb789dd54e0d24cedcb48fc9239e07b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1827862
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: K Moon <kmoon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#700518}

pdf/out_of_process_instance.cc

@@ -24,6 +24,7 @@
 #include "net/base/escape.h"
 #include "net/base/filename_util.h"
 #include "pdf/accessibility.h"
+#include "pdf/document_layout.h"
 #include "pdf/pdf.h"
 #include "pdf/pdf_features.h"
 #include "ppapi/c/dev/ppb_cursor_control_dev.h"
@@ -1284,20 +1285,20 @@ void OutOfProcessInstance::FillRect(const pp::Rect& rect, uint32_t color) {
   }
 }
 
-void OutOfProcessInstance::DocumentSizeUpdated(const pp::Size& size) {
-  document_size_ = size;
+void OutOfProcessInstance::ProposeDocumentLayout(const DocumentLayout& layout) {
+  document_size_ = layout.size();
 
   pp::VarDictionary dimensions;
   dimensions.Set(kType, kJSDocumentDimensionsType);
   dimensions.Set(kJSDocumentWidth, pp::Var(document_size_.width()));
   dimensions.Set(kJSDocumentHeight, pp::Var(document_size_.height()));
   pp::VarArray page_dimensions_array;
-  size_t num_pages = engine_->GetNumberOfPages();
+  size_t num_pages = layout.page_count();
   if (page_is_processed_.size() < num_pages)
     page_is_processed_.resize(num_pages);
 
   for (size_t i = 0; i < num_pages; ++i) {
-    pp::Rect page_rect = engine_->GetPageRect(i);
+    pp::Rect page_rect = layout.page_rect(i);
     pp::VarDictionary page_dimensions;
     page_dimensions.Set(kJSPageX, pp::Var(page_rect.x()));
     page_dimensions.Set(kJSPageY, pp::Var(page_rect.y()));

pdf/out_of_process_instance.h

@@ -97,7 +97,7 @@ class OutOfProcessInstance : public pp::Instance,
   void OnPrint(int32_t);
 
   // PDFEngine::Client implementation.
-  void DocumentSizeUpdated(const pp::Size& size) override;
+  void ProposeDocumentLayout(const DocumentLayout& layout) override;
   void Invalidate(const pp::Rect& rect) override;
   void DidScroll(const pp::Point& point) override;
   void ScrollToX(int x_in_screen_coords) override;

pdf/pdf_engine.h

@@ -55,6 +55,8 @@ class VarDictionary;
 
 namespace chrome_pdf {
 
+class DocumentLayout;
+
 // Do one time initialization of the SDK.
 // If |enable_v8| is false, then the PDFEngine will not be able to run
 // JavaScript.
@@ -136,8 +138,10 @@ class PDFEngine {
    public:
     virtual ~Client() {}
 
-    // Informs the client about the document's size in pixels.
-    virtual void DocumentSizeUpdated(const pp::Size& size) {}
+    // Proposes a document layout to the client.
+    // TODO(crbug.com/885110): Layout still occurs immediately for now. In the
+    // future, the client will need to accept the layout before it takes effect.
+    virtual void ProposeDocumentLayout(const DocumentLayout& layout) = 0;
 
     // Informs the client that the given rect needs to be repainted.
     virtual void Invalidate(const pp::Rect& rect) {}
@@ -340,8 +344,6 @@ class PDFEngine {
       const std::string& destination) = 0;
   // Gets the index of the most visible page, or -1 if none are visible.
   virtual int GetMostVisiblePage() = 0;
-  // Gets the rectangle of the page including shadow.
-  virtual pp::Rect GetPageRect(int index) = 0;
   // Gets the rectangle of the page not including the shadow.
   virtual pp::Rect GetPageBoundsRect(int index) = 0;
   // Gets the rectangle of the page excluding any additional areas.

pdf/pdfium/pdfium_engine.cc

@@ -2223,12 +2223,6 @@ int PDFiumEngine::GetMostVisiblePage() {
   return most_visible_page_;
 }
 
-pp::Rect PDFiumEngine::GetPageRect(int index) {
-  pp::Rect rc(pages_[index]->rect());
-  InsetPage(index, pages_.size(), /*multiplier=*/-1, &rc);
-  return rc;
-}
-
 pp::Rect PDFiumEngine::GetPageBoundsRect(int index) {
   return pages_[index]->rect();
 }
@@ -2556,7 +2550,7 @@ void PDFiumEngine::LoadPageInfo(bool reload) {
   CalculateVisiblePages();
   if (layout_.dirty()) {
     layout_.clear_dirty();
-    client_->DocumentSizeUpdated(layout_.size());
+    client_->ProposeDocumentLayout(layout_);
   }
 }
 

pdf/pdfium/pdfium_engine.h

@@ -105,7 +105,6 @@ class PDFiumEngine : public PDFEngine,
   base::Optional<PDFEngine::NamedDestination> GetNamedDestination(
       const std::string& destination) override;
   int GetMostVisiblePage() override;
-  pp::Rect GetPageRect(int index) override;
   pp::Rect GetPageBoundsRect(int index) override;
   pp::Rect GetPageContentsRect(int index) override;
   pp::Rect GetPageScreenRect(int page_index) const override;

pdf/pdfium/pdfium_engine_unittest.cc

@@ -4,6 +4,7 @@
 
 #include "pdf/pdfium/pdfium_engine.h"
 
+#include "pdf/document_layout.h"
 #include "pdf/pdfium/pdfium_page.h"
 #include "pdf/pdfium/pdfium_test_base.h"
 #include "pdf/test/test_client.h"
@@ -18,10 +19,21 @@ namespace {
 using ::testing::InSequence;
 using ::testing::NiceMock;
 
+MATCHER_P2(LayoutWithSize, width, height, "") {
+  return arg.size() == pp::Size(width, height);
+}
+
 class MockTestClient : public TestClient {
  public:
+  MockTestClient() {
+    ON_CALL(*this, ProposeDocumentLayout)
+        .WillByDefault([this](const DocumentLayout& layout) {
+          TestClient::ProposeDocumentLayout(layout);
+        });
+  }
+
   // TODO(crbug.com/989095): MOCK_METHOD() triggers static_assert on Windows.
-  MOCK_METHOD1(DocumentSizeUpdated, void(const pp::Size& size));
+  MOCK_METHOD1(ProposeDocumentLayout, void(const DocumentLayout& layout));
 };
 
 class PDFiumEngineTest : public PDFiumTestBase {
@@ -37,7 +49,7 @@ class PDFiumEngineTest : public PDFiumTestBase {
 
 TEST_F(PDFiumEngineTest, InitializeWithRectanglesMultiPagesPdf) {
   NiceMock<MockTestClient> client;
-  EXPECT_CALL(client, DocumentSizeUpdated(pp::Size(343, 1664)));
+  EXPECT_CALL(client, ProposeDocumentLayout(LayoutWithSize(343, 1664)));
 
   std::unique_ptr<PDFiumEngine> engine = InitializeEngine(
       &client, FILE_PATH_LITERAL("rectangles_multi_pages.pdf"));
@@ -55,8 +67,8 @@ TEST_F(PDFiumEngineTest, AppendBlankPagesWithFewerPages) {
   NiceMock<MockTestClient> client;
   {
     InSequence normal_then_append;
-    EXPECT_CALL(client, DocumentSizeUpdated(pp::Size(343, 1664)));
-    EXPECT_CALL(client, DocumentSizeUpdated(pp::Size(276, 1037)));
+    EXPECT_CALL(client, ProposeDocumentLayout(LayoutWithSize(343, 1664)));
+    EXPECT_CALL(client, ProposeDocumentLayout(LayoutWithSize(276, 1037)));
   }
 
   std::unique_ptr<PDFiumEngine> engine = InitializeEngine(
@@ -75,8 +87,8 @@ TEST_F(PDFiumEngineTest, AppendBlankPagesWithMorePages) {
   NiceMock<MockTestClient> client;
   {
     InSequence normal_then_append;
-    EXPECT_CALL(client, DocumentSizeUpdated(pp::Size(343, 1664)));
-    EXPECT_CALL(client, DocumentSizeUpdated(pp::Size(276, 2425)));
+    EXPECT_CALL(client, ProposeDocumentLayout(LayoutWithSize(343, 1664)));
+    EXPECT_CALL(client, ProposeDocumentLayout(LayoutWithSize(276, 2425)));
   }
 
   std::unique_ptr<PDFiumEngine> engine = InitializeEngine(

pdf/preview_mode_client.cc

@@ -7,11 +7,16 @@
 #include <stdint.h>
 
 #include "base/logging.h"
+#include "pdf/document_layout.h"
 
 namespace chrome_pdf {
 
 PreviewModeClient::PreviewModeClient(Client* client) : client_(client) {}
 
+void PreviewModeClient::ProposeDocumentLayout(const DocumentLayout& layout) {
+  NOTREACHED();
+}
+
 void PreviewModeClient::Invalidate(const pp::Rect& rect) {
   NOTREACHED();
 }

pdf/preview_mode_client.h

@@ -27,6 +27,7 @@ class PreviewModeClient : public PDFEngine::Client {
   ~PreviewModeClient() override {}
 
   // PDFEngine::Client implementation.
+  void ProposeDocumentLayout(const DocumentLayout& layout) override;
   void Invalidate(const pp::Rect& rect) override;
   void DidScroll(const pp::Point& point) override;
   void ScrollToX(int x_in_screen_coords) override;

pdf/test/test_client.cc

@@ -4,12 +4,18 @@
 
 #include "pdf/test/test_client.h"
 
+#include "pdf/document_layout.h"
+
 namespace chrome_pdf {
 
 TestClient::TestClient() = default;
 
 TestClient::~TestClient() = default;
 
+void TestClient::ProposeDocumentLayout(const DocumentLayout& layout) {
+  CHECK(engine());
+}
+
 bool TestClient::Confirm(const std::string& message) {
   return false;
 }

pdf/test/test_client.h

@@ -25,6 +25,7 @@ class TestClient : public PDFEngine::Client {
   void set_engine(PDFEngine* engine) { engine_ = engine; }
 
   // PDFEngine::Client:
+  void ProposeDocumentLayout(const DocumentLayout& layout) override;
   bool Confirm(const std::string& message) override;
   std::string Prompt(const std::string& question,
                      const std::string& default_answer) override;