Skip to content

Commit

Permalink
[Base] Introduce FindOrNull() and FindPtrOrNull()
Browse files Browse the repository at this point in the history
These helpers simplify the process of finding the stored value for a
given key in STL-like maps.

Bug: None
Change-Id: I0976e6d66ec98269910757990414008ff95a9031
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4926655
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Andrew Rayskiy <greengrape@google.com>
Owners-Override: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1209315}
  • Loading branch information
Andrew Rayskiy authored and Chromium LUCI CQ committed Oct 13, 2023
1 parent 9b8c074 commit 3dfec41
Show file tree
Hide file tree
Showing 11 changed files with 173 additions and 77 deletions.
2 changes: 2 additions & 0 deletions base/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ component("base") {
"containers/linked_list.cc",
"containers/linked_list.h",
"containers/lru_cache.h",
"containers/map_util.h",
"containers/small_map.h",
"containers/span.h",
"containers/stack.h",
Expand Down Expand Up @@ -3130,6 +3131,7 @@ test("base_unittests") {
"containers/intrusive_heap_unittest.cc",
"containers/linked_list_unittest.cc",
"containers/lru_cache_unittest.cc",
"containers/map_util_unittest.cc",
"containers/small_map_unittest.cc",
"containers/span_unittest.cc",
"containers/unique_ptr_adapters_unittest.cc",
Expand Down
69 changes: 69 additions & 0 deletions base/containers/map_util.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef BASE_CONTAINERS_MAP_UTIL_H_
#define BASE_CONTAINERS_MAP_UTIL_H_

#include <memory>

namespace base {

namespace internal {

template <typename Map>
using MappedType = typename Map::mapped_type;

} // namespace internal

// Returns a pointer to the const value associated with the given key if it
// exists, or null otherwise.
template <typename Map, typename Key>
constexpr const internal::MappedType<Map>* FindOrNull(const Map& map,
const Key& key) {
auto it = map.find(key);
return it != map.end() ? &it->second : nullptr;
}

// Returns a pointer to the value associated with the given key if it exists, or
// null otherwise.
template <typename Map, typename Key>
constexpr internal::MappedType<Map>* FindOrNull(Map& map, const Key& key) {
auto it = map.find(key);
return it != map.end() ? &it->second : nullptr;
}

// Returns the const pointer value associated with the given key. If none is
// found, null is returned. The function is designed to be used with a map of
// keys to pointers or smart pointers.
//
// This function does not distinguish between a missing key and a key mapped
// to a null value.
template <typename Map,
typename Key,
typename MappedElementType =
std::pointer_traits<internal::MappedType<Map>>::element_type>
constexpr const MappedElementType* FindPtrOrNull(const Map& map,
const Key& key) {
auto it = map.find(key);
return it != map.end() ? std::to_address(it->second) : nullptr;
}

// Returns the pointer value associated with the given key. If none is found,
// null is returned. The function is designed to be used with a map of keys to
// pointers or smart pointers.
//
// This function does not distinguish between a missing key and a key mapped
// to a null value.
template <typename Map,
typename Key,
typename MappedElementType =
std::pointer_traits<internal::MappedType<Map>>::element_type>
constexpr MappedElementType* FindPtrOrNull(Map& map, const Key& key) {
auto it = map.find(key);
return it != map.end() ? std::to_address(it->second) : nullptr;
}

} // namespace base

#endif // BASE_CONTAINERS_MAP_UTIL_H_
63 changes: 63 additions & 0 deletions base/containers/map_util_unittest.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef BASE_CONTAINERS_MAP_UTIL_UNITTEST_CC_
#define BASE_CONTAINERS_MAP_UTIL_UNITTEST_CC_

#include "base/containers/map_util.h"

#include <memory>
#include <string>

#include "base/containers/flat_map.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace base {

namespace {

using testing::AllOf;
using testing::Eq;
using testing::Pointee;

constexpr char kKey[] = "key";
constexpr char kValue[] = "value";
constexpr char kMissingKey[] = "missing_key";

using StringToStringMap = base::flat_map<std::string, std::string>;
using StringToStringPtrMap = base::flat_map<std::string, std::string*>;
using StringToStringUniquePtrMap =
base::flat_map<std::string, std::unique_ptr<std::string>>;

TEST(MapUtilTest, FindOrNull) {
StringToStringMap mapping({{kKey, kValue}});

EXPECT_THAT(FindOrNull(mapping, kKey), Pointee(Eq(kValue)));
EXPECT_EQ(FindOrNull(mapping, kMissingKey), nullptr);
}

TEST(MapUtilTest, FindPtrOrNullForPointers) {
auto val = std::make_unique<std::string>(kValue);

StringToStringPtrMap mapping({{kKey, val.get()}});

EXPECT_THAT(FindPtrOrNull(mapping, kKey),
AllOf(Eq(val.get()), Pointee(Eq(kValue))));
EXPECT_EQ(FindPtrOrNull(mapping, kMissingKey), nullptr);
}

TEST(MapUtilTest, FindPtrOrNullForPointerLikeValues) {
StringToStringUniquePtrMap mapping;
mapping.insert({kKey, std::make_unique<std::string>(kValue)});

EXPECT_THAT(FindPtrOrNull(mapping, kKey), Pointee(Eq(kValue)));
EXPECT_EQ(FindPtrOrNull(mapping, kMissingKey), nullptr);
}

} // namespace

} // namespace base

#endif // BASE_CONTAINERS_MAP_UTIL_UNITTEST_CC_
8 changes: 3 additions & 5 deletions base/values.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "base/check_op.h"
#include "base/containers/checked_iterators.h"
#include "base/containers/cxx20_erase_vector.h"
#include "base/containers/map_util.h"
#include "base/cxx20_to_address.h"
#include "base/json/json_writer.h"
#include "base/logging.h"
Expand Down Expand Up @@ -420,14 +421,11 @@ void Value::Dict::Merge(Dict dict) {

const Value* Value::Dict::Find(StringPiece key) const {
DCHECK(IsStringUTF8AllowingNoncharacters(key));

auto it = storage_.find(key);
return it != storage_.end() ? it->second.get() : nullptr;
return FindPtrOrNull(storage_, key);
}

Value* Value::Dict::Find(StringPiece key) {
auto it = storage_.find(key);
return it != storage_.end() ? it->second.get() : nullptr;
return FindPtrOrNull(storage_, key);
}

absl::optional<bool> Value::Dict::FindBool(StringPiece key) const {
Expand Down
8 changes: 3 additions & 5 deletions chrome/browser/ash/printing/printers_map.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#include "chrome/browser/ash/printing/printers_map.h"

#include "base/containers/contains.h"
#include "base/containers/map_util.h"
#include "base/types/optional_util.h"

namespace ash {

Expand Down Expand Up @@ -179,11 +181,7 @@ void PrintersMap::SavePrinterStatus(

absl::optional<CupsPrinterStatus> PrintersMap::GetPrinterStatus(
const std::string& printer_id) const {
auto printer_iter = printer_statuses_.find(printer_id);
if (printer_iter != printer_statuses_.end()) {
return printer_iter->second;
}
return absl::nullopt;
return base::OptionalFromPtr(base::FindOrNull(printer_statuses_, printer_id));
}

std::set<std::string> PrintersMap::GetPrinterIdsInClass(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
#include "chrome/browser/nearby_sharing/public/cpp/fake_nearby_connections_manager.h"

#include "base/containers/contains.h"
#include "base/containers/map_util.h"
#include "base/threading/thread_restrictions.h"
#include "base/types/optional_util.h"

FakeNearbyConnectionsManager::FakeNearbyConnectionsManager() = default;

Expand Down Expand Up @@ -120,15 +122,9 @@ void FakeNearbyConnectionsManager::RegisterPayloadPath(
base::File::Flags::FLAG_READ |
base::File::Flags::FLAG_WRITE);
}

auto it = payload_path_status_.find(payload_id);
if (it == payload_path_status_.end()) {
std::move(callback).Run(
nearby::connections::mojom::Status::kPayloadUnknown);
return;
}

std::move(callback).Run(it->second);
auto* status = base::FindOrNull(payload_path_status_, payload_id);
std::move(callback).Run(
status ? *status : nearby::connections::mojom::Status::kPayloadUnknown);
}

FakeNearbyConnectionsManager::Payload*
Expand Down Expand Up @@ -170,26 +166,16 @@ absl::optional<std::string>
FakeNearbyConnectionsManager::GetAuthenticationToken(
const std::string& endpoint_id) {
DCHECK(!is_shutdown());

auto iter = endpoint_auth_tokens_.find(endpoint_id);
if (iter != endpoint_auth_tokens_.end()) {
return iter->second;
}

return absl::nullopt;
return base::OptionalFromPtr(
base::FindOrNull(endpoint_auth_tokens_, endpoint_id));
}

absl::optional<std::vector<uint8_t>>
FakeNearbyConnectionsManager::GetRawAuthenticationToken(
const std::string& endpoint_id) {
DCHECK(!is_shutdown());

auto iter = endpoint_raw_auth_tokens_.find(endpoint_id);
if (iter != endpoint_raw_auth_tokens_.end()) {
return iter->second;
}

return absl::nullopt;
return base::OptionalFromPtr(
base::FindOrNull(endpoint_raw_auth_tokens_, endpoint_id));
}

void FakeNearbyConnectionsManager::SetAuthenticationToken(
Expand Down Expand Up @@ -261,11 +247,9 @@ void FakeNearbyConnectionsManager::SetPayloadPathStatus(
base::WeakPtr<FakeNearbyConnectionsManager::PayloadStatusListener>
FakeNearbyConnectionsManager::GetRegisteredPayloadStatusListener(
int64_t payload_id) {
auto it = payload_status_listeners_.find(payload_id);
if (it != payload_status_listeners_.end()) {
return it->second;
if (auto* ptr = base::FindOrNull(payload_status_listeners_, payload_id)) {
return *ptr;
}

return nullptr;
}

Expand All @@ -281,12 +265,8 @@ bool FakeNearbyConnectionsManager::WasPayloadCanceled(

absl::optional<base::FilePath>
FakeNearbyConnectionsManager::GetRegisteredPayloadPath(int64_t payload_id) {
auto it = registered_payload_paths_.find(payload_id);
if (it == registered_payload_paths_.end()) {
return absl::nullopt;
}

return it->second;
return base::OptionalFromPtr(
base::FindOrNull(registered_payload_paths_, payload_id));
}

void FakeNearbyConnectionsManager::CleanupForProcessStopped() {
Expand Down
19 changes: 6 additions & 13 deletions components/performance_manager/graph/graph_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "base/containers/contains.h"
#include "base/containers/flat_map.h"
#include "base/containers/flat_set.h"
#include "base/containers/map_util.h"
#include "base/functional/bind.h"
#include "base/functional/callback_helpers.h"
#include "base/notreached.h"
Expand Down Expand Up @@ -386,25 +387,17 @@ bool GraphImpl::NodeInGraph(const NodeBase* node) {
return it != nodes_.end();
}

ProcessNodeImpl* GraphImpl::GetProcessNodeByPid(base::ProcessId pid) const {
ProcessNodeImpl* GraphImpl::GetProcessNodeByPid(base::ProcessId pid) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
auto it = processes_by_pid_.find(pid);
if (it == processes_by_pid_.end())
return nullptr;

return it->second;
return base::FindPtrOrNull(processes_by_pid_, pid);
}

FrameNodeImpl* GraphImpl::GetFrameNodeById(
RenderProcessHostId render_process_id,
int render_frame_id) const {
int render_frame_id) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
auto it =
frames_by_id_.find(ProcessAndFrameId(render_process_id, render_frame_id));
if (it == frames_by_id_.end())
return nullptr;

return it->second;
return base::FindPtrOrNull(
frames_by_id_, ProcessAndFrameId(render_process_id, render_frame_id));
}

std::vector<ProcessNodeImpl*> GraphImpl::GetAllProcessNodeImpls() const {
Expand Down
4 changes: 2 additions & 2 deletions components/performance_manager/graph/graph_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -137,11 +137,11 @@ class GraphImpl : public Graph {
const NodeSet& nodes() { return nodes_; }

// Retrieves the process node with PID |pid|, if any.
ProcessNodeImpl* GetProcessNodeByPid(base::ProcessId pid) const;
ProcessNodeImpl* GetProcessNodeByPid(base::ProcessId pid);

// Retrieves the frame node with the routing ids of the process and the frame.
FrameNodeImpl* GetFrameNodeById(RenderProcessHostId render_process_id,
int render_frame_id) const;
int render_frame_id);

// Returns true if |node| is in this graph.
bool NodeInGraph(const NodeBase* node);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

#include "base/check.h"
#include "base/containers/contains.h"
#include "base/containers/map_util.h"
#include "base/types/optional_util.h"
#include "components/subresource_filter/core/mojom/subresource_filter.mojom.h"
#include "content/public/browser/navigation_handle.h"
#include "content/public/browser/render_frame_host.h"
Expand Down Expand Up @@ -74,10 +76,7 @@ void TestSubresourceFilterObserver::DidFinishNavigation(

absl::optional<mojom::ActivationLevel>
TestSubresourceFilterObserver::GetPageActivation(const GURL& url) const {
auto it = page_activations_.find(url);
if (it != page_activations_.end())
return it->second;
return absl::nullopt;
return base::OptionalFromPtr(base::FindOrNull(page_activations_, url));
}

bool TestSubresourceFilterObserver::GetIsAdFrame(int frame_tree_node_id) const {
Expand All @@ -86,10 +85,8 @@ bool TestSubresourceFilterObserver::GetIsAdFrame(int frame_tree_node_id) const {

absl::optional<LoadPolicy>
TestSubresourceFilterObserver::GetChildFrameLoadPolicy(const GURL& url) const {
auto it = child_frame_load_evaluations_.find(url);
if (it != child_frame_load_evaluations_.end())
return it->second;
return absl::optional<LoadPolicy>();
return base::OptionalFromPtr(
base::FindOrNull(child_frame_load_evaluations_, url));
}

absl::optional<mojom::ActivationLevel>
Expand All @@ -99,10 +96,7 @@ TestSubresourceFilterObserver::GetPageActivationForLastCommittedLoad() const {

absl::optional<TestSubresourceFilterObserver::SafeBrowsingCheck>
TestSubresourceFilterObserver::GetSafeBrowsingResult(const GURL& url) const {
auto it = safe_browsing_checks_.find(url);
if (it != safe_browsing_checks_.end())
return it->second;
return absl::optional<SafeBrowsingCheck>();
return base::OptionalFromPtr(base::FindOrNull(safe_browsing_checks_, url));
}

} // namespace subresource_filter

0 comments on commit 3dfec41

Please sign in to comment.