Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(112)

Issue 267553002: FCMify two text-based-repaint tests (Closed)

Created:
6 years, 7 months ago by enne (OOO)
Modified:
6 years, 7 months ago
Reviewers:
ojan, abarth-chromium
CC:
blink-reviews, shans, rjwright, alancutter (OOO until 2018), Mike Lawther (Google), dstockwell, Timothy Loh, darktears, Steve Block, dino_apple.com, Eric Willigers, leviw_travelin_and_unemployed
Visibility:
Public.

Description

FCMify two text-based-repaint tests A previous patch (https://codereview.chromium.org/252353002) tried to make all text-based-repaint tests use FCM, but caused flakiness reported in http://crbug.com/367195 and http://crbug.com/367201. It seems like the issue is that FCM wasn't being enabled soon enough. It was turned on post-onload, after which some repaints may have been generated. (This may no longer be true with repaint-after-layout.) The fix is to enable FCM with the script include, rather than in runRepaintTest. Rather than causing churn on the entire tree, land an FCM version of text-based-repaint.js and use it in two previous flaky tests. This should indicate whether or not this was actually the problem. BUG=368518 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=173040

Patch Set 1 #

Patch Set 2 : Remove image changes #

Patch Set 3 : Two flaky non-image tests #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+47 lines, -26 lines) Patch
M LayoutTests/fast/images/repaint-subrect-grid.html View 1 2 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/fast/images/repaint-subrect-grid-expected.txt View 1 2 1 chunk +31 lines, -23 lines 0 comments Download
A + LayoutTests/fast/repaint/resources/text-based-repaint-fcm.js View 2 chunks +4 lines, -1 line 0 comments Download
M LayoutTests/svg/dynamic-updates/SVGFEComponentTransferElement-dom-slope-attr.html View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/svg/dynamic-updates/SVGFEComponentTransferElement-dom-slope-attr-expected.txt View 1 chunk +10 lines, -0 lines 2 comments Download

Messages

Total messages: 6 (0 generated)
enne (OOO)
6 years, 7 months ago (2014-04-30 22:37:17 UTC) #1
abarth-chromium
LGTM https://codereview.chromium.org/267553002/diff/40001/LayoutTests/svg/dynamic-updates/SVGFEComponentTransferElement-dom-slope-attr-expected.txt File LayoutTests/svg/dynamic-updates/SVGFEComponentTransferElement-dom-slope-attr-expected.txt (right): https://codereview.chromium.org/267553002/diff/40001/LayoutTests/svg/dynamic-updates/SVGFEComponentTransferElement-dom-slope-attr-expected.txt#newcode28 LayoutTests/svg/dynamic-updates/SVGFEComponentTransferElement-dom-slope-attr-expected.txt:28: ) Its it a problem that this text ...
6 years, 7 months ago (2014-04-30 22:43:09 UTC) #2
enne (OOO)
https://codereview.chromium.org/267553002/diff/40001/LayoutTests/svg/dynamic-updates/SVGFEComponentTransferElement-dom-slope-attr-expected.txt File LayoutTests/svg/dynamic-updates/SVGFEComponentTransferElement-dom-slope-attr-expected.txt (right): https://codereview.chromium.org/267553002/diff/40001/LayoutTests/svg/dynamic-updates/SVGFEComponentTransferElement-dom-slope-attr-expected.txt#newcode28 LayoutTests/svg/dynamic-updates/SVGFEComponentTransferElement-dom-slope-attr-expected.txt:28: ) On 2014/04/30 22:43:10, abarth wrote: > Its it ...
6 years, 7 months ago (2014-04-30 22:46:49 UTC) #3
enne (OOO)
The CQ bit was checked by enne@chromium.org
6 years, 7 months ago (2014-04-30 22:55:46 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/enne@chromium.org/267553002/40001
6 years, 7 months ago (2014-04-30 22:56:19 UTC) #5
commit-bot: I haz the power
6 years, 7 months ago (2014-04-30 23:21:31 UTC) #6
Message was sent while issue was closed.
Change committed as 173040

Powered by Google App Engine
This is Rietveld 408576698