Don't animate a progress bar if it isn't animated
The paint path for HTML Progress elements only uses animated
values if the element is "indeterminant", having no "value"
attribute. Yet the LayoutProgress code was always setting up and
using an animation timer, and requesting paint invalidation
repeatedly, even if the value never changed.
Re-enables a test on Mac that should now pass.
Adds a unit test to verify the timer starts and stops as required.
R=wangxianzhu@chromium.org
BUG=570756
Review URL: https://codereview.chromium.org/1721943002
Cr-Commit-Position: refs/heads/master@{#377405}
diff --git a/third_party/WebKit/LayoutTests/NeverFixTests b/third_party/WebKit/LayoutTests/NeverFixTests
index 42b7b516..dba1aaa 100644
--- a/third_party/WebKit/LayoutTests/NeverFixTests
+++ b/third_party/WebKit/LayoutTests/NeverFixTests
@@ -41,9 +41,6 @@
# These tests are only valid on platforms with overlay scrollbars support.
[ SnowLeopard ] inspector-protocol/emulation [ WontFix ]
-# <progress> on Mac is always animated. So it's hard to get a reliable pixel result.
-[ Mac ] fast/dom/HTMLProgressElement/progress-element.html [ WontFix ]
-
# Mac's popup behavior is different.
[ Mac ] fast/forms/select/menulist-onchange-fired-with-key-up-down.html [ WontFix ]
[ Mac ] fast/forms/select/popup-with-display-none-optgroup.html [ WontFix ]
diff --git a/third_party/WebKit/LayoutTests/TestExpectations b/third_party/WebKit/LayoutTests/TestExpectations
index 79e0e25..db8b854 100644
--- a/third_party/WebKit/LayoutTests/TestExpectations
+++ b/third_party/WebKit/LayoutTests/TestExpectations
@@ -1407,6 +1407,8 @@
crbug.com/585724 fast/js/JSON-parse.html [ NeedsManualRebaseline ]
+crbug.com/570756 [ Mac ] fast/dom/HTMLProgressElement/progress-element.html [ NeedsRebaseline ]
+
crbug.com/587136 [ Linux Debug ] http/tests/security/xss-DENIED-cross-origin-stack-overflow.html [ Timeout Pass ]
crbug.com/587593 [ Android ] fast/js/pic/cached-single-entry-transition.html [ Pass Failure ]
diff --git a/third_party/WebKit/Source/core/core.gypi b/third_party/WebKit/Source/core/core.gypi
index e6b6b1d9..f78441a 100644
--- a/third_party/WebKit/Source/core/core.gypi
+++ b/third_party/WebKit/Source/core/core.gypi
@@ -3981,6 +3981,7 @@
'layout/LayoutMultiColumnFlowThreadTest.cpp',
'layout/LayoutObjectTest.cpp',
'layout/LayoutPartTest.cpp',
+ 'layout/LayoutProgressTest.cpp',
'layout/LayoutTableCellTest.cpp',
'layout/LayoutTableRowTest.cpp',
'layout/LayoutTestHelper.cpp',
diff --git a/third_party/WebKit/Source/core/layout/LayoutProgress.cpp b/third_party/WebKit/Source/core/layout/LayoutProgress.cpp
index 5b2c45cf..9ec5d1b 100644
--- a/third_party/WebKit/Source/core/layout/LayoutProgress.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutProgress.cpp
@@ -74,6 +74,16 @@
&& HTMLProgressElement::InvalidPosition != position());
}
+bool LayoutProgress::isAnimationTimerActive() const
+{
+ return m_animationTimer.isActive();
+}
+
+bool LayoutProgress::isAnimating() const
+{
+ return m_animating;
+}
+
void LayoutProgress::animationTimerFired(Timer<LayoutProgress>*)
{
setShouldDoFullPaintInvalidation();
@@ -86,7 +96,7 @@
m_animationDuration = LayoutTheme::theme().animationDurationForProgressBar();
m_animationRepeatInterval = LayoutTheme::theme().animationRepeatIntervalForProgressBar();
- bool animating = style()->hasAppearance() && m_animationDuration > 0;
+ bool animating = !isDeterminate() && style()->hasAppearance() && m_animationDuration > 0;
if (animating == m_animating)
return;
diff --git a/third_party/WebKit/Source/core/layout/LayoutProgress.h b/third_party/WebKit/Source/core/layout/LayoutProgress.h
index c8d8abf..1c30b8e18 100644
--- a/third_party/WebKit/Source/core/layout/LayoutProgress.h
+++ b/third_party/WebKit/Source/core/layout/LayoutProgress.h
@@ -46,6 +46,9 @@
protected:
void willBeDestroyed() override;
+ bool isAnimating() const;
+ bool isAnimationTimerActive() const;
+
private:
bool isOfType(LayoutObjectType type) const override { return type == LayoutObjectProgress || LayoutBlockFlow::isOfType(type); }
@@ -58,6 +61,8 @@
double m_animationDuration;
bool m_animating;
Timer<LayoutProgress> m_animationTimer;
+
+ friend class LayoutProgressTest;
};
DEFINE_LAYOUT_OBJECT_TYPE_CASTS(LayoutProgress, isProgress());
diff --git a/third_party/WebKit/Source/core/layout/LayoutProgressTest.cpp b/third_party/WebKit/Source/core/layout/LayoutProgressTest.cpp
new file mode 100644
index 0000000..eaf410f
--- /dev/null
+++ b/third_party/WebKit/Source/core/layout/LayoutProgressTest.cpp
@@ -0,0 +1,46 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "core/layout/LayoutProgress.h"
+
+#include "core/HTMLNames.h"
+#include "core/html/HTMLElement.h"
+#include "core/layout/LayoutTestHelper.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace blink {
+
+class LayoutProgressTest : public RenderingTest {
+public:
+ static bool isAnimationTimerActive(const LayoutProgress* o) { return o->isAnimationTimerActive(); }
+ static bool isAnimatiing(const LayoutProgress* o) { return o->isAnimating(); }
+};
+
+TEST_F(LayoutProgressTest, AnimationScheduling)
+{
+ RenderingTest::setBodyInnerHTML("<progress id=\"progressElement\" value=0.3 max=1.0></progress>");
+ document().view()->updateAllLifecyclePhases();
+ Element* progressElement = document().getElementById(AtomicString("progressElement"));
+ LayoutProgress* layoutProgress = toLayoutProgress(progressElement->layoutObject());
+
+ // Verify that we do not schedule a timer for a determinant progress element
+ EXPECT_FALSE(LayoutProgressTest::isAnimationTimerActive(layoutProgress));
+ EXPECT_FALSE(LayoutProgressTest::isAnimatiing(layoutProgress));
+
+ progressElement->removeAttribute("value");
+ document().view()->updateAllLifecyclePhases();
+
+ // Verify that we schedule a timer for an indeterminant progress element
+ EXPECT_TRUE(LayoutProgressTest::isAnimationTimerActive(layoutProgress));
+ EXPECT_TRUE(LayoutProgressTest::isAnimatiing(layoutProgress));
+
+ progressElement->setAttribute(HTMLNames::valueAttr, "0.7");
+ document().view()->updateAllLifecyclePhases();
+
+ // Verify that we cancel the timer for a determinant progress element
+ EXPECT_FALSE(LayoutProgressTest::isAnimationTimerActive(layoutProgress));
+ EXPECT_FALSE(LayoutProgressTest::isAnimatiing(layoutProgress));
+}
+
+} // namespace blink
