PDF: Fix potential bad indexes in find results.

Add a new FindTextData class to store the index to avoid all the size_t
to int casts.

BUG=416696

Review URL: https://codereview.chromium.org/691273003

Cr-Commit-Position: refs/heads/master@{#302532}

pdf/instance.cc

@@ -1286,6 +1286,7 @@ void Instance::NotifyNumberOfFindResultsChanged(int total, bool final_result) {
 }
 
 void Instance::NotifySelectedFindResultChanged(int current_find_index) {
+  DCHECK_GE(current_find_index, 0);
   SelectedFindResultChanged(current_find_index);
 }
 

pdf/out_of_process_instance.cc

@@ -906,6 +906,7 @@ void OutOfProcessInstance::NotifyNumberOfFindResultsChanged(int total,
 
 void OutOfProcessInstance::NotifySelectedFindResultChanged(
     int current_find_index) {
+  DCHECK_GE(current_find_index, 0);
   SelectedFindResultChanged(current_find_index);
 }
 

pdf/pdfium/pdfium_engine.cc

@@ -566,8 +566,6 @@ PDFiumEngine::PDFiumEngine(PDFEngine::Client* client)
       next_page_to_search_(-1),
       last_page_to_search_(-1),
       last_character_index_to_search_(-1),
-      current_find_index_(-1),
-      resume_find_index_(-1),
       permissions_(0),
       fpdf_availability_(NULL),
       next_timer_id_(0),
@@ -1686,10 +1684,17 @@ void PDFiumEngine::StartFind(const char* text, bool case_sensitive) {
     client_->NotifyNumberOfFindResultsChanged(find_results_.size(), true);
 
     // When searching is complete, resume finding at a particular index.
-    if (resume_find_index_ != -1) {
-      current_find_index_ = resume_find_index_;
-      resume_find_index_ = -1;
+    // Assuming the user has not clicked the find button in the meanwhile.
+    if (resume_find_index_.valid() && !current_find_index_.valid()) {
+      size_t resume_index = resume_find_index_.GetIndex();
+      if (resume_index >= find_results_.size()) {
+        // This might happen if the PDF has some dynamically generated text?
+        resume_index = 0;
+      }
+      current_find_index_.SetIndex(resume_index);
+      client_->NotifySelectedFindResultChanged(resume_index);
     }
+    resume_find_index_.Invalidate();
   } else {
     pp::CompletionCallback callback =
         find_factory_.NewCallback(&PDFiumEngine::ContinueFind);
@@ -1772,26 +1777,29 @@ void PDFiumEngine::SearchUsingICU(const base::string16& term,
 void PDFiumEngine::AddFindResult(const PDFiumRange& result) {
   // Figure out where to insert the new location, since we could have
   // started searching midway and now we wrapped.
-  size_t i;
+  size_t result_index;
   int page_index = result.page_index();
   int char_index = result.char_index();
-  for (i = 0; i < find_results_.size(); ++i) {
-    if (find_results_[i].page_index() > page_index ||
-        (find_results_[i].page_index() == page_index &&
-         find_results_[i].char_index() > char_index)) {
+  for (result_index = 0; result_index < find_results_.size(); ++result_index) {
+    if (find_results_[result_index].page_index() > page_index ||
+        (find_results_[result_index].page_index() == page_index &&
+         find_results_[result_index].char_index() > char_index)) {
       break;
     }
   }
-  find_results_.insert(find_results_.begin() + i, result);
+  find_results_.insert(find_results_.begin() + result_index, result);
   UpdateTickMarks();
 
-  if (current_find_index_ == -1 && resume_find_index_ == -1) {
-    // Select the first match.
+  if (current_find_index_.valid()) {
+    if (result_index <= current_find_index_.GetIndex()) {
+      // Update the current match index
+      size_t find_index = current_find_index_.IncrementIndex();
+      DCHECK_LT(find_index, find_results_.size());
+      client_->NotifySelectedFindResultChanged(current_find_index_.GetIndex());
+    }
+  } else if (!resume_find_index_.valid()) {
+    // Both indices are invalid. Select the first match.
     SelectFindResult(true);
-  } else if (static_cast<int>(i) <= current_find_index_) {
-    // Update the current match index
-    current_find_index_++;
-    client_->NotifySelectedFindResultChanged(current_find_index_);
   }
   client_->NotifyNumberOfFindResultsChanged(find_results_.size(), false);
 }
@@ -1805,34 +1813,40 @@ bool PDFiumEngine::SelectFindResult(bool forward) {
   SelectionChangeInvalidator selection_invalidator(this);
 
   // Move back/forward through the search locations we previously found.
-  if (forward) {
-    if (++current_find_index_ == static_cast<int>(find_results_.size()))
-      current_find_index_ = 0;
-  } else {
-    if (--current_find_index_ < 0) {
-      current_find_index_ = find_results_.size() - 1;
+  size_t new_index;
+  const size_t last_index = find_results_.size() - 1;
+  if (current_find_index_.valid()) {
+    size_t current_index = current_find_index_.GetIndex();
+    if (forward) {
+      new_index = (current_index >= last_index) ?  0 : current_index + 1;
+    } else {
+      new_index = (current_find_index_.GetIndex() == 0) ?
+          last_index : current_index - 1;
     }
+  } else {
+    new_index = forward ? 0 : last_index;
   }
+  current_find_index_.SetIndex(new_index);
 
   // Update the selection before telling the client to scroll, since it could
   // paint then.
   selection_.clear();
-  selection_.push_back(find_results_[current_find_index_]);
+  selection_.push_back(find_results_[current_find_index_.GetIndex()]);
 
   // If the result is not in view, scroll to it.
-  size_t i;
   pp::Rect bounding_rect;
   pp::Rect visible_rect = GetVisibleRect();
   // Use zoom of 1.0 since visible_rect is without zoom.
-  std::vector<pp::Rect> rects = find_results_[current_find_index_].
-      GetScreenRects(pp::Point(), 1.0, current_rotation_);
-  for (i = 0; i < rects.size(); ++i)
+  std::vector<pp::Rect> rects;
+  rects = find_results_[current_find_index_.GetIndex()].GetScreenRects(
+      pp::Point(), 1.0, current_rotation_);
+  for (size_t i = 0; i < rects.size(); ++i)
     bounding_rect = bounding_rect.Union(rects[i]);
   if (!visible_rect.Contains(bounding_rect)) {
     pp::Point center = bounding_rect.CenterPoint();
     // Make the page centered.
     int new_y = static_cast<int>(center.y() * current_zoom_) -
-                static_cast<int>(visible_rect.height() * current_zoom_ / 2);
+        static_cast<int>(visible_rect.height() * current_zoom_ / 2);
     if (new_y < 0)
       new_y = 0;
     client_->ScrollToY(new_y);
@@ -1840,15 +1854,14 @@ bool PDFiumEngine::SelectFindResult(bool forward) {
     // Only move horizontally if it's not visible.
     if (center.x() < visible_rect.x() || center.x() > visible_rect.right()) {
       int new_x = static_cast<int>(center.x()  * current_zoom_) -
-                  static_cast<int>(visible_rect.width() * current_zoom_ / 2);
+          static_cast<int>(visible_rect.width() * current_zoom_ / 2);
       if (new_x < 0)
         new_x = 0;
       client_->ScrollToX(new_x);
     }
   }
 
-  client_->NotifySelectedFindResultChanged(current_find_index_);
-
+  client_->NotifySelectedFindResultChanged(current_find_index_.GetIndex());
   return true;
 }
 
@@ -1861,7 +1874,7 @@ void PDFiumEngine::StopFind() {
   next_page_to_search_ = -1;
   last_page_to_search_ = -1;
   last_character_index_to_search_ = -1;
-  current_find_index_ = -1;
+  current_find_index_.Invalidate();
   current_find_text_.clear();
   UpdateTickMarks();
   find_factory_.CancelAll();
@@ -1893,30 +1906,12 @@ void PDFiumEngine::ZoomUpdated(double new_zoom_level) {
 
 void PDFiumEngine::RotateClockwise() {
   current_rotation_ = (current_rotation_ + 1) % 4;
-
-  // Store the current find index so that we can resume finding at that
-  // particular index after we have recomputed the find results.
-  std::string current_find_text = current_find_text_;
-  resume_find_index_ = current_find_index_;
-
-  InvalidateAllPages();
-
-  if (!current_find_text.empty())
-    StartFind(current_find_text.c_str(), false);
+  RotateInternal();
 }
 
 void PDFiumEngine::RotateCounterclockwise() {
   current_rotation_ = (current_rotation_ - 1) % 4;
-
-  // Store the current find index so that we can resume finding at that
-  // particular index after we have recomputed the find results.
-  std::string current_find_text = current_find_text_;
-  resume_find_index_ = current_find_index_;
-
-  InvalidateAllPages();
-
-  if (!current_find_text.empty())
-    StartFind(current_find_text.c_str(), false);
+  RotateInternal();
 }
 
 void PDFiumEngine::InvalidateAllPages() {
@@ -2809,6 +2804,32 @@ bool PDFiumEngine::MouseDownState::Matches(
   return false;
 }
 
+PDFiumEngine::FindTextIndex::FindTextIndex()
+    : valid_(false), index_(0) {
+}
+
+PDFiumEngine::FindTextIndex::~FindTextIndex() {
+}
+
+void PDFiumEngine::FindTextIndex::Invalidate() {
+  valid_ = false;
+}
+
+size_t PDFiumEngine::FindTextIndex::GetIndex() const {
+  DCHECK(valid_);
+  return index_;
+}
+
+void PDFiumEngine::FindTextIndex::SetIndex(size_t index) {
+  valid_ = true;
+  index_ = index;
+}
+
+size_t PDFiumEngine::FindTextIndex::IncrementIndex() {
+  DCHECK(valid_);
+  return ++index_;
+}
+
 void PDFiumEngine::DeviceToPage(int page_index,
                                 float device_x,
                                 float device_y,
@@ -2988,6 +3009,24 @@ void PDFiumEngine::OnSelectionChanged() {
     pp::PDF::SetSelectedText(GetPluginInstance(), GetSelectedText().c_str());
 }
 
+void PDFiumEngine::RotateInternal() {
+  // Store the current find index so that we can resume finding at that
+  // particular index after we have recomputed the find results.
+  std::string current_find_text = current_find_text_;
+  if (current_find_index_.valid())
+    resume_find_index_.SetIndex(current_find_index_.GetIndex());
+  else
+    resume_find_index_.Invalidate();
+
+  InvalidateAllPages();
+
+  if (!current_find_text.empty()) {
+    // Clear the UI.
+    client_->NotifyNumberOfFindResultsChanged(0, false);
+    StartFind(current_find_text.c_str(), false);
+  }
+}
+
 void PDFiumEngine::Form_Invalidate(FPDF_FORMFILLINFO* param,
                                    FPDF_PAGE page,
                                    double left,

pdf/pdfium/pdfium_engine.h

@@ -151,6 +151,26 @@ class PDFiumEngine : public PDFEngine,
     DISALLOW_COPY_AND_ASSIGN(MouseDownState);
   };
 
+  // Used to store the state of a text search.
+  class FindTextIndex {
+   public:
+    FindTextIndex();
+    ~FindTextIndex();
+
+    bool valid() const { return valid_; }
+    void Invalidate();
+
+    size_t GetIndex() const;
+    void SetIndex(size_t index);
+    size_t IncrementIndex();
+
+   private:
+    bool valid_;  // Whether |index_| is valid or not.
+    size_t index_;  // The current search result, 0-based.
+
+    DISALLOW_COPY_AND_ASSIGN(FindTextIndex);
+  };
+
   friend class SelectionChangeInvalidator;
 
   struct FileAvail : public FX_FILEAVAIL {
@@ -391,6 +411,9 @@ class PDFiumEngine : public PDFEngine,
   // Called when the selection changes.
   void OnSelectionChanged();
 
+  // Common code shared by RotateClockwise() and RotateCounterclockwise().
+  void RotateInternal();
+
   // FPDF_FORMFILLINFO callbacks.
   static void Form_Invalidate(FPDF_FORMFILLINFO* param,
                               FPDF_PAGE page,
@@ -540,9 +563,9 @@ class PDFiumEngine : public PDFEngine,
   int last_page_to_search_;
   int last_character_index_to_search_;  // -1 if search until end of page.
   // Which result the user has currently selected.
-  int current_find_index_;
+  FindTextIndex current_find_index_;
   // Where to resume searching.
-  int resume_find_index_;
+  FindTextIndex resume_find_index_;
 
   // Permissions bitfield.
   unsigned long permissions_;