Simplify flattening to just write enough to call the factory

src/effects/SkColorFilterImageFilter.cpp

 /*
  * Copyright 2012 The Android Open Source Project
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkColorFilterImageFilter.h"
 #include "SkBitmap.h"
 #include "SkCanvas.h"
@@ skipping 65 matching lines @@
     return SkNEW_ARGS(SkColorFilterImageFilter, (cf, input, cropRect));
 }
 
 SkColorFilterImageFilter::SkColorFilterImageFilter(SkColorFilter* cf,
         SkImageFilter* input, const CropRect* cropRect)
     : INHERITED(1, &input, cropRect), fColorFilter(cf) {
     SkASSERT(cf);
     SkSafeRef(cf);
 }
 
+#ifdef SK_SUPPORT_LEGACY_DEEPFLATTENING
 SkColorFilterImageFilter::SkColorFilterImageFilter(SkReadBuffer& buffer)
   : INHERITED(1, buffer) {
     fColorFilter = buffer.readColorFilter();
 }
+#endif
+
+SkFlattenable* SkColorFilterImageFilter::CreateProc(SkReadBuffer& buffer) {
+    Common common;
+    if (!common.unflatten(buffer, 1)) {
+        return NULL;
+    }
+    SkAutoTUnref<SkColorFilter> cf(buffer.readColorFilter());
+    return Create(cf, common.inputAt(0), &common.cropRect());

[Stephen White, 2014/07/18 15:43:32]

I see you've removed the validate() here (which is good), 
but you've also removed the early return. If the color 
filter fails to read, for example, the cfif Create() function 
will assert. I think we should keep in the early-returns, 
just remove all the validate()s.

Also, Alexis suggests that we might want to do one 
"if (!buffer.isValid()) return NULL;" just before calling each
Create(), since even buffer.readInt() might fail, and
there's no point in constructing an object which we know
will be thrown away anyway.

[reed1, 2014/07/18 16:18:47]

It is a new question about where/when to perform validation on the input
parameters. If we did runtime checks in the Factories (e.g. null-checks,
NaN-checks) in the Factory, we could skip those steps in the CreateProc and only
check for bad-buffers. We would then have all the checks in one place, and our
public factories would be that much saner...

Checking every time before calling a Create isn't wrong it would seem, but I
wonder if it is required. Seems like the final caller of the entire stream
can/should always check the state of the buffer at the end, and can abort the
world if it is invalid. I just want to avoid lots of extra noise in each
CreateProc if we don't absolutely require it. Lets keep thinking on this.

+}
 
 void SkColorFilterImageFilter::flatten(SkWriteBuffer& buffer) const {
-    this->INHERITED::flatten(buffer);
-
+    this->flattenCommon(buffer);
     buffer.writeFlattenable(fColorFilter);
 }
 
 SkColorFilterImageFilter::~SkColorFilterImageFilter() {
     SkSafeUnref(fColorFilter);
 }
 
 bool SkColorFilterImageFilter::onFilterImage(Proxy* proxy, const SkBitmap& source,
                                              const Context& ctx,
                                              SkBitmap* result,
@@ skipping 29 matching lines @@
 bool SkColorFilterImageFilter::asColorFilter(SkColorFilter** filter) const {
     if (!cropRectIsSet()) {
         if (filter) {
             *filter = fColorFilter;
             fColorFilter->ref();
         }
         return true;
     }
     return false;
 }