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..eff23e45000c8ef2920b13740ae85c92d601ed2f 100644
--- a/Source/WebCore/rendering/RenderLayerCompositor.cpp
+++ b/Source/WebCore/rendering/RenderLayerCompositor.cpp
@@ -1931,7 +1931,29 @@ 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 (m_renderView->frameView() && (m_renderView->frameView()->isScrollable()))

[trchen, 2013/04/17 20:38:14]

Do we have to check whether container is a transformed element, not the frame?
If true then we don't need this condition.

[shawnsingh, 2013/04/17 20:57:36]

That's exactly what the if-statement above this new code checks for.  So I feel
it is clean and correct to not worry about it here.  How's that sound?

[trchen, 2013/04/17 21:03:03]

Hmm I didn't realize that transformed container never promote fixed position
layers. I think that's very wrong and needs to be corrected. Anyway it will be
out of scope of this patch. Let's file a bug for later.

[vangelis, 2013/04/17 22:05:29]

Actually that's not necessarily bad. If an element is fixed in a transformed
container then it shouldn't really be fixed to the scrolling viewport so it
doesn't need a separate layer.

+        noScrollableAncestor = false;
+
+    RenderLayer* ancestor = layer->parent();
+    while (ancestor && noScrollableAncestor) {

[trchen, 2013/04/17 20:38:14]

nits: I prefer to use for loop:
for (RenderLayer* ancestor = layer->parent(); ancestor && anceator->renderer()
!= container; ancestor = ancestor->parent())
IMO it is cleaner to see the terminal condition and you won't forget to iterate
when using continue keyword.
Don't have strong opinion though. Either convention is popular.

[shawnsingh, 2013/04/17 20:57:36]

The for-loop version skips the case where ancestor == container.  We need this
loop to include that case, but exit immediately after that.  Rewriting the
for-loop to accomodate that makes it a bit messier, and at that point I think
while loop is actually clearer =)

[trchen, 2013/04/17 21:03:03]

Ah ha you're right. Got your point. :)

+        if (ancestor->hasScrollbars())

[vangelis, 2013/04/17 19:53:15]

I'm wondering whether ancestor->hasScrollbars() will return the same value as
m_renderView->frameView()->isScrollable() when ancestor == container  .  If
that's the case then you could remove lines 1939-1940 . 

[shawnsingh, 2013/04/17 20:15:57]

I had empirically checked, with printf debugging on layout tests: 
When (ancestor == container == the RenderView), FrameView::isScrollable() often
returns "true" when ancestor->hasScrollbars() returns "false".

As I understand the code, the FrameView itself is a ScrollView which takes the
responsibility of having the scrollbars associated with the viewport -- the
RenderView (container) doesn't get those scrollbars.  The implementation of
FrameView::isScrollable() does not explicitly check if the FrameView has
scrollbars, but it does cover that condition indirectly, I believe based on
"condition #1" in that code -
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

[trchen, 2013/04/17 20:38:14]

Shawn is right. RenderView itself, as a RenderBlock, is never scrollable.
RenderLayerCompositor, which is owned by RenderView, handles the top level
scrolling of a frame.

[vangelis, 2013/04/17 22:05:29]

Ah, ok!

+            noScrollableAncestor = false;
+        if (ancestor->renderer() == container)
+            break;
+        ancestor = ancestor->parent();
+    }
+
+    if (noScrollableAncestor) {
+        if (viewportConstrainedNotCompositedReason)
+            *viewportConstrainedNotCompositedReason = RenderLayer::NotCompositedForUnscrollableAncestors;
+        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;