PDF: Make DocumentLoader an interface.

To make it easier to write fake DocumentLoaders for testing. Split the
implementation off into DocumentLoaderImpl.

Change-Id: I44f16c757b8a9be133936c41c231a552a7e05f0a
Reviewed-on: https://chromium-review.googlesource.com/1013998
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: dsinclair <dsinclair@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553816}

pdf/BUILD.gn

@@ -36,8 +36,9 @@ if (enable_pdf) {
 
     sources = [
       "chunk_stream.h",
-      "document_loader.cc",
       "document_loader.h",
+      "document_loader_impl.cc",
+      "document_loader_impl.h",
       "draw_utils.cc",
       "draw_utils.h",
       "out_of_process_instance.cc",
@@ -103,7 +104,7 @@ if (enable_pdf) {
   test("pdf_unittests") {
     sources = [
       "chunk_stream_unittest.cc",
-      "document_loader_unittest.cc",
+      "document_loader_impl_unittest.cc",
       "pdf_transform_unittest.cc",
       "range_set_unittest.cc",
       "run_all_unittests.cc",

pdf/document_loader.h

@@ -11,10 +11,6 @@
 #include <memory>
 #include <string>
 
-#include "base/macros.h"
-#include "pdf/chunk_stream.h"
-#include "ppapi/utility/completion_callback_factory.h"
-
 namespace pp {
 class Instance;
 }
@@ -25,9 +21,6 @@ class URLLoaderWrapper;
 
 class DocumentLoader {
  public:
-  // Number was chosen in crbug.com/78264#c8
-  static constexpr uint32_t kDefaultRequestSize = 65536;
-
   class Client {
    public:
     virtual ~Client() = default;
@@ -48,84 +41,27 @@ class DocumentLoader {
     virtual void CancelBrowserDownload() = 0;
   };
 
-  explicit DocumentLoader(Client* client);
-  ~DocumentLoader();
+  virtual ~DocumentLoader() = default;
 
-  bool Init(std::unique_ptr<URLLoaderWrapper> loader, const std::string& url);
+  virtual bool Init(std::unique_ptr<URLLoaderWrapper> loader,
+                    const std::string& url) = 0;
 
   // Data access interface. Return true if successful.
-  bool GetBlock(uint32_t position, uint32_t size, void* buf) const;
+  virtual bool GetBlock(uint32_t position, uint32_t size, void* buf) const = 0;
 
   // Data availability interface. Return true if data is available.
-  bool IsDataAvailable(uint32_t position, uint32_t size) const;
+  virtual bool IsDataAvailable(uint32_t position, uint32_t size) const = 0;
 
   // Data request interface.
-  void RequestData(uint32_t position, uint32_t size);
+  virtual void RequestData(uint32_t position, uint32_t size) {}
 
-  bool IsDocumentComplete() const;
-  void SetDocumentSize(uint32_t size);
-  uint32_t GetDocumentSize() const;
-  uint32_t bytes_received() const { return bytes_received_; }
+  virtual bool IsDocumentComplete() const = 0;
+  virtual void SetDocumentSize(uint32_t size) {}
+  virtual uint32_t GetDocumentSize() const = 0;
+  virtual uint32_t BytesReceived() const = 0;
 
   // Clear pending requests from the queue.
-  void ClearPendingRequests();
-
-  // Exposed for unit tests.
-  void SetPartialLoadingEnabled(bool enabled);
-  bool is_partial_loader_active() const { return is_partial_loader_active_; }
-
- private:
-  using DataStream = ChunkStream<kDefaultRequestSize>;
-  struct Chunk {
-    Chunk();
-    ~Chunk();
-
-    void Clear();
-
-    uint32_t chunk_index = 0;
-    uint32_t data_size = 0;
-    std::unique_ptr<DataStream::ChunkData> chunk_data;
-  };
-
-  // Called by the completion callback of the document's URLLoader.
-  void DidOpenPartial(int32_t result);
-  // Call to read data from the document's URLLoader.
-  void ReadMore();
-  // Called by the completion callback of the document's URLLoader.
-  void DidRead(int32_t result);
-
-  bool ShouldCancelLoading() const;
-  void ContinueDownload();
-  // Called when we complete server request.
-  void ReadComplete();
-
-  bool SaveBuffer(char* input, uint32_t input_size);
-  void SaveChunkData();
-
-  uint32_t EndOfCurrentChunk() const;
-
-  Client* const client_;
-  std::string url_;
-  std::unique_ptr<URLLoaderWrapper> loader_;
-
-  pp::CompletionCallbackFactory<DocumentLoader> loader_factory_;
-
-  DataStream chunk_stream_;
-  bool partial_loading_enabled_ = true;
-  bool is_partial_loader_active_ = false;
-
-  static constexpr uint32_t kReadBufferSize = 256 * 1024;
-  char buffer_[kReadBufferSize];
-
-  // The current chunk DocumentLoader is working with.
-  Chunk chunk_;
-
-  // In units of Chunks.
-  RangeSet pending_requests_;
-
-  uint32_t bytes_received_ = 0;
-
-  DISALLOW_COPY_AND_ASSIGN(DocumentLoader);
+  virtual void ClearPendingRequests() {}
 };
 
 }  // namespace chrome_pdf

pdf/document_loader.cc → pdf/document_loader_impl.cc

@@ -2,13 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "pdf/document_loader.h"
+#include "pdf/document_loader_impl.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
-#include <memory>
 #include <utility>
 
 #include "base/logging.h"
@@ -53,23 +52,23 @@ bool IsValidContentType(const std::string& type) {
 
 }  // namespace
 
-DocumentLoader::Chunk::Chunk() = default;
+DocumentLoaderImpl::Chunk::Chunk() = default;
 
-DocumentLoader::Chunk::~Chunk() = default;
+DocumentLoaderImpl::Chunk::~Chunk() = default;
 
-void DocumentLoader::Chunk::Clear() {
+void DocumentLoaderImpl::Chunk::Clear() {
   chunk_index = 0;
   data_size = 0;
   chunk_data.reset();
 }
 
-DocumentLoader::DocumentLoader(Client* client)
+DocumentLoaderImpl::DocumentLoaderImpl(Client* client)
     : client_(client), loader_factory_(this) {}
 
-DocumentLoader::~DocumentLoader() = default;
+DocumentLoaderImpl::~DocumentLoaderImpl() = default;
 
-bool DocumentLoader::Init(std::unique_ptr<URLLoaderWrapper> loader,
-                          const std::string& url) {
+bool DocumentLoaderImpl::Init(std::unique_ptr<URLLoaderWrapper> loader,
+                              const std::string& url) {
   DCHECK(url_.empty());
   DCHECK(!loader_);
 
@@ -119,25 +118,29 @@ bool DocumentLoader::Init(std::unique_ptr<URLLoaderWrapper> loader,
   return true;
 }
 
-bool DocumentLoader::IsDocumentComplete() const {
+bool DocumentLoaderImpl::IsDocumentComplete() const {
   return chunk_stream_.IsComplete();
 }
 
-void DocumentLoader::SetDocumentSize(uint32_t size) {
+void DocumentLoaderImpl::SetDocumentSize(uint32_t size) {
   chunk_stream_.set_eof_pos(size);
 }
 
-uint32_t DocumentLoader::GetDocumentSize() const {
+uint32_t DocumentLoaderImpl::GetDocumentSize() const {
   return chunk_stream_.eof_pos();
 }
 
-void DocumentLoader::ClearPendingRequests() {
+uint32_t DocumentLoaderImpl::BytesReceived() const {
+  return bytes_received_;
+}
+
+void DocumentLoaderImpl::ClearPendingRequests() {
   pending_requests_.Clear();
 }
 
-bool DocumentLoader::GetBlock(uint32_t position,
-                              uint32_t size,
-                              void* buf) const {
+bool DocumentLoaderImpl::GetBlock(uint32_t position,
+                                  uint32_t size,
+                                  void* buf) const {
   base::CheckedNumeric<uint32_t> addition_result = position;
   addition_result += size;
   if (!addition_result.IsValid())
@@ -146,7 +149,8 @@ bool DocumentLoader::GetBlock(uint32_t position,
       gfx::Range(position, addition_result.ValueOrDie()), buf);
 }
 
-bool DocumentLoader::IsDataAvailable(uint32_t position, uint32_t size) const {
+bool DocumentLoaderImpl::IsDataAvailable(uint32_t position,
+                                         uint32_t size) const {
   base::CheckedNumeric<uint32_t> addition_result = position;
   addition_result += size;
   if (!addition_result.IsValid())
@@ -155,7 +159,7 @@ bool DocumentLoader::IsDataAvailable(uint32_t position, uint32_t size) const {
       gfx::Range(position, addition_result.ValueOrDie()));
 }
 
-void DocumentLoader::RequestData(uint32_t position, uint32_t size) {
+void DocumentLoaderImpl::RequestData(uint32_t position, uint32_t size) {
   if (size == 0 || IsDataAvailable(position, size))
     return;
 
@@ -190,14 +194,14 @@ void DocumentLoader::RequestData(uint32_t position, uint32_t size) {
   pending_requests_.Union(requested_chunks);
 }
 
-void DocumentLoader::SetPartialLoadingEnabled(bool enabled) {
+void DocumentLoaderImpl::SetPartialLoadingEnabled(bool enabled) {
   partial_loading_enabled_ = enabled;
   if (!enabled) {
     is_partial_loader_active_ = false;
   }
 }
 
-bool DocumentLoader::ShouldCancelLoading() const {
+bool DocumentLoaderImpl::ShouldCancelLoading() const {
   if (!loader_)
     return true;
   if (!partial_loading_enabled_ || pending_requests_.IsEmpty())
@@ -207,7 +211,7 @@ bool DocumentLoader::ShouldCancelLoading() const {
   return !pending_requests_.Intersects(current_range);
 }
 
-void DocumentLoader::ContinueDownload() {
+void DocumentLoaderImpl::ContinueDownload() {
   if (!ShouldCancelLoading())
     return ReadMore();
 
@@ -252,10 +256,10 @@ void DocumentLoader::ContinueDownload() {
 
   loader_->OpenRange(
       url_, url_, start, length,
-      loader_factory_.NewCallback(&DocumentLoader::DidOpenPartial));
+      loader_factory_.NewCallback(&DocumentLoaderImpl::DidOpenPartial));
 }
 
-void DocumentLoader::DidOpenPartial(int32_t result) {
+void DocumentLoaderImpl::DidOpenPartial(int32_t result) {
   if (result != PP_OK) {
     return ReadComplete();
   }
@@ -288,13 +292,13 @@ void DocumentLoader::DidOpenPartial(int32_t result) {
   return ContinueDownload();
 }
 
-void DocumentLoader::ReadMore() {
+void DocumentLoaderImpl::ReadMore() {
   loader_->ReadResponseBody(
       buffer_, sizeof(buffer_),
-      loader_factory_.NewCallback(&DocumentLoader::DidRead));
+      loader_factory_.NewCallback(&DocumentLoaderImpl::DidRead));
 }
 
-void DocumentLoader::DidRead(int32_t result) {
+void DocumentLoaderImpl::DidRead(int32_t result) {
   if (result < 0) {
     // An error occurred.
     // The renderer will detect that we're missing data and will display a
@@ -322,7 +326,7 @@ void DocumentLoader::DidRead(int32_t result) {
   return ContinueDownload();
 }
 
-bool DocumentLoader::SaveBuffer(char* input, uint32_t input_size) {
+bool DocumentLoaderImpl::SaveBuffer(char* input, uint32_t input_size) {
   const uint32_t document_size = GetDocumentSize();
   if (document_size != 0) {
     // If the HTTP server sends more data than expected, then truncate
@@ -368,17 +372,17 @@ bool DocumentLoader::SaveBuffer(char* input, uint32_t input_size) {
   return true;
 }
 
-void DocumentLoader::SaveChunkData() {
+void DocumentLoaderImpl::SaveChunkData() {
   chunk_stream_.SetChunkData(chunk_.chunk_index, std::move(chunk_.chunk_data));
   chunk_.data_size = 0;
   ++chunk_.chunk_index;
 }
 
-uint32_t DocumentLoader::EndOfCurrentChunk() const {
+uint32_t DocumentLoaderImpl::EndOfCurrentChunk() const {
   return chunk_.chunk_index * DataStream::kChunkSize + chunk_.data_size;
 }
 
-void DocumentLoader::ReadComplete() {
+void DocumentLoaderImpl::ReadComplete() {
   if (GetDocumentSize() != 0) {
     // If there is remaining data in |chunk_|, then save whatever can be saved.
     // e.g. In the underrun case.

pdf/document_loader_impl.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef PDF_DOCUMENT_LOADER_IMPL_H_
+#define PDF_DOCUMENT_LOADER_IMPL_H_
+
+#include <memory>
+#include <string>
+
+#include "base/macros.h"
+#include "pdf/chunk_stream.h"
+#include "pdf/document_loader.h"
+#include "ppapi/utility/completion_callback_factory.h"
+
+namespace chrome_pdf {
+
+class DocumentLoaderImpl : public DocumentLoader {
+ public:
+  // Number was chosen in https://crbug.com/78264#c8
+  static constexpr uint32_t kDefaultRequestSize = 65536;
+
+  explicit DocumentLoaderImpl(Client* client);
+  ~DocumentLoaderImpl() override;
+
+  // DocumentLoader:
+  bool Init(std::unique_ptr<URLLoaderWrapper> loader,
+            const std::string& url) override;
+  bool GetBlock(uint32_t position, uint32_t size, void* buf) const override;
+  bool IsDataAvailable(uint32_t position, uint32_t size) const override;
+  void RequestData(uint32_t position, uint32_t size) override;
+  bool IsDocumentComplete() const override;
+  void SetDocumentSize(uint32_t size) override;
+  uint32_t GetDocumentSize() const override;
+  uint32_t BytesReceived() const override;
+  void ClearPendingRequests() override;
+
+  // Exposed for unit tests.
+  void SetPartialLoadingEnabled(bool enabled);
+  bool is_partial_loader_active() const { return is_partial_loader_active_; }
+
+ private:
+  using DataStream = ChunkStream<kDefaultRequestSize>;
+  struct Chunk {
+    Chunk();
+    ~Chunk();
+
+    void Clear();
+
+    uint32_t chunk_index = 0;
+    uint32_t data_size = 0;
+    std::unique_ptr<DataStream::ChunkData> chunk_data;
+  };
+
+  // Called by the completion callback of the document's URLLoader.
+  void DidOpenPartial(int32_t result);
+
+  // Call to read data from the document's URLLoader.
+  void ReadMore();
+
+  // Called by the completion callback of the document's URLLoader.
+  void DidRead(int32_t result);
+
+  bool ShouldCancelLoading() const;
+  void ContinueDownload();
+
+  // Called when we complete server request.
+  void ReadComplete();
+
+  bool SaveBuffer(char* input, uint32_t input_size);
+  void SaveChunkData();
+
+  uint32_t EndOfCurrentChunk() const;
+
+  Client* const client_;
+  std::string url_;
+  std::unique_ptr<URLLoaderWrapper> loader_;
+
+  pp::CompletionCallbackFactory<DocumentLoaderImpl> loader_factory_;
+
+  DataStream chunk_stream_;
+  bool partial_loading_enabled_ = true;
+  bool is_partial_loader_active_ = false;
+
+  static constexpr uint32_t kReadBufferSize = 256 * 1024;
+  char buffer_[kReadBufferSize];
+
+  // The current chunk DocumentLoader is working with.
+  Chunk chunk_;
+
+  // In units of Chunks.
+  RangeSet pending_requests_;
+
+  uint32_t bytes_received_ = 0;
+
+  DISALLOW_COPY_AND_ASSIGN(DocumentLoaderImpl);
+};
+
+}  // namespace chrome_pdf
+
+#endif  // PDF_DOCUMENT_LOADER_IMPL_H_

pdf/pdfium/pdfium_engine.cc

@@ -30,6 +30,7 @@
 #include "gin/array_buffer.h"
 #include "gin/public/gin_embedders.h"
 #include "gin/public/isolate_holder.h"
+#include "pdf/document_loader_impl.h"
 #include "pdf/draw_utils.h"
 #include "pdf/pdf_transform.h"
 #include "pdf/pdfium/pdfium_api_string_buffer_adapter.h"
@@ -1226,7 +1227,7 @@ bool PDFiumEngine::HandleDocumentLoad(const pp::URLLoader& loader) {
       std::make_unique<URLLoaderWrapperImpl>(GetPluginInstance(), loader);
   loader_wrapper->SetResponseHeaders(headers_);
 
-  doc_loader_ = std::make_unique<DocumentLoader>(this);
+  doc_loader_ = std::make_unique<DocumentLoaderImpl>(this);
   if (doc_loader_->Init(std::move(loader_wrapper), url_)) {
     // request initial data.
     doc_loader_->RequestData(0, 1);
@@ -1318,7 +1319,7 @@ void PDFiumEngine::OnPendingRequestComplete() {
 }
 
 void PDFiumEngine::OnNewDataReceived() {
-  client_->DocumentLoadProgress(doc_loader_->bytes_received(),
+  client_->DocumentLoadProgress(doc_loader_->BytesReceived(),
                                 doc_loader_->GetDocumentSize());
 }
 
@@ -1388,7 +1389,7 @@ void PDFiumEngine::FinishLoadingDocument() {
     document_features.is_tagged = FPDFCatalog_IsTagged(doc_);
     document_features.form_type = static_cast<FormType>(FPDF_GetFormType(doc_));
     client_->DocumentLoadComplete(document_features,
-                                  doc_loader_->bytes_received());
+                                  doc_loader_->BytesReceived());
   }
 }