Consolidate background fill layer opaqueness into one method

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

 /*
  * Copyright (C) 1999 Antti Koivisto (koivisto@kde.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
  * This library is distributed in the hope that it will be useful,
@@ skipping 352 matching lines @@
 {
     const FillLayer* curr;
     for (curr = this; curr; curr = curr->next()) {
         if (curr->m_image && !curr->m_image->isLoaded())
             return false;
     }
 
     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;
 
-    // TODO(trchen): Should check blend mode before composite mode.
-    if (m_composite == CompositeClear || m_composite == CompositeCopy)
-        return true;
+    if ((m_repeatX != RepeatFill && m_repeatX != RoundFill)
+        || (m_repeatY != RepeatFill && m_repeatY != RoundFill))
+        return false;
 
     if (m_blendMode != WebBlendModeNormal)
         return false;
 
+    if (m_composite == CompositeClear || m_composite == CompositeCopy)
+        return true;
+
+    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.
     return layer1.image() == layer2.image();
 }
 
 bool FillLayer::imagesIdentical(const FillLayer* layer1, const FillLayer* layer2)
 {
     for (; layer1 && layer2; layer1 = layer1->next(), layer2 = layer2->next()) {
         if (!layerImagesIdentical(*layer1, *layer2))
             return false;
     }
 
     return !layer1 && !layer2;
 }
 
 } // namespace blink