Commit d5830561 authored by Mike West's avatar Mike West Committed by Commit Bot

Revert "Fix false-positives of under-invalidation checking in layout tests"

This reverts commit 206bdc85.

Reason for revert:

Several paint-related tests have begun crashing on "Linux Trusty (dbg)" after landing this patch, flakily hitting a CHECK in
`PaintController.cpp` (see the log in 
https://storage.googleapis.com/chromium-layout-test-archives/WebKit_Linux_Trusty__dbg_/5565/layout-test-results/paint/invalidation/video-mute-repaint-stderr.txt).
It looks like this kind of crash happened while landing the patch as
well, at least on https://storage.googleapis.com/chromium-layout-test-archives/linux_layout_tests_slimming_paint_v2/6682/layout-test-results/results.html).
I'll revert it.


Original change's description:
> Fix false-positives of under-invalidation checking in layout tests
> 
> In the following few cases we intentionally allow under-invalidations in
> cached subsequences:
> - offscreen image animation
> - media buffered range
> 
> We intentionally don't update each time the contents change to improve
> performance or avoid complex implementation of real time change
> notification.
> 
> Now allow cache skipping in cached subsequences.
> 
> Enable under-invalidation checking for tests that would have reported
> under-invalidation with the checking enabled.
> 
> This also helps clusterfuzz not to trigger under-invaldiation checking
> failures when it creates a test for the above cases.
> 
> Bug: 769729
> Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> Change-Id: I2149e9d2304dbad5d7486c822d5452c5dba237fe
> Reviewed-on: https://chromium-review.googlesource.com/690851
> Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
> Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#505281}

TBR=wangxianzhu@chromium.org,chrishtr@chromium.org

Change-Id: Id0ddbc90d9cf4436fe10dc81485d9f13edef6f1a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 769729, 769879
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Reviewed-on: https://chromium-review.googlesource.com/691814Reviewed-by: default avatarMike West <mkwst@chromium.org>
Commit-Queue: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505327}
parent fcf2d29f
......@@ -1116,7 +1116,6 @@ Bug(none) paint/invalidation/compositing/iframe-inside-squashed-layer.html [ Fai
Bug(none) paint/invalidation/compositing/overlap-test-with-filter.html [ Failure ]
Bug(none) paint/invalidation/compositing/should-not-repaint-composited-descendants.html [ Failure ]
Bug(none) paint/invalidation/filter-repaint-accelerated-child-with-filter-child.html [ Failure ]
Bug(none) paint/invalidation/video-paint-invalidation.html [ Failure ]
# Need to support partial invalidation for some changes.
crbug.com/732612 paint/invalidation/canvas-composite-repaint-by-all-imagesource.html [ Failure ]
......@@ -1308,6 +1307,7 @@ crbug.com/768102 compositing/overflow/reparented-overlay-scrollbars-should-respe
crbug.com/644358 fast/css3-text/css3-text-decoration/repaint/repaint-text-decoration-style.html [ Failure ]
crbug.com/644358 paint/invalidation/svg/image-with-clip-path.svg [ Failure ]
crbug.com/644358 paint/invalidation/video-paint-invalidation.html [ Crash ]
Bug(none) compositing/layer-creation/main-thread-scrolling-for-non-composited-fixed-position-if-overflow-hidden.html [ Failure ]
Bug(none) compositing/layer-creation/main-thread-scrolling-for-non-composited-fixed-position.html [ Failure ]
......
......@@ -1534,6 +1534,7 @@ crbug.com/605059 [ Retina ] fast/text/international/rtl-negative-letter-spacing.
crbug.com/610464 [ Linux Win7 Debug ] http/tests/devtools/components/throttler.html [ Failure Pass ]
crbug.com/667560 [ Linux Win7 Debug ] virtual/mojo-loading/http/tests/devtools/components/throttler.html [ Failure Pass ]
crbug.com/654477 paint/invalidation/video-paint-invalidation.html [ Failure Crash ]
crbug.com/654477 [ Win ] compositing/video/video-controls-layer-creation.html [ Pass Failure ]
crbug.com/654477 fast/hidpi/video-controls-in-hidpi.html [ Failure ]
crbug.com/654477 fast/layers/video-layer.html [ Failure ]
......
<script src="../resources/run-after-layout-and-paint.js"></script>
<script>
// Disable under-invalidation checking because we allow short period of
// under-invalidation of buffered ranges.
if (window.internals)
internals.runtimeFlags.paintUnderInvalidationCheckingEnabled = false;
if (window.testRunner)
testRunner.waitUntilDone();
......@@ -33,4 +38,4 @@
<iframe src="content/silence.wav" id="fr" width=380 height=330 onload="frameLoaded()"></iframe>
<script>// To produce the same layout as before iframe was moved down to avoid rebaselines of different platforms.
// https://bugs.webkit.org/show_bug.cgi?id=54942
</script>
</script>
\ No newline at end of file
......@@ -7,6 +7,11 @@
<script>
window.testIsAsync = true;
// Disable under-invalidation checking because the "under-invalidation" of
// offscreen gif animation is intentional.
if (window.internals)
internals.runtimeFlags.paintUnderInvalidationCheckingEnabled = false;
function repaintTest() {
if (window.internals)
internals.advanceImageAnimation(targetImage);
......@@ -25,4 +30,4 @@ onload = function() {
targetImage.onload = targetImageOnload;
targetImage.src = "../../fast/backgrounds/resources/red-green-animated.gif";
}
</script>
</script>
\ No newline at end of file
......@@ -7,6 +7,11 @@
<script>
window.testIsAsync = true;
// Disable under-invalidation checking because the "under-invalidation" of
// offscreen gif animation is intentional.
if (window.internals)
internals.runtimeFlags.paintUnderInvalidationCheckingEnabled = false;
function repaintTest() {
if (window.internals)
internals.advanceImageAnimation(targetImage);
......@@ -24,4 +29,4 @@ window.onload = function() {
targetImage.onload = targetImageOnload;
targetImage.src="../../fast/backgrounds/resources/red-green-animated.gif";
}
</script>
</script>
\ No newline at end of file
......@@ -7,6 +7,11 @@
<script>
window.testIsAsync = true;
// Disable under-invalidation checking because the "under-invalidation" of
// offscreen apng animation is intentional.
if (window.internals)
internals.runtimeFlags.paintUnderInvalidationCheckingEnabled = false;
function repaintTest() {
if (window.internals)
internals.advanceImageAnimation(targetImage);
......
......@@ -7,6 +7,11 @@
<script>
window.testIsAsync = true;
// Disable under-invalidation checking because the "under-invalidation" of
// offscreen webp animation is intentional.
if (window.internals)
internals.runtimeFlags.paintUnderInvalidationCheckingEnabled = false;
function repaintTest() {
if (window.internals)
internals.advanceImageAnimation(targetImage);
......
......@@ -3,6 +3,11 @@
<script>
window.testIsAsync = true;
// Disable under-invalidation checking because the "under-invalidation" of
// offscreen SVG animation is intentional.
if (window.internals)
internals.runtimeFlags.paintUnderInvalidationCheckingEnabled = false;
function repaintTest() {
if (window.internals)
internals.advanceImageAnimation(testTarget);
......
......@@ -3,6 +3,11 @@
<script>
window.testIsAsync = true;
// Disable under-invalidation checking because the "under-invalidation" of
// offscreen SVG animation is intentional.
if (window.internals)
internals.runtimeFlags.paintUnderInvalidationCheckingEnabled = false;
function repaintTest() {
if (window.internals)
internals.advanceImageAnimation(targetImage);
......
......@@ -3,6 +3,11 @@
<script>
window.testIsAsync = true;
// Disable under-invalidation checking because the "under-invalidation" of
// offscreen SVG animation is intentional.
if (window.internals)
internals.runtimeFlags.paintUnderInvalidationCheckingEnabled = false;
function repaintTest() {
if (window.internals)
internals.advanceImageAnimation(targetImage);
......
......@@ -10,6 +10,11 @@
</video>
<script>
// Disable under-invalidation checking because we allow short period of
// under-invalidation of buffered ranges.
if (window.internals)
internals.runtimeFlags.paintUnderInvalidationCheckingEnabled = false;
window.testIsAsync = true;
var video = document.getElementById("v");
......
......@@ -12,6 +12,11 @@
<p>This tests that we repaint the mute button when we change the volume</p>
<script>
// Disable under-invalidation checking because we allow short period of
// under-invalidation of buffered ranges.
if (window.internals)
internals.runtimeFlags.paintUnderInvalidationCheckingEnabled = false;
window.testIsAsync = true;
var video = document.getElementById("v");
......
{
"layers": [
{
"name": "LayoutView #document",
"bounds": [800, 600],
"contentsOpaque": true,
"drawsContent": true
},
{
"name": "LayoutVideo VIDEO id='video'",
"position": [8, 8],
"bounds": [320, 240]
},
{
"name": "Squashing Containment Layer",
"shouldFlattenTransform": false
},
{
"name": "LayoutFlexibleBox (relative positioned) DIV",
"position": [8, 8],
"bounds": [320, 240],
"drawsContent": true
},
{
"name": "Squashing Layer (first squashed layer: LayoutFlexibleBox (relative positioned) DIV)",
"position": [8, 8],
"bounds": [320, 240],
"drawsContent": true
}
],
"objectPaintInvalidations": [
{
"object": "LayoutVideo VIDEO id='video'",
"reason": "full"
}
]
}
{
"bounds": [800, 600],
"children": [
{
"bounds": [800, 600],
"contentsOpaque": true,
"drawsContent": true,
"children": [
{
"position": [8, 8],
"bounds": [320, 240]
},
{
"shouldFlattenTransform": false,
"children": [
{
"position": [8, 8],
"bounds": [320, 240],
"drawsContent": true
},
{
"position": [8, 8],
"bounds": [320, 240],
"drawsContent": true,
"paintInvalidations": [
{
"object": "LayoutSlider INPUT",
"rect": [108, 223, 70, 2],
"reason": "full"
},
{
"object": "LayoutFlexibleBox DIV",
"rect": [108, 223, 69, 2],
"reason": "forced by layout"
},
{
"object": "LayoutBlockFlow DIV id='thumb'",
"rect": [90, 208, 37, 32],
"reason": "full"
},
{
"object": "LayoutBlockFlow DIV id='thumb'",
"rect": [159, 208, 36, 32],
"reason": "full"
},
{
"object": "LayoutButton INPUT",
"rect": [195, 208, 32, 32],
"reason": "full"
},
{
"object": "LayoutButton INPUT",
"rect": [0, 208, 32, 32],
"reason": "full"
},
{
"object": "LayoutSlider INPUT",
"rect": [245, 223, 25, 2],
"reason": "full"
},
{
"object": "LayoutBlockFlow (anonymous)",
"rect": [32, 208, 24, 23],
"reason": "invalidate paint rectangle"
},
{
"object": "LayoutText #text",
"rect": [32, 217, 24, 14],
"reason": "full"
},
{
"object": "InlineTextBox ''",
"reason": "full"
},
{
"object": "InlineTextBox '0:00'",
"reason": "full"
},
{
"object": "RootInlineBox",
"reason": "full"
}
]
}
]
}
]
}
]
}
{
"bounds": [800, 600],
"children": [
{
"bounds": [800, 600],
"contentsOpaque": true,
"drawsContent": true,
"children": [
{
"position": [8, 8],
"bounds": [320, 240]
},
{
"shouldFlattenTransform": false,
"children": [
{
"position": [8, 8],
"bounds": [320, 240],
"drawsContent": true
},
{
"position": [8, 8],
"bounds": [320, 240],
"drawsContent": true,
"paintInvalidations": [
{
"object": "LayoutFlexibleBox DIV",
"rect": [96, 223, 81, 2],
"reason": "forced by layout"
},
{
"object": "LayoutSlider INPUT",
"rect": [96, 223, 81, 2],
"reason": "full"
},
{
"object": "LayoutBlockFlow DIV id='thumb'",
"rect": [159, 208, 36, 32],
"reason": "full"
},
{
"object": "LayoutBlockFlow DIV id='thumb'",
"rect": [78, 208, 36, 32],
"reason": "full"
},
{
"object": "LayoutButton INPUT",
"rect": [195, 208, 32, 32],
"reason": "full"
},
{
"object": "LayoutButton INPUT",
"rect": [0, 208, 32, 32],
"reason": "full"
},
{
"object": "LayoutSlider INPUT",
"rect": [245, 223, 25, 2],
"reason": "full"
},
{
"object": "LayoutBlockFlow (anonymous)",
"rect": [32, 208, 18, 23],
"reason": "invalidate paint rectangle"
},
{
"object": "LayoutText #text",
"rect": [32, 217, 18, 14],
"reason": "full"
},
{
"object": "InlineTextBox ''",
"reason": "full"
},
{
"object": "InlineTextBox '0:00'",
"reason": "full"
},
{
"object": "RootInlineBox",
"reason": "full"
}
]
}
]
}
]
}
]
}
......@@ -977,7 +977,10 @@ void PaintController::ShowUnderInvalidationError(
#else
LOG(ERROR) << "Run debug build to get more details.";
#endif
LOG(ERROR) << "See http://crbug.com/619103.";
LOG(ERROR) << "See http://crbug.com/619103. For media layout tests, this "
"could fail due to change in buffered range. In that case, set "
"internals.runtimeFlags.paintUnderInvalidationCheckingEnabled "
"to false in layout test.";
#ifndef NDEBUG
const PaintRecord* new_record = nullptr;
......@@ -1030,13 +1033,6 @@ void PaintController::CheckUnderInvalidation() {
return;
const DisplayItem& new_item = new_display_item_list_.Last();
if (new_item.SkippedCache()) {
// We allow cache skipping and temporary under-invalidation in cached
// subsequences. See the usage of DisplayItemCacheSkipper in BoxPainter.
under_invalidation_checking_begin_ = under_invalidation_checking_end_;
return;
}
size_t old_item_index = under_invalidation_checking_begin_ +
skipped_probable_under_invalidation_count_;
DisplayItem* old_item =
......
......@@ -11,7 +11,6 @@
#include "platform/graphics/paint/ClipPathRecorder.h"
#include "platform/graphics/paint/ClipRecorder.h"
#include "platform/graphics/paint/CompositingRecorder.h"
#include "platform/graphics/paint/DisplayItemCacheSkipper.h"
#include "platform/graphics/paint/DrawingDisplayItem.h"
#include "platform/graphics/paint/DrawingRecorder.h"
#include "platform/graphics/paint/SubsequenceRecorder.h"
......@@ -2075,10 +2074,7 @@ TEST_P(PaintControllerTest, PartialSkipCache) {
.GetPaintRecord());
}
TEST_P(PaintControllerTest, OptimizeNoopPairs) {
if (RuntimeEnabledFeatures::SlimmingPaintV175Enabled())
return;
TEST_F(PaintControllerTestBase, OptimizeNoopPairs) {
FakeDisplayItemClient first("first");
FakeDisplayItemClient second("second");
FakeDisplayItemClient third("third");
......@@ -2132,7 +2128,7 @@ TEST_P(PaintControllerTest, OptimizeNoopPairs) {
TestControllerDisplayItem(third, kBackgroundDrawingType));
}
TEST_P(PaintControllerTest, SmallPaintControllerHasOnePaintChunk) {
TEST_F(PaintControllerTestBase, SmallPaintControllerHasOnePaintChunk) {
ScopedSlimmingPaintV2ForTest enable_s_pv2(true);
FakeDisplayItemClient client("test client");
......@@ -2172,7 +2168,7 @@ void DrawPath(GraphicsContext& context,
context.DrawPath(path, flags);
}
TEST_P(PaintControllerTest, BeginAndEndFrame) {
TEST_F(PaintControllerTestBase, BeginAndEndFrame) {
class FakeFrame {};
// PaintController should have one null frame in the stack since beginning.
......@@ -2447,30 +2443,6 @@ class PaintControllerUnderInvalidationTest
}
GetPaintController().CommitNewDisplayItems();
}
void TestSkipCacheInSubsequence() {
FakeDisplayItemClient container("container");
FakeDisplayItemClient content("content");
GraphicsContext context(GetPaintController());
{
SubsequenceRecorder r(context, container);
DisplayItemCacheSkipper cache_skipper(context);
DrawRect(context, content, kBackgroundDrawingType,
FloatRect(100, 100, 300, 300));
}
GetPaintController().CommitNewDisplayItems();
{
EXPECT_FALSE(SubsequenceRecorder::UseCachedSubsequenceIfPossible(
context, container));
SubsequenceRecorder r(context, container);
DisplayItemCacheSkipper cache_skipper(context);
DrawRect(context, content, kBackgroundDrawingType,
FloatRect(200, 200, 400, 400));
}
GetPaintController().CommitNewDisplayItems();
}
};
TEST_F(PaintControllerUnderInvalidationTest, ChangeDrawing) {
......@@ -2524,10 +2496,6 @@ TEST_F(PaintControllerUnderInvalidationTest, SubsequenceBecomesEmpty) {
EXPECT_DEATH(TestSubsequenceBecomesEmpty(), "");
}
TEST_F(PaintControllerUnderInvalidationTest, SkipCacheInSubsequence) {
TestSkipCacheInSubsequence();
}
#endif // defined(GTEST_HAS_DEATH_TEST) && !defined(OS_ANDROID)
} // namespace blink
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment