Revert find-in-page back to idle task for M83

We changed find-in-page to use normal tasks instead of idle tasks in
crrev.com/c/2041711, but it's currently causing crashes. To ensure
stability in M83, we're reverting the change (but not all of it, we're
keeping the scheduler-side changes to use later - it won't affect the
revert). We'll investigate this and reland after the M83 branch point.

Bug: 1065371
Change-Id: Ib7b0db989910a5c1c0da82eea65af1ab6f617441
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2127998Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/master@{#754798}

third_party/blink/renderer/core/editing/finder/find_task_controller.cc

@@ -21,48 +21,62 @@
 namespace blink {
 
 namespace {
-
-constexpr base::TimeDelta kFindTaskTimeAllotment =
-    base::TimeDelta::FromMilliseconds(10);
+const int kFindingTimeoutMS = 100;
+constexpr base::TimeDelta kFindTaskTestTimeout =
+    base::TimeDelta::FromSeconds(10);
 }  // namespace
 
-class FindTaskController::FindTask final : public GarbageCollected<FindTask> {
+class FindTaskController::IdleFindTask
+    : public ScriptedIdleTaskController::IdleTask {
  public:
-  FindTask(FindTaskController* controller,
-           Document* document,
-           int identifier,
-           const WebString& search_text,
-           const mojom::blink::FindOptions& options)
+  IdleFindTask(FindTaskController* controller,
+               Document* document,
+               int identifier,
+               const WebString& search_text,
+               const mojom::blink::FindOptions& options)
       : document_(document),
         controller_(controller),
         identifier_(identifier),
         search_text_(search_text),
         options_(options.Clone()) {
     DCHECK(document_);
-    if (options.run_synchronously_for_testing) {
-      Invoke();
-    } else {
-      controller_->GetLocalFrame()
-          ->GetTaskRunner(blink::TaskType::kInternalFindInPage)
-          ->PostTask(FROM_HERE,
-                     WTF::Bind(&FindTask::Invoke, WrapWeakPersistent(this)));
-    }
+    // 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_);
+  }
+
+  void ForceInvocationForTesting() {
+    invoke(MakeGarbageCollected<IdleDeadline>(
+        base::TimeTicks::Now() + kFindTaskTestTimeout,
+        IdleDeadline::CallbackType::kCalledWhenIdle));
   }
 
-  void Trace(Visitor* visitor) {
+  void Trace(Visitor* visitor) override {
     visitor->Trace(controller_);
     visitor->Trace(document_);
+    ScriptedIdleTaskController::IdleTask::Trace(visitor);
   }
 
-  void Invoke() {
+ private:
+  void invoke(IdleDeadline* deadline) override {
     const base::TimeTicks task_start_time = base::TimeTicks::Now();
     if (!controller_)
       return;
-    if (!controller_->ShouldFindMatches(identifier_, search_text_, *options_)) {
+    if (!controller_->ShouldFindMatches(search_text_, *options_)) {
       controller_->DidFinishTask(identifier_, search_text_, *options_,
                                  true /* finished_whole_request */,
                                  PositionInFlatTree(), 0 /* match_count */,
                                  true /* aborted */, task_start_time);
+      return;
     }
     SCOPED_UMA_HISTOGRAM_TIMER("WebCore.FindInPage.TaskDuration");
 
@@ -105,7 +119,6 @@ class FindTaskController::FindTask final : public GarbageCollected<FindTask> {
         (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.
@@ -138,8 +151,7 @@ class FindTaskController::FindTask final : public GarbageCollected<FindTask> {
         break;
       }
       next_task_start_position = search_start;
-      auto time_elapsed = base::TimeTicks::Now() - start_time;
-      if (time_elapsed > kFindTaskTimeAllotment)
+      if (deadline->timeRemaining() <= 0)
         break;
     }
     controller_->DidFinishTask(identifier_, search_text_, *options_,
@@ -150,6 +162,7 @@ class FindTaskController::FindTask final : public GarbageCollected<FindTask> {
 
   Member<Document> document_;
   Member<FindTaskController> controller_;
+  int callback_handle_ = 0;
   const int identifier_;
   const WebString search_text_;
   mojom::blink::FindOptionsPtr options_;
@@ -165,40 +178,41 @@ 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;
-  current_find_identifier_ = identifier;
-  RequestFindTask(identifier, search_text, options);
+  RequestIdleFindTask(identifier, search_text, options);
 }
 
 void FindTaskController::CancelPendingRequest() {
-  if (find_task_)
+  if (find_task_) {
+    find_task_->Dispose();
     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::RequestFindTask(
+void FindTaskController::RequestIdleFindTask(
     int identifier,
     const WebString& search_text,
     const mojom::blink::FindOptions& options) {
   DCHECK_EQ(find_task_, nullptr);
-  DCHECK_EQ(identifier, current_find_identifier_);
-  task_count_for_current_request_++;
-  find_task_ = MakeGarbageCollected<FindTask>(
+  find_task_ = MakeGarbageCollected<IdleFindTask>(
       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(
@@ -210,8 +224,10 @@ void FindTaskController::DidFinishTask(
     int match_count,
     bool aborted,
     base::TimeTicks task_start_time) {
-  if (current_find_identifier_ != identifier)
-    return;
+  if (find_task_) {
+    find_task_->Dispose();
+    find_task_.Clear();
+  }
   total_task_duration_for_current_request_ +=
       base::TimeTicks::Now() - task_start_time;
   if (find_task_)
@@ -234,24 +250,20 @@ void FindTaskController::DidFinishTask(
 
   if (!finished_whole_request) {
     // Task ran out of time, request for another one.
-    RequestFindTask(identifier, search_text, options);
+    RequestIdleFindTask(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_ =
-      !aborted && !current_match_count_;
+  last_find_request_completed_with_no_matches_ = !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_);
@@ -278,11 +290,8 @@ 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();

third_party/blink/renderer/core/editing/finder/find_task_controller.h

@@ -9,16 +9,9 @@
 #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;
@@ -52,11 +45,10 @@ 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(int identifier,
-                         const String& search_text,
+  bool ShouldFindMatches(const String& search_text,
                          const mojom::blink::FindOptions& options);
 
-  // During a run of |idle_find_task|, we found a match.
+  // During a run of |find_task|, we found a match.
   // Updates |current_match_count_| and notifies |text_finder_|.
   void DidFindMatch(int identifier, Range* result_range);
 
@@ -77,20 +69,16 @@ 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 FindTask;
+  class IdleFindTask;
 
   void Trace(Visitor* visitor);
 
   void ResetLastFindRequestCompletedWithNoMatches();
 
-  void InvokeFind(int identifier,
-                  const WebString& search_text_,
-                  mojom::blink::FindOptionsPtr options_);
-
  private:
-  void RequestFindTask(int identifier,
-                       const WebString& search_text,
-                       const mojom::blink::FindOptions& options);
+  void RequestIdleFindTask(int identifier,
+                           const WebString& search_text,
+                           const mojom::blink::FindOptions& options);
 
   enum class RequestEndState {
     // The find-in-page request got aborted before going through every text in
@@ -107,7 +95,7 @@ class CORE_EXPORT FindTaskController final
 
   Member<TextFinder> text_finder_;
 
-  Member<FindTask> find_task_;
+  Member<IdleFindTask> find_task_;
 
   // Keeps track if there is any ongoing find effort or not.
   bool finding_in_progress_;
@@ -127,10 +115,6 @@ 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.