From a1ea4276f87f945dfe922f7c37c08f2d203e4e59 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Tue, 19 Jun 2018 14:37:12 +0000 Subject: fxcrt::{Byte,Wide}String missing move-assign operator This hasn't been a big deal, since no data is copied, but avoids some ref-count churn in the process. Change-Id: I53c059284aa6806793c59a0c19b3e0d7fe4191d6 Reviewed-on: https://pdfium-review.googlesource.com/35350 Commit-Queue: dsinclair Reviewed-by: dsinclair --- core/fxcrt/bytestring.cpp | 18 +++++++++++++++--- core/fxcrt/bytestring.h | 6 +++++- core/fxcrt/bytestring_unittest.cpp | 31 +++++++++++++++++++++++++++++++ core/fxcrt/widestring.cpp | 17 ++++++++++++++--- core/fxcrt/widestring.h | 6 +++++- core/fxcrt/widestring_unittest.cpp | 31 +++++++++++++++++++++++++++++++ 6 files changed, 101 insertions(+), 8 deletions(-) diff --git a/core/fxcrt/bytestring.cpp b/core/fxcrt/bytestring.cpp index 872de065ba..984acf9c69 100644 --- a/core/fxcrt/bytestring.cpp +++ b/core/fxcrt/bytestring.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include "core/fxcrt/cfx_utf8decoder.h" #include "core/fxcrt/fx_codepage.h" @@ -244,9 +245,16 @@ const ByteString& ByteString::operator=(const ByteStringView& stringSrc) { return *this; } -const ByteString& ByteString::operator=(const ByteString& stringSrc) { - if (m_pData != stringSrc.m_pData) - m_pData = stringSrc.m_pData; +const ByteString& ByteString::operator=(const ByteString& that) { + if (m_pData != that.m_pData) + m_pData = that.m_pData; + + return *this; +} + +const ByteString& ByteString::operator=(ByteString&& that) { + if (m_pData != that.m_pData) + m_pData = std::move(that.m_pData); return *this; } @@ -493,6 +501,10 @@ void ByteString::Concat(const char* pSrcData, size_t nSrcLen) { m_pData.Swap(pNewData); } +intptr_t ByteString::ReferenceCountForTesting() const { + return m_pData ? m_pData->m_nRefs : 0; +} + ByteString ByteString::Mid(size_t first, size_t count) const { if (!m_pData) return ByteString(); diff --git a/core/fxcrt/bytestring.h b/core/fxcrt/bytestring.h index 99b87b1d4c..5722c4925d 100644 --- a/core/fxcrt/bytestring.h +++ b/core/fxcrt/bytestring.h @@ -21,6 +21,7 @@ namespace fxcrt { +class ByteString_Assign_Test; class ByteString_Concat_Test; class StringPool_ByteString_Test; class WideString; @@ -133,7 +134,8 @@ class ByteString { const ByteString& operator=(const char* str); const ByteString& operator=(const ByteStringView& bstrc); - const ByteString& operator=(const ByteString& stringSrc); + const ByteString& operator=(const ByteString& that); + const ByteString& operator=(ByteString&& that); const ByteString& operator+=(char ch); const ByteString& operator+=(const char* str); @@ -209,9 +211,11 @@ class ByteString { void AllocCopy(ByteString& dest, size_t nCopyLen, size_t nCopyIndex) const; void AssignCopy(const char* pSrcData, size_t nSrcLen); void Concat(const char* lpszSrcData, size_t nSrcLen); + intptr_t ReferenceCountForTesting() const; RetainPtr m_pData; + friend ByteString_Assign_Test; friend ByteString_Concat_Test; friend class StringPool_ByteString_Test; }; diff --git a/core/fxcrt/bytestring_unittest.cpp b/core/fxcrt/bytestring_unittest.cpp index ed3f375434..2b7d4f7836 100644 --- a/core/fxcrt/bytestring_unittest.cpp +++ b/core/fxcrt/bytestring_unittest.cpp @@ -59,6 +59,37 @@ TEST(ByteString, ElementAccess) { #endif } +TEST(ByteString, Assign) { + { + // Copy-assign. + ByteString string1; + EXPECT_EQ(0, string1.ReferenceCountForTesting()); + { + ByteString string2("abc"); + EXPECT_EQ(1, string2.ReferenceCountForTesting()); + + string1 = string2; + EXPECT_EQ(2, string1.ReferenceCountForTesting()); + EXPECT_EQ(2, string2.ReferenceCountForTesting()); + } + EXPECT_EQ(1, string1.ReferenceCountForTesting()); + } + { + // Move-assign. + ByteString string1; + EXPECT_EQ(0, string1.ReferenceCountForTesting()); + { + ByteString string2("abc"); + EXPECT_EQ(1, string2.ReferenceCountForTesting()); + + string1 = std::move(string2); + EXPECT_EQ(1, string1.ReferenceCountForTesting()); + EXPECT_EQ(0, string2.ReferenceCountForTesting()); + } + EXPECT_EQ(1, string1.ReferenceCountForTesting()); + } +} + TEST(ByteString, OperatorLT) { ByteString empty; ByteString a("a"); diff --git a/core/fxcrt/widestring.cpp b/core/fxcrt/widestring.cpp index 25f253ea11..cde1973d26 100644 --- a/core/fxcrt/widestring.cpp +++ b/core/fxcrt/widestring.cpp @@ -441,9 +441,16 @@ const WideString& WideString::operator=(const WideStringView& stringSrc) { return *this; } -const WideString& WideString::operator=(const WideString& stringSrc) { - if (m_pData != stringSrc.m_pData) - m_pData = stringSrc.m_pData; +const WideString& WideString::operator=(const WideString& that) { + if (m_pData != that.m_pData) + m_pData = that.m_pData; + + return *this; +} + +const WideString& WideString::operator=(WideString&& that) { + if (m_pData != that.m_pData) + m_pData = std::move(that.m_pData); return *this; } @@ -662,6 +669,10 @@ void WideString::Concat(const wchar_t* pSrcData, size_t nSrcLen) { m_pData.Swap(pNewData); } +intptr_t WideString::ReferenceCountForTesting() const { + return m_pData ? m_pData->m_nRefs : 0; +} + ByteString WideString::UTF8Encode() const { return FX_UTF8Encode(AsStringView()); } diff --git a/core/fxcrt/widestring.h b/core/fxcrt/widestring.h index 4b097c4430..b531292c57 100644 --- a/core/fxcrt/widestring.h +++ b/core/fxcrt/widestring.h @@ -23,6 +23,7 @@ namespace fxcrt { class ByteString; class StringPool_WideString_Test; +class WideString_Assign_Test; class WideString_ConcatInPlace_Test; // A mutable string with shared buffers using copy-on-write semantics that @@ -111,8 +112,9 @@ class WideString { bool IsValidLength(size_t length) const { return length <= GetLength(); } const WideString& operator=(const wchar_t* str); - const WideString& operator=(const WideString& stringSrc); const WideString& operator=(const WideStringView& stringSrc); + const WideString& operator=(const WideString& that); + const WideString& operator=(WideString&& that); const WideString& operator+=(const wchar_t* str); const WideString& operator+=(wchar_t ch); @@ -208,10 +210,12 @@ class WideString { void AllocCopy(WideString& dest, size_t nCopyLen, size_t nCopyIndex) const; void AssignCopy(const wchar_t* pSrcData, size_t nSrcLen); void Concat(const wchar_t* lpszSrcData, size_t nSrcLen); + intptr_t ReferenceCountForTesting() const; RetainPtr m_pData; friend WideString_ConcatInPlace_Test; + friend WideString_Assign_Test; friend StringPool_WideString_Test; }; diff --git a/core/fxcrt/widestring_unittest.cpp b/core/fxcrt/widestring_unittest.cpp index 4572305530..b044a81bb0 100644 --- a/core/fxcrt/widestring_unittest.cpp +++ b/core/fxcrt/widestring_unittest.cpp @@ -53,6 +53,37 @@ TEST(WideString, ElementAccess) { #endif } +TEST(WideString, Assign) { + { + // Copy-assign. + WideString string1; + EXPECT_EQ(0, string1.ReferenceCountForTesting()); + { + WideString string2(L"abc"); + EXPECT_EQ(1, string2.ReferenceCountForTesting()); + + string1 = string2; + EXPECT_EQ(2, string1.ReferenceCountForTesting()); + EXPECT_EQ(2, string2.ReferenceCountForTesting()); + } + EXPECT_EQ(1, string1.ReferenceCountForTesting()); + } + { + // Move-assign. + WideString string1; + EXPECT_EQ(0, string1.ReferenceCountForTesting()); + { + WideString string2(L"abc"); + EXPECT_EQ(1, string2.ReferenceCountForTesting()); + + string1 = std::move(string2); + EXPECT_EQ(1, string1.ReferenceCountForTesting()); + EXPECT_EQ(0, string2.ReferenceCountForTesting()); + } + EXPECT_EQ(1, string1.ReferenceCountForTesting()); + } +} + TEST(WideString, OperatorLT) { WideString empty; WideString a(L"a"); -- cgit v1.2.3