Commit de7582f5 authored by Rakina Zata Amni's avatar Rakina Zata Amni Committed by Commit Bot

[reland] Make find-in-page use normal tasks for M84

A reland of parts of crrev.com/c/2041711 that got reverted in
crrev.com/c/2127998 for the M83 branch, with the crash cause fixed (we
previously did not return after the DidFinishTask call when
ShouldFindMatches returns false).

Bug: 1065371
Change-Id: Iac0fa13dd953fd51169cdbcbc0f4c34afb824a2a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2129368Reviewed-by: default avatarYoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756896}
parent 715a1ac0
......@@ -22,15 +22,13 @@
namespace blink {
namespace {
const int kFindingTimeoutMS = 100;
constexpr base::TimeDelta kFindTaskTestTimeout =
base::TimeDelta::FromSeconds(10);
constexpr base::TimeDelta kFindTaskTimeAllotment =
base::TimeDelta::FromMilliseconds(10);
} // namespace
class FindTaskController::IdleFindTask
: public ScriptedIdleTaskController::IdleTask {
class FindTaskController::FindTask final : public GarbageCollected<FindTask> {
public:
IdleFindTask(FindTaskController* controller,
FindTask(FindTaskController* controller,
Document* document,
int identifier,
const WebString& search_text,
......@@ -41,38 +39,26 @@ class FindTaskController::IdleFindTask
search_text_(search_text),
options_(options.Clone()) {
DCHECK(document_);
// We need to add deadline because some webpages might have frames
// that are always busy, resulting in bad experience in find-in-page
// because the scoping tasks are not run.
// See crbug.com/893465.
IdleRequestOptions* request_options = IdleRequestOptions::Create();
request_options->setTimeout(kFindingTimeoutMS);
callback_handle_ = document_->RequestIdleCallback(this, request_options);
}
void Dispose() {
DCHECK_GT(callback_handle_, 0);
document_->CancelIdleCallback(callback_handle_);
if (options.run_synchronously_for_testing) {
Invoke();
} else {
controller_->GetLocalFrame()
->GetTaskRunner(blink::TaskType::kInternalFindInPage)
->PostTask(FROM_HERE,
WTF::Bind(&FindTask::Invoke, WrapWeakPersistent(this)));
}
void ForceInvocationForTesting() {
invoke(MakeGarbageCollected<IdleDeadline>(
base::TimeTicks::Now() + kFindTaskTestTimeout,
IdleDeadline::CallbackType::kCalledWhenIdle));
}
void Trace(Visitor* visitor) override {
void Trace(Visitor* visitor) {
visitor->Trace(controller_);
visitor->Trace(document_);
ScriptedIdleTaskController::IdleTask::Trace(visitor);
}
private:
void invoke(IdleDeadline* deadline) override {
void Invoke() {
const base::TimeTicks task_start_time = base::TimeTicks::Now();
if (!controller_)
return;
if (!controller_->ShouldFindMatches(search_text_, *options_)) {
if (!controller_->ShouldFindMatches(identifier_, search_text_, *options_)) {
controller_->DidFinishTask(identifier_, search_text_, *options_,
true /* finished_whole_request */,
PositionInFlatTree(), 0 /* match_count */,
......@@ -120,6 +106,7 @@ class FindTaskController::IdleFindTask
(options_->forward ? 0 : kBackwards) |
(options_->match_case ? 0 : kCaseInsensitive) |
(options_->find_next ? 0 : kStartInSelection);
auto start_time = base::TimeTicks::Now();
while (search_start != search_end) {
// Find in the whole block.
......@@ -152,7 +139,8 @@ class FindTaskController::IdleFindTask
break;
}
next_task_start_position = search_start;
if (deadline->timeRemaining() <= 0)
auto time_elapsed = base::TimeTicks::Now() - start_time;
if (time_elapsed > kFindTaskTimeAllotment)
break;
}
controller_->DidFinishTask(identifier_, search_text_, *options_,
......@@ -163,7 +151,6 @@ class FindTaskController::IdleFindTask
Member<Document> document_;
Member<FindTaskController> controller_;
int callback_handle_ = 0;
const int identifier_;
const WebString search_text_;
mojom::blink::FindOptionsPtr options_;
......@@ -179,41 +166,40 @@ void FindTaskController::StartRequest(
int identifier,
const WebString& search_text,
const mojom::blink::FindOptions& options) {
TRACE_EVENT_ASYNC_BEGIN0("blink", "FindInPageRequest", identifier);
current_request_start_time_ = base::TimeTicks::Now();
total_task_duration_for_current_request_ = base::TimeDelta();
task_count_for_current_request_ = 0;
DCHECK(!finding_in_progress_);
DCHECK_EQ(current_find_identifier_, kInvalidFindIdentifier);
// This is a brand new search, so we need to reset everything.
finding_in_progress_ = true;
current_match_count_ = 0;
RequestIdleFindTask(identifier, search_text, options);
current_find_identifier_ = identifier;
RequestFindTask(identifier, search_text, options);
}
void FindTaskController::CancelPendingRequest() {
if (find_task_) {
find_task_->Dispose();
if (find_task_)
find_task_.Clear();
}
if (finding_in_progress_) {
RecordRequestMetrics(RequestEndState::ABORTED);
last_find_request_completed_with_no_matches_ = false;
}
finding_in_progress_ = false;
resume_finding_from_range_ = nullptr;
current_find_identifier_ = kInvalidFindIdentifier;
}
void FindTaskController::RequestIdleFindTask(
void FindTaskController::RequestFindTask(
int identifier,
const WebString& search_text,
const mojom::blink::FindOptions& options) {
DCHECK_EQ(find_task_, nullptr);
find_task_ = MakeGarbageCollected<IdleFindTask>(
DCHECK_EQ(identifier, current_find_identifier_);
task_count_for_current_request_++;
find_task_ = MakeGarbageCollected<FindTask>(
this, GetLocalFrame()->GetDocument(), identifier, search_text, options);
// If it's for testing, run the task immediately.
// TODO(rakina): Change to use general solution when it's available.
// https://crbug.com/875203
if (options.run_synchronously_for_testing)
find_task_->ForceInvocationForTesting();
}
void FindTaskController::DidFinishTask(
......@@ -225,10 +211,8 @@ void FindTaskController::DidFinishTask(
int match_count,
bool aborted,
base::TimeTicks task_start_time) {
if (find_task_) {
find_task_->Dispose();
find_task_.Clear();
}
if (current_find_identifier_ != identifier)
return;
total_task_duration_for_current_request_ +=
base::TimeTicks::Now() - task_start_time;
if (find_task_)
......@@ -251,20 +235,24 @@ void FindTaskController::DidFinishTask(
if (!finished_whole_request) {
// Task ran out of time, request for another one.
RequestIdleFindTask(identifier, search_text, options);
RequestFindTask(identifier, search_text, options);
return; // Done for now, resume work later.
}
text_finder_->FinishCurrentScopingEffort(identifier);
RecordRequestMetrics(RequestEndState::ABORTED);
last_find_request_completed_with_no_matches_ = !current_match_count_;
last_find_request_completed_with_no_matches_ =
!aborted && !current_match_count_;
finding_in_progress_ = false;
current_find_identifier_ = kInvalidFindIdentifier;
}
void FindTaskController::RecordRequestMetrics(
RequestEndState request_end_state) {
bool aborted = (request_end_state == RequestEndState::ABORTED);
TRACE_EVENT_ASYNC_END1("blink", "FindInPageRequest", current_find_identifier_,
"aborted", aborted);
if (aborted) {
UMA_HISTOGRAM_MEDIUM_TIMES("WebCore.FindInPage.TotalTaskDuration.Aborted",
total_task_duration_for_current_request_);
......@@ -291,8 +279,11 @@ LocalFrame* FindTaskController::GetLocalFrame() const {
}
bool FindTaskController::ShouldFindMatches(
int identifier,
const String& search_text,
const mojom::blink::FindOptions& options) {
if (identifier != current_find_identifier_)
return false;
// Don't scope if we can't find a frame, a document, or a view.
// The user may have closed the tab/application, so abort.
LocalFrame* frame = GetLocalFrame();
......
......@@ -9,9 +9,16 @@
#include "third_party/blink/renderer/core/core_export.h"
#include "third_party/blink/renderer/core/editing/position.h"
#include "third_party/blink/renderer/platform/heap/garbage_collected.h"
#include "third_party/blink/renderer/platform/scheduler/common/throttling/budget_pool.h"
#include "third_party/blink/renderer/platform/scheduler/common/throttling/budget_pool_controller.h"
#include "third_party/blink/renderer/platform/scheduler/common/throttling/cpu_time_budget_pool.h"
#include "third_party/blink/renderer/platform/scheduler/common/tracing_helper.h"
#include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
namespace blink {
namespace {
const int kInvalidFindIdentifier = -1;
}
class LocalFrame;
class Range;
......@@ -45,10 +52,11 @@ class CORE_EXPORT FindTaskController final
// It is not necessary if the frame is invisible, for example, or if this
// is a repeat search that already returned nothing last time the same prefix
// was searched.
bool ShouldFindMatches(const String& search_text,
bool ShouldFindMatches(int identifier,
const String& search_text,
const mojom::blink::FindOptions& options);
// During a run of |find_task|, we found a match.
// During a run of |idle_find_task|, we found a match.
// Updates |current_match_count_| and notifies |text_finder_|.
void DidFindMatch(int identifier, Range* result_range);
......@@ -69,14 +77,18 @@ class CORE_EXPORT FindTaskController final
// When invoked this will search for a given text and notify us
// whenever a match is found or when it finishes, through FoundMatch and
// DidFinishTask.
class IdleFindTask;
class FindTask;
void Trace(Visitor* visitor);
void ResetLastFindRequestCompletedWithNoMatches();
void InvokeFind(int identifier,
const WebString& search_text_,
mojom::blink::FindOptionsPtr options_);
private:
void RequestIdleFindTask(int identifier,
void RequestFindTask(int identifier,
const WebString& search_text,
const mojom::blink::FindOptions& options);
......@@ -95,7 +107,7 @@ class CORE_EXPORT FindTaskController final
Member<TextFinder> text_finder_;
Member<IdleFindTask> find_task_;
Member<FindTask> find_task_;
// Keeps track if there is any ongoing find effort or not.
bool finding_in_progress_;
......@@ -115,6 +127,10 @@ class CORE_EXPORT FindTaskController final
// without finding any matches in this frame.
bool last_find_request_completed_with_no_matches_;
// The identifier of the current find request, we should only run FindTasks
// that have the same identifier as this.
int current_find_identifier_ = kInvalidFindIdentifier;
// The start time of the current find-in-page request.
base::TimeTicks current_request_start_time_;
// The combined duration of all the tasks done for the current request.
......
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