PDF Viewer - Search results don't work after rotating pdf

pdf/pdfium/pdfium_engine.cc

 // Copyright (c) 2012 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.
 
 #include "pdf/pdfium/pdfium_engine.h"
 
 #include <math.h>
 
 #include "base/json/json_writer.h"
 #include "base/logging.h"
@@ skipping 1611 matching lines @@
       // Only one page but didn't start midway.
       ((pages_.size() == 1 && last_character_index_to_search_ == -1) ||
        // Started midway, but only 1 page and we already looped around.
        (pages_.size() == 1 && !first_search) ||
        // Started midway, and we've just looped around.
        (pages_.size() > 1 && current_page == next_page_to_search_));
 
   if (end_of_search) {
     // Send the final notification.
     client_->NotifyNumberOfFindResultsChanged(find_results_.size(), true);
+
+    // Restore find result state when find is started after rotation.
+    if (find_result_state_.find_index_ != -1) {

[raymes, 2014/09/24 03:57:50]

It doesn't look like we need a new struct, it looks like we can just store the
index to resume the find at?

// When searching is complete, resume finding at a particular index.
if (resume_find_index_ != -1) {
  current_find_index_ = resume_find_index_ - 1;
  SelectFindResult(true);
  resume_find_index_ = -1;
}

[Nikhil, 2014/09/29 08:22:11]

Thanks, I've modified the code accordingly. Please see.

Also, I wanted to remove the call to SelectFindResult() to match Adobe Reader's
behavior but it looks like it is required. The way find operation is implemented
is that whenever we start a find, the first index gets highlighted. This
behavior is not desirable in the given use case and so I maintained the call to
SelectFindResult().

The call to SelectFindResult() causes a slight jitter on some pages, depending
on the content, because of the combination of - 

[*] Rotation
[*] First index is highlighted
[*] Desired index is highlighted

Please take a look and let me know WDYT.

+      current_find_index_ = find_result_state_.find_index_ - 1;
+      SelectFindResult(true);
+      find_result_state_ = FindResultState();
+    }
   } else {
     pp::CompletionCallback callback =
         find_factory_.NewCallback(&PDFiumEngine::ContinueFind);
     pp::Module::Get()->core()->CallOnMainThread(
         0, callback, case_sensitive ? 1 : 0);
   }
 }
 
 void PDFiumEngine::SearchUsingPDFium(const base::string16& term,
                                      bool case_sensitive,
@@ skipping 183 matching lines @@
   CancelPaints();
 
   current_zoom_ = new_zoom_level;
 
   CalculateVisiblePages();
   UpdateTickMarks();
 }
 
 void PDFiumEngine::RotateClockwise() {
   current_rotation_ = (current_rotation_ + 1) % 4;
+  find_result_state_ = FindResultState(current_find_text_, current_find_index_);
+
   InvalidateAllPages();
+
+  // Restore find state if needed.

[raymes, 2014/09/24 03:57:50]

// Store the current find index so that we can resume finding at that 
// particular index after we have recomputed the find results.

[Nikhil, 2014/09/29 08:22:11]

Done.

+  if (!find_result_state_.find_text_.empty())
+    StartFind(find_result_state_.find_text_.c_str(), false);

[raymes, 2014/09/22 05:16:45]

I don't fully understand the find stuff but can we not modify StartFind and
instead to change this code to do:

StartFind(...)
current_find_index_ = ...
SelectFindResult(true)

That way you don't need to add a new member variable or class.

[Nikhil, 2014/09/23 12:18:08]

The problem that I faced earlier with this code flow was that StartFind() is
async and it takes time to complete search. So, I needed a way to store find
index before which rotation happened and update current find index to it once
find operation completes. I introduced new class and member variable for this
purpose.

Updating current find index (once find finishes) to find index prior to rotation
is required to maintain consistency. For example :

[*] Search for text and go to 2nd search item.
[*] Rotate pdf document.
[*] Sarch for previous/next item.
[*] Now we should go to 1st/3rd search item.

Adobe Reader follows the above behavior. 

Please let me know if you see foresee issues with this approach or if there
could be a better way to achieve this.

A few links that might be useful -

[*] Start find :
https://code.google.com/p/chromium/codesearch#chromium/src/pdf/pdfium/pdfium_...

[*] Update find results :
https://code.google.com/p/chromium/codesearch#chromium/src/pdf/pdfium/pdfium_...

[*] End of find :
https://code.google.com/p/chromium/codesearch#chromium/src/pdf/pdfium/pdfium_...

[raymes, 2014/09/24 03:57:50]

Ah I see how StartFind is asynchronous now, thanks for explaining.

[Nikhil, 2014/09/29 08:22:11]

Acknowledged.

 }
 
 void PDFiumEngine::RotateCounterclockwise() {
   current_rotation_ = (current_rotation_ - 1) % 4;
+  find_result_state_ = FindResultState(current_find_text_, current_find_index_);
+
   InvalidateAllPages();
+
+  // Restore find state if needed.
+  if (!find_result_state_.find_text_.empty())
+    StartFind(find_result_state_.find_text_.c_str(), false);
 }
 
 void PDFiumEngine::InvalidateAllPages() {
   selection_.clear();
   find_results_.clear();

[raymes, 2014/09/24 03:57:50]

These two lines and UpdateTickMarks() are already inside StopFind(). 

[Nikhil, 2014/09/29 08:22:11]

Done.

 
+  StopFind();

[raymes, 2014/09/24 03:57:50]

You might want to move this after CancelPaints() to ensure that it doesn't
interfere with it.

[Nikhil, 2014/09/29 08:22:11]

Done.

   CancelPaints();
   LoadPageInfo(true);
   UpdateTickMarks();
   client_->Invalidate(pp::Rect(plugin_size_));
 }
 
 std::string PDFiumEngine::GetSelectedText() {
   base::string16 result;
   for (size_t i = 0; i < selection_.size(); ++i) {
     if (i > 0 &&
@@ skipping 1569 matching lines @@
     double* height) {
   FPDF_DOCUMENT doc = FPDF_LoadMemDocument(pdf_buffer, pdf_buffer_size, NULL);
   if (!doc)
     return false;
   bool success = FPDF_GetPageSizeByIndex(doc, page_number, width, height) != 0;
   FPDF_CloseDocument(doc);
   return success;
 }
 
 }  // namespace chrome_pdf