Move off SK_SUPPORT_LEGACY_DATA_FACTORIES.

include/core/SkData.h

 /*
  * Copyright 2011 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #ifndef SkData_DEFINED
 #define SkData_DEFINED
 
 #include <stdio.h>
 
 #include "SkRefCnt.h"
 
 class SkStream;
 
-#ifndef SK_SUPPORT_LEGACY_DATA_FACTORIES
-#define SK_SUPPORT_LEGACY_DATA_FACTORIES
-#endif
-
 /**
  *  SkData holds an immutable data buffer. Not only is the data immutable,
  *  but the actual ptr that is returned (by data() or bytes()) is guaranteed
  *  to always be the same for the life of this instance.
  */
 class SK_API SkData : public SkRefCnt {
 public:
     /**
      *  Returns the number of bytes stored.
      */
@@ skipping 33 matching lines @@
      *  length to the size of the data. If buffer is NULL, it is ignored, and
      *  only the computed number of bytes is returned.
      */
     size_t copyRange(size_t offset, size_t length, void* buffer) const;
 
     /**
      *  Returns true if these two objects have the same length and contents,
      *  effectively returning 0 == memcmp(...)
      */
     bool equals(const SkData* other) const;
+#ifdef SK_SUPPORT_LEGACY_DATA_FACTORIES

[reed1, 2016/08/02 21:11:25]

why is this guy guarded?

[bungeman-skia, 2016/08/02 21:29:56]

This has caused some rather awkward code in Chromium when they attempted to use
it. It has unexpected and rather surprising meaning.

For example, this will not bind to a 'sk_sp<SkData>' value at all. That would
require the parameter type to be 'const sk_sp<const SkData>&'. But not having
this signature is actually a good thing, because if someone did call this
signature with an sk_sp<SkData> there would be a ref/unref pair at the call
site. However, almost no one actually has an sk_sp<const SkData> since they're
generally considered const (it does have writable_data(), but we generally
assume no one but the creator of the SkData is going to call it). Generally,
taking smart pointers by reference turns out not to work very well.

It turns out there was just one test file that used this at all, and there is
only one very awkward user in Chromium which would be way better off calling the
other 'equals'. If it turns out we really need something like this in the
future, we can see about adding something back that isn't as awkward.

     bool equals(sk_sp<const SkData>& other) const { return this->equals(other.get()); }
+#endif
 
     /**
      *  Function that, if provided, will be called when the SkData goes out
      *  of scope, allowing for custom allocation/freeing of the data's contents.
      */
     typedef void (*ReleaseProc)(const void* ptr, void* context);
 
     /**
      *  Create a new dataref by copying the specified data
      */
@@ skipping 137 matching lines @@
 
     typedef SkRefCnt INHERITED;
 };
 
 #ifdef SK_SUPPORT_LEGACY_DATA_FACTORIES
 /** Typedef of SkAutoTUnref<SkData> for automatically unref-ing a SkData. */
 typedef SkAutoTUnref<SkData> SkAutoDataUnref;
 #endif
 
 #endif