Move ScopedSetInsertion to core/fxcrt/scoped_set_insertion.h. Move it out of third_party/base/stl_util.h since it does not exist in Chromium's base/stl_util.h. Along the way: - Add a unit test. - Disallow copy, assign, and heap allocations. Change-Id: Ie04cb53059805117da6b9906063f34d96d0629e7 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/80350 Commit-Queue: Lei Zhang <thestig@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fpdfapi/page/cpdf_colorspace.cpp b/core/fpdfapi/page/cpdf_colorspace.cpp index 8da89a6..13061b5 100644 --- a/core/fpdfapi/page/cpdf_colorspace.cpp +++ b/core/fpdfapi/page/cpdf_colorspace.cpp
@@ -33,6 +33,7 @@ #include "core/fxcodec/icc/iccmodule.h" #include "core/fxcrt/fx_safe_types.h" #include "core/fxcrt/maybe_owned.h" +#include "core/fxcrt/scoped_set_insertion.h" #include "third_party/base/check.h" #include "third_party/base/check_op.h" #include "third_party/base/notreached.h" @@ -500,7 +501,7 @@ if (pdfium::Contains(*pVisited, pObj)) return nullptr; - pdfium::ScopedSetInsertion<const CPDF_Object*> insertion(pVisited, pObj); + ScopedSetInsertion<const CPDF_Object*> insertion(pVisited, pObj); if (pObj->IsName()) return ColorspaceFromName(pObj->GetString());
diff --git a/core/fpdfapi/page/cpdf_docpagedata.cpp b/core/fpdfapi/page/cpdf_docpagedata.cpp index 38c41ac..921a3c8 100644 --- a/core/fpdfapi/page/cpdf_docpagedata.cpp +++ b/core/fpdfapi/page/cpdf_docpagedata.cpp
@@ -32,6 +32,7 @@ #include "core/fxcrt/fx_codepage.h" #include "core/fxcrt/fx_memory.h" #include "core/fxcrt/fx_safe_types.h" +#include "core/fxcrt/scoped_set_insertion.h" #include "core/fxge/cfx_font.h" #include "core/fxge/cfx_fontmapper.h" #include "core/fxge/cfx_substfont.h" @@ -262,8 +263,7 @@ if (pdfium::Contains(*pVisitedInternal, pCSObj)) return nullptr; - pdfium::ScopedSetInsertion<const CPDF_Object*> insertion(pVisitedInternal, - pCSObj); + ScopedSetInsertion<const CPDF_Object*> insertion(pVisitedInternal, pCSObj); if (pCSObj->IsName()) { ByteString name = pCSObj->GetString();
diff --git a/core/fpdfapi/page/cpdf_function.cpp b/core/fpdfapi/page/cpdf_function.cpp index ae0b8fb..ed2283b 100644 --- a/core/fpdfapi/page/cpdf_function.cpp +++ b/core/fpdfapi/page/cpdf_function.cpp
@@ -17,6 +17,7 @@ #include "core/fpdfapi/parser/cpdf_stream.h" #include "core/fpdfapi/parser/fpdf_parser_utility.h" #include "core/fxcrt/fx_safe_types.h" +#include "core/fxcrt/scoped_set_insertion.h" #include "third_party/base/stl_util.h" namespace { @@ -51,7 +52,7 @@ if (pdfium::Contains(*pVisited, pFuncObj)) return nullptr; - pdfium::ScopedSetInsertion<const CPDF_Object*> insertion(pVisited, pFuncObj); + ScopedSetInsertion<const CPDF_Object*> insertion(pVisited, pFuncObj); int iType = -1; if (const CPDF_Stream* pStream = pFuncObj->AsStream())
diff --git a/core/fpdfapi/page/cpdf_streamcontentparser.cpp b/core/fpdfapi/page/cpdf_streamcontentparser.cpp index 9435f40..e9422a0 100644 --- a/core/fpdfapi/page/cpdf_streamcontentparser.cpp +++ b/core/fpdfapi/page/cpdf_streamcontentparser.cpp
@@ -36,6 +36,7 @@ #include "core/fpdfapi/parser/fpdf_parser_utility.h" #include "core/fxcrt/autonuller.h" #include "core/fxcrt/fx_safe_types.h" +#include "core/fxcrt/scoped_set_insertion.h" #include "core/fxge/cfx_graphstatedata.h" #include "third_party/base/check.h" #include "third_party/base/no_destructor.h" @@ -1498,8 +1499,8 @@ m_StreamStartOffsets = stream_start_offsets; - pdfium::ScopedSetInsertion<const uint8_t*> scopedInsert(m_ParsedSet.Get(), - pDataStart); + ScopedSetInsertion<const uint8_t*> scopedInsert(m_ParsedSet.Get(), + pDataStart); uint32_t init_obj_count = m_pObjectHolder->GetPageObjectCount(); AutoNuller<std::unique_ptr<CPDF_StreamParser>> auto_clearer(&m_pSyntax);
diff --git a/core/fpdfapi/parser/cpdf_document.cpp b/core/fpdfapi/parser/cpdf_document.cpp index b80043f..4a403fe 100644 --- a/core/fpdfapi/parser/cpdf_document.cpp +++ b/core/fpdfapi/parser/cpdf_document.cpp
@@ -16,6 +16,7 @@ #include "core/fpdfapi/parser/cpdf_reference.h" #include "core/fxcodec/jbig2/JBig2_DocumentContext.h" #include "core/fxcrt/fx_codepage.h" +#include "core/fxcrt/scoped_set_insertion.h" #include "third_party/base/check.h" #include "third_party/base/stl_util.h" @@ -38,8 +39,7 @@ continue; if (pKid->KeyExist("Kids")) { // Use |visited_pages| to help detect circular references of pages. - pdfium::ScopedSetInsertion<CPDF_Dictionary*> local_add(visited_pages, - pKid); + ScopedSetInsertion<CPDF_Dictionary*> local_add(visited_pages, pKid); count += CountPages(pKid, visited_pages); } else { // This page is a leaf node. @@ -424,7 +424,7 @@ if (pdfium::Contains(*pVisited, pKid)) return false; - pdfium::ScopedSetInsertion<CPDF_Dictionary*> insertion(pVisited, pKid); + ScopedSetInsertion<CPDF_Dictionary*> insertion(pVisited, pKid); if (!InsertDeletePDFPage(pKid, nPagesToGo, pPageDict, bInsert, pVisited)) return false;
diff --git a/core/fpdfapi/parser/cpdf_parser.cpp b/core/fpdfapi/parser/cpdf_parser.cpp index b269776..f131121 100644 --- a/core/fpdfapi/parser/cpdf_parser.cpp +++ b/core/fpdfapi/parser/cpdf_parser.cpp
@@ -28,6 +28,7 @@ #include "core/fxcrt/fx_extension.h" #include "core/fxcrt/fx_memory_wrappers.h" #include "core/fxcrt/fx_safe_types.h" +#include "core/fxcrt/scoped_set_insertion.h" #include "third_party/base/check.h" #include "third_party/base/check_op.h" #include "third_party/base/notreached.h" @@ -865,7 +866,7 @@ if (pdfium::Contains(m_ParsingObjNums, objnum)) return nullptr; - pdfium::ScopedSetInsertion<uint32_t> local_insert(&m_ParsingObjNums, objnum); + ScopedSetInsertion<uint32_t> local_insert(&m_ParsingObjNums, objnum); if (GetObjectType(objnum) == ObjectType::kNotCompressed) { FX_FILESIZE pos = GetObjectPositionOrZero(objnum); if (pos <= 0) @@ -888,8 +889,7 @@ if (pdfium::Contains(m_ParsingObjNums, object_number)) return nullptr; - pdfium::ScopedSetInsertion<uint32_t> local_insert(&m_ParsingObjNums, - object_number); + ScopedSetInsertion<uint32_t> local_insert(&m_ParsingObjNums, object_number); auto it = m_ObjectStreamMap.find(object_number); if (it != m_ObjectStreamMap.end())
diff --git a/core/fxcrt/BUILD.gn b/core/fxcrt/BUILD.gn index a9ba304..c3220b7 100644 --- a/core/fxcrt/BUILD.gn +++ b/core/fxcrt/BUILD.gn
@@ -62,6 +62,7 @@ "pauseindicator_iface.h", "retain_ptr.h", "retained_tree_node.h", + "scoped_set_insertion.h", "shared_copy_on_write.h", "string_data_template.cpp", "string_data_template.h", @@ -155,6 +156,7 @@ "pdfium_span_unittest.cpp", "retain_ptr_unittest.cpp", "retained_tree_node_unittest.cpp", + "scoped_set_insertion_unittest.cpp", "shared_copy_on_write_unittest.cpp", "string_pool_template_unittest.cpp", "tree_node_unittest.cpp",
diff --git a/core/fxcrt/scoped_set_insertion.h b/core/fxcrt/scoped_set_insertion.h new file mode 100644 index 0000000..890e75f --- /dev/null +++ b/core/fxcrt/scoped_set_insertion.h
@@ -0,0 +1,41 @@ +// Copyright 2021 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_SCOPED_SET_INSERTION_H_ +#define CORE_FXCRT_SCOPED_SET_INSERTION_H_ + +#include <set> +#include <utility> + +#include "third_party/base/check.h" + +namespace fxcrt { + +// Track the addition of an object to a set, removing it automatically when +// the ScopedSetInsertion goes out of scope. +template <typename T> +class ScopedSetInsertion { + public: + ScopedSetInsertion(std::set<T>* org_set, const T& elem) + : set_(org_set), insert_results_(set_->insert(elem)) { + CHECK(insert_results_.second); + } + ScopedSetInsertion(const ScopedSetInsertion&) = delete; + ScopedSetInsertion& operator=(const ScopedSetInsertion&) = delete; + ~ScopedSetInsertion() { set_->erase(insert_results_.first); } + + // Stack allocated only. + void* operator new(size_t) = delete; + void* operator new(size_t, void*) = delete; + + private: + std::set<T>* const set_; + const std::pair<typename std::set<T>::iterator, bool> insert_results_; +}; + +} // namespace fxcrt + +using fxcrt::ScopedSetInsertion; + +#endif // CORE_FXCRT_SCOPED_SET_INSERTION_H_
diff --git a/core/fxcrt/scoped_set_insertion_unittest.cpp b/core/fxcrt/scoped_set_insertion_unittest.cpp new file mode 100644 index 0000000..ad7e232 --- /dev/null +++ b/core/fxcrt/scoped_set_insertion_unittest.cpp
@@ -0,0 +1,24 @@ +// Copyright 2021 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/scoped_set_insertion.h" + +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +TEST(fxcrt, ScopedSetInsertion) { + std::set<int> container; + { + ScopedSetInsertion<int> insertion(&container, 5); + EXPECT_THAT(container, testing::UnorderedElementsAreArray({5})); + + { + ScopedSetInsertion<int> insertion2(&container, 6); + EXPECT_THAT(container, testing::UnorderedElementsAreArray({5, 6})); + } + + EXPECT_THAT(container, testing::UnorderedElementsAreArray({5})); + } + EXPECT_TRUE(container.empty()); +}
diff --git a/third_party/base/stl_util.h b/third_party/base/stl_util.h index 17fdcd8..40ae622 100644 --- a/third_party/base/stl_util.h +++ b/third_party/base/stl_util.h
@@ -8,12 +8,9 @@ #include <algorithm> #include <iterator> #include <memory> -#include <set> #include <type_traits> -#include <utility> #include <vector> -#include "third_party/base/check.h" #include "third_party/base/numerics/safe_conversions.h" #include "third_party/base/numerics/safe_math.h" #include "third_party/base/template_util.h" @@ -136,22 +133,6 @@ return index >= 0 && index < CollectionSize<IndexType>(collection); } -// Track the addition of an object to a set, removing it automatically when -// the ScopedSetInsertion goes out of scope. -template <typename T> -class ScopedSetInsertion { - public: - ScopedSetInsertion(std::set<T>* org_set, const T& elem) - : set_(org_set), insert_results_(set_->insert(elem)) { - CHECK(insert_results_.second); - } - ~ScopedSetInsertion() { set_->erase(insert_results_.first); } - - private: - std::set<T>* const set_; - const std::pair<typename std::set<T>::iterator, bool> insert_results_; -}; - // std::clamp(), some day. template <class T> constexpr const T& clamp(const T& v, const T& lo, const T& hi) {