Commit 6522ddf2 authored by Lukasz Anforowicz's avatar Lukasz Anforowicz Committed by Commit Bot

Don't silently drop find-in-page request when PDF has not yet loaded.

This CL tweaks so that PDFiumEngine::StartFind doesn't *silently* drop
the find-in-page request when the PDF has not loaded yet.  Instead,
after the CL, the find-in-page client will be notified that the search
has completed with 0 results.

Bug: 964364, 965254
Change-Id: Ib528d407d1be34179aca2f04a792c582925f90a6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1629709
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: default avatarLei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#665279}
parent 65bb962f
...@@ -1035,8 +1035,7 @@ class SavePageOriginalVsSavedComparisonTest ...@@ -1035,8 +1035,7 @@ class SavePageOriginalVsSavedComparisonTest
const GURL& url, const GURL& url,
int expected_number_of_frames_in_original_page, int expected_number_of_frames_in_original_page,
int expected_number_of_frames_in_mhtml_page, int expected_number_of_frames_in_mhtml_page,
const std::vector<std::string>& expected_substrings, const std::vector<std::string>& expected_substrings) {
bool skip_find_in_page = false) {
// Navigate to the test page and verify if test expectations // Navigate to the test page and verify if test expectations
// are met (this is mostly a sanity check - a failure to meet // are met (this is mostly a sanity check - a failure to meet
// expectations would probably mean that there is a test bug // expectations would probably mean that there is a test bug
...@@ -1045,8 +1044,7 @@ class SavePageOriginalVsSavedComparisonTest ...@@ -1045,8 +1044,7 @@ class SavePageOriginalVsSavedComparisonTest
DLOG(INFO) << "Verifying test expectations for original page... : " DLOG(INFO) << "Verifying test expectations for original page... : "
<< GetCurrentTab(browser())->GetLastCommittedURL(); << GetCurrentTab(browser())->GetLastCommittedURL();
AssertExpectationsAboutCurrentTab( AssertExpectationsAboutCurrentTab(
expected_number_of_frames_in_original_page, expected_substrings, expected_number_of_frames_in_original_page, expected_substrings);
skip_find_in_page);
// Save the page. // Save the page.
base::FilePath full_file_name, dir; base::FilePath full_file_name, dir;
...@@ -1074,7 +1072,7 @@ class SavePageOriginalVsSavedComparisonTest ...@@ -1074,7 +1072,7 @@ class SavePageOriginalVsSavedComparisonTest
expected_number_of_frames_in_mhtml_page : expected_number_of_frames_in_mhtml_page :
expected_number_of_frames_in_original_page; expected_number_of_frames_in_original_page;
AssertExpectationsAboutCurrentTab(expected_number_of_frames_in_saved_page, AssertExpectationsAboutCurrentTab(expected_number_of_frames_in_saved_page,
expected_substrings, skip_find_in_page); expected_substrings);
if (GetParam() == content::SAVE_PAGE_TYPE_AS_MHTML) { if (GetParam() == content::SAVE_PAGE_TYPE_AS_MHTML) {
std::set<url::Origin> origins; std::set<url::Origin> origins;
...@@ -1091,7 +1089,7 @@ class SavePageOriginalVsSavedComparisonTest ...@@ -1091,7 +1089,7 @@ class SavePageOriginalVsSavedComparisonTest
chrome::GoBack(browser(), WindowOpenDisposition::CURRENT_TAB); chrome::GoBack(browser(), WindowOpenDisposition::CURRENT_TAB);
content::WaitForLoadStop(GetCurrentTab(browser())); content::WaitForLoadStop(GetCurrentTab(browser()));
AssertExpectationsAboutCurrentTab(expected_number_of_frames_in_saved_page, AssertExpectationsAboutCurrentTab(expected_number_of_frames_in_saved_page,
expected_substrings, skip_find_in_page); expected_substrings);
} }
// Helper method to deduplicate some code across 2 tests. // Helper method to deduplicate some code across 2 tests.
...@@ -1120,42 +1118,34 @@ class SavePageOriginalVsSavedComparisonTest ...@@ -1120,42 +1118,34 @@ class SavePageOriginalVsSavedComparisonTest
"frames-nested2.htm: 6d23dc47-f283-4977-96ec-66bcf72301a4", "frames-nested2.htm: 6d23dc47-f283-4977-96ec-66bcf72301a4",
"text-object.txt: ae52dd09-9746-4b7e-86a6-6ada5e2680c2", "text-object.txt: ae52dd09-9746-4b7e-86a6-6ada5e2680c2",
"svg: 0875fd06-131d-4708-95e1-861853c6b8dc", "svg: 0875fd06-131d-4708-95e1-861853c6b8dc",
// TODO(lukasza): Consider also verifying presence of "PDF test file"
// from <object data="pdf.pdf">. This requires ensuring that the PDF is
// loaded before continuing with the test.
}; };
// TODO(lukasza): crbug.com/553478: Enable <object> testing of MHTML. // TODO(lukasza): crbug.com/553478: Enable <object> testing of MHTML.
if (save_page_type == content::SAVE_PAGE_TYPE_AS_MHTML) if (save_page_type == content::SAVE_PAGE_TYPE_AS_MHTML)
return; return;
// TODO(lukasza): https://crbug.com/965254: Re-enable find-in-page aspect of
// the test.
bool skip_find_in_page =
content::MimeHandlerViewMode::UsesCrossProcessFrame();
TestOriginalVsSavedPage(save_page_type, url, expected_number_of_frames, TestOriginalVsSavedPage(save_page_type, url, expected_number_of_frames,
expected_number_of_frames, expected_substrings, expected_number_of_frames, expected_substrings);
skip_find_in_page);
} }
private: private:
void AssertExpectationsAboutCurrentTab( void AssertExpectationsAboutCurrentTab(
int expected_number_of_frames, int expected_number_of_frames,
const std::vector<std::string>& expected_substrings, const std::vector<std::string>& expected_substrings) {
bool skip_find_in_page) {
int actual_number_of_frames = 0; int actual_number_of_frames = 0;
GetCurrentTab(browser())->ForEachFrame(base::BindRepeating( GetCurrentTab(browser())->ForEachFrame(base::BindRepeating(
&IncrementInteger, base::Unretained(&actual_number_of_frames))); &IncrementInteger, base::Unretained(&actual_number_of_frames)));
EXPECT_EQ(expected_number_of_frames, actual_number_of_frames); EXPECT_EQ(expected_number_of_frames, actual_number_of_frames);
// TODO(lukasza): https://crbug.com/965254: Re-enable find-in-page aspect of
// the test.
if (skip_find_in_page)
return;
for (const auto& expected_substring : expected_substrings) { for (const auto& expected_substring : expected_substrings) {
int actual_number_of_matches = ui_test_utils::FindInPage( int actual_number_of_matches = ui_test_utils::FindInPage(
GetCurrentTab(browser()), base::UTF8ToUTF16(expected_substring), GetCurrentTab(browser()), base::UTF8ToUTF16(expected_substring),
true, // |forward| true, // |forward|
true, // |case_sensitive| false, // |case_sensitive|
nullptr, nullptr); nullptr, nullptr);
EXPECT_EQ(1, actual_number_of_matches) EXPECT_EQ(1, actual_number_of_matches)
...@@ -1173,7 +1163,7 @@ class SavePageOriginalVsSavedComparisonTest ...@@ -1173,7 +1163,7 @@ class SavePageOriginalVsSavedComparisonTest
for (const auto& forbidden_substring : forbidden_substrings) { for (const auto& forbidden_substring : forbidden_substrings) {
int actual_number_of_matches = ui_test_utils::FindInPage( int actual_number_of_matches = ui_test_utils::FindInPage(
GetCurrentTab(browser()), base::UTF8ToUTF16(forbidden_substring), GetCurrentTab(browser()), base::UTF8ToUTF16(forbidden_substring),
true, // |forward| true, // |forward|
false, // |case_sensitive| false, // |case_sensitive|
nullptr, nullptr); nullptr, nullptr);
EXPECT_EQ(0, actual_number_of_matches) EXPECT_EQ(0, actual_number_of_matches)
......
...@@ -1838,8 +1838,10 @@ void PDFiumEngine::StartFind(const std::string& text, bool case_sensitive) { ...@@ -1838,8 +1838,10 @@ void PDFiumEngine::StartFind(const std::string& text, bool case_sensitive) {
// If StartFind() gets called before we have any page information (i.e. // If StartFind() gets called before we have any page information (i.e.
// before the first call to LoadDocument has happened). Handle this case. // before the first call to LoadDocument has happened). Handle this case.
if (pages_.empty()) if (pages_.empty()) {
client_->NotifyNumberOfFindResultsChanged(0, true);
return; return;
}
bool first_search = (current_find_text_ != text); bool first_search = (current_find_text_ != text);
int character_to_start_searching_from = 0; int character_to_start_searching_from = 0;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment