Avoid compositing fixed-position elements if they cannot scroll

Source/WebCore/rendering/RenderLayerCompositor.cpp

 /*
  * Copyright (C) 2009, 2010 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 1913 matching lines @@
         return false;
     }
 
     // Don't promote fixed position elements that are descendants of a non-view container, e.g. transformed elements.
     // They will stay fixed wrt the container rather than the enclosing frame.
     if (container != m_renderView) {
         if (viewportConstrainedNotCompositedReason)
             *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;
         return layer->isComposited();
     }
 
     bool paintsContent = layer->isVisuallyNonEmpty() || layer->hasVisibleDescendant();
     if (!paintsContent) {
         if (viewportConstrainedNotCompositedReason)
             *viewportConstrainedNotCompositedReason = RenderLayer::NotCompositedForNoVisibleContent;
@@ skipping 833 matching lines @@
     info.addMember(m_layerForOverhangAreas, "layerForOverhangAreas");
     info.addMember(m_contentShadowLayer, "contentShadowLayer");
     info.addMember(m_layerForTopOverhangArea, "layerForTopOverhangArea");
     info.addMember(m_layerForBottomOverhangArea, "layerForBottomOverhangArea");
     info.addMember(m_layerForHeader, "layerForHeader");
     info.addMember(m_layerForFooter, "layerForFooter");
 #endif
 }
 
 } // namespace WebCore