New issue
Advanced search Search tips

Issue 893465 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 9
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 890622



Sign in to add a comment

Idle tasks for some frames are not fired.

Project Member Reported by rakina@chromium.org, Oct 9

Issue description

Find-in-page was recently refactored to use idle tasks for counting the number of matches in all frames. However some frames' idle task never ran which means they don't send any result or update to the browser, leading to bugs such as https://bugs.chromium.org/p/chromium/issues/detail?id=890622 that depend on having all the result from all of the frames. 
 
Components: Blink>Scheduling
I wrote the script to register idle task to IFRAME and main window.

Only jsfiddle works as expected == console prints "iframe1", "iframg2", "main"
Note: Firefox works as expected for all vases.

Note: Idle task is pushed into thread's schedule queue instead of per-frame  see
ScriptedIdleTaskController::ScheduleCallback()

* https://jsfiddle.net/7gqhy5mu/
 => "iframe1", "iframe2" "main"
* https://jsbin.com/kedewodeke/edit?html,js,output
 => "main"
* file://c://tmp/bar.html (local file)
 => "main"
* http://127.0.0.1:8000 by python -m SimpleHTTPServer
 => "main"

<iframe id=iframe1 srcdoc="foo"></iframe>
<iframe id=iframe2 srcdoc="bar"></iframe>
<script>
const iframe1 = document.getElementById('iframe1');
const iframe2 = document.getElementById('iframe2');

iframe1.contentWindow.requestIdleCallback(() => console.log('iframe1'));
iframe2.contentWindow.requestIdleCallback(() => console.log('iframe2'));
window.requestIdleCallback(() => console.log('main'));
</script>


void ScriptedIdleTaskController::ScheduleCallback(
    scoped_refptr<internal::IdleRequestCallbackWrapper> callback_wrapper,
    long long timeout_millis) {
  scheduler_->PostIdleTask(
      FROM_HERE, WTF::Bind(&internal::IdleRequestCallbackWrapper::IdleTaskFired,
                           callback_wrapper));
  if (timeout_millis > 0) {
    GetExecutionContext()
        ->GetTaskRunner(TaskType::kIdleTask)
        ->PostDelayedTask(
            FROM_HERE,
            WTF::Bind(&internal::IdleRequestCallbackWrapper::TimeoutFired,
                      callback_wrapper),
            TimeDelta::FromMilliseconds(timeout_millis));
  }
}



When I changed the script in #c2 to call requestIdelCallback() after window.onload, All sites work as expected.

<iframe id=iframe1 srcdoc="foo"></iframe>
<iframe id=iframe2 srcdoc="bar"></iframe>
<script>
const iframe1 = document.getElementById('iframe1');
const iframe2 = document.getElementById('iframe2');

window.onload = () => {
  iframe1.contentWindow.requestIdleCallback(() => console.log('iframe1'));
  iframe2.contentWindow.requestIdleCallback(() => console.log('iframe2'));
  window.requestIdleCallback(() => console.log('main'));
};
</script>

According to #3, I guess requestIdleCallback doesn't work when the document doesn't exist yet.
Owner: skyos...@chromium.org
Find-in-Page request idle time callback as follows:


"third_party/blink/renderer/core/editing/finder/text_finder.cc"
  IdleScopeStringMatchesCallback(TextFinder* text_finder,
                                 int identifier,
                                 const WebString& search_text,
                                 const mojom::blink::FindOptions& options)
      : text_finder_(text_finder),... {
    callback_handle_ =
        text_finder->GetFrame()->GetDocument()->RequestIdleCallback(
            this, IdleRequestOptions());
  }
Blocking: 890622
Cc: rakina@chromium.org
Labels: -Pri-2 Pri-1
Just changing the deadline of the posted tasks for Find-in-page (and nothing else) makes the linked Find-in-page bug not reproduce anymore (https://chromium-review.googlesource.com/c/chromium/src/+/1270196). I've checked the calls to the task creation call and all of the frames got to the creation of the task for the frame, but only a few frames' task eventually fires.

I'll post the call stack with explanation soon.
The part of Find-in-page that posts the idle task is the IdleScopeStringMatchesCallback constructor in text_finder.cc. https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/editing/finder/text_finder.cc?sq=package:chromium&g=0&l=108.

Find-in-page is done independently for each frame, and each frame owns a TextFinder that manages the idle task. The idle task's job is to go through the content of its owning frame, finding matches. When a FIP request happens, all frames in the webpage will create an idle task, except those that doesn't have visible content (through this check https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/frame/find_in_page.cc?q=find_in_page.cc&sq=package:chromium&g=0&l=81)

I tried doing Find-in-page in the example page in the original FIP bug: https://blog.cloudflare.com/esni/, after the page is loaded and I haven't interacted with the page yet (Ctrl+F immediately), with the text "green" (not typed, but copy pasted the whole word directly to make sure it's the first request instead of g -> gr -> gre -> ...) that exists in the comment section on the page.

There were 9 RequestIdleCallback calls from the FIP code, and only 3 got invoked. After that I scrolled a bit until around the top image part of the article, and 4 other callbacks got invoked. After I scrolled nearing the comment/disqus iframe, the callback for the comment section got called and it updates the amount of find-in-page matches to 1. Even after I scrolled back and forth through the whole page, at the end only 8 callbacks got invoked. No Dispose/CancelIdleCallback call is ever done in TextFinder. 

Is this expected behavior? Why are the idle tasks not done until scrolled into, and why at the end there are some remaining callbacks that are never called?

BTW, if I add a deadline of 100ms to the idle tasks they are working as intended, all callbacks are called without any interaction to the page and yields correct result. https://chromium-review.googlesource.com/c/chromium/src/+/1270196
Note: Requesting idle task via IFRAME.contentWindow works as expected:
http://jsfiddle.net/z6kqu5wr/

Components: -Blink>Scheduling Blink>Editing>Serialization
Owner: rakina@chromium.org
Status: Assigned (was: Untriaged)
It seems the root cause is https://disqus.com/embed/comments/.
We should investigate why disqus.com prevents not to make idle time.

In devtool's Performance view, there are idle periods. Or Find-in-page fails to handle contents change.

It seems loading contents from discus.com takes more than 1000ms.

Thanks for digging into this! It might be useful to record a Chrome trace[1] to check whether all CPU time is indeed getting used for non-idle work. Since we're competing with arbitrary web content here, setting a deadline -- say 1 second -- for the idle tasks seems appropriate.

[1] https://www.chromium.org/developers/how-tos/trace-event-profiling-tool/recording-tracing-runs
Trace for trying to find "green" in the page https://blog.cloudflare.com/esni/ is "trace_fip.json.gz"
Trace for trying to find "rfc" while focusing not on the article frame (but on the comment frame) is "trace_rfc.json.gz"

I tried it on canary and it looks like it took a few seconds for the "green" one got all the frames' response at the end, but the "rfc" one is still waiting for some frame's response after a while (meaning idle tasks for frame with no reply are not run)

I do think adding a long enough deadline, like 1 second, is reasonable to make sure that even on sites with a lot of loads, FIP will get the right result in a fairly reasonable time instead of waiting a long time to get idle time.
trace_fip.json.gz
6.1 MB Download
trace_rfc.json.gz
9.6 MB Download
Project Member

Comment 12 by bugdroid1@chromium.org, Oct 15

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0df2607f9809b189f7b7177df1d19d7cf8429f3c

commit 0df2607f9809b189f7b7177df1d19d7cf8429f3c
Author: Rakina Zata Amni <rakina@chromium.org>
Date: Mon Oct 15 03:58:13 2018

Add deadline to Find-in-page idle tasks

In some cases such as multiple frame webpages, idle task might never
execute or take a noticably long time to execute. This might lead to
incorrect number of find-in-page result and not highlighting matches
because the scoping is never trigerred. This CL adds a deadline to
scoping idle tasks to ensure all scoping happens in a timely manner.

Bug:  890622 , 893465 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I8ec998d4c2f42377361c1f7b95c1497a80368ef6
Reviewed-on: https://chromium-review.googlesource.com/c/1270196
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599562}
[modify] https://crrev.com/0df2607f9809b189f7b7177df1d19d7cf8429f3c/third_party/blink/renderer/core/editing/finder/text_finder.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Oct 16

Labels: merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/370d5b4e71b5d71106bbd3f89817df4cce97b299

commit 370d5b4e71b5d71106bbd3f89817df4cce97b299
Author: Rakina Zata Amni <rakina@chromium.org>
Date: Tue Oct 16 22:21:08 2018

Add deadline to Find-in-page idle tasks

In some cases such as multiple frame webpages, idle task might never
execute or take a noticably long time to execute. This might lead to
incorrect number of find-in-page result and not highlighting matches
because the scoping is never trigerred. This CL adds a deadline to
scoping idle tasks to ensure all scoping happens in a timely manner.

Bug:  890622 , 893465 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I8ec998d4c2f42377361c1f7b95c1497a80368ef6
Reviewed-on: https://chromium-review.googlesource.com/c/1270196
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#599562}(cherry picked from commit 0df2607f9809b189f7b7177df1d19d7cf8429f3c)
Reviewed-on: https://chromium-review.googlesource.com/c/1285149
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#69}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/370d5b4e71b5d71106bbd3f89817df4cce97b299/third_party/blink/renderer/core/editing/finder/text_finder.cc

Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/370d5b4e71b5d71106bbd3f89817df4cce97b299

Commit: 370d5b4e71b5d71106bbd3f89817df4cce97b299
Author: rakina@chromium.org
Commiter: rakina@chromium.org
Date: 2018-10-16 22:21:08 +0000 UTC

Add deadline to Find-in-page idle tasks

In some cases such as multiple frame webpages, idle task might never
execute or take a noticably long time to execute. This might lead to
incorrect number of find-in-page result and not highlighting matches
because the scoping is never trigerred. This CL adds a deadline to
scoping idle tasks to ensure all scoping happens in a timely manner.

Bug:  890622 , 893465 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I8ec998d4c2f42377361c1f7b95c1497a80368ef6
Reviewed-on: https://chromium-review.googlesource.com/c/1270196
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#599562}(cherry picked from commit 0df2607f9809b189f7b7177df1d19d7cf8429f3c)
Reviewed-on: https://chromium-review.googlesource.com/c/1285149
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#69}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
Status: Fixed (was: Assigned)

Sign in to add a comment