Title: [225052] trunk
- Revision
- 225052
- Author
- [email protected]
- Date
- 2017-11-20 09:18:57 -0800 (Mon, 20 Nov 2017)
Log Message
Remove slow repaint object from FrameView when style changes.
https://bugs.webkit.org/show_bug.cgi?id=179871
Reviewed by Antti Koivisto.
Source/WebCore:
The "oldStyleSlowScroll" value does not need to be computed. We already know its value
by checking the HashSet. This code is also unnecessarily complicated and error prone
(could lead to UAF errors by leaving stale renderers in the slow paint list).
Test: fast/repaint/slow-repaint-object-crash.html
* rendering/RenderElement.cpp:
(WebCore::RenderElement::styleWillChange):
LayoutTests:
* fast/repaint/slow-repaint-object-crash-expected.txt: Added.
* fast/repaint/slow-repaint-object-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (225051 => 225052)
--- trunk/LayoutTests/ChangeLog 2017-11-20 15:57:57 UTC (rev 225051)
+++ trunk/LayoutTests/ChangeLog 2017-11-20 17:18:57 UTC (rev 225052)
@@ -1,3 +1,13 @@
+2017-11-20 Zalan Bujtas <[email protected]>
+
+ Remove slow repaint object from FrameView when style changes.
+ https://bugs.webkit.org/show_bug.cgi?id=179871
+
+ Reviewed by Antti Koivisto.
+
+ * fast/repaint/slow-repaint-object-crash-expected.txt: Added.
+ * fast/repaint/slow-repaint-object-crash.html: Added.
+
2017-11-19 Ms2ger <[email protected]>
[WPE] Enable the XMLHttpRequest/ directory of web-platform-tests.
Added: trunk/LayoutTests/fast/repaint/slow-repaint-object-crash-expected.txt (0 => 225052)
--- trunk/LayoutTests/fast/repaint/slow-repaint-object-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/repaint/slow-repaint-object-crash-expected.txt 2017-11-20 17:18:57 UTC (rev 225052)
@@ -0,0 +1 @@
+
Added: trunk/LayoutTests/fast/repaint/slow-repaint-object-crash.html (0 => 225052)
--- trunk/LayoutTests/fast/repaint/slow-repaint-object-crash.html (rev 0)
+++ trunk/LayoutTests/fast/repaint/slow-repaint-object-crash.html 2017-11-20 17:18:57 UTC (rev 225052)
@@ -0,0 +1,24 @@
+<html>
+<head>
+<style>
+html, body {
+ background: url() fixed;
+}
+</style>
+<script>
+if (window.testRunner)
+ testRunner.dumpAsText();
+
+function runTest() {
+ var range = document.createRange();
+ range.setEnd(span, 1);
+ range.extractContents();
+ window.scrollBy(0, 100);
+ document.body.style.display = "none";
+}
+</script>
+</head>
+<body _onload_=runTest()>
+<span id=span>Pass if no crash or assert.</span>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (225051 => 225052)
--- trunk/Source/WebCore/ChangeLog 2017-11-20 15:57:57 UTC (rev 225051)
+++ trunk/Source/WebCore/ChangeLog 2017-11-20 17:18:57 UTC (rev 225052)
@@ -1,3 +1,19 @@
+2017-11-20 Zalan Bujtas <[email protected]>
+
+ Remove slow repaint object from FrameView when style changes.
+ https://bugs.webkit.org/show_bug.cgi?id=179871
+
+ Reviewed by Antti Koivisto.
+
+ The "oldStyleSlowScroll" value does not need to be computed. We already know its value
+ by checking the HashSet. This code is also unnecessarily complicated and error prone
+ (could lead to UAF errors by leaving stale renderers in the slow paint list).
+
+ Test: fast/repaint/slow-repaint-object-crash.html
+
+ * rendering/RenderElement.cpp:
+ (WebCore::RenderElement::styleWillChange):
+
2017-11-20 Carlos Alberto Lopez Perez <[email protected]>
[WPE] GLContextEGLWPE.cpp:44:96: error: invalid cast from type ‘GLNativeWindowType {aka long long unsigned int}’ to type ‘EGLNativeWindowType {aka unsigned int}
Modified: trunk/Source/WebCore/page/FrameView.cpp (225051 => 225052)
--- trunk/Source/WebCore/page/FrameView.cpp 2017-11-20 15:57:57 UTC (rev 225051)
+++ trunk/Source/WebCore/page/FrameView.cpp 2017-11-20 17:18:57 UTC (rev 225052)
@@ -1448,7 +1448,7 @@
updateCanBlitOnScrollRecursively();
}
-void FrameView::addSlowRepaintObject(RenderElement* o)
+void FrameView::addSlowRepaintObject(RenderElement& renderer)
{
bool hadSlowRepaintObjects = hasSlowRepaintObjects();
@@ -1455,32 +1455,33 @@
if (!m_slowRepaintObjects)
m_slowRepaintObjects = std::make_unique<HashSet<const RenderElement*>>();
- m_slowRepaintObjects->add(o);
+ m_slowRepaintObjects->add(&renderer);
+ if (hadSlowRepaintObjects)
+ return;
- if (!hadSlowRepaintObjects) {
- updateCanBlitOnScrollRecursively();
+ updateCanBlitOnScrollRecursively();
- if (Page* page = frame().page()) {
- if (ScrollingCoordinator* scrollingCoordinator = page->scrollingCoordinator())
- scrollingCoordinator->frameViewHasSlowRepaintObjectsDidChange(*this);
- }
+ if (auto* page = frame().page()) {
+ if (auto* scrollingCoordinator = page->scrollingCoordinator())
+ scrollingCoordinator->frameViewHasSlowRepaintObjectsDidChange(*this);
}
}
-void FrameView::removeSlowRepaintObject(RenderElement* o)
+void FrameView::removeSlowRepaintObject(RenderElement& renderer)
{
if (!m_slowRepaintObjects)
return;
- m_slowRepaintObjects->remove(o);
- if (m_slowRepaintObjects->isEmpty()) {
- m_slowRepaintObjects = nullptr;
- updateCanBlitOnScrollRecursively();
+ m_slowRepaintObjects->remove(&renderer);
+ if (!m_slowRepaintObjects->isEmpty())
+ return;
- if (Page* page = frame().page()) {
- if (ScrollingCoordinator* scrollingCoordinator = page->scrollingCoordinator())
- scrollingCoordinator->frameViewHasSlowRepaintObjectsDidChange(*this);
- }
+ m_slowRepaintObjects = nullptr;
+ updateCanBlitOnScrollRecursively();
+
+ if (auto* page = frame().page()) {
+ if (auto* scrollingCoordinator = page->scrollingCoordinator())
+ scrollingCoordinator->frameViewHasSlowRepaintObjectsDidChange(*this);
}
}
Modified: trunk/Source/WebCore/page/FrameView.h (225051 => 225052)
--- trunk/Source/WebCore/page/FrameView.h 2017-11-20 15:57:57 UTC (rev 225051)
+++ trunk/Source/WebCore/page/FrameView.h 2017-11-20 17:18:57 UTC (rev 225052)
@@ -273,8 +273,8 @@
void setIsOverlapped(bool);
void setContentIsOpaque(bool);
- void addSlowRepaintObject(RenderElement*);
- void removeSlowRepaintObject(RenderElement*);
+ void addSlowRepaintObject(RenderElement&);
+ void removeSlowRepaintObject(RenderElement&);
bool hasSlowRepaintObject(const RenderElement& renderer) const { return m_slowRepaintObjects && m_slowRepaintObjects->contains(&renderer); }
bool hasSlowRepaintObjects() const { return m_slowRepaintObjects && m_slowRepaintObjects->size(); }
Modified: trunk/Source/WebCore/rendering/RenderElement.cpp (225051 => 225052)
--- trunk/Source/WebCore/rendering/RenderElement.cpp 2017-11-20 15:57:57 UTC (rev 225051)
+++ trunk/Source/WebCore/rendering/RenderElement.cpp 2017-11-20 17:18:57 UTC (rev 225052)
@@ -913,32 +913,20 @@
s_noLongerAffectsParentBlock = false;
}
- bool newStyleUsesFixedBackgrounds = newStyle.hasFixedBackgroundImage();
- bool oldStyleUsesFixedBackgrounds = m_style.hasFixedBackgroundImage();
- if (newStyleUsesFixedBackgrounds || oldStyleUsesFixedBackgrounds) {
- bool repaintFixedBackgroundsOnScroll = !settings().fixedBackgroundsPaintRelativeToDocument();
- bool newStyleSlowScroll = repaintFixedBackgroundsOnScroll && newStyleUsesFixedBackgrounds;
- bool oldStyleSlowScroll = oldStyle && repaintFixedBackgroundsOnScroll && oldStyleUsesFixedBackgrounds;
+ bool newStyleSlowScroll = false;
+ if (newStyle.hasFixedBackgroundImage() && !settings().fixedBackgroundsPaintRelativeToDocument()) {
+ newStyleSlowScroll = true;
bool drawsRootBackground = isDocumentElementRenderer() || (isBody() && !rendererHasBackground(document().documentElement()->renderer()));
- if (drawsRootBackground && repaintFixedBackgroundsOnScroll) {
- if (view().compositor().supportsFixedRootBackgroundCompositing()) {
- if (newStyleSlowScroll && newStyle.hasEntirelyFixedBackground())
- newStyleSlowScroll = false;
+ if (drawsRootBackground && newStyle.hasEntirelyFixedBackground() && view().compositor().supportsFixedRootBackgroundCompositing())
+ newStyleSlowScroll = false;
+ }
- if (oldStyleSlowScroll && m_style.hasEntirelyFixedBackground())
- oldStyleSlowScroll = false;
- }
- }
+ if (view().frameView().hasSlowRepaintObject(*this)) {
+ if (!newStyleSlowScroll)
+ view().frameView().removeSlowRepaintObject(*this);
+ } else if (newStyleSlowScroll)
+ view().frameView().addSlowRepaintObject(*this);
- if (oldStyleSlowScroll != newStyleSlowScroll) {
- if (oldStyleSlowScroll)
- view().frameView().removeSlowRepaintObject(this);
-
- if (newStyleSlowScroll)
- view().frameView().addSlowRepaintObject(this);
- }
- }
-
if (isDocumentElementRenderer() || isBody())
view().frameView().updateExtendBackgroundIfNecessary();
}
@@ -1124,7 +1112,7 @@
void RenderElement::willBeDestroyed()
{
if (m_style.hasFixedBackgroundImage() && !settings().fixedBackgroundsPaintRelativeToDocument())
- view().frameView().removeSlowRepaintObject(this);
+ view().frameView().removeSlowRepaintObject(*this);
unregisterForVisibleInViewportCallback();
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
