Skip to content

Commit

Permalink
[base] Remove base::Contains{Key,Value}
Browse files Browse the repository at this point in the history
This change removes base::ContainsKey() and base::ContainsValue() from
the codebase. In addition, it migrates remaining usages of these methods
to using base::Contains instead. These usages were not caught by the
search and replace CLs, because they either were added afterwards, or
were unqualified and used ADL.

TBR=rockot@google.com,blundell@chromium.org

Bug: 970209
Change-Id: I5de98dd6b823b14adb81d0b4f2026b155aeb303c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1649478
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#668329}
  • Loading branch information
jdoerrie authored and Commit Bot committed Jun 12, 2019
1 parent 7deed1e commit 73c901e
Show file tree
Hide file tree
Showing 30 changed files with 55 additions and 92 deletions.
37 changes: 0 additions & 37 deletions base/stl_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,17 +79,6 @@ struct HasContains<Container,
void_t<decltype(std::declval<const Container&>().contains(
std::declval<const Element&>()))>> : std::true_type {};

// Utility type trait to detect whether a given container type has a nested
// mapped_type typedef. Used below to disable ContainsValue() for map like
// types.
// TOOD(crbug.com/970209): Remove once ContainsValue() is no longer used.
template <typename Container, typename = void>
struct HasMappedType : std::false_type {};

template <typename Container>
struct HasMappedType<Container, void_t<typename Container::mapped_type>>
: std::true_type {};

} // namespace internal

// C++14 implementation of C++17's std::size():
Expand Down Expand Up @@ -230,32 +219,6 @@ bool Contains(const Container& container, const Value& value) {
return container.contains(value);
}

// Test to see if a set or map contains a particular key.
// Returns true if the key is in the collection.
// TODO(crbug.com/970209): Replace usages of ContainsKey() with Contains() and
// remove this method.
template <typename Collection, typename Key>
bool ContainsKey(const Collection& collection, const Key& key) {
return Contains(collection, key);
}

// Test to see if a collection like a vector contains a particular value.
// Returns true if the value is in the collection.
//
// Note: This method is disabled for std::map-like types to avoid confusion.
// Since ContainsValue() would invoke std::map::find() through Contains(),
// usages of keys would be reported, not values.
//
// TODO(crbug.com/970209): Replace usages of ContainsValue() with Contains() and
// remove this method.
template <
typename Collection,
typename Value,
std::enable_if_t<!internal::HasMappedType<Collection>::value>* = nullptr>
bool ContainsValue(const Collection& collection, const Value& value) {
return Contains(collection, value);
}

// O(1) implementation of const casting an iterator for any sequence,
// associative or unordered associative container in the STL.
//
Expand Down
4 changes: 2 additions & 2 deletions base/test/launcher/test_launcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1259,7 +1259,7 @@ bool TestLauncher::ProcessAndValidateTests() {
std::vector<TestInfo> test_sequence;
test_sequence.push_back(test_info);
// Move Pre Tests prior to final test in order.
while (base::ContainsKey(pre_tests, test_sequence.back().GetPreName())) {
while (base::Contains(pre_tests, test_sequence.back().GetPreName())) {
test_sequence.push_back(pre_tests[test_sequence.back().GetPreName()]);
pre_tests.erase(test_sequence.back().GetDisabledStrippedName());
}
Expand Down Expand Up @@ -1397,7 +1397,7 @@ bool TestLauncher::RunRetryTests() {
// Retry all tests that depend on a failing test.
std::vector<std::string> test_names;
for (const TestInfo& test_info : tests_) {
if (base::ContainsKey(tests_to_retry_, test_info.GetPrefixStrippedName()))
if (base::Contains(tests_to_retry_, test_info.GetPrefixStrippedName()))
test_names.push_back(test_info.GetFullName());
}
tests_to_retry_.clear();
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/complex_tasks/task_tab_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class TaskTabHelper : public content::WebContentsObserver,
content::WebContents* web_contents);
const sessions::ContextRecordTaskId* get_context_record_task_id(
int nav_id) const {
if (!ContainsKey(local_context_record_task_id_map_, nav_id))
if (!base::Contains(local_context_record_task_id_map_, nav_id))
return nullptr;
return &local_context_record_task_id_map_.find(nav_id)->second;
}
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/devtools/global_confirm_info_bar.cc
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ void GlobalConfirmInfoBar::MaybeAddInfoBar(content::WebContents* web_contents) {
InfoBarService::FromWebContents(web_contents);
// WebContents from the tab strip must have the infobar service.
DCHECK(infobar_service);
if (ContainsKey(proxies_, infobar_service))
if (base::Contains(proxies_, infobar_service))
return;

std::unique_ptr<GlobalConfirmInfoBar::DelegateProxy> proxy(
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/extensions/content_verifier_test_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ void DownloaderTestDelegate::StartUpdateCheck(
requests_.push_back(std::move(fetch_data));
const ManifestFetchData* data = requests_.back().get();
for (const auto& id : data->extension_ids()) {
if (ContainsKey(responses_, id)) {
if (base::Contains(responses_, id)) {
// We use PostTask here instead of calling OnExtensionDownloadFinished
// immeditately, because the calling code isn't expecting a synchronous
// response (in non-test situations there are at least 2 network
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -295,8 +295,8 @@ TEST_F(MediaDrmOriginIdManagerTest, OriginIdNotInList) {
DVLOG(1) << "Checking preference " << kMediaDrmOriginIds;
auto* dict = GetDictionary(kMediaDrmOriginIds);
auto* list = dict->FindKey(kAvailableOriginIds);
EXPECT_FALSE(ContainsValue(list->GetList(),
CreateUnguessableTokenValue(origin_id.value())));
EXPECT_FALSE(base::Contains(list->GetList(),
CreateUnguessableTokenValue(origin_id.value())));
}

TEST_F(MediaDrmOriginIdManagerTest, ProvisioningFail) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ void MediaStreamCaptureIndicator::ExecuteCommand(int command_id,
DCHECK_LE(0, index);
DCHECK_GT(static_cast<int>(command_targets_.size()), index);
WebContents* web_contents = command_targets_[index];
if (ContainsKey(usage_map_, web_contents))
if (base::Contains(usage_map_, web_contents))
web_contents->GetDelegate()->ActivateContents(web_contents);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -793,7 +793,7 @@ void MTPDeviceDelegateImplLinux::AddWatcher(
const auto it = subscribers_.find(file_path);
if (it != subscribers_.end()) {
// Adds to existing origin callback map.
if (ContainsKey(it->second, origin)) {
if (base::Contains(it->second, origin)) {
callback.Run(base::File::FILE_ERROR_EXISTS);
return;
}
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/plugins/chrome_plugin_service_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,8 @@ bool ChromePluginServiceFilter::CanLoadPlugin(int render_process_id,
if (!details)
return false;

return (ContainsKey(details->authorized_plugins, path) ||
ContainsKey(details->authorized_plugins, base::FilePath()));
return (base::Contains(details->authorized_plugins, path) ||
base::Contains(details->authorized_plugins, base::FilePath()));
}

ChromePluginServiceFilter::ChromePluginServiceFilter() {
Expand Down
6 changes: 3 additions & 3 deletions chrome/browser/profiles/profile_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -240,14 +240,14 @@ void ProfileSizeTask(const base::FilePath& path, int enabled_app_count) {
// Schedule a profile for deletion if it isn't already scheduled.
// Returns whether the profile has been newly scheduled.
bool ScheduleProfileDirectoryForDeletion(const base::FilePath& path) {
if (ContainsKey(ProfilesToDelete(), path))
if (base::Contains(ProfilesToDelete(), path))
return false;
ProfilesToDelete()[path] = ProfileDeletionStage::SCHEDULING;
return true;
}

void MarkProfileDirectoryForDeletion(const base::FilePath& path) {
DCHECK(!ContainsKey(ProfilesToDelete(), path) ||
DCHECK(!base::Contains(ProfilesToDelete(), path) ||
ProfilesToDelete()[path] == ProfileDeletionStage::SCHEDULING);
ProfilesToDelete()[path] = ProfileDeletionStage::MARKED;
// Remember that this profile was deleted and files should have been deleted
Expand All @@ -260,7 +260,7 @@ void MarkProfileDirectoryForDeletion(const base::FilePath& path) {
// Cancel a scheduling deletion, so ScheduleProfileDirectoryForDeletion can be
// called again successfully.
void CancelProfileDeletion(const base::FilePath& path) {
DCHECK(!ContainsKey(ProfilesToDelete(), path) ||
DCHECK(!base::Contains(ProfilesToDelete(), path) ||
ProfilesToDelete()[path] == ProfileDeletionStage::SCHEDULING);
ProfilesToDelete().erase(path);
ProfileMetrics::LogProfileDeleteUser(ProfileMetrics::DELETE_PROFILE_ABORTED);
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/sessions/session_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,8 @@ void SessionService::SetTabGroup(const SessionID& window_id,

// Tabs get ungrouped as they close. However, if the whole window is closing
// tabs should stay in their groups. So, ignore this call in that case.
if (base::ContainsKey(pending_window_close_ids_, window_id) ||
base::ContainsKey(window_closing_ids_, window_id))
if (base::Contains(pending_window_close_ids_, window_id) ||
base::Contains(window_closing_ids_, window_id))
return;

ScheduleCommand(sessions::CreateTabGroupCommand(tab_id, group));
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/tabs/tab_strip_model.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1007,7 +1007,7 @@ void TabStripModel::AddToGroupForRestore(const std::vector<int>& indices,
DCHECK(!reentrancy_guard_);
base::AutoReset<bool> resetter(&reentrancy_guard_, true);

const bool group_exists = base::ContainsKey(group_data_, group);
const bool group_exists = base::Contains(group_data_, group);
if (group_exists)
AddToExistingGroupImpl(indices, group);
else
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/ui/tabs/tab_strip_model_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3317,7 +3317,7 @@ bool GroupsAreContiguous(
// |seen_groups| tells us which.
if (group_per_tab[tab_index].has_value() &&
group_per_tab[tab_index] != last_seen_group) {
if (base::ContainsKey(seen_groups, group_per_tab[tab_index].value()))
if (base::Contains(seen_groups, group_per_tab[tab_index].value()))
return false;
seen_groups.insert(group_per_tab[tab_index].value());
}
Expand Down Expand Up @@ -3346,7 +3346,7 @@ std::string PrintGroupsToString(
continue;
}

if (!base::ContainsKey(group_to_output, cur.value()))
if (!base::Contains(group_to_output, cur.value()))
group_to_output[cur.value()] = next_group++;

result += base::NumberToString(group_to_output[cur.value()]) + " ";
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/unload_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ bool UnloadController::TabsNeedBeforeUnloadFired() {
bool should_fire_beforeunload =
contents->NeedToFireBeforeUnload() ||
DevToolsWindow::NeedsToInterceptBeforeUnload(contents);
if (!ContainsKey(tabs_needing_unload_fired_, contents) &&
if (!base::Contains(tabs_needing_unload_fired_, contents) &&
should_fire_beforeunload) {
tabs_needing_before_unload_fired_.insert(contents);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ void DbusPropertiesInterface::OnGetAllProperties(
dbus::Response::FromMethodCall(method_call);
dbus::MessageWriter writer(response.get());

if (base::ContainsKey(properties_, interface)) {
if (base::Contains(properties_, interface)) {
dbus::MessageWriter array_writer(nullptr);
dbus::MessageWriter dict_entry_writer(nullptr);
writer.OpenArray("{sv}", &array_writer);
Expand Down Expand Up @@ -112,8 +112,8 @@ void DbusPropertiesInterface::OnGetProperty(
std::string interface;
std::string property_name;
if (!reader.PopString(&interface) || !reader.PopString(&property_name) ||
!base::ContainsKey(properties_, interface) ||
!base::ContainsKey(properties_[interface], property_name)) {
!base::Contains(properties_, interface) ||
!base::Contains(properties_[interface], property_name)) {
response_sender.Run(nullptr);
return;
}
Expand Down
2 changes: 1 addition & 1 deletion chrome/service/cloud_print/cloud_print_connector.cc
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ void CloudPrintConnector::InitJobHandlerForPrinter(
DCHECK(!printer_info_cloud.printer_id.empty());
VLOG(1) << "CP_CONNECTOR: Init job handler"
<< ", printer id: " << printer_info_cloud.printer_id;
if (ContainsKey(job_handler_map_, printer_info_cloud.printer_id))
if (base::Contains(job_handler_map_, printer_info_cloud.printer_id))
return; // Nothing to do if we already have a job handler for this printer.

printing::PrinterBasicInfo printer_info;
Expand Down
2 changes: 1 addition & 1 deletion chrome/services/cups_proxy/ipp_validator_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class FakeServiceDelegate
}

base::Optional<Printer> GetPrinter(const std::string& id) override {
if (!base::ContainsKey(known_printers_, id)) {
if (!base::Contains(known_printers_, id)) {
return base::nullopt;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,15 @@ void ClientPolicyControllerTest::TearDown() {

void ClientPolicyControllerTest::ExpectTemporary(std::string name_space) {
EXPECT_TRUE(
base::ContainsValue(controller()->GetTemporaryNamespaces(), name_space))
base::Contains(controller()->GetTemporaryNamespaces(), name_space))
<< "Namespace " << name_space
<< " had incorrect lifetime type when getting temporary namespaces.";
EXPECT_TRUE(controller()->IsTemporary(name_space))
<< "Namespace " << name_space
<< " had incorrect lifetime type setting when directly checking"
" if it is temporary.";
EXPECT_FALSE(
base::ContainsValue(controller()->GetPersistentNamespaces(), name_space))
base::Contains(controller()->GetPersistentNamespaces(), name_space))
<< "Namespace " << name_space
<< " had incorrect lifetime type when getting persistent namespaces.";
EXPECT_FALSE(controller()->IsPersistent(name_space))
Expand Down Expand Up @@ -122,8 +122,8 @@ TEST_F(ClientPolicyControllerTest, FallbackTest) {
EXPECT_EQ(policy.name_space, kDefaultNamespace);
EXPECT_TRUE(isTemporary(policy));
ExpectTemporary(kDefaultNamespace);
EXPECT_FALSE(base::ContainsValue(controller()->GetTemporaryNamespaces(),
kUndefinedNamespace));
EXPECT_FALSE(base::Contains(controller()->GetTemporaryNamespaces(),
kUndefinedNamespace));
EXPECT_TRUE(controller()->IsTemporary(kUndefinedNamespace));
ExpectDownloadSupport(kUndefinedNamespace, false);
ExpectDownloadSupport(kDefaultNamespace, false);
Expand Down
2 changes: 1 addition & 1 deletion content/browser/frame_host/render_frame_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3409,7 +3409,7 @@ void RenderFrameHostImpl::OnEnterFullscreen(
node = node->parent()) {
SiteInstance* parent_site_instance =
node->parent()->current_frame_host()->GetSiteInstance();
if (ContainsKey(notified_instances, parent_site_instance))
if (base::Contains(notified_instances, parent_site_instance))
continue;

RenderFrameProxyHost* child_proxy =
Expand Down
4 changes: 2 additions & 2 deletions content/browser/service_worker/service_worker_context_core.cc
Original file line number Diff line number Diff line change
Expand Up @@ -361,13 +361,13 @@ void ServiceWorkerContextCore::HasMainFrameProviderHost(
void ServiceWorkerContextCore::RegisterProviderHostByClientID(
const std::string& client_uuid,
ServiceWorkerProviderHost* provider_host) {
DCHECK(!ContainsKey(*provider_by_uuid_, client_uuid));
DCHECK(!base::Contains(*provider_by_uuid_, client_uuid));
(*provider_by_uuid_)[client_uuid] = provider_host;
}

void ServiceWorkerContextCore::UnregisterProviderHostByClientID(
const std::string& client_uuid) {
DCHECK(ContainsKey(*provider_by_uuid_, client_uuid));
DCHECK(base::Contains(*provider_by_uuid_, client_uuid));
provider_by_uuid_->erase(client_uuid);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -997,8 +997,8 @@ TEST_F(ServiceWorkerContextTest, ProviderHostIterator) {
results.insert(it->GetProviderHost());
}
EXPECT_EQ(2u, results.size());
EXPECT_TRUE(ContainsKey(results, host1_raw));
EXPECT_TRUE(ContainsKey(results, host3_raw));
EXPECT_TRUE(base::Contains(results, host1_raw));
EXPECT_TRUE(base::Contains(results, host3_raw));

// Iterate over the provider hosts that belong to kOrigin2.
// (This should not include host4 as it's not for controllee.)
Expand All @@ -1009,7 +1009,7 @@ TEST_F(ServiceWorkerContextTest, ProviderHostIterator) {
results.insert(it->GetProviderHost());
}
EXPECT_EQ(1u, results.size());
EXPECT_TRUE(ContainsKey(results, host2_raw));
EXPECT_TRUE(base::Contains(results, host2_raw));

context()->RemoveProviderHost(host1->provider_id());
context()->RemoveProviderHost(host2->provider_id());
Expand Down
2 changes: 1 addition & 1 deletion content/browser/web_contents/web_contents_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2165,7 +2165,7 @@ void WebContentsImpl::OnWebContentsDestroyed(WebContentsImpl* web_contents) {
}

void WebContentsImpl::AddDestructionObserver(WebContentsImpl* web_contents) {
if (!ContainsKey(destruction_observers_, web_contents)) {
if (!base::Contains(destruction_observers_, web_contents)) {
destruction_observers_[web_contents] =
std::make_unique<DestructionObserver>(this, web_contents);
}
Expand Down
6 changes: 3 additions & 3 deletions extensions/browser/api/declarative/rules_registry_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ scoped_refptr<RulesRegistry> RulesRegistryService::GetRulesRegistry(

scoped_refptr<RulesRegistry> registry = RegisterWebRequestRulesRegistry(
rules_registry_id, RulesCacheDelegate::Type::kEphemeral);
DCHECK(ContainsKey(rule_registries_, key));
DCHECK(base::Contains(rule_registries_, key));
return registry;
}

Expand Down Expand Up @@ -184,7 +184,7 @@ RulesRegistryService::RegisterWebRequestRulesRegistry(
int rules_registry_id,
RulesCacheDelegate::Type cache_delegate_type) {
DCHECK(browser_context_);
DCHECK(!ContainsKey(
DCHECK(!base::Contains(
rule_registries_,
RulesRegistryKey(declarative_webrequest_constants::kOnRequest,
rules_registry_id)));
Expand All @@ -208,7 +208,7 @@ RulesRegistryService::RegisterWebRequestRulesRegistry(

void RulesRegistryService::EnsureDefaultRulesRegistriesRegistered() {
DCHECK(browser_context_);
DCHECK(!ContainsKey(
DCHECK(!base::Contains(
rule_registries_,
RulesRegistryKey(declarative_webrequest_constants::kOnRequest,
kDefaultRulesRegistryID)));
Expand Down
10 changes: 5 additions & 5 deletions extensions/browser/api/device_permissions_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -538,8 +538,8 @@ void DevicePermissionsManager::AllowUsbDevice(

device_permissions->entries_.insert(device_entry);
SaveDevicePermissionEntry(context_, extension_id, device_entry);
} else if (!ContainsKey(device_permissions->ephemeral_usb_devices_,
device_info.guid)) {
} else if (!base::Contains(device_permissions->ephemeral_usb_devices_,
device_info.guid)) {
// Non-persistent devices cannot be reliably identified when they are
// reconnected so such devices are only remembered until disconnect.
// Register an observer here so that this set doesn't grow undefinitely.
Expand Down Expand Up @@ -576,8 +576,8 @@ void DevicePermissionsManager::AllowHidDevice(

device_permissions->entries_.insert(device_entry);
SaveDevicePermissionEntry(context_, extension_id, device_entry);
} else if (!ContainsKey(device_permissions->ephemeral_hid_devices_,
device.guid)) {
} else if (!base::Contains(device_permissions->ephemeral_hid_devices_,
device.guid)) {
// Non-persistent devices cannot be reliably identified when they are
// reconnected so such devices are only remembered until disconnect.
// Register an observer here so that this set doesn't grow undefinitely.
Expand Down Expand Up @@ -609,7 +609,7 @@ void DevicePermissionsManager::RemoveEntry(
DCHECK(thread_checker_.CalledOnValidThread());
DevicePermissions* device_permissions = GetInternal(extension_id);
DCHECK(device_permissions);
DCHECK(ContainsKey(device_permissions->entries_, entry));
DCHECK(base::Contains(device_permissions->entries_, entry));
device_permissions->entries_.erase(entry);
if (entry->IsPersistent()) {
RemoveDevicePermissionEntry(context_, extension_id, entry);
Expand Down
Loading

0 comments on commit 73c901e

Please sign in to comment.