Fix some nits in chrome_pdf::DocumentLoader.

Add helper functions to make code more readable as well.

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

pdf/document_loader.cc

@@ -98,16 +98,15 @@ bool DocumentLoader::Init(std::unique_ptr<URLLoaderWrapper> loader,
   url_ = url;
   loader_ = std::move(loader);
 
-  if (!loader_->IsContentEncoded()) {
-    chunk_stream_.set_eof_pos(std::max(0, loader_->GetContentLength()));
-  }
+  if (!loader_->IsContentEncoded())
+    SetDocumentSize(std::max(0, loader_->GetContentLength()));
+
   int64_t bytes_received = 0;
   int64_t total_bytes_to_be_received = 0;
-  if (!chunk_stream_.eof_pos() &&
+  if (GetDocumentSize() == 0 &&
       loader_->GetDownloadProgress(&bytes_received,
                                    &total_bytes_to_be_received)) {
-    chunk_stream_.set_eof_pos(
-        std::max(0, static_cast<int>(total_bytes_to_be_received)));
+    SetDocumentSize(std::max(0, static_cast<int>(total_bytes_to_be_received)));
   }
 
   SetPartialLoadingEnabled(
@@ -124,6 +123,10 @@ bool DocumentLoader::IsDocumentComplete() const {
   return chunk_stream_.IsComplete();
 }
 
+void DocumentLoader::SetDocumentSize(uint32_t size) {
+  chunk_stream_.set_eof_pos(size);
+}
+
 uint32_t DocumentLoader::GetDocumentSize() const {
   return chunk_stream_.eof_pos();
 }
@@ -153,22 +156,22 @@ bool DocumentLoader::IsDataAvailable(uint32_t position, uint32_t size) const {
 }
 
 void DocumentLoader::RequestData(uint32_t position, uint32_t size) {
-  if (!size || IsDataAvailable(position, size)) {
+  if (size == 0 || IsDataAvailable(position, size))
     return;
-  }
-  {
-    // Check integer overflow.
+
+  const uint32_t document_size = GetDocumentSize();
+  if (document_size != 0) {
+    // Check for integer overflow.
     base::CheckedNumeric<uint32_t> addition_result = position;
     addition_result += size;
     if (!addition_result.IsValid())
       return;
+
+    if (addition_result.ValueOrDie() > document_size)
+      return;
   }
 
-  if (GetDocumentSize() && (position + size > GetDocumentSize())) {
-    return;
-  }
-
-  // We have some artefact request from
+  // We have some artifact request from
   // PDFiumEngine::OnDocumentComplete() -> FPDFAvail_IsPageAvail after
   // document is complete.
   // We need this fix in PDFIum. Adding this as a work around.
@@ -207,9 +210,10 @@ bool DocumentLoader::ShouldCancelLoading() const {
 void DocumentLoader::ContinueDownload() {
   if (!ShouldCancelLoading())
     return ReadMore();
+
   DCHECK(partial_loading_enabled_);
   DCHECK(!IsDocumentComplete());
-  DCHECK(GetDocumentSize());
+  DCHECK_GT(GetDocumentSize(), 0U);
 
   const uint32_t range_start =
       pending_requests_.IsEmpty() ? 0 : pending_requests_.First().start();
@@ -241,7 +245,7 @@ void DocumentLoader::ContinueDownload() {
 
   const uint32_t start = next_request.start() * DataStream::kChunkSize;
   const uint32_t length =
-      std::min(chunk_stream_.eof_pos() - start,
+      std::min(GetDocumentSize() - start,
                next_request.length() * DataStream::kChunkSize);
 
   loader_ = client_->CreateURLLoader();
@@ -319,27 +323,26 @@ void DocumentLoader::DidRead(int32_t result) {
 }
 
 bool DocumentLoader::SaveChunkData(char* input, uint32_t input_size) {
-  count_of_bytes_received_ += input_size;
+  bytes_received_ += input_size;
   bool chunk_saved = false;
   bool loading_pending_request = pending_requests_.Contains(chunk_.chunk_index);
   while (input_size > 0) {
-    if (chunk_.data_size == 0) {
+    if (chunk_.data_size == 0)
       chunk_.chunk_data = std::make_unique<DataStream::ChunkData>();
-    }
+
     const uint32_t new_chunk_data_len =
         std::min(DataStream::kChunkSize - chunk_.data_size, input_size);
     memcpy(chunk_.chunk_data->data() + chunk_.data_size, input,
            new_chunk_data_len);
     chunk_.data_size += new_chunk_data_len;
     if (chunk_.data_size == DataStream::kChunkSize ||
-        chunk_stream_.eof_pos() ==
-            chunk_.chunk_index * DataStream::kChunkSize + chunk_.data_size) {
+        GetDocumentSize() == EndOfCurrentChunk()) {
       chunk_stream_.SetChunkData(chunk_.chunk_index,
                                  std::move(chunk_.chunk_data));
       pending_requests_.Subtract(
           gfx::Range(chunk_.chunk_index, chunk_.chunk_index + 1));
       chunk_.data_size = 0;
-      ++(chunk_.chunk_index);
+      ++chunk_.chunk_index;
       chunk_saved = true;
     }
 
@@ -362,17 +365,20 @@ bool DocumentLoader::SaveChunkData(char* input, uint32_t input_size) {
   return true;
 }
 
+uint32_t DocumentLoader::EndOfCurrentChunk() const {
+  return chunk_.chunk_index * DataStream::kChunkSize + chunk_.data_size;
+}
+
 void DocumentLoader::ReadComplete() {
-  if (!GetDocumentSize()) {
-    uint32_t eof =
-        chunk_.chunk_index * DataStream::kChunkSize + chunk_.data_size;
+  if (GetDocumentSize() == 0) {
+    uint32_t eof = EndOfCurrentChunk();
     if (!chunk_stream_.filled_chunks().IsEmpty()) {
       eof = std::max(
           chunk_stream_.filled_chunks().Last().end() * DataStream::kChunkSize,
           eof);
     }
-    chunk_stream_.set_eof_pos(eof);
-    if (eof == chunk_.chunk_index * DataStream::kChunkSize + chunk_.data_size) {
+    SetDocumentSize(eof);
+    if (eof == EndOfCurrentChunk()) {
       chunk_stream_.SetChunkData(chunk_.chunk_index,
                                  std::move(chunk_.chunk_data));
     }

pdf/document_loader.h

@@ -11,6 +11,7 @@
 #include <memory>
 #include <string>
 
+#include "base/macros.h"
 #include "pdf/chunk_stream.h"
 #include "ppapi/utility/completion_callback_factory.h"
 
@@ -62,8 +63,9 @@ class DocumentLoader {
   void RequestData(uint32_t position, uint32_t size);
 
   bool IsDocumentComplete() const;
+  void SetDocumentSize(uint32_t size);
   uint32_t GetDocumentSize() const;
-  uint32_t count_of_bytes_received() const { return count_of_bytes_received_; }
+  uint32_t bytes_received() const { return bytes_received_; }
 
   // Clear pending requests from the queue.
   void ClearPendingRequests();
@@ -99,6 +101,8 @@ class DocumentLoader {
 
   bool SaveChunkData(char* input, uint32_t input_size);
 
+  uint32_t EndOfCurrentChunk() const;
+
   Client* const client_;
   std::string url_;
   std::unique_ptr<URLLoaderWrapper> loader_;
@@ -112,9 +116,15 @@ class DocumentLoader {
   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 count_of_bytes_received_ = 0;
+
+  uint32_t bytes_received_ = 0;
+
+  DISALLOW_COPY_AND_ASSIGN(DocumentLoader);
 };
 
 }  // namespace chrome_pdf

pdf/pdfium/pdfium_engine.cc

@@ -1309,7 +1309,7 @@ void PDFiumEngine::OnPendingRequestComplete() {
 }
 
 void PDFiumEngine::OnNewDataReceived() {
-  client_->DocumentLoadProgress(doc_loader_->count_of_bytes_received(),
+  client_->DocumentLoadProgress(doc_loader_->bytes_received(),
                                 doc_loader_->GetDocumentSize());
 }