0

Change PDFEngine::StartFind() to take a std::u16string.

Create a UTF-16 string and pass that all the way down to the text search
code, instead of creating a UTF-8 string first and converting it to
UTF-16 later.

Change-Id: I845c9659230fadb2772e97938ea61d09207c4d9c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4137082
Reviewed-by: K. Moon <kmoon@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1089378}
This commit is contained in:
Lei Zhang
2023-01-05 19:50:29 +00:00
committed by Chromium LUCI CQ
parent 954f75e06e
commit ac5ca90ad1
5 changed files with 19 additions and 21 deletions

@@ -307,7 +307,7 @@ class PDFEngine {
const std::vector<int>& page_index, const std::vector<int>& page_index,
const blink::WebPrintParams& print_params) = 0; const blink::WebPrintParams& print_params) = 0;
virtual void PrintEnd() = 0; virtual void PrintEnd() = 0;
virtual void StartFind(const std::string& text, bool case_sensitive) = 0; virtual void StartFind(const std::u16string& text, bool case_sensitive) = 0;
virtual bool SelectFindResult(bool forward) = 0; virtual bool SelectFindResult(bool forward) = 0;
virtual void StopFind() = 0; virtual void StopFind() = 0;
virtual void ZoomUpdated(double new_zoom_level) = 0; virtual void ZoomUpdated(double new_zoom_level) = 0;

@@ -693,7 +693,7 @@ bool PdfViewWebPlugin::StartFind(const blink::WebString& search_text,
int identifier) { int identifier) {
ResetRecentlySentFindUpdate(); ResetRecentlySentFindUpdate();
find_identifier_ = identifier; find_identifier_ = identifier;
engine_->StartFind(search_text.Utf8(), case_sensitive); engine_->StartFind(search_text.Utf16(), case_sensitive);
return true; return true;
} }

@@ -92,7 +92,7 @@ TEST_P(FindTextTest, FindText) {
ASSERT_TRUE(engine); ASSERT_TRUE(engine);
ExpectInitialSearchResults(client, 10); ExpectInitialSearchResults(client, 10);
engine->StartFind("o", /*case_sensitive=*/true); engine->StartFind(u"o", /*case_sensitive=*/true);
} }
TEST_P(FindTextTest, FindHyphenatedText) { TEST_P(FindTextTest, FindHyphenatedText) {
@@ -102,7 +102,7 @@ TEST_P(FindTextTest, FindHyphenatedText) {
ASSERT_TRUE(engine); ASSERT_TRUE(engine);
ExpectInitialSearchResults(client, 6); ExpectInitialSearchResults(client, 6);
engine->StartFind("application", /*case_sensitive=*/true); engine->StartFind(u"application", /*case_sensitive=*/true);
} }
TEST_P(FindTextTest, FindLineBreakText) { TEST_P(FindTextTest, FindLineBreakText) {
@@ -112,7 +112,7 @@ TEST_P(FindTextTest, FindLineBreakText) {
ASSERT_TRUE(engine); ASSERT_TRUE(engine);
ExpectInitialSearchResults(client, 1); ExpectInitialSearchResults(client, 1);
engine->StartFind("is the first system", /*case_sensitive=*/true); engine->StartFind(u"is the first system", /*case_sensitive=*/true);
} }
TEST_P(FindTextTest, FindSimpleQuotationMarkText) { TEST_P(FindTextTest, FindSimpleQuotationMarkText) {
@@ -122,7 +122,7 @@ TEST_P(FindTextTest, FindSimpleQuotationMarkText) {
ASSERT_TRUE(engine); ASSERT_TRUE(engine);
ExpectInitialSearchResults(client, 2); ExpectInitialSearchResults(client, 2);
engine->StartFind("don't", /*case_sensitive=*/true); engine->StartFind(u"don't", /*case_sensitive=*/true);
} }
TEST_P(FindTextTest, FindFancyQuotationMarkText) { TEST_P(FindTextTest, FindFancyQuotationMarkText) {
@@ -134,8 +134,7 @@ TEST_P(FindTextTest, FindFancyQuotationMarkText) {
ExpectInitialSearchResults(client, 2); ExpectInitialSearchResults(client, 2);
// don't, using right apostrophe instead of a single quotation mark // don't, using right apostrophe instead of a single quotation mark
std::u16string term = {'d', 'o', 'n', 0x2019, 't'}; engine->StartFind(u"don\u2019t", /*case_sensitive=*/true);
engine->StartFind(base::UTF16ToUTF8(term), /*case_sensitive=*/true);
} }
TEST_P(FindTextTest, FindHiddenCroppedText) { TEST_P(FindTextTest, FindHiddenCroppedText) {
@@ -146,7 +145,7 @@ TEST_P(FindTextTest, FindHiddenCroppedText) {
// The word "Hello" is cropped out. // The word "Hello" is cropped out.
ExpectInitialSearchResults(client, 0); ExpectInitialSearchResults(client, 0);
engine->StartFind("Hello", /*case_sensitive=*/true); engine->StartFind(u"Hello", /*case_sensitive=*/true);
} }
TEST_P(FindTextTest, FindVisibleCroppedText) { TEST_P(FindTextTest, FindVisibleCroppedText) {
@@ -157,7 +156,7 @@ TEST_P(FindTextTest, FindVisibleCroppedText) {
// Only one instance of the word "world" is visible. The other is cropped out. // Only one instance of the word "world" is visible. The other is cropped out.
ExpectInitialSearchResults(client, 1); ExpectInitialSearchResults(client, 1);
engine->StartFind("world", /*case_sensitive=*/true); engine->StartFind(u"world", /*case_sensitive=*/true);
} }
TEST_P(FindTextTest, FindVisibleCroppedTextRepeatedly) { TEST_P(FindTextTest, FindVisibleCroppedTextRepeatedly) {
@@ -169,9 +168,9 @@ TEST_P(FindTextTest, FindVisibleCroppedTextRepeatedly) {
// Only one instance of the word "world" is visible. The other is cropped out. // Only one instance of the word "world" is visible. The other is cropped out.
// These 2 find operations should not trigger https://crbug.com/1344057. // These 2 find operations should not trigger https://crbug.com/1344057.
ExpectInitialSearchResults(client, 1); ExpectInitialSearchResults(client, 1);
engine->StartFind("worl", /*case_sensitive=*/true); engine->StartFind(u"worl", /*case_sensitive=*/true);
ExpectInitialSearchResults(client, 1); ExpectInitialSearchResults(client, 1);
engine->StartFind("world", /*case_sensitive=*/true); engine->StartFind(u"world", /*case_sensitive=*/true);
} }
TEST_P(FindTextTest, SelectFindResult) { TEST_P(FindTextTest, SelectFindResult) {
@@ -181,7 +180,7 @@ TEST_P(FindTextTest, SelectFindResult) {
ASSERT_TRUE(engine); ASSERT_TRUE(engine);
ExpectInitialSearchResults(client, 4); ExpectInitialSearchResults(client, 4);
engine->StartFind("world", /*case_sensitive=*/true); engine->StartFind(u"world", /*case_sensitive=*/true);
ASSERT_TRUE(engine->SelectFindResult(/*forward=*/true)); ASSERT_TRUE(engine->SelectFindResult(/*forward=*/true));
EXPECT_CALL(client, NotifyNumberOfFindResultsChanged(_, _)).Times(0); EXPECT_CALL(client, NotifyNumberOfFindResultsChanged(_, _)).Times(0);
@@ -206,7 +205,7 @@ TEST_P(FindTextTest, SelectFindResultAndSwitchToTwoUpView) {
ASSERT_TRUE(engine); ASSERT_TRUE(engine);
ExpectInitialSearchResults(client, 4); ExpectInitialSearchResults(client, 4);
engine->StartFind("world", /*case_sensitive=*/false); engine->StartFind(u"world", /*case_sensitive=*/false);
{ {
InSequence sequence; InSequence sequence;

@@ -1757,7 +1757,7 @@ bool PDFiumEngine::OnChar(const blink::WebKeyboardEvent& event) {
return rv; return rv;
} }
void PDFiumEngine::StartFind(const std::string& text, bool case_sensitive) { void PDFiumEngine::StartFind(const std::u16string& text, bool case_sensitive) {
// If the caller asks StartFind() to search for no text, then this is an // If the caller asks StartFind() to search for no text, then this is an
// error on the part of the caller. The `blink::FindInPage` code guarantees it // error on the part of the caller. The `blink::FindInPage` code guarantees it
// is not empty, so this should never happen. // is not empty, so this should never happen.
@@ -1795,15 +1795,14 @@ void PDFiumEngine::StartFind(const std::string& text, bool case_sensitive) {
int current_page = next_page_to_search_; int current_page = next_page_to_search_;
if (pages_[current_page]->available()) { if (pages_[current_page]->available()) {
std::u16string str = base::UTF8ToUTF16(text);
// Don't use PDFium to search for now, since it doesn't support unicode // Don't use PDFium to search for now, since it doesn't support unicode
// text. Leave the code for now to avoid bit-rot, in case it's fixed later. // text. Leave the code for now to avoid bit-rot, in case it's fixed later.
// The extra parens suppress a -Wunreachable-code warning. // The extra parens suppress a -Wunreachable-code warning.
if ((false)) { if ((false)) {
SearchUsingPDFium(str, case_sensitive, first_search, SearchUsingPDFium(text, case_sensitive, first_search,
character_to_start_searching_from, current_page); character_to_start_searching_from, current_page);
} else { } else {
SearchUsingICU(str, case_sensitive, first_search, SearchUsingICU(text, case_sensitive, first_search,
character_to_start_searching_from, current_page); character_to_start_searching_from, current_page);
} }
@@ -3779,7 +3778,7 @@ gfx::Size PDFiumEngine::ApplyDocumentLayout(
// Store the current find index so that we can resume finding at that // Store the current find index so that we can resume finding at that
// particular index after we have recomputed the find results. // particular index after we have recomputed the find results.
std::string current_find_text = current_find_text_; std::u16string current_find_text = current_find_text_;
resume_find_index_ = current_find_index_; resume_find_index_ = current_find_index_;
// Save the current page. // Save the current page.

@@ -106,7 +106,7 @@ class PDFiumEngine : public PDFEngine,
const std::vector<int>& page_numbers, const std::vector<int>& page_numbers,
const blink::WebPrintParams& print_params) override; const blink::WebPrintParams& print_params) override;
void PrintEnd() override; void PrintEnd() override;
void StartFind(const std::string& text, bool case_sensitive) override; void StartFind(const std::u16string& text, bool case_sensitive) override;
bool SelectFindResult(bool forward) override; bool SelectFindResult(bool forward) override;
void StopFind() override; void StopFind() override;
void ZoomUpdated(double new_zoom_level) override; void ZoomUpdated(double new_zoom_level) override;
@@ -723,7 +723,7 @@ class PDFiumEngine : public PDFEngine,
gfx::Point mouse_middle_button_last_position_; gfx::Point mouse_middle_button_last_position_;
// The current text used for searching. // The current text used for searching.
std::string current_find_text_; std::u16string current_find_text_;
// The results found. // The results found.
std::vector<PDFiumRange> find_results_; std::vector<PDFiumRange> find_results_;
// Whether a search is in progress. // Whether a search is in progress.