Consolidate background fill layer opaqueness into one method

third_party/WebKit/Source/core/style/FillLayer.cpp

Index: third_party/WebKit/Source/core/style/FillLayer.cpp
diff --git a/third_party/WebKit/Source/core/style/FillLayer.cpp b/third_party/WebKit/Source/core/style/FillLayer.cpp
index 47df687455c70a50f07c791c79b9ecc19026ea42..edb268968bc39dfbf3864da211987c53f8cc370e 100644
--- a/third_party/WebKit/Source/core/style/FillLayer.cpp
+++ b/third_party/WebKit/Source/core/style/FillLayer.cpp
@@ -370,29 +370,30 @@ bool FillLayer::imagesAreLoaded() const
     return true;
 }
 
-bool FillLayer::hasOpaqueImage(const LayoutObject* layoutObject) const
+bool FillLayer::backgroundImageIsOpaque(const LayoutObject& imageClient, const float effectiveZoom) const

[trchen, 2016/03/11 22:13:33]

I think the complexity of this function is getting unmanageable.

Usually a boolean function checks a few positive conditions to return true, and
return false if none of the conditions are met. (i.e. each functions being CNF.)

Mixing positive and negative conditions make it really difficult to comprehend.
Is there a reason why we can't we split these criteria into logical groups as I
recommended on crbug?

[Stephen Chennney, 2016/03/14 14:59:35]

I'm wary of exposing multiple methods that a caller then has to combine. I agree
on the "all if's return true, return false at end" or vice versa in this case,
so I'll aim to get things into that form.

 {
-    if (!m_image)
+    if (!m_image || !m_image->canRender())
+        return false;
+
+    if ((m_repeatX != RepeatFill && m_repeatX != RoundFill)
+        || (m_repeatY != RepeatFill && m_repeatY != RoundFill))
+        return false;
+
+    if (m_blendMode != WebBlendModeNormal)
         return false;
 
-    // TODO(trchen): Should check blend mode before composite mode.
     if (m_composite == CompositeClear || m_composite == CompositeCopy)
         return true;
 
-    if (m_blendMode != WebBlendModeNormal)
+    if (m_image->imageSize(&imageClient, effectiveZoom, LayoutSize()).isEmpty())
         return false;
 
     if (m_composite == CompositeSourceOver)
-        return m_image->knownToBeOpaque(layoutObject);
+        return m_image->knownToBeOpaque(&imageClient);
 
     return false;
 }
 
-bool FillLayer::hasRepeatXY() const
-{
-    return m_repeatX == RepeatFill && m_repeatY == RepeatFill;
-}
-
 static inline bool layerImagesIdentical(const FillLayer& layer1, const FillLayer& layer2)
 {
     // We just care about pointer equivalency.