New issue
Advanced search Search tips

Issue 836295 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug



Sign in to add a comment

reinserting-svg-into-document.html flaky on win

Project Member Reported by xidac...@chromium.org, Apr 24 2018

Issue description

Project Member

Comment 1 by bugdroid1@chromium.org, Apr 24 2018

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

commit 63b09b6990567ec7162d8105b0b45a374ffca19c
Author: Xida Chen <xidachen@chromium.org>
Date: Tue Apr 24 17:04:55 2018

Mark reinserting-svg-into-document.html flaky on win

TBR=thestig@chromium.org
NOTRY=true

Bug:  836295 
Change-Id: Ie96425b623849d7ca24bfba40b472605be01903f
Reviewed-on: https://chromium-review.googlesource.com/1026158
Reviewed-by: Xida Chen <xidachen@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553169}
[modify] https://crrev.com/63b09b6990567ec7162d8105b0b45a374ffca19c/third_party/WebKit/LayoutTests/TestExpectations

Components: Blink>Animation
Labels: Test-Layout Test-Flaky OS-Windows
Looks like an old test that still uses setTimeout (actually, looks like a *lot* of the svg tests still do :(...) and on Windows the timeout was delayed.

I will attempt to figure out a way to fix the test.
Status: Started (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 26 2018

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

commit 3c2d1550cb8ace4b6511679dc1a8f8b4c67d8484
Author: Stephen McGruer <smcgruer@chromium.org>
Date: Thu Apr 26 15:02:20 2018

Fix flake in svg/animations/reinserting-svg-into-document.html

This test was very old, and appeared to rely on setTimeout(func, 1)
taking close enough to 1ms for the following to hold:

setCurrentTime(1);  // Note this is seconds.
setTimeout(function() {
  getCurrentTime() ~= 1
}, 1);

This is not guaranteed by setTimeout, however, and occasionally
in LayoutTest runs the setTimeout took long enough that the behavior
didn't hold. This CL updates the test to only require that the
currentTime is >= what was previously set, which will hold.

Alongside this, I also:

  * Converted the test to the modern jsharness style,
  * Removed the outer setTimeout which didn't seem required at all, and
  * Replaced the inner setTimeout with a requestAnimationFrame

Bug:  836295 

Change-Id: I8d5fc2b3ed544b3085b999efeb37979e8d070a5b
Reviewed-on: https://chromium-review.googlesource.com/1025855
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554031}
[modify] https://crrev.com/3c2d1550cb8ace4b6511679dc1a8f8b4c67d8484/third_party/WebKit/LayoutTests/TestExpectations
[delete] https://crrev.com/544122c6009ae3e632e9346223716df3e83ee68d/third_party/WebKit/LayoutTests/svg/animations/reinserting-svg-into-document-expected.txt
[modify] https://crrev.com/3c2d1550cb8ace4b6511679dc1a8f8b4c67d8484/third_party/WebKit/LayoutTests/svg/animations/reinserting-svg-into-document.html

Status: Fixed (was: Started)

Sign in to add a comment