New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 872239 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 21
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 867133



Sign in to add a comment

maps_pixel_test captures screenshot too early

Project Member Reported by altimin@chromium.org, Aug 8

Issue description

In maps_pixel_test new tiles are obtained from MessagePort. Before the recent scheduling change MessagePort events starved out the rest of the work by continuing to process events until we run out of them. 

This was fixed in crbug.com/867133 and message port events started to yield properly to other tasks, including compositing and requestAnimationFrames. 

Previously, the _SpinWaitOnRAF(3) hack was employed, which was brittle and stopped working now.
 
Blocking: 867133
Labels: -Pri-2 Needs-Feedback Pri-1
I disagree with this diagnosis. maps_integration_test.py is a simple harness around the Maps team's test, which renders roughly 600 frames (10 seconds at 60 FPS) before setting the JavaScript variable window.testDone to true. Our harness waits for window.testDone to be set:

https://cs.chromium.org/chromium/src/content/test/gpu/gpu_tests/maps_integration_test.py?q=_SpinWaitOnRAF&sq=package:chromium&g=0&l=99

before calling this _SpinWaitOnRAF hack.

At the time _SpinWaitOnRAF is called, there isn't (or shouldn't be) any deferred JavaScript work going on.

Have you actually demonstrated that this is the problem? Happy to change the harness if so, but the intent of waiting for three rAFs was to make sure that all of the rendered frames have propagated through Chrome's graphics pipeline, and just waiting for one idle callback won't necessarily do that.

Asked the same question on https://chromium-review.googlesource.com/1167053 .

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 15

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

commit e3495620c3ca9677f073295fa48fa2c40978ef2e
Author: Alexander Timin <altimin@chromium.org>
Date: Wed Aug 15 16:04:09 2018

Reland [blink] Add a time limit to message processing in MessagePort

In addition to existing limit of 200 messages in a single task also
limit it to 50 milliseconds to avoid starvation.

This logic will be removed when message-per-task mojo dispatching
will be implemented.

This patch also modifies maps_pixel_test to use requestIdleCallback
for detecting the end of the page load instead of three requestAnimationFrames.

BUG=867133, 872239 
R=mek@chromium.org,kbr@chromium.org

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: Ib2de605bc6d069c23c3a991edcc0dccc995a4632
Reviewed-on: https://chromium-review.googlesource.com/1167053
Commit-Queue: Alexander Timin <altimin@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583267}
[modify] https://crrev.com/e3495620c3ca9677f073295fa48fa2c40978ef2e/content/test/gpu/gpu_tests/maps_integration_test.py
[modify] https://crrev.com/e3495620c3ca9677f073295fa48fa2c40978ef2e/third_party/blink/renderer/core/messaging/message_port.cc
[modify] https://crrev.com/e3495620c3ca9677f073295fa48fa2c40978ef2e/third_party/blink/renderer/core/messaging/message_port.h

Cc: kbr@chromium.org
Owner: altimin@chromium.org
Status: Fixed (was: Assigned)
It looks like this issue was fixed in https://chromium-review.googlesource.com/1167053 so reassigning and closing.

Sign in to add a comment