From 81b224669647c9dd3c5ea2e013ee3cc109744bb8 Mon Sep 17 00:00:00 2001 From: tsepez Date: Wed, 23 Nov 2016 14:20:19 -0800 Subject: 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 --- core/fxcrt/cfx_maybe_owned.h | 88 ++++++++++++++++ core/fxcrt/cfx_maybe_owned_unittest.cpp | 179 ++++++++++++++++++++++++++++++++ 2 files changed, 267 insertions(+) create mode 100644 core/fxcrt/cfx_maybe_owned.h create mode 100644 core/fxcrt/cfx_maybe_owned_unittest.cpp (limited to 'core/fxcrt') 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 +#include +#include + +#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 > +class CFX_MaybeOwned { + public: + CFX_MaybeOwned() : m_pObj(nullptr) {} + explicit CFX_MaybeOwned(T* ptr) : m_pObj(ptr) {} + explicit CFX_MaybeOwned(std::unique_ptr 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 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 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 ptr) { + Reset(std::move(ptr)); + return *this; + } + + bool operator==(const CFX_MaybeOwned& that) const { + return Get() == that.Get(); + } + bool operator==(const std::unique_ptr& 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 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 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 +#include + +#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 ptr1; + EXPECT_FALSE(ptr1.IsOwned()); + EXPECT_FALSE(ptr1); + EXPECT_EQ(nullptr, ptr1.Get()); + + CFX_MaybeOwned ptr2; + EXPECT_TRUE(ptr1 == ptr2); + EXPECT_FALSE(ptr1 != ptr2); +} + +TEST(fxcrt, MaybeOwnedNotOwned) { + int delete_count = 0; + PseudoDeletable thing1(100, &delete_count); + { + CFX_MaybeOwned 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 empty; + EXPECT_FALSE(ptr == empty); + EXPECT_TRUE(ptr != empty); + } + EXPECT_EQ(0, delete_count); + + delete_count = 0; + PseudoDeletable thing2(200, &delete_count); + { + CFX_MaybeOwned 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 ptr(&thing1); + EXPECT_EQ(100, ptr->GetID()); + ptr = pdfium::MakeUnique(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 ptr( + pdfium::MakeUnique(100, &delete_count)); + EXPECT_TRUE(ptr.IsOwned()); + EXPECT_EQ(100, ptr->GetID()); + + CFX_MaybeOwned empty; + EXPECT_FALSE(ptr == empty); + EXPECT_TRUE(ptr != empty); + } + EXPECT_EQ(1, delete_count); + + delete_count = 0; + { + CFX_MaybeOwned ptr( + pdfium::MakeUnique(200, &delete_count)); + ptr = pdfium::MakeUnique(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 ptr( + pdfium::MakeUnique(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 stolen; + { + CFX_MaybeOwned ptr( + pdfium::MakeUnique(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 ptr1(&thing1); + CFX_MaybeOwned ptr2( + pdfium::MakeUnique(200, &delete_count)); + EXPECT_FALSE(ptr1.IsOwned()); + EXPECT_TRUE(ptr2.IsOwned()); + + CFX_MaybeOwned ptr3(std::move(ptr1)); + CFX_MaybeOwned 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 ptr5; + CFX_MaybeOwned 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); +} -- cgit v1.2.3