remove unneeded readbuffer flags

- buffers are always 'cross-process'
- readbuffer is always validating

Bug:796107
Change-Id: I59614e9c29490c0b029c60d2aafe2806671bc9e1
Reviewed-on: https://skia-review.googlesource.com/90560
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Reed <reed@google.com>

include/core/SkPicture.h

@@ -160,10 +160,11 @@ private:
     // V57: Sweep tiling info.
     // V58: No more 2pt conical flipping.
     // V59: No more LocalSpace option on PictureImageFilter
+    // V60: Remove flags in picture header
 
     // Only SKPs within the min/current picture version range (inclusive) can be read.
     static const uint32_t     MIN_PICTURE_VERSION = 51;     // Produced by Chrome ~M56.
-    static const uint32_t CURRENT_PICTURE_VERSION = 59;
+    static const uint32_t CURRENT_PICTURE_VERSION = 60;
 
     static bool IsValidPictInfo(const SkPictInfo& info);
     static sk_sp<SkPicture> Forwardport(const SkPictInfo&,

src/core/SkPaint.cpp

@@ -1865,17 +1865,16 @@ static FlatFlags unpack_paint_flags(SkPaint* paint, uint32_t packed) {
     it if there are not tricky elements like shaders, etc.
  */
 void SkPaint::flatten(SkWriteBuffer& buffer) const {
-    // If the writer is xprocess, then we force recording our typeface, even if its "default"
-    // since the other process may have a different notion of default.
     SkTypeface* tf = this->getTypeface();
-    if (!tf && buffer.isCrossProcess()) {
+    if (!tf) {
+        // We force recording our typeface, even if its "default" since the receiver process
+        // may have a different notion of default.
         tf = SkTypeface::GetDefaultTypeface();
+        SkASSERT(tf);
     }
 
-    uint8_t flatFlags = 0;
+    uint8_t flatFlags = kHasTypeface_FlatFlag;
-    if (tf) {
+
-        flatFlags |= kHasTypeface_FlatFlag;
-    }
     if (asint(this->getPathEffect()) |
         asint(this->getShader()) |
         asint(this->getMaskFilter()) |
@@ -1899,11 +1898,8 @@ void SkPaint::flatten(SkWriteBuffer& buffer) const {
                             (this->getStyle() << 4) | this->getTextEncoding(),
                             fBlendMode));
 
-    // now we're done with ptr and the (pre)reserved space. If we need to write
+    buffer.writeTypeface(tf);
-    // additional fields, use the buffer directly
+
-    if (flatFlags & kHasTypeface_FlatFlag) {
-        buffer.writeTypeface(tf);
-    }
     if (flatFlags & kHasEffects_FlatFlag) {
         buffer.writeFlattenable(this->getPathEffect());
         buffer.writeFlattenable(this->getShader());

src/core/SkPicture.cpp

@@ -66,13 +66,6 @@ SkPictInfo SkPicture::createHeader() const {
     // Set picture info after magic bytes in the header
     info.setVersion(CURRENT_PICTURE_VERSION);
     info.fCullRect = this->cullRect();
-    info.fFlags = SkPictInfo::kCrossProcess_Flag;
-    // TODO: remove this flag, since we're always float (now)
-    info.fFlags |= SkPictInfo::kScalarIsFloat_Flag;
-
-    if (8 == sizeof(void*)) {
-        info.fFlags |= SkPictInfo::kPtrIs64Bit_Flag;
-    }
     return info;
 }
 
@@ -102,7 +95,9 @@ bool SkPicture::StreamIsSKP(SkStream* stream, SkPictInfo* pInfo) {
     info.fCullRect.fTop    = stream->readScalar();
     info.fCullRect.fRight  = stream->readScalar();
     info.fCullRect.fBottom = stream->readScalar();
-    info.fFlags            = stream->readU32();
+    if (info.getVersion() < SkReadBuffer::kRemoveHeaderFlags_Version) {
+        (void)stream->readU32();    // used to be flags
+    }
 
     if (IsValidPictInfo(info)) {
         if (pInfo) { *pInfo = info; }
@@ -123,7 +118,9 @@ bool SkPicture::BufferIsSKP(SkReadBuffer* buffer, SkPictInfo* pInfo) {
 
     info.setVersion(buffer->readUInt());
     buffer->readRect(&info.fCullRect);
-    info.fFlags = buffer->readUInt();
+    if (info.getVersion() < SkReadBuffer::kRemoveHeaderFlags_Version) {
+        (void)buffer->readUInt();   // used to be flags
+    }
 
     if (IsValidPictInfo(info)) {
         if (pInfo) { *pInfo = info; }
@@ -306,7 +303,6 @@ void SkPicture::flatten(SkWriteBuffer& buffer) const {
     buffer.writeByteArray(&info.fMagic, sizeof(info.fMagic));
     buffer.writeUInt(info.getVersion());
     buffer.writeRect(info.fCullRect);
-    buffer.writeUInt(info.fFlags);
 
     if (auto custom = custom_serialize(this, buffer.fProcs)) {
         int32_t size = SkToS32(custom->size());

src/core/SkPictureData.cpp

@@ -279,7 +279,7 @@ void SkPictureData::serialize(SkWStream* stream, const SkSerialProcs& procs,
     // We delay serializing the bulk of our data until after we've serialized
     // factories and typefaces by first serializing to an in-memory write buffer.
     SkFactorySet factSet;  // buffer refs factSet, so factSet must come first.
-    SkBinaryWriteBuffer buffer(SkBinaryWriteBuffer::kCrossProcess_Flag);
+    SkBinaryWriteBuffer buffer;
     buffer.setFactoryRecorder(&factSet);
     buffer.setSerialProcs(procs);
     buffer.setTypefaceRecorder(typefaceSet);
@@ -344,29 +344,6 @@ void SkPictureData::flatten(SkWriteBuffer& buffer) const {
 
 ///////////////////////////////////////////////////////////////////////////////
 
-/**
- *  Return the corresponding SkReadBuffer flags, given a set of
- *  SkPictInfo flags.
- */
-static uint32_t pictInfoFlagsToReadBufferFlags(uint32_t pictInfoFlags) {
-    static const struct {
-        uint32_t    fSrc;
-        uint32_t    fDst;
-    } gSD[] = {
-        { SkPictInfo::kCrossProcess_Flag,   SkReadBuffer::kCrossProcess_Flag },
-        { SkPictInfo::kScalarIsFloat_Flag,  SkReadBuffer::kScalarIsFloat_Flag },
-        { SkPictInfo::kPtrIs64Bit_Flag,     SkReadBuffer::kPtrIs64Bit_Flag },
-    };
-
-    uint32_t rbMask = 0;
-    for (size_t i = 0; i < SK_ARRAY_COUNT(gSD); ++i) {
-        if (pictInfoFlags & gSD[i].fSrc) {
-            rbMask |= gSD[i].fDst;
-        }
-    }
-    return rbMask;
-}
-
 bool SkPictureData::parseStreamTag(SkStream* stream,
                                    uint32_t tag,
                                    uint32_t size,
@@ -437,7 +414,6 @@ bool SkPictureData::parseStreamTag(SkStream* stream,
             }
 
             SkReadBuffer buffer(storage.get(), size);
-            buffer.setFlags(pictInfoFlagsToReadBufferFlags(fInfo.fFlags));
             buffer.setVersion(fInfo.getVersion());
 
             if (!fFactoryPlayback) {

src/core/SkPictureData.h

@@ -28,12 +28,6 @@ class SkReadBuffer;
 class SkTextBlob;
 
 struct SkPictInfo {
-    enum Flags {
-        kCrossProcess_Flag      = 1 << 0,
-        kScalarIsFloat_Flag     = 1 << 1,
-        kPtrIs64Bit_Flag        = 1 << 2,
-    };
-
     SkPictInfo() : fVersion(~0U) {}
 
     uint32_t getVersion() const {
@@ -52,7 +46,6 @@ private:
     uint32_t    fVersion;
 public:
     SkRect      fCullRect;
-    uint32_t    fFlags;
 };
 
 #define SK_PICT_READER_TAG     SkSetFourByteTag('r', 'e', 'a', 'd')

src/core/SkReadBuffer.cpp

@@ -36,17 +36,7 @@ namespace {
 } // anonymous namespace
 
 
-static uint32_t default_flags() {
-    uint32_t flags = 0;
-    flags |= SkReadBuffer::kScalarIsFloat_Flag;
-    if (8 == sizeof(void*)) {
-        flags |= SkReadBuffer::kPtrIs64Bit_Flag;
-    }
-    return flags;
-}
-
 SkReadBuffer::SkReadBuffer() {
-    fFlags = default_flags();
     fVersion = 0;
     fMemoryPtr = nullptr;
 
@@ -61,7 +51,6 @@ SkReadBuffer::SkReadBuffer() {
 }
 
 SkReadBuffer::SkReadBuffer(const void* data, size_t size) {
-    fFlags = default_flags();
     fVersion = 0;
     this->setMemory(data, size);
     fMemoryPtr = nullptr;

src/core/SkReadBuffer.h

@@ -76,6 +76,7 @@ public:
         kTileInfoInSweepGradient_Version   = 57,
         k2PtConicalNoFlip_Version          = 58,
         kRemovePictureImageFilterLocalSpace = 59,
+        kRemoveHeaderFlags_Version         = 60,
     };
 
     /**
@@ -94,23 +95,6 @@ public:
         fVersion = version;
     }
 
-    enum Flags {
-        kCrossProcess_Flag  = 1 << 0,
-        kScalarIsFloat_Flag = 1 << 1,
-        kPtrIs64Bit_Flag    = 1 << 2,
-        kValidation_Flag    = 1 << 3,
-    };
-
-    void setFlags(uint32_t flags) { fFlags = flags; }
-    uint32_t getFlags() const { return fFlags; }
-
-    bool isCrossProcess() const {
-        return this->isValidating() || SkToBool(fFlags & kCrossProcess_Flag);
-    }
-    bool isScalarFloat() const { return SkToBool(fFlags & kScalarIsFloat_Flag); }
-    bool isPtr64Bit() const { return SkToBool(fFlags & kPtrIs64Bit_Flag); }
-    bool isValidating() const { return SkToBool(fFlags & kValidation_Flag); }
-
     size_t size() { return fReader.size(); }
     size_t offset() { return fReader.offset(); }
     bool eof() { return fReader.eof(); }
@@ -281,7 +265,6 @@ private:
     bool readArray(void* value, size_t size, size_t elementSize);
     void setMemory(const void*, size_t);
 
-    uint32_t fFlags;
     int fVersion;
 
     void* fMemoryPtr;

src/core/SkRecordedDrawable.cpp

@@ -77,7 +77,6 @@ sk_sp<SkFlattenable> SkRecordedDrawable::CreateProc(SkReadBuffer& buffer) {
     SkPictInfo info;
     info.setVersion(buffer.getVersion());
     info.fCullRect = bounds;
-    info.fFlags = 0;    // ???
     std::unique_ptr<SkPictureData> pictureData(SkPictureData::CreateFromBuffer(buffer, info));
     if (!pictureData) {
         return nullptr;

src/core/SkWriteBuffer.cpp

@@ -16,15 +16,13 @@
 
 ///////////////////////////////////////////////////////////////////////////////////////////////////
 
-SkBinaryWriteBuffer::SkBinaryWriteBuffer(uint32_t flags)
+SkBinaryWriteBuffer::SkBinaryWriteBuffer()
-    : fFlags(flags)
+    : fFactorySet(nullptr)
-    , fFactorySet(nullptr)
     , fTFSet(nullptr) {
 }
 
-SkBinaryWriteBuffer::SkBinaryWriteBuffer(void* storage, size_t storageSize, uint32_t flags)
+SkBinaryWriteBuffer::SkBinaryWriteBuffer(void* storage, size_t storageSize)
-    : fFlags(flags)
+    : fFactorySet(nullptr)
-    , fFactorySet(nullptr)
     , fWriter(storage, storageSize)
     , fTFSet(nullptr) {
 }

src/core/SkWriteBuffer.h

@@ -25,8 +25,6 @@ public:
     SkWriteBuffer() {}
     virtual ~SkWriteBuffer() {}
 
-    virtual bool isCrossProcess() const = 0;
-
     virtual void writePad32(const void* buffer, size_t bytes) = 0;
 
     virtual void writeByteArray(const void* data, size_t size) = 0;
@@ -77,18 +75,10 @@ protected:
  */
 class SkBinaryWriteBuffer : public SkWriteBuffer {
 public:
-    enum Flags {
+    SkBinaryWriteBuffer();
-        kCrossProcess_Flag = 1 << 0,
+    SkBinaryWriteBuffer(void* initialStorage, size_t storageSize);
-    };
-
-    SkBinaryWriteBuffer(uint32_t flags = 0);
-    SkBinaryWriteBuffer(void* initialStorage, size_t storageSize, uint32_t flags = 0);
     ~SkBinaryWriteBuffer() override;
 
-    bool isCrossProcess() const override {
-        return SkToBool(fFlags & kCrossProcess_Flag);
-    }
-
     void write(const void* buffer, size_t bytes) {
         fWriter.write(buffer, bytes);
     }
@@ -135,7 +125,6 @@ public:
     SkRefCntSet* setTypefaceRecorder(SkRefCntSet*);
 
 private:
-    const uint32_t fFlags;
     SkFactorySet* fFactorySet;
     SkWriter32 fWriter;
 

src/effects/SkPictureImageFilter.cpp

@@ -56,7 +56,7 @@ sk_sp<SkFlattenable> SkPictureImageFilter::CreateProc(SkReadBuffer& buffer) {
     sk_sp<SkPicture> picture;
     SkRect cropRect;
 
-    if (buffer.isCrossProcess() && SkPicture::PictureIOSecurityPrecautionsEnabled()) {
+    if (SkPicture::PictureIOSecurityPrecautionsEnabled()) {
         buffer.validate(!buffer.readBool());
     } else {
         if (buffer.readBool()) {
@@ -77,7 +77,7 @@ sk_sp<SkFlattenable> SkPictureImageFilter::CreateProc(SkReadBuffer& buffer) {
 }
 
 void SkPictureImageFilter::flatten(SkWriteBuffer& buffer) const {
-    if (buffer.isCrossProcess() && SkPicture::PictureIOSecurityPrecautionsEnabled()) {
+    if (SkPicture::PictureIOSecurityPrecautionsEnabled()) {
         buffer.writeBool(false);
     } else {
         bool hasPicture = (fPicture != nullptr);

src/shaders/SkPictureShader.cpp

@@ -162,9 +162,7 @@ sk_sp<SkFlattenable> SkPictureShader::CreateProc(SkReadBuffer& buffer) {
 
     sk_sp<SkPicture> picture;
 
-    if (buffer.isCrossProcess() && SkPicture::PictureIOSecurityPrecautionsEnabled()) {
+    if (SkPicture::PictureIOSecurityPrecautionsEnabled()) {
-        // Newer code won't serialize pictures in disallow-cross-process-picture mode.
-        // Assert that they didn't serialize anything except a false here.
         buffer.validate(!buffer.readBool());
     } else {
         bool didSerialize = buffer.readBool();
@@ -183,7 +181,7 @@ void SkPictureShader::flatten(SkWriteBuffer& buffer) const {
 
     // The deserialization code won't trust that our serialized picture is safe to deserialize.
     // So write a 'false' telling it that we're not serializing a picture.
-    if (buffer.isCrossProcess() && SkPicture::PictureIOSecurityPrecautionsEnabled()) {
+    if (SkPicture::PictureIOSecurityPrecautionsEnabled()) {
         buffer.writeBool(false);
     } else {
         buffer.writeBool(true);

tools/debugger/SkJsonWriteBuffer.h

@@ -21,8 +21,6 @@ public:
         : fUrlDataManager(urlDataManager)
         , fJson(Json::objectValue) {}
 
-    bool isCrossProcess() const override { return false; }
-
     void writePad32(const void* buffer, size_t bytes) override;
     void writeByteArray(const void* data, size_t size) override;
     void writeBool(bool value) override;

tools/skpinfo.cpp

@@ -64,28 +64,6 @@ int main(int argc, char** argv) {
                  info.fCullRect.fLeft, info.fCullRect.fTop,
                  info.fCullRect.fRight, info.fCullRect.fBottom);
     }
-    if (FLAGS_flags && !FLAGS_quiet) {
-        SkDebugf("Flags: ");
-        bool needsSeparator = false;
-        if (info.fFlags & SkPictInfo::kCrossProcess_Flag) {
-            SkDebugf("kCrossProcess");
-            needsSeparator = true;
-        }
-        if (info.fFlags & SkPictInfo::kScalarIsFloat_Flag) {
-            if (needsSeparator) {
-                SkDebugf("|");
-            }
-            SkDebugf("kScalarIsFloat");
-            needsSeparator = true;
-        }
-        if (info.fFlags & SkPictInfo::kPtrIs64Bit_Flag) {
-            if (needsSeparator) {
-                SkDebugf("|");
-            }
-            SkDebugf("kPtrIs64Bit");
-        }
-        SkDebugf("\n");
-    }
 
     if (!stream.readBool()) {
         // If we read true there's a picture playback object flattened