Skip to content

Commit

Permalink
[CodeHealth] Avoid unwanted copies with base::flat_{tree,set,map}<>
Browse files Browse the repository at this point in the history
Applies one or more of the following:
-- std::move(argument) where possible.
-- remove stray 'const' where not helpful.
-- pass by const ref where helpful.
-- initalise in place to select for NRVO.

This CL was uploaded by git cl split.
  • Loading branch information
cdesouza-chromium committed Oct 7, 2024
1 parent 9809f6b commit f5e192a
Show file tree
Hide file tree
Showing 29 changed files with 110 additions and 126 deletions.
4 changes: 0 additions & 4 deletions browser/brave_shields/brave_shields_web_contents_observer.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,6 @@ class BraveShieldsWebContentsObserver
friend class content::WebContentsUserData<BraveShieldsWebContentsObserver>;
friend class BraveShieldsWebContentsObserverBrowserTest;

using BraveShieldsRemotesMap = base::flat_map<
content::RenderFrameHost*,
mojo::AssociatedRemote<brave_shields::mojom::BraveShields>>;

// Allows indicating a implementor of brave_shields::mojom::BraveShieldsHost
// other than this own class, for testing purposes only.
static void SetReceiverImplForTesting(BraveShieldsWebContentsObserver* impl);
Expand Down
22 changes: 7 additions & 15 deletions browser/brave_wallet/brave_wallet_tab_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,33 +37,25 @@ BraveWalletTabHelper::~BraveWalletTabHelper() {
void BraveWalletTabHelper::AddSolanaConnectedAccount(
const content::GlobalRenderFrameHostId& id,
const std::string& account) {
base::flat_set<std::string> connection_set;
if (solana_connected_accounts_.contains(id)) {
connection_set = solana_connected_accounts_.at(id);
}
connection_set.insert(account);
solana_connected_accounts_[id] = std::move(connection_set);
solana_connected_accounts_[id].insert(account);
}

void BraveWalletTabHelper::RemoveSolanaConnectedAccount(
const content::GlobalRenderFrameHostId& id,
const std::string& account) {
if (!solana_connected_accounts_.contains(id)) {
auto it = solana_connected_accounts_.find(id);
if (it == solana_connected_accounts_.end()) {
return;
}
auto connection_set = solana_connected_accounts_.at(id);
connection_set.erase(account);
solana_connected_accounts_[id] = std::move(connection_set);
it->second.erase(account);
}

bool BraveWalletTabHelper::IsSolanaAccountConnected(
const content::GlobalRenderFrameHostId& id,
const std::string& account) {
if (!solana_connected_accounts_.contains(id)) {
return false;
}
auto connection_set = solana_connected_accounts_.at(id);
return connection_set.contains(account);
auto it = solana_connected_accounts_.find(id);
return it == solana_connected_accounts_.end() ? false
: it->second.contains(account);
}

void BraveWalletTabHelper::ClearSolanaConnectedAccounts(
Expand Down
2 changes: 1 addition & 1 deletion browser/ui/commands/accelerator_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ AcceleratorService::AcceleratorService(
base::flat_set<ui::Accelerator> system_managed)
: pref_manager_(pref_service, commands::GetCommands()),
default_accelerators_(std::move(default_accelerators)),
system_managed_(system_managed) {
system_managed_(std::move(system_managed)) {
Initialize();
}

Expand Down
4 changes: 2 additions & 2 deletions browser/ui/commands/accelerator_service_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ KeyedService* AcceleratorServiceFactory::BuildServiceInstanceFor(
DCHECK(profile);

auto [accelerators, system_managed] = GetDefaultAccelerators();
return new AcceleratorService(profile->GetPrefs(), accelerators,
system_managed);
return new AcceleratorService(profile->GetPrefs(), std::move(accelerators),
std::move(system_managed));
}

} // namespace commands
10 changes: 5 additions & 5 deletions browser/ui/views/commands/default_accelerators_views.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@
// You can obtain one at https://mozilla.org/MPL/2.0/.

#include <utility>

#include "base/containers/flat_set.h"
#include "base/ranges/algorithm.h"
#include "brave/browser/ui/commands/default_accelerators.h"

#include "brave/browser/ui/commands/accelerator_service.h"
#include "brave/browser/ui/commands/default_accelerators.h"
#include "chrome/browser/ui/views/accelerator_table.h"
#include "ui/base/accelerators/accelerator.h"

Expand All @@ -20,8 +20,8 @@ namespace commands {

std::pair<Accelerators, base::flat_set<ui::Accelerator>>
GetDefaultAccelerators() {
Accelerators defaults;
base::flat_set<ui::Accelerator> system_commands;
std::pair<Accelerators, base::flat_set<ui::Accelerator>> result;
auto& [defaults, system_commands] = result;

auto add_to_accelerators = [&defaults](const AcceleratorMapping& mapping) {
defaults[mapping.command_id].push_back(
Expand All @@ -39,7 +39,7 @@ GetDefaultAccelerators() {
mapping.keycode, mapping.modifiers));
});
#endif // BUILDFLAG(IS_MAC)
return std::tie(defaults, system_commands);
return result;
}

} // namespace commands
36 changes: 18 additions & 18 deletions build/ios/mojom/cpp_transformations.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ NS_INLINE NSArray<NSNumber*>* NSArrayFromVector(std::vector<T> v) {
typedef NSNumber* (*NSNumberCall)(id, SEL, T);
NSNumberCall call = (NSNumberCall)method_getImplementation(method);

for (auto t : v) {
for (const auto& t : v) {
NSNumber* number =
reinterpret_cast<NSNumber*>(call(NSNumber.class, selector, t));
[a addObject:number];
Expand Down Expand Up @@ -102,7 +102,7 @@ NS_INLINE std::vector<T> VectorFromNSArray(NSArray<NSNumber*>* a) {
/// Convert a vector storing strings to an array of NSString's
NS_INLINE NSArray<NSString*>* NSArrayFromVector(std::vector<std::string> v) {
const auto a = [NSMutableArray new];
for (auto s : v) {
for (const auto& s : v) {
[a addObject:[NSString stringWithCString:s.c_str()
encoding:NSUTF8StringEncoding]];
}
Expand Down Expand Up @@ -175,12 +175,12 @@ NS_INLINE NSNumber* NumberFromPrimitive(T t) {
/// NSNumber *>
template <typename T>
NS_INLINE NSDictionary<NSString*, NSNumber*>* NSDictionaryFromMap(
std::map<std::string, T> m) {
const std::map<std::string, T>& m) {
const auto d = [NSMutableDictionary new];
if (m.empty()) {
return @{};
}
for (auto item : m) {
for (const auto& item : m) {
d[[NSString stringWithCString:item.first.c_str()
encoding:NSUTF8StringEncoding]] =
NumberFromPrimitive(item.second);
Expand All @@ -192,12 +192,12 @@ NS_INLINE NSDictionary<NSString*, NSNumber*>* NSDictionaryFromMap(
/// NSNumber *>
template <typename T>
NS_INLINE NSDictionary<NSString*, NSNumber*>* NSDictionaryFromMap(
base::flat_map<std::string, T> m) {
const base::flat_map<std::string, T>& m) {
const auto d = [NSMutableDictionary new];
if (m.empty()) {
return @{};
}
for (auto item : m) {
for (const auto& item : m) {
d[[NSString stringWithCString:item.first.c_str()
encoding:NSUTF8StringEncoding]] =
NumberFromPrimitive(item.second);
Expand All @@ -207,12 +207,12 @@ NS_INLINE NSDictionary<NSString*, NSNumber*>* NSDictionaryFromMap(

/// Convert a String to String mapping to an NSDictionary
NS_INLINE NSDictionary<NSString*, NSString*>* NSDictionaryFromMap(
std::map<std::string, std::string> m) {
const std::map<std::string, std::string>& m) {
const auto d = [NSMutableDictionary new];
if (m.empty()) {
return @{};
}
for (auto item : m) {
for (const auto& item : m) {
d[[NSString stringWithCString:item.first.c_str()
encoding:NSUTF8StringEncoding]] =
[NSString stringWithCString:item.second.c_str()
Expand All @@ -223,12 +223,12 @@ NS_INLINE NSDictionary<NSString*, NSString*>* NSDictionaryFromMap(

/// Convert a String to String mapping to an NSDictionary
NS_INLINE NSDictionary<NSString*, NSString*>* NSDictionaryFromMap(
base::flat_map<std::string, std::string> m) {
const base::flat_map<std::string, std::string>& m) {
const auto d = [NSMutableDictionary new];
if (m.empty()) {
return @{};
}
for (auto item : m) {
for (const auto& item : m) {
d[[NSString stringWithCString:item.first.c_str()
encoding:NSUTF8StringEncoding]] =
[NSString stringWithCString:item.second.c_str()
Expand All @@ -241,13 +241,13 @@ NS_INLINE NSDictionary<NSString*, NSString*>* NSDictionaryFromMap(
/// objects
template <typename V, typename ObjCObj>
NS_INLINE NSDictionary<NSString*, ObjCObj>* NSDictionaryFromMap(
std::map<std::string, V> m,
const std::map<std::string, V>& m,
ObjCObj (^transformValue)(V)) {
const auto d = [NSMutableDictionary new];
if (m.empty()) {
return @{};
}
for (auto item : m) {
for (const auto& item : m) {
d[[NSString stringWithCString:item.first.c_str()
encoding:NSUTF8StringEncoding]] =
transformValue(item.second);
Expand All @@ -259,13 +259,13 @@ NS_INLINE NSDictionary<NSString*, ObjCObj>* NSDictionaryFromMap(
/// objects
template <typename V, typename ObjCObj>
NS_INLINE NSDictionary<NSString*, ObjCObj>* NSDictionaryFromMap(
base::flat_map<std::string, V> m,
const base::flat_map<std::string, V>& m,
ObjCObj (^transformValue)(V)) {
const auto d = [NSMutableDictionary new];
if (m.empty()) {
return @{};
}
for (auto item : m) {
for (const auto& item : m) {
d[[NSString stringWithCString:item.first.c_str()
encoding:NSUTF8StringEncoding]] =
transformValue(item.second);
Expand All @@ -277,14 +277,14 @@ NS_INLINE NSDictionary<NSString*, ObjCObj>* NSDictionaryFromMap(
/// the key and the value types to Obj-C types
template <typename K, typename KObjC, typename V, typename VObjC>
NS_INLINE NSDictionary<KObjC, VObjC>* NSDictionaryFromMap(
std::map<K, V> m,
const std::map<K, V>& m,
KObjC (^transformKey)(K),
VObjC (^transformValue)(V)) {
const auto d = [NSMutableDictionary new];
if (m.empty()) {
return @{};
}
for (auto item : m) {
for (const auto& item : m) {
d[transformKey(item.first)] = transformValue(item.second);
}
return d;
Expand All @@ -294,14 +294,14 @@ NS_INLINE NSDictionary<KObjC, VObjC>* NSDictionaryFromMap(
/// the key and the value types to Obj-C types
template <typename K, typename KObjC, typename V, typename VObjC>
NS_INLINE NSDictionary<KObjC, VObjC>* NSDictionaryFromMap(
base::flat_map<K, V> m,
const base::flat_map<K, V>& m,
KObjC (^transformKey)(K),
VObjC (^transformValue)(V)) {
const auto d = [NSMutableDictionary new];
if (m.empty()) {
return @{};
}
for (auto item : m) {
for (const auto& item : m) {
d[transformKey(item.first)] = transformValue(item.second);
}
return d;
Expand Down
2 changes: 0 additions & 2 deletions components/ai_chat/content/browser/page_content_fetcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,6 @@ class PageContentFetcherInternal {
std::string invalidation_token,
std::unique_ptr<std::string> response_body) {
auto response_code = -1;
base::flat_map<std::string, std::string> headers;
if (loader->ResponseInfo()) {
auto headers_list = loader->ResponseInfo()->headers;
if (headers_list) {
Expand Down Expand Up @@ -328,7 +327,6 @@ class PageContentFetcherInternal {
std::unique_ptr<network::SimpleURLLoader> loader,
std::unique_ptr<std::string> response_body) {
auto response_code = -1;
base::flat_map<std::string, std::string> headers;
if (loader->ResponseInfo()) {
auto headers_list = loader->ResponseInfo()->headers;
if (headers_list) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,8 @@ EngineConsumerClaudeRemote::EngineConsumerClaudeRemote(
base::flat_set<std::string_view> stop_sequences(kStopSequences.begin(),
kStopSequences.end());
api_ = std::make_unique<RemoteCompletionClient>(
model_options.name, stop_sequences, url_loader_factory,
credential_manager);
model_options.name, std::move(stop_sequences),
std::move(url_loader_factory), credential_manager);

max_page_content_length_ = model_options.max_page_content_length;
}
Expand Down Expand Up @@ -222,7 +222,7 @@ void EngineConsumerClaudeRemote::GenerateQuestionSuggestions(
stop_sequences.push_back("</response>");

api_->QueryPrompt(
prompt, stop_sequences,
prompt, std::move(stop_sequences),
base::BindOnce(
&EngineConsumerClaudeRemote::OnGenerateQuestionSuggestionsResponse,
weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -330,8 +330,8 @@ EngineConsumerLlamaRemote::EngineConsumerLlamaRemote(
base::flat_set<std::string_view> stop_sequences(kStopSequences.begin(),
kStopSequences.end());
api_ = std::make_unique<RemoteCompletionClient>(
model_options.name, stop_sequences, url_loader_factory,
credential_manager);
model_options.name, std::move(stop_sequences),
std::move(url_loader_factory), credential_manager);

max_page_content_length_ = model_options.max_page_content_length;

Expand Down Expand Up @@ -374,7 +374,7 @@ void EngineConsumerLlamaRemote::GenerateQuestionSuggestions(
stop_sequences.push_back("</ul>");
DCHECK(api_);
api_->QueryPrompt(
prompt, stop_sequences,
prompt, std::move(stop_sequences),
base::BindOnce(
&EngineConsumerLlamaRemote::OnGenerateQuestionSuggestionsResponse,
weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class MockRemoteCompletionClient : public RemoteCompletionClient {
MOCK_METHOD(void,
QueryPrompt,
(const std::string&,
const std::vector<std::string>&,
std::vector<std::string>,
GenerationCompletedCallback,
GenerationDataCallback),
(override));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,25 +120,26 @@ GURL GetEndpointUrl(bool premium, const std::string& path) {

RemoteCompletionClient::RemoteCompletionClient(
const std::string& model_name,
const base::flat_set<std::string_view>& stop_sequences,
base::flat_set<std::string_view> stop_sequences,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
AIChatCredentialManager* credential_manager)
: model_name_(model_name),
stop_sequences_(stop_sequences),
api_request_helper_(GetNetworkTrafficAnnotationTag(), url_loader_factory),
stop_sequences_(std::move(stop_sequences)),
api_request_helper_(GetNetworkTrafficAnnotationTag(),
std::move(url_loader_factory)),
credential_manager_(credential_manager) {}

RemoteCompletionClient::~RemoteCompletionClient() = default;

void RemoteCompletionClient::QueryPrompt(
const std::string& prompt,
const std::vector<std::string>& extra_stop_sequences,
std::vector<std::string> extra_stop_sequences,
GenerationCompletedCallback data_completed_callback,
GenerationDataCallback
data_received_callback /* = base::NullCallback() */) {
auto callback = base::BindOnce(
&RemoteCompletionClient::OnFetchPremiumCredential,
weak_ptr_factory_.GetWeakPtr(), prompt, extra_stop_sequences,
weak_ptr_factory_.GetWeakPtr(), prompt, std::move(extra_stop_sequences),
std::move(data_completed_callback), std::move(data_received_callback));
credential_manager_->FetchPremiumCredential(std::move(callback));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class RemoteCompletionClient {

RemoteCompletionClient(
const std::string& model_name,
const base::flat_set<std::string_view>& stop_sequences,
base::flat_set<std::string_view> stop_sequences,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
AIChatCredentialManager* credential_manager);

Expand All @@ -50,7 +50,7 @@ class RemoteCompletionClient {
// In non-SSE cases, only the data_completed_callback will be triggered.
virtual void QueryPrompt(
const std::string& prompt,
const std::vector<std::string>& stop_sequences,
std::vector<std::string> stop_sequences,
GenerationCompletedCallback data_completed_callback,
GenerationDataCallback data_received_callback = base::NullCallback());
// Clears all in-progress requests
Expand Down
2 changes: 1 addition & 1 deletion components/brave_ads/browser/ads_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1016,7 +1016,7 @@ void AdsServiceImpl::URLRequestCallback(
mojom_url_response->url = url_loader->GetFinalURL();
mojom_url_response->status_code = response_code;
mojom_url_response->body = response_body ? *response_body : "";
mojom_url_response->headers = headers;
mojom_url_response->headers = std::move(headers);

std::move(callback).Run(std::move(mojom_url_response));
}
Expand Down
9 changes: 4 additions & 5 deletions components/brave_news/browser/peeking_card.cc
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ base::flat_set<std::string> GetTopStoryUrls(

std::optional<size_t> PickPeekingCardWithMax(
SubscriptionsSnapshot subscriptions,
base::flat_set<std::string> top_story_urls,
const base::flat_set<std::string>& top_story_urls,
const ArticleInfos& articles,
size_t max_candidates) {
// Store now, so it's consistent for everything.
Expand Down Expand Up @@ -217,11 +217,10 @@ std::optional<size_t> PickPeekingCardWithMax(

std::optional<size_t> PickPeekingCard(
SubscriptionsSnapshot subscriptions,
base::flat_set<std::string> top_story_urls,
const base::flat_set<std::string>& top_story_urls,
const ArticleInfos& articles) {
return PickPeekingCardWithMax(std::move(subscriptions),
std::move(top_story_urls), articles,
kMaxPeekingCardCandidates);
return PickPeekingCardWithMax(std::move(subscriptions), top_story_urls,
articles, kMaxPeekingCardCandidates);
}

} // namespace brave_news
Loading

0 comments on commit f5e192a

Please sign in to comment.