Improve CPDF_ReadVaildator

Merge CPDF_ReadValidator::IsXXXAvailable and CPDF_ReadValidator::SheduleXXXDowload 
methods into one CheckXXXAndRequestIfUnavailable.

Change-Id: Ib108d5bc3589df17269c841b0024dd4690909304
Reviewed-on: https://pdfium-review.googlesource.com/13730
Reviewed-by: dsinclair <dsinclair@chromium.org>
Commit-Queue: Art Snake <art-snake@yandex-team.ru>

6 files changed, 48 insertions, 53 deletions

diff --git a/core/fpdfapi/parser/cpdf_data_avail.cpp b/core/fpdfapi/parser/cpdf_data_avail.cpp
index 8b378aab0e..b2415915c4 100644
--- a/core/fpdfapi/parser/cpdf_data_avail.cpp
+++ b/core/fpdfapi/parser/cpdf_data_avail.cpp
@@ -322,11 +322,10 @@ bool CPDF_DataAvail::CheckPageStatus() {
 }
 
 bool CPDF_DataAvail::LoadAllFile() {
-  if (GetValidator()->IsWholeFileAvailable()) {
+  if (GetValidator()->CheckWholeFileAndRequestIfUnavailable()) {
     m_docStatus = PDF_DATAAVAIL_DONE;
     return true;
   }
-  GetValidator()->ScheduleDownloadWholeFile();
   return false;
 }
 
@@ -590,18 +589,17 @@ bool CPDF_DataAvail::CheckFirstPage() {
     dwEnd = (uint32_t)m_dwFileLen;
 
   const FX_FILESIZE start_pos = m_dwFileLen > 1024 ? 1024 : m_dwFileLen;
-  const uint32_t data_size = dwEnd > 1024 ? dwEnd - 1024 : 0;
-  if (!GetValidator()->IsDataRangeAvailable(start_pos, data_size)) {
-    GetValidator()->ScheduleDataDownload(start_pos, data_size);
+  const size_t data_size = dwEnd > 1024 ? static_cast<size_t>(dwEnd - 1024) : 0;
+  if (!GetValidator()->CheckDataRangeAndRequestIfUnavailable(start_pos,
+                                                             data_size))
     return false;
-  }
 
   m_docStatus =
       m_bSupportHintTable ? PDF_DATAAVAIL_HINTTABLE : PDF_DATAAVAIL_DONE;
   return true;
 }
 
-bool CPDF_DataAvail::IsDataAvail(FX_FILESIZE offset, uint32_t size) {
+bool CPDF_DataAvail::IsDataAvail(FX_FILESIZE offset, size_t size) {
   if (offset < 0 || offset > m_dwFileLen)
     return true;
 
@@ -613,10 +611,9 @@ bool CPDF_DataAvail::IsDataAvail(FX_FILESIZE offset, uint32_t size) {
   else
     size += 512;
 
-  if (!GetValidator()->IsDataRangeAvailable(offset, size)) {
-    GetValidator()->ScheduleDataDownload(offset, size);
+  if (!GetValidator()->CheckDataRangeAndRequestIfUnavailable(offset, size))
     return false;
-  }
+
   return true;
 }
 
@@ -1207,17 +1204,14 @@ CPDF_DataAvail::DocAvailStatus CPDF_DataAvail::CheckLinearizedData() {
     return DataError;
 
   if (!m_bMainXRefLoadTried) {
-    FX_SAFE_UINT32 data_size = m_dwFileLen;
+    FX_SAFE_SIZE_T data_size = m_dwFileLen;
     data_size -= m_pLinearized->GetLastXRefOffset();
     if (!data_size.IsValid())
       return DataError;
 
-    if (!GetValidator()->IsDataRangeAvailable(
-            m_pLinearized->GetLastXRefOffset(), data_size.ValueOrDie())) {
-      GetValidator()->ScheduleDataDownload(m_pLinearized->GetLastXRefOffset(),
-                                           data_size.ValueOrDie());
+    if (!GetValidator()->CheckDataRangeAndRequestIfUnavailable(
+            m_pLinearized->GetLastXRefOffset(), data_size.ValueOrDie()))
       return DataNotAvailable;
-    }
 
     CPDF_Parser::Error eRet =
         m_pDocument->GetParser()->LoadLinearizedMainXRefTable();
diff --git a/core/fpdfapi/parser/cpdf_data_avail.h b/core/fpdfapi/parser/cpdf_data_avail.h
index ab7f045fec..24b5b7f357 100644
--- a/core/fpdfapi/parser/cpdf_data_avail.h
+++ b/core/fpdfapi/parser/cpdf_data_avail.h
@@ -84,13 +84,13 @@ class CPDF_DataAvail final {
   class FileAvail {
    public:
     virtual ~FileAvail();
-    virtual bool IsDataAvail(FX_FILESIZE offset, uint32_t size) = 0;
+    virtual bool IsDataAvail(FX_FILESIZE offset, size_t size) = 0;
   };
 
   class DownloadHints {
    public:
     virtual ~DownloadHints();
-    virtual void AddSegment(FX_FILESIZE offset, uint32_t size) = 0;
+    virtual void AddSegment(FX_FILESIZE offset, size_t size) = 0;
   };
 
   CPDF_DataAvail(FileAvail* pFileAvail,
@@ -98,7 +98,7 @@ class CPDF_DataAvail final {
                  bool bSupportHintTable);
   ~CPDF_DataAvail();
 
-  bool IsDataAvail(FX_FILESIZE offset, uint32_t size);
+  bool IsDataAvail(FX_FILESIZE offset, size_t size);
   DocAvailStatus IsDocAvail(DownloadHints* pHints);
   void SetDocument(CPDF_Document* pDoc);
   DocAvailStatus IsPageAvail(uint32_t dwPage, DownloadHints* pHints);
diff --git a/core/fpdfapi/parser/cpdf_read_validator.cpp b/core/fpdfapi/parser/cpdf_read_validator.cpp
index bf086008ca..0f7eaa70a2 100644
--- a/core/fpdfapi/parser/cpdf_read_validator.cpp
+++ b/core/fpdfapi/parser/cpdf_read_validator.cpp
@@ -59,22 +59,18 @@ void CPDF_ReadValidator::ResetErrors() {
 bool CPDF_ReadValidator::ReadBlock(void* buffer,
                                    FX_FILESIZE offset,
                                    size_t size) {
-  // correct values checks:
-  if (!pdfium::base::IsValueInRangeForNumericType<uint32_t>(size))
-    return false;
-
   FX_SAFE_FILESIZE end_offset = offset;
   end_offset += size;
   if (!end_offset.IsValid() || end_offset.ValueOrDie() > GetSize())
     return false;
 
-  if (!file_avail_ ||
-      file_avail_->IsDataAvail(offset, static_cast<uint32_t>(size))) {
-    if (file_read_->ReadBlock(buffer, offset, size))
-      return true;
-    read_error_ = true;
-  }
-  has_unavailable_data_ = true;
+  if (!CheckDataRangeAndRequestIfUnavailable(offset, size))
+    return false;
+
+  if (file_read_->ReadBlock(buffer, offset, size))
+    return true;
+
+  read_error_ = true;
   ScheduleDownload(offset, size);
   return false;
 }
@@ -84,13 +80,7 @@ FX_FILESIZE CPDF_ReadValidator::GetSize() {
 }
 
 void CPDF_ReadValidator::ScheduleDownload(FX_FILESIZE offset, size_t size) {
-  const FX_SAFE_UINT32 safe_size = size;
-  if (safe_size.IsValid())
-    ScheduleDataDownload(offset, safe_size.ValueOrDie());
-}
-
-void CPDF_ReadValidator::ScheduleDataDownload(FX_FILESIZE offset,
-                                              uint32_t size) {
+  has_unavailable_data_ = true;
   if (!hints_ || size == 0)
     return;
 
@@ -104,7 +94,7 @@ void CPDF_ReadValidator::ScheduleDataDownload(FX_FILESIZE offset,
   end_segment_offset =
       std::min(GetSize(), AlignUp(end_segment_offset.ValueOrDie()));
 
-  FX_SAFE_UINT32 segment_size = end_segment_offset;
+  FX_SAFE_SIZE_T segment_size = end_segment_offset;
   segment_size -= start_segment_offset;
   if (!segment_size.IsValid()) {
     NOTREACHED();
@@ -113,19 +103,30 @@ void CPDF_ReadValidator::ScheduleDataDownload(FX_FILESIZE offset,
   hints_->AddSegment(start_segment_offset, segment_size.ValueOrDie());
 }
 
-void CPDF_ReadValidator::ScheduleDownloadWholeFile() {
-  const FX_SAFE_UINT32 safe_size = GetSize();
-  if (safe_size.IsValid())
-    ScheduleDataDownload(0, safe_size.ValueOrDie());
-}
-
 bool CPDF_ReadValidator::IsDataRangeAvailable(FX_FILESIZE offset,
-                                              uint32_t size) const {
+                                              size_t size) const {
   return !file_avail_ || file_avail_->IsDataAvail(offset, size);
 }
 
 bool CPDF_ReadValidator::IsWholeFileAvailable() {
-  const FX_SAFE_UINT32 safe_size = GetSize();
+  const FX_SAFE_SIZE_T safe_size = GetSize();
   return safe_size.IsValid() ? IsDataRangeAvailable(0, safe_size.ValueOrDie())
                              : false;
 }
+
+bool CPDF_ReadValidator::CheckDataRangeAndRequestIfUnavailable(
+    FX_FILESIZE offset,
+    size_t size) {
+  if (IsDataRangeAvailable(offset, size))
+    return true;
+
+  ScheduleDownload(offset, size);
+  return false;
+}
+
+bool CPDF_ReadValidator::CheckWholeFileAndRequestIfUnavailable() {
+  const FX_SAFE_SIZE_T safe_size = GetSize();
+  return safe_size.IsValid()
+             ? CheckDataRangeAndRequestIfUnavailable(0, safe_size.ValueOrDie())
+             : false;
+}
diff --git a/core/fpdfapi/parser/cpdf_read_validator.h b/core/fpdfapi/parser/cpdf_read_validator.h
index 0bbf6ab924..0fb15b20f8 100644
--- a/core/fpdfapi/parser/cpdf_read_validator.h
+++ b/core/fpdfapi/parser/cpdf_read_validator.h
@@ -37,11 +37,10 @@ class CPDF_ReadValidator : public IFX_SeekableReadStream {
 
   void ResetErrors();
 
-  bool IsDataRangeAvailable(FX_FILESIZE offset, uint32_t size) const;
   bool IsWholeFileAvailable();
 
-  void ScheduleDataDownload(FX_FILESIZE offset, uint32_t size);
-  void ScheduleDownloadWholeFile();
+  bool CheckDataRangeAndRequestIfUnavailable(FX_FILESIZE offset, size_t size);
+  bool CheckWholeFileAndRequestIfUnavailable();
 
   // IFX_SeekableReadStream overrides:
   bool ReadBlock(void* buffer, FX_FILESIZE offset, size_t size) override;
@@ -54,6 +53,7 @@ class CPDF_ReadValidator : public IFX_SeekableReadStream {
 
  private:
   void ScheduleDownload(FX_FILESIZE offset, size_t size);
+  bool IsDataRangeAvailable(FX_FILESIZE offset, size_t size) const;
 
   RetainPtr<IFX_SeekableReadStream> file_read_;
   UnownedPtr<CPDF_DataAvail::FileAvail> file_avail_;
diff --git a/core/fpdfapi/parser/cpdf_read_validator_unittest.cpp b/core/fpdfapi/parser/cpdf_read_validator_unittest.cpp
index 308704f762..13ace078d3 100644
--- a/core/fpdfapi/parser/cpdf_read_validator_unittest.cpp
+++ b/core/fpdfapi/parser/cpdf_read_validator_unittest.cpp
@@ -26,7 +26,7 @@ class MockFileAvail : public CPDF_DataAvail::FileAvail {
   MockFileAvail() : available_range_(0, 0) {}
   ~MockFileAvail() override {}
 
-  bool IsDataAvail(FX_FILESIZE offset, uint32_t size) override {
+  bool IsDataAvail(FX_FILESIZE offset, size_t size) override {
     return available_range_.first <= offset &&
            available_range_.second >= static_cast<FX_FILESIZE>(offset + size);
   }
@@ -48,7 +48,7 @@ class MockDownloadHints : public CPDF_DataAvail::DownloadHints {
   MockDownloadHints() : last_requested_range_(0, 0) {}
   ~MockDownloadHints() override {}
 
-  void AddSegment(FX_FILESIZE offset, uint32_t size) override {
+  void AddSegment(FX_FILESIZE offset, size_t size) override {
     last_requested_range_.first = offset;
     last_requested_range_.second = offset + size;
   }
diff --git a/fpdfsdk/fpdf_dataavail.cpp b/fpdfsdk/fpdf_dataavail.cpp
index 3e1f713477..0a74c9c245 100644
--- a/fpdfsdk/fpdf_dataavail.cpp
+++ b/fpdfsdk/fpdf_dataavail.cpp
@@ -51,7 +51,7 @@ class FPDF_FileAvailContext : public CPDF_DataAvail::FileAvail {
   void Set(FX_FILEAVAIL* pfileAvail) { m_pfileAvail = pfileAvail; }
 
   // CPDF_DataAvail::FileAvail:
-  bool IsDataAvail(FX_FILESIZE offset, uint32_t size) override {
+  bool IsDataAvail(FX_FILESIZE offset, size_t size) override {
     return !!m_pfileAvail->IsDataAvail(m_pfileAvail, offset, size);
   }
 
@@ -91,7 +91,7 @@ class FPDF_DownloadHintsContext : public CPDF_DataAvail::DownloadHints {
 
  public:
   // IFX_DownloadHints
-  void AddSegment(FX_FILESIZE offset, uint32_t size) override {
+  void AddSegment(FX_FILESIZE offset, size_t size) override {
     if (m_pDownloadHints)
       m_pDownloadHints->AddSegment(m_pDownloadHints, offset, size);
   }