Commit d45e3a59 authored by Xiaocheng Hu's avatar Xiaocheng Hu Committed by Commit Bot

[Find-in-Page] Ensure timeout check in TextFinder::ScopeStringMatches()

The function's main loop is supposed to halt after timeout, but contains
a branch that continues directly without checking timeout, which leads
to renderer hang in some edge/buggy cases.

This patch implements yoichio's idea (https://goo.gl/EQED8Y) to fix the
issue, by rewriting loop conditions and ensuring timeout checking in
all branches.

Note: no test in this patch, because the only repro case we have hits
a DCHECK earlier before entering the problematic branch.

Bug: 862648
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I050909bd625f122c0df7ed905184c32e133170ce
Reviewed-on: https://chromium-review.googlesource.com/1148645Reviewed-by: default avatarYoichi Osato <yoichio@chromium.org>
Reviewed-by: default avatarRakina Zata Amni <rakina@chromium.org>
Reviewed-by: default avatarYoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577918}
parent e1d017d5
...@@ -443,7 +443,7 @@ void TextFinder::ScopeStringMatches(int identifier, ...@@ -443,7 +443,7 @@ void TextFinder::ScopeStringMatches(int identifier,
const double kMaxScopingDuration = 0.1; // seconds const double kMaxScopingDuration = 0.1; // seconds
int match_count = 0; int match_count = 0;
bool timed_out = false; bool full_range_searched = false;
double start_time = CurrentTime(); double start_time = CurrentTime();
PositionInFlatTree next_scoping_start; PositionInFlatTree next_scoping_start;
do { do {
...@@ -457,6 +457,7 @@ void TextFinder::ScopeStringMatches(int identifier, ...@@ -457,6 +457,7 @@ void TextFinder::ScopeStringMatches(int identifier,
search_text, options.match_case ? 0 : kCaseInsensitive); search_text, options.match_case ? 0 : kCaseInsensitive);
if (result.IsCollapsed()) { if (result.IsCollapsed()) {
// Not found. // Not found.
full_range_searched = true;
break; break;
} }
Range* result_range = Range::Create( Range* result_range = Range::Create(
...@@ -516,8 +517,7 @@ void TextFinder::ScopeStringMatches(int identifier, ...@@ -516,8 +517,7 @@ void TextFinder::ScopeStringMatches(int identifier,
search_start = result.EndPosition(); search_start = result.EndPosition();
next_scoping_start = search_start; next_scoping_start = search_start;
timed_out = (CurrentTime() - start_time) >= kMaxScopingDuration; } while (CurrentTime() - start_time < kMaxScopingDuration);
} while (!timed_out);
if (next_scoping_start.IsNotNull()) { if (next_scoping_start.IsNotNull()) {
resume_scoping_from_range_ = resume_scoping_from_range_ =
...@@ -540,7 +540,7 @@ void TextFinder::ScopeStringMatches(int identifier, ...@@ -540,7 +540,7 @@ void TextFinder::ScopeStringMatches(int identifier,
IncreaseMatchCount(identifier, match_count); IncreaseMatchCount(identifier, match_count);
} }
if (timed_out) { if (!full_range_searched) {
// If we found anything during this pass, we should redraw. However, we // If we found anything during this pass, we should redraw. However, we
// don't want to spam too much if the page is extremely long, so if we // don't want to spam too much if the page is extremely long, so if we
// reach a certain point we start throttling the redraw requests. // reach a certain point we start throttling the redraw requests.
......
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