SkPDF: Refactor SkPDFObject heiararchy.

src/pdf/SkPDFTypes.cpp

Index: src/pdf/SkPDFTypes.cpp
diff --git a/src/pdf/SkPDFTypes.cpp b/src/pdf/SkPDFTypes.cpp
index cd130cfc520b46f36918b80b016f7da5bf713da1..e1f155fcf6b17e1b021ed5a78433efbd87999439 100644
--- a/src/pdf/SkPDFTypes.cpp
+++ b/src/pdf/SkPDFTypes.cpp
@@ -6,7 +6,6 @@
  * found in the LICENSE file.
  */
 
-
 #include "SkPDFTypes.h"
 #include "SkStream.h"
 
@@ -18,61 +17,265 @@
 
 ////////////////////////////////////////////////////////////////////////////////
 
-SkPDFObjRef::SkPDFObjRef(SkPDFObject* obj) : fObj(obj) {
-    SkSafeRef(obj);
+SkString* pun(char* x) { return reinterpret_cast<SkString*>(x); }
+const SkString* pun(const char* x) {
+    return reinterpret_cast<const SkString*>(x);
 }
 
-SkPDFObjRef::~SkPDFObjRef() {}
+SkPDFUnion::SkPDFUnion(Type t) : fType(t) {}
 
-void SkPDFObjRef::emitObject(SkWStream* stream,
-                             const SkPDFObjNumMap& objNumMap,
-                             const SkPDFSubstituteMap& substitutes) {
-    SkPDFObject* obj = substitutes.getSubstitute(fObj);
-    stream->writeDecAsText(objNumMap.getObjectNumber(obj));
-    stream->writeText(" 0 R");  // Generation number is always 0.
+SkPDFUnion::~SkPDFUnion() {
+    switch (fType) {
+        case Type::kNameSkS:
+        case Type::kStringSkS:
+            pun(fSkString)->~SkString();
+            return;
+        case Type::kObjRef:
+        case Type::kObject:
+            SkSafeUnref(fObject);
+            return;
+        default:
+            return;
+    }
 }
 
-void SkPDFObjRef::addResources(SkPDFObjNumMap* catalog,
-                               const SkPDFSubstituteMap& substitutes) const {
-    SkPDFObject* obj = substitutes.getSubstitute(fObj);
-    SkASSERT(obj);
-    if (catalog->addObject(obj)) {
-        obj->addResources(catalog, substitutes);
+SkPDFUnion& SkPDFUnion::operator=(SkPDFUnion&& other) {
+    if (this != &other) {

[tomhudson, 2015/04/24 21:32:29]

Never worked with && before, so I'm assuming you've got this right / tested it
explicitly / it's isomorphic to **.

[hal.canary, 2015/04/24 21:51:46]

It is not isomorphic to **.

This is C++11 notation for a r-value reference, that is: a temporary value.

+        this->~SkPDFUnion();
+        SkNEW_PLACEMENT_ARGS(this, SkPDFUnion, (other.move()));
     }
+    return *this;
+}
+
+SkPDFUnion::SkPDFUnion(SkPDFUnion&& other) {
+    SkASSERT(this != &other);
+    memcpy(this, &other, sizeof(*this));
+    other.fType = Type::kDestroyed;
+}
+
+#if 0
+SkPDFUnion SkPDFUnion::copy() const {
+    SkPDFUnion u(fType);
+    memcpy(&u, this, sizeof(u));
+    switch (fType) {
+        case Type::kNameSkS:
+        case Type::kStringSkS:
+            SkNEW_PLACEMENT_ARGS(pun(u.fSkString), SkString,
+                                 (*pun(fSkString)));
+            return u.move();
+        case Type::kObjRef:
+        case Type::kObject:
+            SkRef(u.fObject);
+            return u.move();
+        default:
+            return u.move();
+    }
+}
+SkPDFUnion& SkPDFUnion::operator=(const SkPDFUnion& other) {
+    return *this = other.copy();
 }
+SkPDFUnion::SkPDFUnion(const SkPDFUnion& other) {
+    *this = other.copy();
+}
+#endif
 
-////////////////////////////////////////////////////////////////////////////////
+bool SkPDFUnion::isName() const {
+    return Type::kName == fType || Type::kNameSkS == fType;
+}
 
-SkPDFInt::SkPDFInt(int32_t value) : fValue(value) {}
-SkPDFInt::~SkPDFInt() {}
+bool SkPDFUnion::IsValidName(const char* n) {
+    static const char kControlChars[] = "/%()<>[]{}";
+    while (*n) {
+        if (*n < '!' || *n > '~' || strchr(kControlChars, *n)) {
+            return false;
+        }
+        ++n;
+    }
+    return true;
+}
 
-void SkPDFInt::emitObject(SkWStream* stream,
-                          const SkPDFObjNumMap&,
-                          const SkPDFSubstituteMap&) {
-    stream->writeDecAsText(fValue);
+SkString SkPDFUnion::EscapeName(const char* name, size_t len) {
+    static const char kToEscape[] = "#/%()<>[]{}";
+    int count = 0;
+    const char* const end = &name[len];
+    for (const char* n = name; n != end; ++n) {
+        if (*n < '!' || *n > '~' || strchr(kToEscape, *n)) {
+            count += 2;
+        }
+        ++count;
+    }
+    SkString result(count);
+    char* s = result.writable_str();
+    static const char kHex[] = "0123456789ABCDEF";
+    for (const char* n = name; n != end; ++n) {
+        if (*n < '!' || *n > '~' || strchr(kToEscape, *n)) {
+            *s++ = '#';
+            *s++ = kHex[(*n >> 4) & 0xF];
+            *s++ = kHex[*n & 0xF];
+        } else {
+            *s++ = *n;
+        }
+    }
+    SkASSERT(&result.writable_str()[count] == s);  // don't over-write
+    return result;                                 // allocated space
 }
 
-////////////////////////////////////////////////////////////////////////////////
+static void write_string(SkWStream* o, const SkString& s) {
+    o->write(s.c_str(), s.size());
+}
+
+static SkString format_string(const SkString& s) {
+    return SkPDFString::FormatString(s.c_str(), s.size());
+}
 
-SkPDFBool::SkPDFBool(bool value) : fValue(value) {}
-SkPDFBool::~SkPDFBool() {}
+static SkString format_string(const char* s) {
+    return SkPDFString::FormatString(s, strlen(s));
+}
 
-void SkPDFBool::emitObject(SkWStream* stream,
-                          const SkPDFObjNumMap&,
-                          const SkPDFSubstituteMap&) {
-    stream->writeText(fValue ? "true" : "false");
+SkString escape_name(const SkString& name) {
+    return SkPDFUnion::EscapeName(name.c_str(), name.size());
 }
 
-////////////////////////////////////////////////////////////////////////////////
+void SkPDFUnion::emitObject(SkWStream* stream,
+                            const SkPDFObjNumMap& objNumMap,
+                            const SkPDFSubstituteMap& substitutes) const {
+    switch (fType) {
+        case Type::kInt:
+            stream->writeDecAsText(fIntValue);
+            return;
+        case Type::kBool:
+            stream->writeText(fBoolValue ? "true" : "false");
+            return;
+        case Type::kScalar:
+            SkPDFScalar::Append(fScalarValue, stream);
+            return;
+        case Type::kName:
+            stream->writeText("/");
+            SkASSERT(SkPDFUnion::IsValidName(fStaticString));
+            stream->writeText(fStaticString);
+            return;
+        case Type::kString:
+            SkASSERT(fStaticString);
+            write_string(stream, format_string(fStaticString));
+            return;
+        case Type::kNameSkS:
+            stream->writeText("/");
+            write_string(stream, escape_name(*pun(fSkString)));
+            return;
+        case Type::kStringSkS:
+            write_string(stream, format_string(*pun(fSkString)));
+            return;
+        case Type::kObjRef:
+            stream->writeDecAsText(objNumMap.getObjectNumber(
+                    substitutes.getSubstitute(fObject)));
+            stream->writeText(" 0 R");  // Generation number is always 0.
+            return;
+        case Type::kObject:
+            fObject->emitObject(stream, objNumMap, substitutes);
+            return;
+        default:
+            SkDEBUGFAIL("SkPDFUnion::emitObject with bad type");
+    }
+}
+
+void SkPDFUnion::addResources(SkPDFObjNumMap* objNumMap,
+                              const SkPDFSubstituteMap& substituteMap) const {
+    switch (fType) {
+        case Type::kInt:
+        case Type::kBool:
+        case Type::kScalar:
+        case Type::kName:
+        case Type::kString:
+        case Type::kNameSkS:
+        case Type::kStringSkS:
+            return;  // These have no resources.
+        case Type::kObjRef: {
+            SkPDFObject* obj = substituteMap.getSubstitute(fObject);
+            if (objNumMap->addObject(obj)) {
+                obj->addResources(objNumMap, substituteMap);
+            }
+            return;
+        }
+        case Type::kObject:
+            fObject->addResources(objNumMap, substituteMap);
+            return;
+        default:
+            SkDEBUGFAIL("SkPDFUnion::addResources with bad type");
+    }
+}
+
+SkPDFUnion SkPDFUnion::Int(int32_t value) {
+    SkPDFUnion u(Type::kInt);
+    u.fIntValue = value;
+    return u.move();
+}
+
+SkPDFUnion SkPDFUnion::Bool(bool value) {
+    SkPDFUnion u(Type::kBool);
+    u.fBoolValue = value;
+    return u.move();
+}
+
+SkPDFUnion SkPDFUnion::Scalar(SkScalar value) {
+    SkPDFUnion u(Type::kScalar);
+    u.fScalarValue = value;
+    return u.move();
+}
+
+SkPDFUnion SkPDFUnion::Name(const char* value) {
+    SkPDFUnion u(Type::kName);
+    SkASSERT(value);
+    SkASSERT(SkPDFUnion::IsValidName(value));
+    u.fStaticString = value;
+    return u.move();
+}
+
+SkPDFUnion SkPDFUnion::String(const char* value) {
+    SkPDFUnion u(Type::kString);
+    SkASSERT(value);
+    u.fStaticString = value;
+    return u.move();
+}
+
+SkPDFUnion SkPDFUnion::Name(const SkString& s) {
+    SkPDFUnion u(Type::kNameSkS);
+    SkNEW_PLACEMENT_ARGS(pun(u.fSkString), SkString, (s));
+    return u.move();
+}
+
+SkPDFUnion SkPDFUnion::String(const SkString& s) {
+    SkPDFUnion u(Type::kStringSkS);
+    SkNEW_PLACEMENT_ARGS(pun(u.fSkString), SkString, (s));
+    return u.move();
+}
+
+SkPDFUnion SkPDFUnion::ObjRef(SkPDFObject* ptr) {
+    SkPDFUnion u(Type::kObjRef);
+    SkASSERT(ptr);
+    u.fObject = ptr;
+    return u.move();
+}
+
+SkPDFUnion SkPDFUnion::Object(SkPDFObject* ptr) {
+    SkPDFUnion u(Type::kObject);
+    SkASSERT(ptr);
+    u.fObject = ptr;
+    return u.move();
+}
 
-SkPDFScalar::SkPDFScalar(SkScalar value) : fValue(value) {}
-SkPDFScalar::~SkPDFScalar() {}
+////////////////////////////////////////////////////////////////////////////////
 
-void SkPDFScalar::emitObject(SkWStream* stream,
-                             const SkPDFObjNumMap&,
-                             const SkPDFSubstituteMap&) {
-    SkPDFScalar::Append(fValue, stream);
+void SkPDFAtom::emitObject(SkWStream* stream,
+                           const SkPDFObjNumMap& objNumMap,
+                           const SkPDFSubstituteMap& substitutes) {
+    fValue.emitObject(stream, objNumMap, substitutes);
 }
+void SkPDFAtom::addResources(SkPDFObjNumMap* map,
+                             const SkPDFSubstituteMap& substitutes) const {
+    fValue.addResources(map, substitutes);
+}
+
+////////////////////////////////////////////////////////////////////////////////
 
 // static
 void SkPDFScalar::Append(SkScalar value, SkWStream* stream) {
@@ -125,23 +328,8 @@ void SkPDFScalar::Append(SkScalar value, SkWStream* stream) {
 
 ////////////////////////////////////////////////////////////////////////////////
 
-SkPDFString::SkPDFString(const char value[])
-    : fValue(FormatString(value, strlen(value))) {
-}
-
-SkPDFString::SkPDFString(const SkString& value)
-    : fValue(FormatString(value.c_str(), value.size())) {
-}
-
-SkPDFString::~SkPDFString() {}
-
-void SkPDFString::emitObject(SkWStream* stream,
-                             const SkPDFObjNumMap&,
-                             const SkPDFSubstituteMap&) {
-    stream->write(fValue.c_str(), fValue.size());
-}
-
 // static
+
 SkString SkPDFString::FormatString(const char* cin, size_t len) {
     SkASSERT(len <= kMaxLen);
 
@@ -187,55 +375,25 @@ SkString SkPDFString::FormatString(const char* cin, size_t len) {
 
 ////////////////////////////////////////////////////////////////////////////////
 
-SkPDFName::SkPDFName(const char name[]) : fValue(FormatName(SkString(name))) {}
-SkPDFName::SkPDFName(const SkString& name) : fValue(FormatName(name)) {}
-SkPDFName::~SkPDFName() {}
-
-bool SkPDFName::operator==(const SkPDFName& b) const {
-    return fValue == b.fValue;
-}
-
-void SkPDFName::emitObject(SkWStream* stream,
-                             const SkPDFObjNumMap&,
-                             const SkPDFSubstituteMap&) {
-    stream->write(fValue.c_str(), fValue.size());
-}
-
-// static
-SkString SkPDFName::FormatName(const SkString& input) {
-    SkASSERT(input.size() <= kMaxLen);
-    // TODO(vandebo) If more escaping is needed, improve the linear scan.
-    static const char escaped[] = "#/%()<>[]{}";
-
-    SkString result("/");
-    for (size_t i = 0; i < input.size(); i++) {
-        if (input[i] & 0x80 || input[i] < '!' || strchr(escaped, input[i])) {
-            result.append("#");
-            // Mask with 0xFF to avoid sign extension. i.e. #FFFFFF81
-            result.appendHex(input[i] & 0xFF, 2);
-        } else {
-            result.append(input.c_str() + i, 1);
-        }
+SkPDFArray::SkPDFArray() {}
+SkPDFArray::~SkPDFArray() {
+    for (SkPDFUnion& value : fValues) {
+        value.~SkPDFUnion();
     }
-
-    return result;
+    fValues.reset();
 }
 
-////////////////////////////////////////////////////////////////////////////////
+int SkPDFArray::size() const { return fValues.count(); }
 
-SkPDFArray::SkPDFArray() {}
-SkPDFArray::~SkPDFArray() {
-    fValue.unrefAll();
-}
+void SkPDFArray::reserve(int length) { fValues.setReserve(length); }
 
 void SkPDFArray::emitObject(SkWStream* stream,
                              const SkPDFObjNumMap& objNumMap,
                              const SkPDFSubstituteMap& substitutes) {
     stream->writeText("[");
-    for (int i = 0; i < fValue.count(); i++) {
-        SkASSERT(substitutes.getSubstitute(fValue[i]) == fValue[i]);

[tomhudson, 2015/04/24 21:32:29]

Not worth maintaining this invariant?

[hal.canary, 2015/04/25 13:41:02]

Only top level objects (referred to via ObjRefs) should have substitutes.  I put
this assert in place to make sure I understood the code.  Now I am confident it
is unnecessary.

-        fValue[i]->emitObject(stream, objNumMap, substitutes);
-        if (i + 1 < fValue.count()) {
+    for (int i = 0; i < fValues.count(); i++) {
+        fValues[i].emitObject(stream, objNumMap, substitutes);
+        if (i + 1 < fValues.count()) {
             stream->writeText(" ");
         }
     }
@@ -244,68 +402,78 @@ void SkPDFArray::emitObject(SkWStream* stream,
 
 void SkPDFArray::addResources(SkPDFObjNumMap* catalog,
                               const SkPDFSubstituteMap& substitutes) const {
-    for (int i = 0; i < fValue.count(); i++) {
-        SkASSERT(substitutes.getSubstitute(fValue[i]) == fValue[i]);
-        fValue[i]->addResources(catalog, substitutes);
+    for (const SkPDFUnion& value : fValues) {
+        value.addResources(catalog, substitutes);
     }
 }
 
-void SkPDFArray::reserve(int length) {
-    SkASSERT(length <= kMaxLen);
-    fValue.setReserve(length);
+// void SkPDFArray::append(const SkPDFUnion& value) {
+//     SkNEW_PLACEMENT_ARGS(fValues.append(), SkPDFUnion, (value));
+// }

[tomhudson, 2015/04/24 21:32:29]

Commented out code - delete?

[hal.canary, 2015/04/25 13:41:02]

Done.

+
+void SkPDFArray::append(SkPDFUnion&& value) {
+    SkNEW_PLACEMENT_ARGS(fValues.append(), SkPDFUnion, (value.move()));
 }
 
 SkPDFObject* SkPDFArray::append(SkPDFObject* value) {
-    SkASSERT(fValue.count() < kMaxLen);
-    value->ref();
-    fValue.push(value);
+    // DEPRECATED
+    this->append(SkPDFUnion::Object(SkRef(value)));
     return value;
 }
 
 void SkPDFArray::appendInt(int32_t value) {
-    SkASSERT(fValue.count() < kMaxLen);
-    fValue.push(new SkPDFInt(value));
+    this->append(SkPDFUnion::Int(value));
+}
+
+void SkPDFArray::appendBool(bool value) {
+    this->append(SkPDFUnion::Bool(value));
 }
 
 void SkPDFArray::appendScalar(SkScalar value) {
-    SkASSERT(fValue.count() < kMaxLen);
-    fValue.push(new SkPDFScalar(value));
+    this->append(SkPDFUnion::Scalar(value));
 }
 
 void SkPDFArray::appendName(const char name[]) {
-    SkASSERT(fValue.count() < kMaxLen);
-    fValue.push(new SkPDFName(name));
+    this->append(SkPDFUnion::Name(SkString(name)));
 }
 
-///////////////////////////////////////////////////////////////////////////////
+void SkPDFArray::appendName(const SkString& name) {
+    this->append(SkPDFUnion::Name(name));
+}
 
-SkPDFDict::SkPDFDict() {}
+void SkPDFArray::appendString(const SkString& value) {
+    this->append(SkPDFUnion::String(value));
+}
 
-SkPDFDict::SkPDFDict(const char type[]) {
-    insertName("Type", type);
+void SkPDFArray::appendString(const char value[]) {
+    this->append(SkPDFUnion::String(value));
 }
 
-SkPDFDict::~SkPDFDict() {
-    clear();
+void SkPDFArray::appendObject(SkPDFObject* value) {
+    this->append(SkPDFUnion::Object(value));
 }
 
-int SkPDFDict::size() const {
-    return fValue.count();
+void SkPDFArray::appendObjRef(SkPDFObject* value) {
+    this->append(SkPDFUnion::ObjRef(value));
 }
 
+///////////////////////////////////////////////////////////////////////////////
+
+SkPDFDict::SkPDFDict() {}
+
+SkPDFDict::~SkPDFDict() { this->clear(); }
+
+SkPDFDict::SkPDFDict(const char type[]) { this->insertName("Type", type); }
+
 void SkPDFDict::emitObject(SkWStream* stream,
                            const SkPDFObjNumMap& objNumMap,
                            const SkPDFSubstituteMap& substitutes) {
     stream->writeText("<<");
-    for (int i = 0; i < fValue.count(); i++) {
-        SkASSERT(fValue[i].key);
-        SkASSERT(fValue[i].value);
-        SkASSERT(substitutes.getSubstitute(fValue[i].key) == fValue[i].key);
-        SkASSERT(substitutes.getSubstitute(fValue[i].value) == fValue[i].value);
-        fValue[i].key->emitObject(stream, objNumMap, substitutes);
+    for (int i = 0; i < fRecords.count(); i++) {
+        fRecords[i].fKey.emitObject(stream, objNumMap, substitutes);
         stream->writeText(" ");
-        fValue[i].value->emitObject(stream, objNumMap, substitutes);
-        if (i + 1 < fValue.count()) {
+        fRecords[i].fValue.emitObject(stream, objNumMap, substitutes);
+        if (i + 1 < fRecords.count()) {
             stream->writeText("\n");
         }
     }
@@ -314,72 +482,75 @@ void SkPDFDict::emitObject(SkWStream* stream,
 
 void SkPDFDict::addResources(SkPDFObjNumMap* catalog,
                              const SkPDFSubstituteMap& substitutes) const {
-    for (int i = 0; i < fValue.count(); i++) {
-        SkASSERT(fValue[i].key);
-        SkASSERT(fValue[i].value);
-        fValue[i].key->addResources(catalog, substitutes);
-        SkASSERT(substitutes.getSubstitute(fValue[i].value) == fValue[i].value);
-        fValue[i].value->addResources(catalog, substitutes);
+    for (int i = 0; i < fRecords.count(); i++) {
+        fRecords[i].fKey.addResources(catalog, substitutes);
+        fRecords[i].fValue.addResources(catalog, substitutes);
     }
 }
 
-SkPDFObject*  SkPDFDict::append(SkPDFName* key, SkPDFObject* value) {
-    SkASSERT(key);
-    SkASSERT(value);
-    *(fValue.append()) = Rec(key, value);
-    return value;
+void SkPDFDict::set(SkPDFUnion&& name, SkPDFUnion&& value) {
+    Record* rec = fRecords.append();
+    SkASSERT(name.isName());
+    SkNEW_PLACEMENT_ARGS(&rec->fKey, SkPDFUnion, (name.move()));
+    SkNEW_PLACEMENT_ARGS(&rec->fValue, SkPDFUnion, (value.move()));
+}
+
+int SkPDFDict::size() const { return fRecords.count(); }
+
+void SkPDFDict::insertObjRef(const char key[], SkPDFObject* value) {
+    this->set(SkPDFUnion::Name(key), SkPDFUnion::ObjRef(value));
+}
+void SkPDFDict::insertObjRef(const SkString& key, SkPDFObject* value) {
+    this->set(SkPDFUnion::Name(key), SkPDFUnion::ObjRef(value));
 }
 
-SkPDFObject* SkPDFDict::insert(SkPDFName* key, SkPDFObject* value) {
-    return this->append(SkRef(key), SkRef(value));
+void SkPDFDict::insertObject(const char key[], SkPDFObject* value) {
+    this->set(SkPDFUnion::Name(key), SkPDFUnion::Object(value));
+}
+void SkPDFDict::insertObject(const SkString& key, SkPDFObject* value) {
+    this->set(SkPDFUnion::Name(key), SkPDFUnion::Object(value));
 }
 
+// DEPRECATED
 SkPDFObject* SkPDFDict::insert(const char key[], SkPDFObject* value) {
-    return this->append(new SkPDFName(key), SkRef(value));
+    this->set(SkPDFUnion::Name(key), SkPDFUnion::Object(SkRef(value)));
+    return value;
 }
 
 void SkPDFDict::insertInt(const char key[], int32_t value) {
-    (void)this->append(new SkPDFName(key), new SkPDFInt(value));
+    this->set(SkPDFUnion::Name(key), SkPDFUnion::Int(value));
+}
+
+void SkPDFDict::insertInt(const char key[], size_t value) {
+    this->insertInt(key, SkToS32(value));
 }
 
 void SkPDFDict::insertScalar(const char key[], SkScalar value) {
-    (void)this->append(new SkPDFName(key), new SkPDFScalar(value));
+    this->set(SkPDFUnion::Name(key), SkPDFUnion::Scalar(value));
 }
 
 void SkPDFDict::insertName(const char key[], const char name[]) {
-    (void)this->append(new SkPDFName(key), new SkPDFName(name));
+    this->set(SkPDFUnion::Name(key), SkPDFUnion::Name(name));
 }
 
-void SkPDFDict::clear() {
-    for (int i = 0; i < fValue.count(); i++) {
-        SkASSERT(fValue[i].key);
-        SkASSERT(fValue[i].value);
-        fValue[i].key->unref();
-        fValue[i].value->unref();
-    }
-    fValue.reset();
-}
-
-void SkPDFDict::remove(const char key[]) {
-    SkASSERT(key);
-    SkPDFName name(key);
-    for (int i = 0; i < fValue.count(); i++) {
-        SkASSERT(fValue[i].key);
-        if (*(fValue[i].key) == name) {
-            fValue[i].key->unref();
-            SkASSERT(fValue[i].value);
-            fValue[i].value->unref();
-            fValue.removeShuffle(i);
-            return;
-        }
-    }
+void SkPDFDict::insertName(const char key[], const SkString& name) {
+    this->set(SkPDFUnion::Name(key), SkPDFUnion::Name(name));
+}
+
+void SkPDFDict::insertString(const char key[], const char value[]) {
+    this->set(SkPDFUnion::Name(key), SkPDFUnion::String(value));
+}
+
+void SkPDFDict::insertString(const char key[], const SkString& value) {
+    this->set(SkPDFUnion::Name(key), SkPDFUnion::String(value));
 }
 
-void SkPDFDict::mergeFrom(const SkPDFDict& other) {
-    for (int i = 0; i < other.fValue.count(); i++) {
-        *(fValue.append()) =
-                Rec(SkRef(other.fValue[i].key), SkRef(other.fValue[i].value));
+void SkPDFDict::clear() {
+    for (Record& rec : fRecords) {
+        rec.fKey.~SkPDFUnion();
+        rec.fValue.~SkPDFUnion();
     }
+    fRecords.reset();
 }
 
 ////////////////////////////////////////////////////////////////////////////////