New issue
Advanced search Search tips

Issue 824792 link

Starred by 1 user
Status: Fixed
Owner:
robertma@chromium.org
Closed: Mar 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Frame load callbacks are accidentally dumped in some layout tests (e.g. wpt)

Project Member Reported by robertma@chromium.org, Mar 22 2018 
https://cs.chromium.org/chromium/src/content/shell/test_runner/test_interfaces.cc?l=99&rcl=8ace0e3a1a09c4a0c764c8a8af9d189b4d211111

The line above sets ShouldDumpFrameLoadCallbacks when 'loading/' is found in the URL of a test.

I think this was meant for http/tests/loading/*. The pattern is overly broad and also matches stuff like external/wpt/css/css-font-loading/*, which causes frame load callbacks to be dumped unnecessarily and makes the tests flaky.

Example flake: https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=webkit_layout_tests&tests=external%2Fwpt%2Fcss%2Fcss-font-loading%2Fidlharness.https.html
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 23 2018 

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

commit 5648f52c5dde1b11e20a6e9414f0da97ad2ea574
Author: Robert Ma <robertma@chromium.org>
Date: Fri Mar 23 09:59:55 2018

[test_runner] Only dump frame load callbacks in /loading/

The target directory is http/tests/loading/, which is served via httpd
and becomes /loading/. The previous pattern was too broad and missed
the leading slash, which would accidentally match other places like
external/wpt/css/css-font-loading.

Bug:  824792 
Change-Id: I4029b3848b590fe1fe431a58bf40c560ae5f9b56
Reviewed-on: https://chromium-review.googlesource.com/975903
Commit-Queue: Philip Jägenstedt <foolip@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545397}
[modify] https://crrev.com/5648f52c5dde1b11e20a6e9414f0da97ad2ea574/content/shell/test_runner/test_interfaces.cc
[modify] https://crrev.com/5648f52c5dde1b11e20a6e9414f0da97ad2ea574/third_party/WebKit/LayoutTests/external/wpt/css/css-font-loading/idlharness.https-expected.txt
[delete] https://crrev.com/fe008b46c2e7b2c52876c77b1747606ed2af3b56/third_party/WebKit/LayoutTests/platform/linux/external/wpt/css/css-font-loading/idlharness.https-expected.txt
[delete] https://crrev.com/fe008b46c2e7b2c52876c77b1747606ed2af3b56/third_party/WebKit/LayoutTests/platform/win/external/wpt/css/css-font-loading/idlharness.https-expected.txt
[delete] https://crrev.com/fe008b46c2e7b2c52876c77b1747606ed2af3b56/third_party/WebKit/LayoutTests/platform/win7/external/wpt/css/css-font-loading/idlharness.https-expected.txt

Comment 2 by robertma@chromium.org, Mar 23 2018

Status: Fixed (was: Started)
►

Sign in to add a comment