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

Source/platform/graphics/filters/FEColorMatrix.cpp

Index: Source/platform/graphics/filters/FEColorMatrix.cpp
diff --git a/Source/platform/graphics/filters/FEColorMatrix.cpp b/Source/platform/graphics/filters/FEColorMatrix.cpp
index 689edd37ddcbccd11181990a53e63f3a79619200..3389827adadc0635cd22ad1a172b2751d4bb3c63 100644
--- a/Source/platform/graphics/filters/FEColorMatrix.cpp
+++ b/Source/platform/graphics/filters/FEColorMatrix.cpp
@@ -63,7 +63,6 @@ const Vector<float>& FEColorMatrix::values() const
 
 bool FEColorMatrix::setValues(const Vector<float> &values)
 {
-    ASSERT(values.size() == 20);
     if (m_values == values)
         return false;
     m_values = values;
@@ -118,26 +117,37 @@ static void luminanceToAlphaMatrix(SkScalar matrix[20])
     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);
@@ -150,13 +160,13 @@ 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));
 }