summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authortsepez <tsepez@chromium.org>2016-11-23 14:20:19 -0800
committerCommit bot <commit-bot@chromium.org>2016-11-23 14:20:19 -0800
commit81b224669647c9dd3c5ea2e013ee3cc109744bb8 (patch)
treeeb3d718b2be01b04fab9586c0b9f94035d1077d6
parent9067fd683ebf8d6467f8cc5aa7daf5e1f950f846 (diff)
downloadpdfium-81b224669647c9dd3c5ea2e013ee3cc109744bb8.tar.xz
Add CFX_MaybeOwned<> template.
This will allow us to get rid of more .release()s of unique_ptrs, as shown by the changed cpdf_colorspace usage. Review-Url: https://codereview.chromium.org/2526903002
-rw-r--r--BUILD.gn2
-rw-r--r--core/fpdfapi/page/cpdf_colorspace.cpp21
-rw-r--r--core/fxcrt/cfx_maybe_owned.h88
-rw-r--r--core/fxcrt/cfx_maybe_owned_unittest.cpp179
4 files changed, 277 insertions, 13 deletions
diff --git a/BUILD.gn b/BUILD.gn
index 29572e72c5..37ae9e0580 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -703,6 +703,7 @@ config("fxge_warnings") {
static_library("fxcrt") {
sources = [
+ "core/fxcrt/cfx_maybe_owned.h",
"core/fxcrt/cfx_observable.h",
"core/fxcrt/cfx_retain_ptr.h",
"core/fxcrt/cfx_shared_copy_on_write.h",
@@ -1734,6 +1735,7 @@ test("pdfium_unittests") {
"core/fpdftext/fpdf_text_int_unittest.cpp",
"core/fxcodec/codec/fx_codec_jpx_unittest.cpp",
"core/fxcodec/jbig2/JBig2_Image_unittest.cpp",
+ "core/fxcrt/cfx_maybe_owned_unittest.cpp",
"core/fxcrt/cfx_observable_unittest.cpp",
"core/fxcrt/cfx_retain_ptr_unittest.cpp",
"core/fxcrt/cfx_shared_copy_on_write_unittest.cpp",
diff --git a/core/fpdfapi/page/cpdf_colorspace.cpp b/core/fpdfapi/page/cpdf_colorspace.cpp
index b1675d87a7..6cd0075e8e 100644
--- a/core/fpdfapi/page/cpdf_colorspace.cpp
+++ b/core/fpdfapi/page/cpdf_colorspace.cpp
@@ -7,6 +7,7 @@
#include "core/fpdfapi/page/cpdf_colorspace.h"
#include <memory>
+#include <utility>
#include "core/fpdfapi/cpdf_modulemgr.h"
#include "core/fpdfapi/page/cpdf_docpagedata.h"
@@ -20,6 +21,8 @@
#include "core/fpdfapi/parser/cpdf_stream_acc.h"
#include "core/fpdfapi/parser/cpdf_string.h"
#include "core/fxcodec/fx_codec.h"
+#include "core/fxcrt/cfx_maybe_owned.h"
+#include "core/fxcrt/fx_memory.h"
namespace {
@@ -175,11 +178,10 @@ class CPDF_ICCBasedCS : public CPDF_ColorSpace {
int image_height,
bool bTransMask = false) const override;
- CPDF_ColorSpace* m_pAlterCS;
+ CFX_MaybeOwned<CPDF_ColorSpace> m_pAlterCS;
CPDF_IccProfile* m_pProfile;
uint8_t* m_pCache;
FX_FLOAT* m_pRanges;
- bool m_bOwn;
};
class CPDF_IndexedCS : public CPDF_ColorSpace {
@@ -815,17 +817,13 @@ void CPDF_LabCS::TranslateImageLine(uint8_t* pDestBuf,
CPDF_ICCBasedCS::CPDF_ICCBasedCS(CPDF_Document* pDoc)
: CPDF_ColorSpace(pDoc, PDFCS_ICCBASED, 0),
- m_pAlterCS(nullptr),
m_pProfile(nullptr),
m_pCache(nullptr),
- m_pRanges(nullptr),
- m_bOwn(false) {}
+ m_pRanges(nullptr) {}
CPDF_ICCBasedCS::~CPDF_ICCBasedCS() {
FX_Free(m_pCache);
FX_Free(m_pRanges);
- if (m_pAlterCS && m_bOwn)
- m_pAlterCS->Release();
if (m_pProfile && m_pDocument)
m_pDocument->GetPageData()->ReleaseIccProfile(m_pProfile);
}
@@ -852,8 +850,7 @@ bool CPDF_ICCBasedCS::v_Load(CPDF_Document* pDoc, CPDF_Array* pArray) {
if (m_nComponents == 0) { // NO valid ICC profile
if (pAlterCS->CountComponents() > 0) { // Use Alternative colorspace
m_nComponents = pAlterCS->CountComponents();
- m_pAlterCS = pAlterCS.release();
- m_bOwn = true;
+ m_pAlterCS = std::move(pAlterCS);
} else { // No valid alternative colorspace
int32_t nDictComponents = pDict ? pDict->GetIntegerFor("N") : 0;
if (nDictComponents != 1 && nDictComponents != 3 &&
@@ -863,10 +860,8 @@ bool CPDF_ICCBasedCS::v_Load(CPDF_Document* pDoc, CPDF_Array* pArray) {
m_nComponents = nDictComponents;
}
} else { // Using sRGB
- if (pAlterCS->CountComponents() == m_nComponents) {
- m_pAlterCS = pAlterCS.release();
- m_bOwn = true;
- }
+ if (pAlterCS->CountComponents() == m_nComponents)
+ m_pAlterCS = std::move(pAlterCS);
}
}
}
diff --git a/core/fxcrt/cfx_maybe_owned.h b/core/fxcrt/cfx_maybe_owned.h
new file mode 100644
index 0000000000..76bd580e28
--- /dev/null
+++ b/core/fxcrt/cfx_maybe_owned.h
@@ -0,0 +1,88 @@
+// Copyright 2016 PDFium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CORE_FXCRT_CFX_MAYBE_OWNED_H_
+#define CORE_FXCRT_CFX_MAYBE_OWNED_H_
+
+#include <algorithm>
+#include <memory>
+#include <utility>
+
+#include "core/fxcrt/fx_memory.h"
+#include "core/fxcrt/fx_system.h"
+
+// A template that can hold either owned or unowned references, and cleans up
+// appropriately. Possibly the most pernicious anti-pattern imaginable, but
+// it crops up throughout the codebase due to a desire to avoid copying-in
+// objects or data.
+template <typename T, typename D = std::default_delete<T>>
+class CFX_MaybeOwned {
+ public:
+ CFX_MaybeOwned() : m_pObj(nullptr) {}
+ explicit CFX_MaybeOwned(T* ptr) : m_pObj(ptr) {}
+ explicit CFX_MaybeOwned(std::unique_ptr<T> ptr)
+ : m_pOwnedObj(std::move(ptr)), m_pObj(m_pOwnedObj.get()) {}
+
+ CFX_MaybeOwned(const CFX_MaybeOwned& that) = delete;
+ CFX_MaybeOwned(CFX_MaybeOwned&& that)
+ : m_pOwnedObj(that.m_pOwnedObj.release()), m_pObj(that.m_pObj) {
+ that.m_pObj = nullptr;
+ }
+
+ void Reset(std::unique_ptr<T> ptr) {
+ m_pOwnedObj = std::move(ptr);
+ m_pObj = m_pOwnedObj.get();
+ }
+ void Reset(T* ptr = nullptr) {
+ m_pOwnedObj.reset();
+ m_pObj = ptr;
+ }
+
+ bool IsOwned() const { return !!m_pOwnedObj; }
+ T* Get() const { return m_pObj; }
+ std::unique_ptr<T> Release() {
+ ASSERT(IsOwned());
+ return std::move(m_pOwnedObj);
+ }
+
+ CFX_MaybeOwned& operator=(const CFX_MaybeOwned& that) = delete;
+ CFX_MaybeOwned& operator=(CFX_MaybeOwned&& that) {
+ m_pOwnedObj = std::move(that.m_pOwnedObj);
+ m_pObj = that.m_pObj;
+ that.m_pObj = nullptr;
+ return *this;
+ }
+ CFX_MaybeOwned& operator=(T* ptr) {
+ Reset(ptr);
+ return *this;
+ }
+ CFX_MaybeOwned& operator=(std::unique_ptr<T> ptr) {
+ Reset(std::move(ptr));
+ return *this;
+ }
+
+ bool operator==(const CFX_MaybeOwned& that) const {
+ return Get() == that.Get();
+ }
+ bool operator==(const std::unique_ptr<T>& ptr) const {
+ return Get() == ptr.get();
+ }
+ bool operator==(T* ptr) const { return Get() == ptr; }
+
+ bool operator!=(const CFX_MaybeOwned& that) const { return !(*this == that); }
+ bool operator!=(const std::unique_ptr<T> ptr) const {
+ return !(*this == ptr);
+ }
+ bool operator!=(T* ptr) const { return !(*this == ptr); }
+
+ explicit operator bool() const { return !!m_pObj; }
+ T& operator*() const { return *m_pObj; }
+ T* operator->() const { return m_pObj; }
+
+ private:
+ std::unique_ptr<T, D> m_pOwnedObj;
+ T* m_pObj;
+};
+
+#endif // CORE_FXCRT_CFX_MAYBE_OWNED_H_
diff --git a/core/fxcrt/cfx_maybe_owned_unittest.cpp b/core/fxcrt/cfx_maybe_owned_unittest.cpp
new file mode 100644
index 0000000000..8f513a358d
--- /dev/null
+++ b/core/fxcrt/cfx_maybe_owned_unittest.cpp
@@ -0,0 +1,179 @@
+// Copyright 2016 PDFium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "core/fxcrt/cfx_maybe_owned.h"
+
+#include <memory>
+#include <utility>
+
+#include "core/fxcrt/fx_memory.h"
+#include "testing/fx_string_testhelpers.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/base/ptr_util.h"
+
+namespace {
+
+class PseudoDeletable {
+ public:
+ explicit PseudoDeletable(int id, int* count_location)
+ : id_(id), count_location_(count_location) {}
+ ~PseudoDeletable() { ++(*count_location_); }
+ int GetID() const { return id_; }
+
+ private:
+ int id_;
+ int* count_location_;
+};
+
+} // namespace
+
+TEST(fxcrt, MaybeOwnedNull) {
+ CFX_MaybeOwned<PseudoDeletable> ptr1;
+ EXPECT_FALSE(ptr1.IsOwned());
+ EXPECT_FALSE(ptr1);
+ EXPECT_EQ(nullptr, ptr1.Get());
+
+ CFX_MaybeOwned<PseudoDeletable> ptr2;
+ EXPECT_TRUE(ptr1 == ptr2);
+ EXPECT_FALSE(ptr1 != ptr2);
+}
+
+TEST(fxcrt, MaybeOwnedNotOwned) {
+ int delete_count = 0;
+ PseudoDeletable thing1(100, &delete_count);
+ {
+ CFX_MaybeOwned<PseudoDeletable> ptr(&thing1);
+ EXPECT_FALSE(ptr.IsOwned());
+ EXPECT_EQ(ptr.Get(), &thing1);
+ EXPECT_EQ(100, ptr->GetID());
+ EXPECT_TRUE(ptr == &thing1);
+ EXPECT_FALSE(ptr != &thing1);
+
+ CFX_MaybeOwned<PseudoDeletable> empty;
+ EXPECT_FALSE(ptr == empty);
+ EXPECT_TRUE(ptr != empty);
+ }
+ EXPECT_EQ(0, delete_count);
+
+ delete_count = 0;
+ PseudoDeletable thing2(200, &delete_count);
+ {
+ CFX_MaybeOwned<PseudoDeletable> ptr(&thing1);
+ ptr = &thing2;
+ EXPECT_FALSE(ptr.IsOwned());
+ EXPECT_EQ(ptr.Get(), &thing2);
+ EXPECT_EQ(200, ptr->GetID());
+ }
+ EXPECT_EQ(0, delete_count);
+
+ delete_count = 0;
+ int owned_delete_count = 0;
+ {
+ CFX_MaybeOwned<PseudoDeletable> ptr(&thing1);
+ EXPECT_EQ(100, ptr->GetID());
+ ptr = pdfium::MakeUnique<PseudoDeletable>(300, &owned_delete_count);
+ EXPECT_TRUE(ptr.IsOwned());
+ EXPECT_EQ(300, ptr->GetID());
+ }
+ EXPECT_EQ(0, delete_count);
+ EXPECT_EQ(1, owned_delete_count);
+}
+
+TEST(fxcrt, MaybeOwnedOwned) {
+ int delete_count = 0;
+ {
+ CFX_MaybeOwned<PseudoDeletable> ptr(
+ pdfium::MakeUnique<PseudoDeletable>(100, &delete_count));
+ EXPECT_TRUE(ptr.IsOwned());
+ EXPECT_EQ(100, ptr->GetID());
+
+ CFX_MaybeOwned<PseudoDeletable> empty;
+ EXPECT_FALSE(ptr == empty);
+ EXPECT_TRUE(ptr != empty);
+ }
+ EXPECT_EQ(1, delete_count);
+
+ delete_count = 0;
+ {
+ CFX_MaybeOwned<PseudoDeletable> ptr(
+ pdfium::MakeUnique<PseudoDeletable>(200, &delete_count));
+ ptr = pdfium::MakeUnique<PseudoDeletable>(300, &delete_count);
+ EXPECT_TRUE(ptr.IsOwned());
+ EXPECT_EQ(300, ptr->GetID());
+ EXPECT_EQ(1, delete_count);
+ }
+ EXPECT_EQ(2, delete_count);
+
+ delete_count = 0;
+ int unowned_delete_count = 0;
+ PseudoDeletable thing2(400, &unowned_delete_count);
+ {
+ CFX_MaybeOwned<PseudoDeletable> ptr(
+ pdfium::MakeUnique<PseudoDeletable>(500, &delete_count));
+ ptr = &thing2;
+ EXPECT_FALSE(ptr.IsOwned());
+ EXPECT_EQ(400, ptr->GetID());
+ EXPECT_EQ(1, delete_count);
+ EXPECT_EQ(0, unowned_delete_count);
+ }
+ EXPECT_EQ(1, delete_count);
+ EXPECT_EQ(0, unowned_delete_count);
+}
+
+TEST(fxcrt, MaybeOwnedRelease) {
+ int delete_count = 0;
+ {
+ std::unique_ptr<PseudoDeletable> stolen;
+ {
+ CFX_MaybeOwned<PseudoDeletable> ptr(
+ pdfium::MakeUnique<PseudoDeletable>(100, &delete_count));
+ EXPECT_TRUE(ptr.IsOwned());
+ stolen = ptr.Release();
+ EXPECT_FALSE(ptr.IsOwned());
+ EXPECT_EQ(ptr, stolen);
+ EXPECT_EQ(0, delete_count);
+ }
+ EXPECT_EQ(0, delete_count);
+ }
+ EXPECT_EQ(1, delete_count);
+}
+
+TEST(fxcrt, MaybeOwnedMove) {
+ int delete_count = 0;
+ PseudoDeletable thing1(100, &delete_count);
+ {
+ CFX_MaybeOwned<PseudoDeletable> ptr1(&thing1);
+ CFX_MaybeOwned<PseudoDeletable> ptr2(
+ pdfium::MakeUnique<PseudoDeletable>(200, &delete_count));
+ EXPECT_FALSE(ptr1.IsOwned());
+ EXPECT_TRUE(ptr2.IsOwned());
+
+ CFX_MaybeOwned<PseudoDeletable> ptr3(std::move(ptr1));
+ CFX_MaybeOwned<PseudoDeletable> ptr4(std::move(ptr2));
+ EXPECT_FALSE(ptr1.IsOwned());
+ EXPECT_FALSE(ptr2.IsOwned());
+ EXPECT_FALSE(ptr3.IsOwned());
+ EXPECT_TRUE(ptr4.IsOwned());
+ EXPECT_EQ(0, delete_count);
+ EXPECT_EQ(nullptr, ptr1.Get());
+ EXPECT_EQ(nullptr, ptr2.Get());
+ EXPECT_EQ(100, ptr3->GetID());
+ EXPECT_EQ(200, ptr4->GetID());
+
+ CFX_MaybeOwned<PseudoDeletable> ptr5;
+ CFX_MaybeOwned<PseudoDeletable> ptr6;
+ ptr5 = std::move(ptr3);
+ ptr6 = std::move(ptr4);
+ EXPECT_FALSE(ptr3.IsOwned());
+ EXPECT_FALSE(ptr4.IsOwned());
+ EXPECT_FALSE(ptr5.IsOwned());
+ EXPECT_TRUE(ptr6.IsOwned());
+ EXPECT_EQ(0, delete_count);
+ EXPECT_EQ(nullptr, ptr3.Get());
+ EXPECT_EQ(nullptr, ptr4.Get());
+ EXPECT_EQ(100, ptr5->GetID());
+ EXPECT_EQ(200, ptr6->GetID());
+ }
+ EXPECT_EQ(1, delete_count);
+}