Diff
Modified: trunk/LayoutTests/ChangeLog (185309 => 185310)
--- trunk/LayoutTests/ChangeLog 2015-06-08 01:39:35 UTC (rev 185309)
+++ trunk/LayoutTests/ChangeLog 2015-06-08 02:39:15 UTC (rev 185310)
@@ -1,3 +1,25 @@
+2015-06-07 Darin Adler <[email protected]>
+
+ REGRESSION (r181720): Unnecessary layout triggered any time animated GIF advances to a new frame
+ https://bugs.webkit.org/show_bug.cgi?id=145733
+
+ Reviewed by Andreas Kling.
+
+ * TestExpectations: Expect image failures on the animated GIF tests (the one
+ old one I am fixing and the one new one I am adding) because they don't yet work
+ under DumpRenderTree.
+
+ * fast/images/animated-gif-no-layout-expected.html: Added.
+ * fast/images/animated-gif-no-layout.html: Added.
+
+ * fast/images/gif-loop-count-expected.html: Added. This test was worthless as a render
+ tree dump test, and only valuable as a pixel test. And that hid the fact that it was
+ failing under WebKit1. Changing it to a reference test makes it a valuable test again.
+ * fast/images/gif-loop-count-expected.png: Removed.
+ * fast/images/gif-loop-count-expected.txt: Removed.
+
+ * platform/wk2/TestExpectations: Expect successes on these two tests.
+
2015-06-06 Alexey Proskuryakov <[email protected]>
Clean up tests for blocking mixed content
Modified: trunk/LayoutTests/TestExpectations (185309 => 185310)
--- trunk/LayoutTests/TestExpectations 2015-06-08 01:39:35 UTC (rev 185309)
+++ trunk/LayoutTests/TestExpectations 2015-06-08 02:39:15 UTC (rev 185310)
@@ -546,3 +546,6 @@
webkit.org/b/145090 inspector/debugger/break-on-uncaught-exception-throw-in-promise-rethrow-in-catch.html [ Skip ]
webkit.org/b/145090 inspector/debugger/break-on-uncaught-exception-window-onerror.html [ Skip ]
+# DumpRenderTree does not allow GIFs to animate, thus animated GIF tests don't work in WebKit1.
+fast/images/animated-gif-no-layout.html [ ImageOnlyFailure ]
+fast/images/gif-loop-count.html [ ImageOnlyFailure ]
Added: trunk/LayoutTests/fast/images/animated-gif-no-layout-expected.html (0 => 185310)
--- trunk/LayoutTests/fast/images/animated-gif-no-layout-expected.html (rev 0)
+++ trunk/LayoutTests/fast/images/animated-gif-no-layout-expected.html 2015-06-08 02:39:15 UTC (rev 185310)
@@ -0,0 +1,2 @@
+<div style="position:absolute;left:0;top:0;width:100;height:100;background-color:#21B14D"></div>
+<img src="" style="position:absolute;left:0;top:150">
Added: trunk/LayoutTests/fast/images/animated-gif-no-layout.html (0 => 185310)
--- trunk/LayoutTests/fast/images/animated-gif-no-layout.html (rev 0)
+++ trunk/LayoutTests/fast/images/animated-gif-no-layout.html 2015-06-08 02:39:15 UTC (rev 185310)
@@ -0,0 +1,43 @@
+<script>
+ // This test waits for a GIF repaint and double checks that there is a repaint only,
+ // no layout. If the test passes, there are two green squares. If it fails, then one
+ // or both of the squares is red instead.
+
+ var layoutCountBeforeTimer;
+
+ function animationComplete()
+ {
+ document.body.offsetWidth; // Explicitly trigger a layout.
+
+ if (window.internals && window.testRunner) {
+ var count = internals.layoutCount - layoutCountBeforeTimer;
+ if (count == 0) {
+ var indicator = document.getElementById("indicator");
+ indicator.addEventListener("load", function() { testRunner.notifyDone(); })
+ indicator.src = ""
+ return;
+ }
+ testRunner.notifyDone();
+ }
+ }
+
+ function test()
+ {
+ document.body.offsetWidth; // Explicitly trigger a layout.
+
+ if (window.internals && window.testRunner) {
+ testRunner.waitUntilDone();
+ layoutCountBeforeTimer = internals.layoutCount;
+ }
+
+ // The 200ms value here is longer than the time it takes for gif-loop-count.gif
+ // to advance from its first frame to its second. There should be no race because
+ // the GIF timer should already be scheduled and this timer should share the
+ // same time base.
+ setTimeout(animationComplete, 200);
+ }
+</script>
+<body _onload_="test()">
+<img src="" style="position:absolute;left:0;top:0">
+<img id="indicator" src="" style="position:absolute;left:0;top:150">
+</body>
Added: trunk/LayoutTests/fast/images/gif-loop-count-expected.html (0 => 185310)
--- trunk/LayoutTests/fast/images/gif-loop-count-expected.html (rev 0)
+++ trunk/LayoutTests/fast/images/gif-loop-count-expected.html 2015-06-08 02:39:15 UTC (rev 185310)
@@ -0,0 +1 @@
+<div style="position:absolute;left:0;top:0;width:100;height:100;background-color:#21B14D"></div>
Deleted: trunk/LayoutTests/fast/images/gif-loop-count-expected.png
(Binary files differ)
Deleted: trunk/LayoutTests/fast/images/gif-loop-count-expected.txt (185309 => 185310)
--- trunk/LayoutTests/fast/images/gif-loop-count-expected.txt 2015-06-08 01:39:35 UTC (rev 185309)
+++ trunk/LayoutTests/fast/images/gif-loop-count-expected.txt 2015-06-08 02:39:15 UTC (rev 185310)
@@ -1,9 +0,0 @@
-layer at (0,0) size 800x600
- RenderView at (0,0) size 800x600
-layer at (0,0) size 800x600
- RenderBlock {HTML} at (0,0) size 800x600
- RenderBody {BODY} at (8,8) size 784x584
-layer at (0,0) size 100x100
- RenderImage {IMG} at (0,0) size 100x100
-layer at (0,0) size 100x100
- RenderImage {IMG} at (0,0) size 100x100
Modified: trunk/LayoutTests/platform/wk2/TestExpectations (185309 => 185310)
--- trunk/LayoutTests/platform/wk2/TestExpectations 2015-06-08 01:39:35 UTC (rev 185309)
+++ trunk/LayoutTests/platform/wk2/TestExpectations 2015-06-08 02:39:15 UTC (rev 185310)
@@ -36,6 +36,7 @@
### (2) Classified failures without bug reports (yet)
### (3) Unclassified failures
### (4) Features that are not supported in WebKit2 and likely never will be
+### (5) Progressions, expected successes that are expected failures in WebKit1.
########################################
@@ -680,7 +681,16 @@
### END OF (4) Features that are not supported in WebKit2 and likely never will be
########################################
+########################################
+### START OF (5) Progressions, expected successes that are expected failures in WebKit1.
+# DumpRenderTree does not allow GIFs to animate, thus animated GIF tests don't work in WebKit1.
+fast/images/animated-gif-no-layout.html [ Pass ]
+fast/images/gif-loop-count.html [ Pass ]
+
+### END OF (5) Progressions, expected successes that are expected failures in WebKit1.
+########################################
+
###### This file has four sections. When adding new tests, make sure to
###### add to the right section:
@@ -688,3 +698,4 @@
### (2) Classified failures without bug reports (yet)
### (3) Unclassified failures
### (4) Features that are not supported in WebKit2 and likely never will be
+### (5) Progressions, expected successes that are expected failures in WebKit1.
Modified: trunk/Source/WebCore/ChangeLog (185309 => 185310)
--- trunk/Source/WebCore/ChangeLog 2015-06-08 01:39:35 UTC (rev 185309)
+++ trunk/Source/WebCore/ChangeLog 2015-06-08 02:39:15 UTC (rev 185310)
@@ -1,3 +1,24 @@
+2015-06-07 Darin Adler <[email protected]>
+
+ REGRESSION (r181720): Unnecessary layout triggered any time animated GIF advances to a new frame
+ https://bugs.webkit.org/show_bug.cgi?id=145733
+
+ Reviewed by Andreas Kling.
+
+ Test: fast/images/animated-gif-no-layout.html
+
+ * rendering/RenderImage.cpp:
+ (WebCore::RenderImage::styleDidChange): Correctly pass ImageSizeChangeNone in cases
+ where we don't need to report a change in intrinsic size that happened outside the
+ repaintOrMarkForLayout function.
+ (WebCore::RenderImage::repaintOrMarkForLayout): Move work that should only be done
+ when size changed inside the if statement.
+
+ * testing/Internals.cpp:
+ (WebCore::Internals::layoutCount): Added.
+ * testing/Internals.h: Added layoutCount.
+ * testing/Internals.idl: Ditto.
+
2015-06-07 Michael Catanzaro <[email protected]>
Fix strncpy use in WebCore::Text::formatForDebugger
Modified: trunk/Source/WebCore/rendering/RenderImage.cpp (185309 => 185310)
--- trunk/Source/WebCore/rendering/RenderImage.cpp 2015-06-08 01:39:35 UTC (rev 185309)
+++ trunk/Source/WebCore/rendering/RenderImage.cpp 2015-06-08 02:39:15 UTC (rev 185310)
@@ -214,7 +214,7 @@
}
#if ENABLE(CSS_IMAGE_ORIENTATION)
if (diff == StyleDifferenceLayout && oldStyle->imageOrientation() != style().imageOrientation())
- return repaintOrMarkForLayout(ImageSizeChangeForAltText);
+ return repaintOrMarkForLayout(ImageSizeChangeNone);
#endif
#if ENABLE(CSS_IMAGE_RESOLUTION)
@@ -222,7 +222,7 @@
&& (oldStyle->imageResolution() != style().imageResolution()
|| oldStyle->imageResolutionSnap() != style().imageResolutionSnap()
|| oldStyle->imageResolutionSource() != style().imageResolutionSource()))
- repaintOrMarkForLayout(ImageSizeChangeForAltText);
+ repaintOrMarkForLayout(ImageSizeChangeNone);
#endif
}
@@ -304,26 +304,28 @@
bool imageSourceHasChangedSize = oldIntrinsicSize != newIntrinsicSize || imageSizeChange != ImageSizeChangeNone;
- if (imageSourceHasChangedSize)
+ if (imageSourceHasChangedSize) {
setPreferredLogicalWidthsDirty(true);
- // If the actual area occupied by the image has changed and it is not constrained by style then a layout is required.
- bool imageSizeIsConstrained = style().logicalWidth().isSpecified() && style().logicalHeight().isSpecified();
- bool needsLayout = !imageSizeIsConstrained && imageSourceHasChangedSize;
+ // If the actual area occupied by the image has changed and it is not constrained by style then a layout is required.
+ bool imageSizeIsConstrained = style().logicalWidth().isSpecified() && style().logicalHeight().isSpecified();
- // FIXME: We only need to recompute the containing block's preferred size
- // if the containing block's size depends on the image's size (i.e., the container uses shrink-to-fit sizing).
- // There's no easy way to detect that shrink-to-fit is needed, always force a layout.
- bool containingBlockNeedsToRecomputePreferredSize =
- style().logicalWidth().isPercentOrCalculated()
- || style().logicalMaxWidth().isPercentOrCalculated()
- || style().logicalMinWidth().isPercentOrCalculated();
+ // FIXME: We only need to recompute the containing block's preferred size
+ // if the containing block's size depends on the image's size (i.e., the container uses shrink-to-fit sizing).
+ // There's no easy way to detect that shrink-to-fit is needed, always force a layout.
+ bool containingBlockNeedsToRecomputePreferredSize =
+ style().logicalWidth().isPercentOrCalculated()
+ || style().logicalMaxWidth().isPercentOrCalculated()
+ || style().logicalMinWidth().isPercentOrCalculated();
- bool layoutSizeDependsOnIntrinsicSize = style().aspectRatioType() == AspectRatioFromIntrinsic;
+ bool layoutSizeDependsOnIntrinsicSize = style().aspectRatioType() == AspectRatioFromIntrinsic;
- if (needsLayout || containingBlockNeedsToRecomputePreferredSize || layoutSizeDependsOnIntrinsicSize) {
- setNeedsLayout();
- return;
+ if (!imageSizeIsConstrained || containingBlockNeedsToRecomputePreferredSize || layoutSizeDependsOnIntrinsicSize) {
+ // FIXME: It's not clear that triggering a layout guarantees a repaint in all cases.
+ // But many callers do depend on this code causing a layout.
+ setNeedsLayout();
+ return;
+ }
}
if (everHadLayout() && !selfNeedsLayout()) {
Modified: trunk/Source/WebCore/testing/Internals.cpp (185309 => 185310)
--- trunk/Source/WebCore/testing/Internals.cpp 2015-06-08 01:39:35 UTC (rev 185309)
+++ trunk/Source/WebCore/testing/Internals.cpp 2015-06-08 02:39:15 UTC (rev 185310)
@@ -2232,6 +2232,14 @@
document->updateLayoutIgnorePendingStylesheets(Document::RunPostLayoutTasks::Synchronously);
}
+unsigned Internals::layoutCount() const
+{
+ Document* document = contextDocument();
+ if (!document || !document->view())
+ return 0;
+ return document->view()->layoutCount();
+}
+
#if !PLATFORM(IOS)
static const char* cursorTypeToString(Cursor::Type cursorType)
{
Modified: trunk/Source/WebCore/testing/Internals.h (185309 => 185310)
--- trunk/Source/WebCore/testing/Internals.h 2015-06-08 01:39:35 UTC (rev 185309)
+++ trunk/Source/WebCore/testing/Internals.h 2015-06-08 02:39:15 UTC (rev 185310)
@@ -317,6 +317,7 @@
void updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks(ExceptionCode&);
void updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks(Node*, ExceptionCode&);
+ unsigned layoutCount() const;
PassRefPtr<ArrayBuffer> serializeObject(PassRefPtr<SerializedScriptValue>) const;
PassRefPtr<SerializedScriptValue> deserializeBuffer(PassRefPtr<ArrayBuffer>) const;
Modified: trunk/Source/WebCore/testing/Internals.idl (185309 => 185310)
--- trunk/Source/WebCore/testing/Internals.idl 2015-06-08 01:39:35 UTC (rev 185309)
+++ trunk/Source/WebCore/testing/Internals.idl 2015-06-08 02:39:15 UTC (rev 185310)
@@ -298,6 +298,8 @@
// specified without security checks. Unspecified means this document.
[RaisesException] void updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks(optional Node node);
+ readonly attribute unsigned long layoutCount;
+
// Returns a string with information about the mouse cursor used at the specified client location.
[RaisesException] DOMString getCurrentCursorInfo();
