Rework the checks for too-few values in feColorMatrix filter.

Source/platform/graphics/filters/FEColorMatrix.cpp

 /*
  * Copyright (C) 2004, 2005, 2006, 2007 Nikolas Zimmermann <zimmermann@kde.org>
  * Copyright (C) 2004, 2005 Rob Buis <buis@kde.org>
  * Copyright (C) 2005 Eric Seidel <eric@webkit.org>
  * Copyright (C) 2009 Dirk Schulze <krit@webkit.org>
  * Copyright (C) 2013 Google 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
@@ skipping 45 matching lines @@
     return true;
 }
 
 const Vector<float>& FEColorMatrix::values() const
 {
     return m_values;
 }
 
 bool FEColorMatrix::setValues(const Vector<float> &values)
 {
-    ASSERT(values.size() == 20);
     if (m_values == values)
         return false;
     m_values = values;
     return true;
 }
 
 static void saturateMatrix(float s, SkScalar matrix[20])
 {
     matrix[0] = 0.213f + 0.787f * s;
     matrix[1] = 0.715f - 0.715f * s;
@@ skipping 34 matching lines @@
 }
 
 static void luminanceToAlphaMatrix(SkScalar matrix[20])
 {
     memset(matrix, 0, 20 * sizeof(SkScalar));
     matrix[15] = 0.2125f;
     matrix[16] = 0.7154f;
     matrix[17] = 0.0721f;
 }
 
-static SkColorFilter* createColorFilter(ColorMatrixType type, const float* values)
+static SkColorFilter* createColorFilter(ColorMatrixType type, const Vector<float>& values)
 {
+    // Use defaults if values contains too few values. See SVGFEColorMatrixElement::build

[fs, 2015/04/15 22:07:27]

Presumably said method would be made much simpler with this in place - else
there'd be one case where things would work (valid -> invalid) and one where it
wouldn't (invalid -> invalid) - for non.obvious reasons.

     SkScalar matrix[20];
     switch (type) {
     case FECOLORMATRIX_TYPE_UNKNOWN:
         break;
     case FECOLORMATRIX_TYPE_MATRIX:
-        for (int i = 0; i < 20; ++i)
-            matrix[i] = values[i];
-
+        if (values.size() < 20) {

[pdr., 2015/04/15 18:34:22]

Named constant please :)

[fs, 2015/04/15 22:07:27]

arraysize(matrix)? =)

[Stephen Chennney, 2015/04/16 18:12:19]

The named constant lets us use it everywhere, including the declarations.

+            for (size_t i = 0; i < 20; i++)

[pdr., 2015/04/15 18:34:22]

One of these for loops uses a size_t with i++, the other an int with ++i.

[Stephen Chennney, 2015/04/16 18:12:19]

Good point. That's what I get for copy-paste.

+                matrix[i] = ((i % 6) ? 0 : 1);
+        } else {
+            for (int i = 0; i < 20; ++i)
+                matrix[i] = values[i];
+        }
         matrix[4] *= SkScalar(255);
         matrix[9] *= SkScalar(255);
         matrix[14] *= SkScalar(255);
         matrix[19] *= SkScalar(255);
         break;
     case FECOLORMATRIX_TYPE_SATURATE:
-        saturateMatrix(values[0], matrix);
+        if (values.size() < 1)
+            saturateMatrix(1, matrix);

[fs, 2015/04/15 22:07:27]

Since all these "error" cases end up generating an identity matrix, it'd also be
possible to do that up front and then just break on the conditions in each case.
It's a matter of how uch you want to avoid double writes I guess =).

[Stephen Chennney, 2015/04/16 18:12:19]

Done.

+        else
+            saturateMatrix(values[0], matrix);
         break;
     case FECOLORMATRIX_TYPE_HUEROTATE:
-        hueRotateMatrix(values[0], matrix);
+        if (values.size() < 1)
+            hueRotateMatrix(0, matrix);
+        else
+            hueRotateMatrix(values[0], matrix);
         break;
     case FECOLORMATRIX_TYPE_LUMINANCETOALPHA:
         luminanceToAlphaMatrix(matrix);
         break;
     }
     return SkColorMatrixFilter::Create(matrix);
 }
 
 bool FEColorMatrix::affectsTransparentPixels()
 {
     // Because the input pixels are premultiplied, the only way clear pixels can be
     // painted is if the additive component for the alpha is not 0.
-    return m_type == FECOLORMATRIX_TYPE_MATRIX && m_values[19] > 0;
+    return m_type == FECOLORMATRIX_TYPE_MATRIX && m_values.size() > 19 && m_values[19] > 0;
 }
 
 PassRefPtr<SkImageFilter> FEColorMatrix::createImageFilter(SkiaImageFilterBuilder* builder)
 {
     RefPtr<SkImageFilter> input(builder->build(inputEffect(0), operatingColorSpace()));
-    SkAutoTUnref<SkColorFilter> filter(createColorFilter(m_type, m_values.data()));
+    SkAutoTUnref<SkColorFilter> filter(createColorFilter(m_type, m_values));
     SkImageFilter::CropRect rect = getCropRect(builder->cropOffset());
     return adoptRef(SkColorFilterImageFilter::Create(filter, input.get(), &rect));
 }
 
 static TextStream& operator<<(TextStream& ts, const ColorMatrixType& type)
 {
     switch (type) {
     case FECOLORMATRIX_TYPE_UNKNOWN:
         ts << "UNKNOWN";
         break;
@@ skipping 30 matching lines @@
                 ts << " ";
         }
         ts << "\"";
     }
     ts << "]\n";
     inputEffect(0)->externalRepresentation(ts, indent + 1);
     return ts;
 }
 
 } // namespace blink