[M135][PDF] Fix crash when double clicking at the end of a page
Double-click text selection at the end of a page is a literal edge case
that PDFiumEngine::OnMultipleClick() does not handle correctly. This
method creates a PDFiumRange where the character index is out of bounds.
In older code, this would have triggered a DCHECK() failure, but not
crash when DCHECKs are turned off. After https://crrev.com/1417508, the
DCHECK() failure still happens. When DCHECKs are turned off, code
execution eventually triggers a crash, because that CL assumed the
character index is never out of bounds.
Add a test case that triggers this crash, and fix the crash by
calculating the character index correctly in OnMultipleClick().
Additionally:
1) Add another test case for double clicking on a blank page, to help
provide more test coverage, and to show OnMultipleClick() does not
need to handle pages with no text.
2) Upgrade a cheap DCHECK_GE() to CHECK_GE().
(cherry picked from commit ac8c360346
)
Bug: 401992988, 404242972
Change-Id: I60e1a4afb7cbb956205ce8f304b15701cf762ce7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6347717
Reviewed-by: Andy Phan <andyphan@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1431938}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6360751
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/7049@{#853}
Cr-Branched-From: 2dab7846d0951a552bdc4f350dad497f986e6fed-refs/heads/main@{#1427262}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
3af40fb443
commit
180adb7e42
@ -1341,33 +1341,39 @@ void PDFiumEngine::OnSingleClick(int page_index, int char_index) {
|
||||
void PDFiumEngine::OnMultipleClick(int click_count,
|
||||
int page_index,
|
||||
int char_index) {
|
||||
DCHECK_GE(click_count, 2);
|
||||
bool is_double_click = click_count == 2;
|
||||
CHECK_GE(click_count, 2);
|
||||
const bool is_double_click = click_count == 2;
|
||||
|
||||
const int char_count = pages_[page_index]->GetCharCount();
|
||||
CHECK_GT(char_count, 0);
|
||||
|
||||
// It would be more efficient if the SDK could support finding a space, but
|
||||
// now it doesn't.
|
||||
int start_index = char_index;
|
||||
do {
|
||||
uint32_t cur = pages_[page_index]->GetCharUnicode(start_index);
|
||||
if (FindMultipleClickBoundary(is_double_click, cur))
|
||||
if (FindMultipleClickBoundary(is_double_click, cur)) {
|
||||
break;
|
||||
}
|
||||
} while (--start_index >= 0);
|
||||
if (start_index)
|
||||
if (start_index && start_index < char_count - 1) {
|
||||
start_index++;
|
||||
}
|
||||
|
||||
int end_index = char_index;
|
||||
const int total = pages_[page_index]->GetCharCount();
|
||||
for (; end_index < total; ++end_index) {
|
||||
for (; end_index < char_count; ++end_index) {
|
||||
uint32_t cur = pages_[page_index]->GetCharUnicode(end_index);
|
||||
if (FindMultipleClickBoundary(is_double_click, cur))
|
||||
if (FindMultipleClickBoundary(is_double_click, cur)) {
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
selection_.push_back(PDFiumRange(pages_[page_index].get(), start_index,
|
||||
end_index - start_index));
|
||||
|
||||
if (handling_long_press_)
|
||||
if (handling_long_press_) {
|
||||
client_->NotifyTouchSelectionOccurred();
|
||||
}
|
||||
}
|
||||
|
||||
bool PDFiumEngine::OnLeftMouseDown(const blink::WebMouseEvent& event) {
|
||||
|
@ -1139,6 +1139,40 @@ TEST_P(PDFiumEngineTest, SelectTextAcrossEmptyPage) {
|
||||
EXPECT_EQ(kExpectedAllSelection, engine->GetSelectedText());
|
||||
}
|
||||
|
||||
TEST_P(PDFiumEngineTest, SelectTextWithDoubleClickOnEmptyPage) {
|
||||
NiceMock<MockTestClient> client;
|
||||
std::unique_ptr<PDFiumEngine> engine =
|
||||
InitializeEngine(&client, FILE_PATH_LITERAL("blank.pdf"));
|
||||
ASSERT_TRUE(engine);
|
||||
|
||||
// Plugin size chosen so all pages of the document are visible.
|
||||
engine->PluginSizeUpdated({1024, 4096});
|
||||
|
||||
constexpr gfx::PointF kPosition(100, 100);
|
||||
EXPECT_TRUE(engine->HandleInputEvent(MouseEventBuilder()
|
||||
.CreateLeftClickAtPosition(kPosition)
|
||||
.SetClickCount(2)
|
||||
.Build()));
|
||||
EXPECT_THAT(engine->GetSelectedText(), IsEmpty());
|
||||
}
|
||||
|
||||
TEST_P(PDFiumEngineTest, SelectTextWithDoubleClickAtEndOfPage) {
|
||||
NiceMock<MockTestClient> client;
|
||||
std::unique_ptr<PDFiumEngine> engine =
|
||||
InitializeEngine(&client, FILE_PATH_LITERAL("hello_world2.pdf"));
|
||||
ASSERT_TRUE(engine);
|
||||
|
||||
// Plugin size chosen so all pages of the document are visible.
|
||||
engine->PluginSizeUpdated({1024, 4096});
|
||||
|
||||
constexpr gfx::PointF kPosition(195, 130);
|
||||
EXPECT_TRUE(engine->HandleInputEvent(MouseEventBuilder()
|
||||
.CreateLeftClickAtPosition(kPosition)
|
||||
.SetClickCount(2)
|
||||
.Build()));
|
||||
EXPECT_THAT(engine->GetSelectedText(), IsEmpty());
|
||||
}
|
||||
|
||||
TEST_P(PDFiumEngineTest, DrawTextSelectionsHelloWorld) {
|
||||
constexpr int kPageIndex = 0;
|
||||
NiceMock<MockTestClient> client;
|
||||
|
Reference in New Issue
Block a user