Avoid compositing fixed-position elements if they cannot scroll

Source/WebCore/rendering/RenderLayerCompositor.cpp

Index: Source/WebCore/rendering/RenderLayerCompositor.cpp
diff --git a/Source/WebCore/rendering/RenderLayerCompositor.cpp b/Source/WebCore/rendering/RenderLayerCompositor.cpp
index b27fea85a626c964804447d14aff9d3925888857..bcb90dbfa7e8d819bc7cb231d084db11b0cae20c 100644
--- a/Source/WebCore/rendering/RenderLayerCompositor.cpp
+++ b/Source/WebCore/rendering/RenderLayerCompositor.cpp
@@ -1931,7 +1931,26 @@ bool RenderLayerCompositor::requiresCompositingForPosition(RenderObject* rendere
             *viewportConstrainedNotCompositedReason = RenderLayer::NotCompositedForNonViewContainer;
         return false;
     }
-    
+
+    // If the fixed-position element does not have any scrollable ancestor
+    // between it and its container, then we do not need to spend compositor
+    // resources for it.
+    bool noScrollableAncestor = true;
+    if (container == m_renderView && m_renderView->frameView() && (m_renderView->frameView()->isScrollable()))

[vangelis, 2013/04/16 06:54:23]

Because of the code right above, we're guaranteed that (container ==
m_renderView) if we get here.

[shawnsingh, 2013/04/16 20:20:06]

Oops - forgot to remove this based on your previous review.  Will be gone in
next patch.

+        noScrollableAncestor = false;

[vangelis, 2013/04/16 06:54:23]

Should you set the value of viewportConstrainedNotCompositedReason here and
below? 

[shawnsingh, 2013/04/16 20:20:06]

Thanks for catching this - will do it.

+
+    RenderLayer* ancestor = layer->parent();
+    while (ancestor && noScrollableAncestor) {
+        if (ancestor->hasHorizontalScrollbar() || ancestor->hasVerticalScrollbar())

[vangelis, 2013/04/16 06:54:23]

Since you know you need to iterate until you get back to the container which is
the m_renderView, it seems that you can eliminate the code in lines 1939-1940
and just use this loop. 

For the RenderView, is ancestor->hasHorizontalScrollbar() equivalent to
m_renderView->frameView()->isScrollable() ? 

[trchen, 2013/04/16 19:36:21]

I have the same concern. For historical reason we often use scrollbar existance
as scrollability. For example, overflow:hidden doesn't have scrollbars, but they
are actually scrollable (you can set scrollLeft/scrollTop with javascript). In
contrast, overflow:visible can be never scrolled, even if you assign a
scrollLeft/scrollTop, the readback will be always 0.

Another commonly confused concepts are scrollability and user-scrollability. For
example, although overflow:hidden can be scrolled by javascript, you can't
scroll those with mouse or gesture.

In this case, I believe what you want is user-scrollability. Looking at
scrollbar existance doesn't look right.

[shawnsingh, 2013/04/16 20:20:06]

I don't think we can eliminate those lines of code.  I found that the FrameView
can have scrollbars while the rest of the RenderLayer hierarchy does not.  We
want to make sure we catch these cases, or else we will opt-out of compositing
too often.

Another option is to revert back to the RenderObject version of this patch,
which checks RenderStyle directly for the overflow status.  That version works
without needing to specially check the FrameView as well.

+            noScrollableAncestor = false;
+        if (ancestor->renderer() == container)
+            break;
+        ancestor = ancestor->parent();
+    }
+
+    if (noScrollableAncestor)
+        return false;
+
     // Subsequent tests depend on layout. If we can't tell now, just keep things the way they are until layout is done.
     if (!m_inPostLayoutUpdate) {
         m_reevaluateCompositingAfterLayout = true;