Cleanup PDF/PPAPI SearchString code.

- Results count should be unsigned. - return results vector instead of using an old parameter in pdf code. Change-Id: I207ed15ca364679e5d8c54edb384fff548d59caf Reviewed-on: https://chromium-review.googlesource.com/727702 Commit-Queue: Lei Zhang <thestig@chromium.org> Reviewed-by: Bill Budge <bbudge@chromium.org> Cr-Commit-Position: refs/heads/master@{#510608}
Denger-Network · Oct 20, 2017 · 49baa88 · 49baa88
1 parent a14f781
commit 49baa88
Show file tree

Hide file tree

Showing 14 changed files with 45 additions and 46 deletions.
diff --git a/pdf/out_of_process_instance.cc b/pdf/out_of_process_instance.cc
@@ -1409,25 +1409,26 @@ void OutOfProcessInstance::ScheduleTouchTimerCallback(int id,
                                               id);
 }
 
-void OutOfProcessInstance::SearchString(
-    const base::char16* string,
-    const base::char16* term,
-    bool case_sensitive,
-    std::vector<SearchStringResult>* results) {
+std::vector<PDFEngine::Client::SearchStringResult>
+OutOfProcessInstance::SearchString(const base::char16* string,
+                                   const base::char16* term,
+                                   bool case_sensitive) {
   PP_PrivateFindResult* pp_results;
-  int count = 0;
+  uint32_t count = 0;
   pp::PDF::SearchString(this, reinterpret_cast<const unsigned short*>(string),
                         reinterpret_cast<const unsigned short*>(term),
                         case_sensitive, &pp_results, &count);
 
-  results->resize(count);
-  for (int i = 0; i < count; ++i) {
-    (*results)[i].start_index = pp_results[i].start_index;
-    (*results)[i].length = pp_results[i].length;
+  std::vector<SearchStringResult> results(count);
+  for (uint32_t i = 0; i < count; ++i) {
+    results[i].start_index = pp_results[i].start_index;
+    results[i].length = pp_results[i].length;
   }
 
   pp::Memory_Dev memory;
   memory.MemFree(pp_results);
+
+  return results;
 }
 
 void OutOfProcessInstance::DocumentPaintOccurred() {}

diff --git a/pdf/out_of_process_instance.h b/pdf/out_of_process_instance.h
@@ -126,10 +126,9 @@ class OutOfProcessInstance : public pp::Instance,
   pp::URLLoader CreateURLLoader() override;
   void ScheduleCallback(int id, base::TimeDelta delay) override;
   void ScheduleTouchTimerCallback(int id, base::TimeDelta delay) override;
-  void SearchString(const base::char16* string,
-                    const base::char16* term,
-                    bool case_sensitive,
-                    std::vector<SearchStringResult>* results) override;
+  std::vector<SearchStringResult> SearchString(const base::char16* string,
+                                               const base::char16* term,
+                                               bool case_sensitive) override;
   void DocumentPaintOccurred() override;
   void DocumentLoadComplete(
       const PDFEngine::DocumentFeatures& document_features) override;

diff --git a/pdf/pdf_engine.h b/pdf/pdf_engine.h
@@ -167,10 +167,10 @@ class PDFEngine {
       int start_index;
       int length;
     };
-    virtual void SearchString(const base::char16* string,
-                              const base::char16* term,
-                              bool case_sensitive,
-                              std::vector<SearchStringResult>* results) = 0;
+    virtual std::vector<SearchStringResult> SearchString(
+        const base::char16* string,
+        const base::char16* term,
+        bool case_sensitive) = 0;
 
     // Notifies the client that the engine has painted a page from the document.
     virtual void DocumentPaintOccurred() = 0;

diff --git a/pdf/pdfium/pdfium_engine.cc b/pdf/pdfium/pdfium_engine.cc
@@ -2325,9 +2325,8 @@ void PDFiumEngine::SearchUsingICU(const base::string16& term,
                                  text_length + 1, data);
   api_string_adapter.Close(written);
 
-  std::vector<PDFEngine::Client::SearchStringResult> results;
-  client_->SearchString(page_text.c_str(), term.c_str(), case_sensitive,
-                        &results);
+  std::vector<PDFEngine::Client::SearchStringResult> results =
+      client_->SearchString(page_text.c_str(), term.c_str(), case_sensitive);
   for (const auto& result : results) {
     // Need to map the indexes from the page text, which may have generated
     // characters like space etc, to character indices from the page.

diff --git a/pdf/preview_mode_client.cc b/pdf/preview_mode_client.cc
@@ -116,11 +116,12 @@ void PreviewModeClient::ScheduleTouchTimerCallback(int id,
   NOTREACHED();
 }
 
-void PreviewModeClient::SearchString(const base::char16* string,
-                                     const base::char16* term,
-                                     bool case_sensitive,
-                                     std::vector<SearchStringResult>* results) {
+std::vector<PDFEngine::Client::SearchStringResult>
+PreviewModeClient::SearchString(const base::char16* string,
+                                const base::char16* term,
+                                bool case_sensitive) {
   NOTREACHED();
+  return std::vector<SearchStringResult>();
 }
 
 void PreviewModeClient::DocumentPaintOccurred() {

diff --git a/pdf/preview_mode_client.h b/pdf/preview_mode_client.h
@@ -58,10 +58,9 @@ class PreviewModeClient : public PDFEngine::Client {
   pp::URLLoader CreateURLLoader() override;
   void ScheduleCallback(int id, base::TimeDelta delay) override;
   void ScheduleTouchTimerCallback(int id, base::TimeDelta delay) override;
-  void SearchString(const base::char16* string,
-                    const base::char16* term,
-                    bool case_sensitive,
-                    std::vector<SearchStringResult>* results) override;
+  std::vector<SearchStringResult> SearchString(const base::char16* string,
+                                               const base::char16* term,
+                                               bool case_sensitive) override;
   void DocumentPaintOccurred() override;
   void DocumentLoadComplete(
       const PDFEngine::DocumentFeatures& document_features) override;

diff --git a/ppapi/c/private/ppb_pdf.h b/ppapi/c/private/ppb_pdf.h
@@ -92,13 +92,12 @@ struct PPB_PDF {
 
   // Search the given string using ICU.  Use PPB_Core's MemFree on results when
   // done.
-  void (*SearchString)(
-     PP_Instance instance,
-     const unsigned short* string,
-     const unsigned short* term,
-     bool case_sensitive,
-     struct PP_PrivateFindResult** results,
-     int* count);
+  void (*SearchString)(PP_Instance instance,
+                       const unsigned short* string,
+                       const unsigned short* term,
+                       bool case_sensitive,
+                       struct PP_PrivateFindResult** results,
+                       uint32_t* count);
 
   // Since WebFrame doesn't know about PPAPI requests, it'll think the page has
   // finished loading even if there are outstanding requests by the plugin.

diff --git a/ppapi/cpp/private/pdf.cc b/ppapi/cpp/private/pdf.cc
@@ -55,7 +55,7 @@ void PDF::SearchString(const InstanceHandle& instance,
                        const unsigned short* term,
                        bool case_sensitive,
                        PP_PrivateFindResult** results,
-                       int* count) {
+                       uint32_t* count) {
   if (has_interface<PPB_PDF>()) {
     get_interface<PPB_PDF>()->SearchString(instance.pp_instance(), string,
         term, case_sensitive, results, count);

diff --git a/ppapi/cpp/private/pdf.h b/ppapi/cpp/private/pdf.h
@@ -36,7 +36,7 @@ class PDF {
                            const unsigned short* term,
                            bool case_sensitive,
                            PP_PrivateFindResult** results,
-                           int* count);
+                           uint32_t* count);
   static void DidStartLoading(const InstanceHandle& instance);
   static void DidStopLoading(const InstanceHandle& instance);
   static void SetContentRestriction(const InstanceHandle& instance,

diff --git a/ppapi/proxy/pdf_resource.cc b/ppapi/proxy/pdf_resource.cc
@@ -55,7 +55,8 @@ thunk::PPB_PDF_API* PDFResource::AsPPB_PDF_API() {
 void PDFResource::SearchString(const unsigned short* input_string,
                                const unsigned short* input_term,
                                bool case_sensitive,
-                               PP_PrivateFindResult** results, int* count) {
+                               PP_PrivateFindResult** results,
+                               uint32_t* count) {
   if (locale_.empty())
     locale_ = GetLocale();
   const base::char16* string =

diff --git a/ppapi/proxy/pdf_resource.h b/ppapi/proxy/pdf_resource.h
@@ -37,7 +37,7 @@ class PPAPI_PROXY_EXPORT PDFResource
                     const unsigned short* input_term,
                     bool case_sensitive,
                     PP_PrivateFindResult** results,
-                    int* count) override;
+                    uint32_t* count) override;
   void DidStartLoading() override;
   void DidStopLoading() override;
   void SetContentRestriction(int restrictions) override;

diff --git a/ppapi/proxy/pdf_resource_unittest.cc b/ppapi/proxy/pdf_resource_unittest.cc
@@ -30,8 +30,8 @@ typedef PluginProxyTest PDFResourceTest;
 TEST_F(PDFResourceTest, SearchString) {
   ProxyAutoLock lock;
   // Instantiate a resource explicitly so we can specify the locale.
-  scoped_refptr<PDFResource> pdf_resource(
-      new PDFResource(Connection(&sink(), &sink(), 0), pp_instance()));
+  auto pdf_resource = base::MakeRefCounted<PDFResource>(
+      Connection(&sink(), &sink(), 0), pp_instance());
   pdf_resource->SetLocaleForTest("en-US");
 
   base::string16 input;
@@ -40,15 +40,15 @@ TEST_F(PDFResourceTest, SearchString) {
   base::UTF8ToUTF16("bc", 2, &term);
 
   PP_PrivateFindResult* results;
-  int count = 0;
+  uint32_t count = 0;
   pdf_resource->SearchString(
       reinterpret_cast<const unsigned short*>(input.c_str()),
       reinterpret_cast<const unsigned short*>(term.c_str()),
       true,
       &results,
       &count);
 
-  ASSERT_EQ(2, count);
+  ASSERT_EQ(2U, count);
   ASSERT_EQ(1, results[0].start_index);
   ASSERT_EQ(2, results[0].length);
   ASSERT_EQ(7, results[1].start_index);

diff --git a/ppapi/thunk/ppb_pdf_api.h b/ppapi/thunk/ppb_pdf_api.h
@@ -17,7 +17,7 @@ class PPB_PDF_API {
                             const unsigned short* input_term,
                             bool case_sensitive,
                             PP_PrivateFindResult** results,
-                            int* count) = 0;
+                            uint32_t* count) = 0;
   virtual void DidStartLoading() = 0;
   virtual void DidStopLoading() = 0;
   virtual void SetContentRestriction(int restrictions) = 0;

diff --git a/ppapi/thunk/ppb_pdf_thunk.cc b/ppapi/thunk/ppb_pdf_thunk.cc
@@ -49,7 +49,7 @@ void SearchString(PP_Instance instance,
                   const unsigned short* term,
                   bool case_sensitive,
                   PP_PrivateFindResult** results,
-                  int* count) {
+                  uint32_t* count) {
   EnterInstanceAPI<PPB_PDF_API> enter(instance);
   if (enter.failed())
     return;