Commit 0e998e39 authored by Sidney San Martín's avatar Sidney San Martín Committed by Commit Bot

[Mac] Validate the selection range before using it for text suggestions.

It looks like Blink can report an invalid selection range back to the
browser. This change treats invalid ranges like the empty string for the
purpose of generating suggestions, and adds a DCHECK for this condition.

Bug: 893038
Change-Id: I2afa5065d7eeebb6cc61030277f86b488a69388b
Reviewed-on: https://chromium-review.googlesource.com/c/1351530
Commit-Queue: Sidney San Martín <sdy@chromium.org>
Reviewed-by: default avatarElly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612882}
parent 09bb5f10
......@@ -11,6 +11,7 @@
#include "base/bind.h"
#include "base/command_line.h"
#include "base/debug/dump_without_crashing.h"
#include "base/feature_list.h"
#include "base/logging.h"
#include "base/mac/mac_util.h"
......@@ -655,6 +656,24 @@ void RenderWidgetHostViewMac::OnTextSelectionChanged(
const TextInputManager::TextSelection* selection = GetTextSelection();
if (!selection)
return;
if (!selection->range().IsValid()) {
LOG(ERROR) << "Invalid selection range.";
base::debug::DumpWithoutCrashing();
return;
}
gfx::Range text_range(selection->offset(),
selection->text().length() + selection->offset());
if (selection->range().GetMin() < text_range.GetMin() ||
selection->range().GetMax() > text_range.GetMax()) {
LOG(ERROR) << "Selection with range " << selection->range().ToString()
<< " in a chunk of text which only covers "
<< text_range.ToString() << ".";
base::debug::DumpWithoutCrashing();
return;
}
ns_view_bridge_->SetTextSelection(selection->text(), selection->offset(),
selection->range());
}
......
......@@ -187,9 +187,10 @@ requestCandidatesForSelectedRange:(NSRange)selectedRange
return sequenceNumber_;
}
- (void (^)(NSInteger sequenceNumber,
NSArray<NSTextCheckingResult*>* candidates))lastCompletionHandler {
return lastCompletionHandler_;
- (base::mac::ScopedBlock<void (^)(NSInteger sequenceNumber,
NSArray<NSTextCheckingResult*>* candidates)>)
takeCompletionHandler {
return std::move(lastCompletionHandler_);
}
@end
......@@ -2123,9 +2124,7 @@ TEST_F(InputMethodMacTest, TouchBarTextSuggestionsReplacement) {
tab_view()->SelectionChanged(kOriginalString, 3, gfx::Range(3, 3));
NSInteger firstSequenceNumber = [spellChecker sequenceNumber];
base::mac::ScopedBlock<void (^)(NSInteger sequenceNumber,
NSArray<NSTextCheckingResult*>* candidates)>
firstCompletionHandler([[spellChecker lastCompletionHandler] retain]);
auto firstCompletionHandler = [spellChecker takeCompletionHandler];
EXPECT_NE(nil, (id)firstCompletionHandler.get());
EXPECT_EQ(0U, candidate_list_item().candidates.count);
......@@ -2143,7 +2142,7 @@ TEST_F(InputMethodMacTest, TouchBarTextSuggestionsReplacement) {
EXPECT_EQ(0U, candidate_list_item().candidates.count);
// But calling the current handler should work.
[spellChecker lastCompletionHandler](
[spellChecker takeCompletionHandler].get()(
[spellChecker sequenceNumber],
@[ static_cast<NSTextCheckingResult*>(fakeResult) ]);
base::RunLoop().RunUntilIdle();
......@@ -2178,6 +2177,11 @@ TEST_F(InputMethodMacTest, TouchBarTextSuggestionsInvalidSelection) {
SetTextInputType(tab_view(), ui::TEXT_INPUT_TYPE_TEXT);
candidate_list_item().allowsCollapsing = NO;
if (auto completionHandler = [spellChecker takeCompletionHandler]) {
completionHandler.get()([spellChecker sequenceNumber], @[]);
base::RunLoop().RunUntilIdle();
}
FakeTextCheckingResult* fakeResult =
[FakeTextCheckingResult resultWithRange:NSMakeRange(0, 3)
replacementString:@"foo"];
......@@ -2187,10 +2191,12 @@ TEST_F(InputMethodMacTest, TouchBarTextSuggestionsInvalidSelection) {
tab_view()->SelectionChanged(kOriginalString, 3,
gfx::Range::InvalidRange());
[spellChecker lastCompletionHandler](
[spellChecker sequenceNumber],
@[ static_cast<NSTextCheckingResult*>(fakeResult) ]);
base::RunLoop().RunUntilIdle();
if (auto completionHandler = [spellChecker takeCompletionHandler]) {
completionHandler.get()(
[spellChecker sequenceNumber],
@[ static_cast<NSTextCheckingResult*>(fakeResult) ]);
base::RunLoop().RunUntilIdle();
}
EXPECT_NE(nil, candidate_list_item().candidates);
EXPECT_EQ(0U, candidate_list_item().candidates.count);
......
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