Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify image-loading-subpixel-clip.html. #22403

Merged
merged 1 commit into from Mar 25, 2020

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

Verified that it still fails with [1] applied and [2] reverted.

This may help with deflaking WPT [3]

[1] https://chromium-review.googlesource.com/c/chromium/src/+/2111906
[2] https://chromium-review.googlesource.com/c/chromium/src/+/2092698
[3] #22364

TBR:szager@chromium.org

Change-Id: Ie668c43c4d03ea442c37075c0e80be5ba1481fff
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2115932
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Stefan Zager <szager@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#752511}

Verified that it still fails with [1] applied and [2] reverted.

This may help with deflaking WPT [3]

[1] https://chromium-review.googlesource.com/c/chromium/src/+/2111906
[2] https://chromium-review.googlesource.com/c/chromium/src/+/2092698
[3] #22364

TBR:szager@chromium.org

Change-Id: Ie668c43c4d03ea442c37075c0e80be5ba1481fff
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2115932
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Stefan Zager <szager@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#752511}
Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The review process for this patch is being conducted in the Chromium project.

@KyleJu
Copy link
Contributor

KyleJu commented Mar 24, 2020

@chrishtr Looks like this test still times out on Firefox and Chrome. Would you mind taking a second look?

Test Subtest Results Messages
/html/semantics/embedded-content/the-img-element/image-loading-subpixel-clip.html PASS: 8/10, TIMEOUT: 2/10

@chrishtr
Copy link
Contributor

Will try to repro now.

@stephenmcgruer
Copy link
Contributor

stephenmcgruer commented Mar 25, 2020

If it helps, I can reproduce on a fresh checkout of WPT via:

$ git checkout chromium-export-bb6643e9d8
$ ./wpt run --log-mach-level=info --log-mach=- --verify --verify-log-full --channel=experimental chrome html/semantics/embedded-content/the-img-element/image-loading-subpixel-clip.html 
...
 1:29.90 INFO ## Unstable results ##

 1:29.90 INFO |                                         Test                                        | Subtest |            Results            | Messages |
 1:29.90 INFO |-------------------------------------------------------------------------------------|---------|-------------------------------|----------|
 1:29.90 INFO | `/html/semantics/embedded-content/the-img-element/image-loading-subpixel-clip.html` |         | **PASS: 6/10, TIMEOUT: 4/10** |          |

Looking at the test - does target.onload fire even if the target has already loaded (if that is possible) when that script executes?

EDIT: if you want to pass a specific chrome binary in, you can pass the --binary=/path/to/chrome flag to wpt run. Note that you still need to set the 'product' as chrome, e.g. it looks like:

./wpt run --binary=/path/to/chrome --log-mach-level=info --log-mach=- --verify --verify-log-full --channel=experimental chrome html/semantics/embedded-content/the-img-element/image-loading-subpixel-clip.html

@chrishtr
Copy link
Contributor

@stephenmcgruer I think you figured it out thanks. I reproduced the flakiness locally and I think
fixed it by setting the src attribute after adding the onload handler. I'll make a Chromium patch
now.

@stephenmcgruer
Copy link
Contributor

Glad to hear it. Admin-merging this so that the following patch can be successfully exported. Note that you can check on the stabiltity checks/etc before merging a patch Chromium side. After sending it out for review look for the wpt-pr-bot to comment on it saying its been uploaded to GH (should contain a link to the PR) :).

@stephenmcgruer stephenmcgruer merged commit a82dbcb into master Mar 25, 2020
@stephenmcgruer stephenmcgruer deleted the chromium-export-bb6643e9d8 branch March 25, 2020 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants