Refactoring RenderLayerCompositor to avoid redundant checks for Compositing reasons

Source/core/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 1558 matching lines @@
 CompositingReasons RenderLayerCompositor::directReasonsForCompositing(const RenderLayer* layer) const
 {
     RenderObject* renderer = layer->renderer();
     CompositingReasons directReasons = CompositingReasonNone;
 
     if (requiresCompositingForTransform(renderer))
         directReasons |= CompositingReason3DTransform;
 
     if (requiresCompositingForVideo(renderer))
         directReasons |= CompositingReasonVideo;
-
-    if (requiresCompositingForCanvas(renderer))
+    else if (requiresCompositingForCanvas(renderer))

[shawnsingh, 2013/07/01 06:23:28]

Let's add a brief comment at the beginning of this group of if-else statements,
saying "Only zero or one of the following conditions will be true for a given
RenderLayer."

I'm curious - is this just a cleanup refactoring you thought would be
appropriate?  Or is there a deeper reason that we need to use else-statements
here?  Can you please explain?

[pals, 2013/07/01 07:41:57]

Done.
This is just cleanup refactoring which I came accross while going through the
code.

         directReasons |= CompositingReasonCanvas;
-
-    if (requiresCompositingForPlugin(renderer))
+    else if (requiresCompositingForPlugin(renderer))
         directReasons |= CompositingReasonPlugin;
-
-    if (requiresCompositingForFrame(renderer))
+    else if (requiresCompositingForFrame(renderer))
         directReasons |= CompositingReasonIFrame;
     
     if (requiresCompositingForBackfaceVisibilityHidden(renderer))
         directReasons |= CompositingReasonBackfaceVisibilityHidden;
 
-    if (requiresCompositingForAnimation(renderer))
-        directReasons |= CompositingReasonAnimation;
-
-    if (requiresCompositingForTransition(renderer))
+    if (requiresCompositingForTransition(renderer) || requiresCompositingForAnimation(renderer))

[shawnsingh, 2013/07/01 06:23:28]

This particular line, let's not combine these.  We only combined them
temporarily while we were limited to 32 reasons in a 32-bit field.  We're
migrating to 64-bit already, and we should be able to add the
CompositingReasonTransition reason again.

[pals, 2013/07/01 07:41:57]

Done.

         directReasons |= CompositingReasonAnimation;
 
     if (requiresCompositingForFilters(renderer))
         directReasons |= CompositingReasonFilters;
 
     if (requiresCompositingForPosition(renderer, layer))
         directReasons |= renderer->style()->position() == FixedPosition ? CompositingReasonPositionFixed : CompositingReasonPositionSticky;
 
     if (requiresCompositingForOverflowScrolling(layer))
         directReasons |= CompositingReasonOverflowScrollingTouch;
@@ skipping 1146 matching lines @@
     info.addMember(m_layerForScrollCorner, "layerForScrollCorner");
 #if ENABLE(RUBBER_BANDING)
     info.addMember(m_layerForOverhangAreas, "layerForOverhangAreas");
     info.addMember(m_contentShadowLayer, "contentShadowLayer");
     info.addMember(m_layerForTopOverhangArea, "layerForTopOverhangArea");
     info.addMember(m_layerForBottomOverhangArea, "layerForBottomOverhangArea");
 #endif
 }
 
 } // namespace WebCore