From f6fb877de915f93d352c2eb732a3998c1425ee1d Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 25 Jul 2019 10:19:04 -0500 Subject: [PATCH] chore: pass base::StringPiece args by value (#19432) https://cs.chromium.org/chromium/src/base/strings/string_piece.h?l=14 discusses this, saying "Prefer passing StringPieces by value" because "[p]assing by value generates slightly smaller code." --- native_mate/native_mate/converter.cc | 7 +++---- native_mate/native_mate/converter.h | 7 +++---- native_mate/native_mate/dictionary.h | 14 ++++++------- .../native_mate/object_template_builder.cc | 7 +++---- .../native_mate/object_template_builder.h | 12 +++++------ shell/browser/api/atom_api_screen.cc | 4 ++-- shell/browser/api/event_emitter.h | 10 ++++----- shell/browser/web_contents_preferences.cc | 21 ++++++++----------- shell/browser/web_contents_preferences.h | 9 ++++---- .../crash_reporter/crash_reporter_crashpad.cc | 4 ++-- .../crash_reporter/crash_reporter_crashpad.h | 3 +-- shell/common/gin_util.h | 2 +- 12 files changed, 46 insertions(+), 54 deletions(-) diff --git a/native_mate/native_mate/converter.cc b/native_mate/native_mate/converter.cc index 464dc14cb6b..31bd30662ff 100644 --- a/native_mate/native_mate/converter.cc +++ b/native_mate/native_mate/converter.cc @@ -138,9 +138,8 @@ v8::Local Converter::ToV8(v8::Isolate* isolate, .ToLocalChecked(); } -v8::Local Converter::ToV8( - v8::Isolate* isolate, - const base::StringPiece& val) { +v8::Local Converter::ToV8(v8::Isolate* isolate, + base::StringPiece val) { return v8::String::NewFromUtf8(isolate, val.data(), v8::NewStringType::kNormal, static_cast(val.length())) @@ -260,7 +259,7 @@ bool Converter>::FromV8(v8::Isolate* isolate, } v8::Local StringToSymbol(v8::Isolate* isolate, - const base::StringPiece& val) { + base::StringPiece val) { return v8::String::NewFromUtf8(isolate, val.data(), v8::NewStringType::kInternalized, static_cast(val.length())) diff --git a/native_mate/native_mate/converter.h b/native_mate/native_mate/converter.h index ac9167eade5..f8a736f59c8 100644 --- a/native_mate/native_mate/converter.h +++ b/native_mate/native_mate/converter.h @@ -123,8 +123,7 @@ struct Converter { template <> struct Converter { - static v8::Local ToV8(v8::Isolate* isolate, - const base::StringPiece& val); + static v8::Local ToV8(v8::Isolate* isolate, base::StringPiece val); // No conversion out is possible because StringPiece does not contain storage. }; @@ -138,7 +137,7 @@ struct Converter { }; v8::Local StringToSymbol(v8::Isolate* isolate, - const base::StringPiece& input); + base::StringPiece input); template <> struct Converter> { @@ -343,7 +342,7 @@ bool ConvertFromV8(v8::Isolate* isolate, } inline v8::Local StringToV8(v8::Isolate* isolate, - const base::StringPiece& input) { + base::StringPiece input) { return ConvertToV8(isolate, input).As(); } diff --git a/native_mate/native_mate/dictionary.h b/native_mate/native_mate/dictionary.h index 8e2969ba860..2c2a275fb5c 100644 --- a/native_mate/native_mate/dictionary.h +++ b/native_mate/native_mate/dictionary.h @@ -41,7 +41,7 @@ class Dictionary { static Dictionary CreateEmpty(v8::Isolate* isolate); template - bool Get(const base::StringPiece& key, T* out) const { + bool Get(base::StringPiece key, T* out) const { // Check for existence before getting, otherwise this method will always // returns true when T == v8::Local. v8::Local context = isolate_->GetCurrentContext(); @@ -56,7 +56,7 @@ class Dictionary { } template - bool GetHidden(const base::StringPiece& key, T* out) const { + bool GetHidden(base::StringPiece key, T* out) const { v8::Local context = isolate_->GetCurrentContext(); v8::Local privateKey = v8::Private::ForApi(isolate_, StringToV8(isolate_, key)); @@ -69,7 +69,7 @@ class Dictionary { } template - bool Set(const base::StringPiece& key, const T& val) { + bool Set(base::StringPiece key, const T& val) { v8::Local v8_value; if (!TryConvertToV8(isolate_, val, &v8_value)) return false; @@ -79,7 +79,7 @@ class Dictionary { } template - bool SetHidden(const base::StringPiece& key, T val) { + bool SetHidden(base::StringPiece key, T val) { v8::Local v8_value; if (!TryConvertToV8(isolate_, val, &v8_value)) return false; @@ -92,7 +92,7 @@ class Dictionary { } template - bool SetReadOnly(const base::StringPiece& key, T val) { + bool SetReadOnly(base::StringPiece key, T val) { v8::Local v8_value; if (!TryConvertToV8(isolate_, val, &v8_value)) return false; @@ -103,7 +103,7 @@ class Dictionary { } template - bool SetMethod(const base::StringPiece& key, const T& callback) { + bool SetMethod(base::StringPiece key, const T& callback) { return GetHandle() ->Set(isolate_->GetCurrentContext(), StringToV8(isolate_, key), CallbackTraits::CreateTemplate(isolate_, callback) @@ -112,7 +112,7 @@ class Dictionary { .ToChecked(); } - bool Delete(const base::StringPiece& key) { + bool Delete(base::StringPiece key) { v8::Maybe result = GetHandle()->Delete(isolate_->GetCurrentContext(), StringToV8(isolate_, key)); return !result.IsNothing() && result.FromJust(); diff --git a/native_mate/native_mate/object_template_builder.cc b/native_mate/native_mate/object_template_builder.cc index 90da13d1af0..cb6d1ec9e89 100644 --- a/native_mate/native_mate/object_template_builder.cc +++ b/native_mate/native_mate/object_template_builder.cc @@ -13,15 +13,14 @@ ObjectTemplateBuilder::ObjectTemplateBuilder( ObjectTemplateBuilder::~ObjectTemplateBuilder() {} -ObjectTemplateBuilder& ObjectTemplateBuilder::SetImpl( - const base::StringPiece& name, - v8::Local val) { +ObjectTemplateBuilder& ObjectTemplateBuilder::SetImpl(base::StringPiece name, + v8::Local val) { template_->Set(StringToSymbol(isolate_, name), val); return *this; } ObjectTemplateBuilder& ObjectTemplateBuilder::SetPropertyImpl( - const base::StringPiece& name, + base::StringPiece name, v8::Local getter, v8::Local setter) { template_->SetAccessorProperty(StringToSymbol(isolate_, name), getter, diff --git a/native_mate/native_mate/object_template_builder.h b/native_mate/native_mate/object_template_builder.h index 1f502c02d7e..93bb1f0d62b 100644 --- a/native_mate/native_mate/object_template_builder.h +++ b/native_mate/native_mate/object_template_builder.h @@ -72,7 +72,7 @@ class ObjectTemplateBuilder { // poetic license here in order that all calls to Set() can be via the '.' // operator and line up nicely. template - ObjectTemplateBuilder& SetValue(const base::StringPiece& name, T val) { + ObjectTemplateBuilder& SetValue(base::StringPiece name, T val) { return SetImpl(name, ConvertToV8(isolate_, val)); } @@ -81,17 +81,17 @@ class ObjectTemplateBuilder { // use one of the first two options. Also see mate::CreateFunctionTemplate() // for creating raw function templates. template - ObjectTemplateBuilder& SetMethod(const base::StringPiece& name, T callback) { + ObjectTemplateBuilder& SetMethod(base::StringPiece name, T callback) { return SetImpl(name, CallbackTraits::CreateTemplate(isolate_, callback)); } template - ObjectTemplateBuilder& SetProperty(const base::StringPiece& name, T getter) { + ObjectTemplateBuilder& SetProperty(base::StringPiece name, T getter) { return SetPropertyImpl(name, CallbackTraits::CreateTemplate(isolate_, getter), v8::Local()); } template - ObjectTemplateBuilder& SetProperty(const base::StringPiece& name, + ObjectTemplateBuilder& SetProperty(base::StringPiece name, T getter, U setter) { return SetPropertyImpl(name, @@ -105,10 +105,10 @@ class ObjectTemplateBuilder { v8::Local Build(); private: - ObjectTemplateBuilder& SetImpl(const base::StringPiece& name, + ObjectTemplateBuilder& SetImpl(base::StringPiece name, v8::Local val); ObjectTemplateBuilder& SetPropertyImpl( - const base::StringPiece& name, + base::StringPiece name, v8::Local getter, v8::Local setter); diff --git a/shell/browser/api/atom_api_screen.cc b/shell/browser/api/atom_api_screen.cc index 783910b9527..4ce35048806 100644 --- a/shell/browser/api/atom_api_screen.cc +++ b/shell/browser/api/atom_api_screen.cc @@ -52,13 +52,13 @@ std::vector MetricsToArray(uint32_t metrics) { } void DelayEmit(Screen* screen, - const base::StringPiece& name, + base::StringPiece name, const display::Display& display) { screen->Emit(name, display); } void DelayEmitWithMetrics(Screen* screen, - const base::StringPiece& name, + base::StringPiece name, const display::Display& display, const std::vector& metrics) { screen->Emit(name, display, metrics); diff --git a/shell/browser/api/event_emitter.h b/shell/browser/api/event_emitter.h index 3f19452cd75..ac39ee21c8b 100644 --- a/shell/browser/api/event_emitter.h +++ b/shell/browser/api/event_emitter.h @@ -53,7 +53,7 @@ class EventEmitter : public Wrappable { // this.emit(name, event, args...); template - bool EmitCustomEvent(const base::StringPiece& name, + bool EmitCustomEvent(base::StringPiece name, v8::Local event, Args&&... args) { return EmitWithEvent( @@ -63,7 +63,7 @@ class EventEmitter : public Wrappable { // this.emit(name, new Event(flags), args...); template - bool EmitWithFlags(const base::StringPiece& name, int flags, Args&&... args) { + bool EmitWithFlags(base::StringPiece name, int flags, Args&&... args) { return EmitCustomEvent(name, internal::CreateEventFromFlags(isolate(), flags), std::forward(args)...); @@ -71,7 +71,7 @@ class EventEmitter : public Wrappable { // this.emit(name, new Event(), args...); template - bool Emit(const base::StringPiece& name, Args&&... args) { + bool Emit(base::StringPiece name, Args&&... args) { return EmitWithSender(name, nullptr, base::nullopt, std::forward(args)...); } @@ -79,7 +79,7 @@ class EventEmitter : public Wrappable { // this.emit(name, new Event(sender, message), args...); template bool EmitWithSender( - const base::StringPiece& name, + base::StringPiece name, content::RenderFrameHost* sender, base::Optional callback, @@ -101,7 +101,7 @@ class EventEmitter : public Wrappable { private: // this.emit(name, event, args...); template - bool EmitWithEvent(const base::StringPiece& name, + bool EmitWithEvent(base::StringPiece name, v8::Local event, Args&&... args) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); diff --git a/shell/browser/web_contents_preferences.cc b/shell/browser/web_contents_preferences.cc index 40e85650581..e8489d8e420 100644 --- a/shell/browser/web_contents_preferences.cc +++ b/shell/browser/web_contents_preferences.cc @@ -33,7 +33,7 @@ namespace { bool GetAsString(const base::Value* val, - const base::StringPiece& path, + base::StringPiece path, std::string* out) { if (val) { auto* found = val->FindKeyOfType(path, base::Value::Type::STRING); @@ -46,7 +46,7 @@ bool GetAsString(const base::Value* val, } bool GetAsString(const base::Value* val, - const base::StringPiece& path, + base::StringPiece path, base::string16* out) { if (val) { auto* found = val->FindKeyOfType(path, base::Value::Type::STRING); @@ -58,9 +58,7 @@ bool GetAsString(const base::Value* val, return false; } -bool GetAsInteger(const base::Value* val, - const base::StringPiece& path, - int* out) { +bool GetAsInteger(const base::Value* val, base::StringPiece path, int* out) { if (val) { auto* found = val->FindKey(path); if (found && found->is_int()) { @@ -74,7 +72,7 @@ bool GetAsInteger(const base::Value* val, } bool GetAsAutoplayPolicy(const base::Value* val, - const base::StringPiece& path, + base::StringPiece path, content::AutoplayPolicy* out) { std::string policy_str; if (GetAsString(val, path, &policy_str)) { @@ -180,9 +178,8 @@ void WebContentsPreferences::SetDefaults() { last_preference_ = preference_.Clone(); } -bool WebContentsPreferences::SetDefaultBoolIfUndefined( - const base::StringPiece& key, - bool val) { +bool WebContentsPreferences::SetDefaultBoolIfUndefined(base::StringPiece key, + bool val) { auto* current_value = preference_.FindKeyOfType(key, base::Value::Type::BOOLEAN); if (current_value) { @@ -193,11 +190,11 @@ bool WebContentsPreferences::SetDefaultBoolIfUndefined( } } -void WebContentsPreferences::SetBool(const base::StringPiece& key, bool value) { +void WebContentsPreferences::SetBool(base::StringPiece key, bool value) { preference_.SetKey(key, base::Value(value)); } -bool WebContentsPreferences::IsEnabled(const base::StringPiece& name, +bool WebContentsPreferences::IsEnabled(base::StringPiece name, bool default_value) const { auto* current_value = preference_.FindKeyOfType(name, base::Value::Type::BOOLEAN); @@ -218,7 +215,7 @@ void WebContentsPreferences::Clear() { static_cast(&preference_)->Clear(); } -bool WebContentsPreferences::GetPreference(const base::StringPiece& name, +bool WebContentsPreferences::GetPreference(base::StringPiece name, std::string* value) const { return GetAsString(&preference_, name, value); } diff --git a/shell/browser/web_contents_preferences.h b/shell/browser/web_contents_preferences.h index 5aad37b7672..45f425d76f7 100644 --- a/shell/browser/web_contents_preferences.h +++ b/shell/browser/web_contents_preferences.h @@ -40,8 +40,7 @@ class WebContentsPreferences void SetDefaults(); // A simple way to know whether a Boolean property is enabled. - bool IsEnabled(const base::StringPiece& name, - bool default_value = false) const; + bool IsEnabled(base::StringPiece name, bool default_value = false) const; // $.extend(|web_preferences|, |new_web_preferences|). void Merge(const base::DictionaryValue& new_web_preferences); @@ -57,7 +56,7 @@ class WebContentsPreferences void Clear(); // Return true if the particular preference value exists. - bool GetPreference(const base::StringPiece& name, std::string* value) const; + bool GetPreference(base::StringPiece name, std::string* value) const; // Whether to enable the remote module bool IsRemoteModuleEnabled() const; @@ -77,10 +76,10 @@ class WebContentsPreferences static content::WebContents* GetWebContentsFromProcessID(int process_id); // Set preference value to given bool if user did not provide value - bool SetDefaultBoolIfUndefined(const base::StringPiece& key, bool val); + bool SetDefaultBoolIfUndefined(base::StringPiece key, bool val); // Set preference value to given bool - void SetBool(const base::StringPiece& key, bool value); + void SetBool(base::StringPiece key, bool value); static std::vector instances_; diff --git a/shell/common/crash_reporter/crash_reporter_crashpad.cc b/shell/common/crash_reporter/crash_reporter_crashpad.cc index 979e6270341..3c82ccf4535 100644 --- a/shell/common/crash_reporter/crash_reporter_crashpad.cc +++ b/shell/common/crash_reporter/crash_reporter_crashpad.cc @@ -34,8 +34,8 @@ void CrashReporterCrashpad::SetUploadToServer(const bool upload_to_server) { } } -void CrashReporterCrashpad::SetCrashKeyValue(const base::StringPiece& key, - const base::StringPiece& value) { +void CrashReporterCrashpad::SetCrashKeyValue(base::StringPiece key, + base::StringPiece value) { simple_string_dictionary_->SetKeyValue(key.data(), value.data()); } diff --git a/shell/common/crash_reporter/crash_reporter_crashpad.h b/shell/common/crash_reporter/crash_reporter_crashpad.h index bc51fe2cf65..d54d990eb6d 100644 --- a/shell/common/crash_reporter/crash_reporter_crashpad.h +++ b/shell/common/crash_reporter/crash_reporter_crashpad.h @@ -32,8 +32,7 @@ class CrashReporterCrashpad : public CrashReporter { ~CrashReporterCrashpad() override; void SetUploadsEnabled(bool enable_uploads); - void SetCrashKeyValue(const base::StringPiece& key, - const base::StringPiece& value); + void SetCrashKeyValue(base::StringPiece key, base::StringPiece value); void SetInitialCrashKeyValues(); std::vector GetUploadedReports( diff --git a/shell/common/gin_util.h b/shell/common/gin_util.h index 875fc5b5925..3cb286444a4 100644 --- a/shell/common/gin_util.h +++ b/shell/common/gin_util.h @@ -12,7 +12,7 @@ namespace gin_util { template bool SetMethod(v8::Local recv, - const base::StringPiece& key, + base::StringPiece key, const T& callback) { v8::Isolate* isolate = v8::Isolate::GetCurrent(); auto context = isolate->GetCurrentContext();