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."
This commit is contained in:
Charles Kerr 2019-07-25 10:19:04 -05:00 committed by GitHub
parent 539078f281
commit f6fb877de9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 46 additions and 54 deletions

View file

@ -138,9 +138,8 @@ v8::Local<v8::Value> Converter<const char*>::ToV8(v8::Isolate* isolate,
.ToLocalChecked(); .ToLocalChecked();
} }
v8::Local<v8::Value> Converter<base::StringPiece>::ToV8( v8::Local<v8::Value> Converter<base::StringPiece>::ToV8(v8::Isolate* isolate,
v8::Isolate* isolate, base::StringPiece val) {
const base::StringPiece& val) {
return v8::String::NewFromUtf8(isolate, val.data(), return v8::String::NewFromUtf8(isolate, val.data(),
v8::NewStringType::kNormal, v8::NewStringType::kNormal,
static_cast<uint32_t>(val.length())) static_cast<uint32_t>(val.length()))
@ -260,7 +259,7 @@ bool Converter<v8::Local<v8::Value>>::FromV8(v8::Isolate* isolate,
} }
v8::Local<v8::String> StringToSymbol(v8::Isolate* isolate, v8::Local<v8::String> StringToSymbol(v8::Isolate* isolate,
const base::StringPiece& val) { base::StringPiece val) {
return v8::String::NewFromUtf8(isolate, val.data(), return v8::String::NewFromUtf8(isolate, val.data(),
v8::NewStringType::kInternalized, v8::NewStringType::kInternalized,
static_cast<uint32_t>(val.length())) static_cast<uint32_t>(val.length()))

View file

@ -123,8 +123,7 @@ struct Converter<const char*> {
template <> template <>
struct Converter<base::StringPiece> { struct Converter<base::StringPiece> {
static v8::Local<v8::Value> ToV8(v8::Isolate* isolate, static v8::Local<v8::Value> ToV8(v8::Isolate* isolate, base::StringPiece val);
const base::StringPiece& val);
// No conversion out is possible because StringPiece does not contain storage. // No conversion out is possible because StringPiece does not contain storage.
}; };
@ -138,7 +137,7 @@ struct Converter<std::string> {
}; };
v8::Local<v8::String> StringToSymbol(v8::Isolate* isolate, v8::Local<v8::String> StringToSymbol(v8::Isolate* isolate,
const base::StringPiece& input); base::StringPiece input);
template <> template <>
struct Converter<v8::Local<v8::Function>> { struct Converter<v8::Local<v8::Function>> {
@ -343,7 +342,7 @@ bool ConvertFromV8(v8::Isolate* isolate,
} }
inline v8::Local<v8::String> StringToV8(v8::Isolate* isolate, inline v8::Local<v8::String> StringToV8(v8::Isolate* isolate,
const base::StringPiece& input) { base::StringPiece input) {
return ConvertToV8(isolate, input).As<v8::String>(); return ConvertToV8(isolate, input).As<v8::String>();
} }

View file

@ -41,7 +41,7 @@ class Dictionary {
static Dictionary CreateEmpty(v8::Isolate* isolate); static Dictionary CreateEmpty(v8::Isolate* isolate);
template <typename T> template <typename T>
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 // Check for existence before getting, otherwise this method will always
// returns true when T == v8::Local<v8::Value>. // returns true when T == v8::Local<v8::Value>.
v8::Local<v8::Context> context = isolate_->GetCurrentContext(); v8::Local<v8::Context> context = isolate_->GetCurrentContext();
@ -56,7 +56,7 @@ class Dictionary {
} }
template <typename T> template <typename T>
bool GetHidden(const base::StringPiece& key, T* out) const { bool GetHidden(base::StringPiece key, T* out) const {
v8::Local<v8::Context> context = isolate_->GetCurrentContext(); v8::Local<v8::Context> context = isolate_->GetCurrentContext();
v8::Local<v8::Private> privateKey = v8::Local<v8::Private> privateKey =
v8::Private::ForApi(isolate_, StringToV8(isolate_, key)); v8::Private::ForApi(isolate_, StringToV8(isolate_, key));
@ -69,7 +69,7 @@ class Dictionary {
} }
template <typename T> template <typename T>
bool Set(const base::StringPiece& key, const T& val) { bool Set(base::StringPiece key, const T& val) {
v8::Local<v8::Value> v8_value; v8::Local<v8::Value> v8_value;
if (!TryConvertToV8(isolate_, val, &v8_value)) if (!TryConvertToV8(isolate_, val, &v8_value))
return false; return false;
@ -79,7 +79,7 @@ class Dictionary {
} }
template <typename T> template <typename T>
bool SetHidden(const base::StringPiece& key, T val) { bool SetHidden(base::StringPiece key, T val) {
v8::Local<v8::Value> v8_value; v8::Local<v8::Value> v8_value;
if (!TryConvertToV8(isolate_, val, &v8_value)) if (!TryConvertToV8(isolate_, val, &v8_value))
return false; return false;
@ -92,7 +92,7 @@ class Dictionary {
} }
template <typename T> template <typename T>
bool SetReadOnly(const base::StringPiece& key, T val) { bool SetReadOnly(base::StringPiece key, T val) {
v8::Local<v8::Value> v8_value; v8::Local<v8::Value> v8_value;
if (!TryConvertToV8(isolate_, val, &v8_value)) if (!TryConvertToV8(isolate_, val, &v8_value))
return false; return false;
@ -103,7 +103,7 @@ class Dictionary {
} }
template <typename T> template <typename T>
bool SetMethod(const base::StringPiece& key, const T& callback) { bool SetMethod(base::StringPiece key, const T& callback) {
return GetHandle() return GetHandle()
->Set(isolate_->GetCurrentContext(), StringToV8(isolate_, key), ->Set(isolate_->GetCurrentContext(), StringToV8(isolate_, key),
CallbackTraits<T>::CreateTemplate(isolate_, callback) CallbackTraits<T>::CreateTemplate(isolate_, callback)
@ -112,7 +112,7 @@ class Dictionary {
.ToChecked(); .ToChecked();
} }
bool Delete(const base::StringPiece& key) { bool Delete(base::StringPiece key) {
v8::Maybe<bool> result = GetHandle()->Delete(isolate_->GetCurrentContext(), v8::Maybe<bool> result = GetHandle()->Delete(isolate_->GetCurrentContext(),
StringToV8(isolate_, key)); StringToV8(isolate_, key));
return !result.IsNothing() && result.FromJust(); return !result.IsNothing() && result.FromJust();

View file

@ -13,15 +13,14 @@ ObjectTemplateBuilder::ObjectTemplateBuilder(
ObjectTemplateBuilder::~ObjectTemplateBuilder() {} ObjectTemplateBuilder::~ObjectTemplateBuilder() {}
ObjectTemplateBuilder& ObjectTemplateBuilder::SetImpl( ObjectTemplateBuilder& ObjectTemplateBuilder::SetImpl(base::StringPiece name,
const base::StringPiece& name,
v8::Local<v8::Data> val) { v8::Local<v8::Data> val) {
template_->Set(StringToSymbol(isolate_, name), val); template_->Set(StringToSymbol(isolate_, name), val);
return *this; return *this;
} }
ObjectTemplateBuilder& ObjectTemplateBuilder::SetPropertyImpl( ObjectTemplateBuilder& ObjectTemplateBuilder::SetPropertyImpl(
const base::StringPiece& name, base::StringPiece name,
v8::Local<v8::FunctionTemplate> getter, v8::Local<v8::FunctionTemplate> getter,
v8::Local<v8::FunctionTemplate> setter) { v8::Local<v8::FunctionTemplate> setter) {
template_->SetAccessorProperty(StringToSymbol(isolate_, name), getter, template_->SetAccessorProperty(StringToSymbol(isolate_, name), getter,

View file

@ -72,7 +72,7 @@ class ObjectTemplateBuilder {
// poetic license here in order that all calls to Set() can be via the '.' // poetic license here in order that all calls to Set() can be via the '.'
// operator and line up nicely. // operator and line up nicely.
template <typename T> template <typename T>
ObjectTemplateBuilder& SetValue(const base::StringPiece& name, T val) { ObjectTemplateBuilder& SetValue(base::StringPiece name, T val) {
return SetImpl(name, ConvertToV8(isolate_, val)); return SetImpl(name, ConvertToV8(isolate_, val));
} }
@ -81,17 +81,17 @@ class ObjectTemplateBuilder {
// use one of the first two options. Also see mate::CreateFunctionTemplate() // use one of the first two options. Also see mate::CreateFunctionTemplate()
// for creating raw function templates. // for creating raw function templates.
template <typename T> template <typename T>
ObjectTemplateBuilder& SetMethod(const base::StringPiece& name, T callback) { ObjectTemplateBuilder& SetMethod(base::StringPiece name, T callback) {
return SetImpl(name, CallbackTraits<T>::CreateTemplate(isolate_, callback)); return SetImpl(name, CallbackTraits<T>::CreateTemplate(isolate_, callback));
} }
template <typename T> template <typename T>
ObjectTemplateBuilder& SetProperty(const base::StringPiece& name, T getter) { ObjectTemplateBuilder& SetProperty(base::StringPiece name, T getter) {
return SetPropertyImpl(name, return SetPropertyImpl(name,
CallbackTraits<T>::CreateTemplate(isolate_, getter), CallbackTraits<T>::CreateTemplate(isolate_, getter),
v8::Local<v8::FunctionTemplate>()); v8::Local<v8::FunctionTemplate>());
} }
template <typename T, typename U> template <typename T, typename U>
ObjectTemplateBuilder& SetProperty(const base::StringPiece& name, ObjectTemplateBuilder& SetProperty(base::StringPiece name,
T getter, T getter,
U setter) { U setter) {
return SetPropertyImpl(name, return SetPropertyImpl(name,
@ -105,10 +105,10 @@ class ObjectTemplateBuilder {
v8::Local<v8::ObjectTemplate> Build(); v8::Local<v8::ObjectTemplate> Build();
private: private:
ObjectTemplateBuilder& SetImpl(const base::StringPiece& name, ObjectTemplateBuilder& SetImpl(base::StringPiece name,
v8::Local<v8::Data> val); v8::Local<v8::Data> val);
ObjectTemplateBuilder& SetPropertyImpl( ObjectTemplateBuilder& SetPropertyImpl(
const base::StringPiece& name, base::StringPiece name,
v8::Local<v8::FunctionTemplate> getter, v8::Local<v8::FunctionTemplate> getter,
v8::Local<v8::FunctionTemplate> setter); v8::Local<v8::FunctionTemplate> setter);

View file

@ -52,13 +52,13 @@ std::vector<std::string> MetricsToArray(uint32_t metrics) {
} }
void DelayEmit(Screen* screen, void DelayEmit(Screen* screen,
const base::StringPiece& name, base::StringPiece name,
const display::Display& display) { const display::Display& display) {
screen->Emit(name, display); screen->Emit(name, display);
} }
void DelayEmitWithMetrics(Screen* screen, void DelayEmitWithMetrics(Screen* screen,
const base::StringPiece& name, base::StringPiece name,
const display::Display& display, const display::Display& display,
const std::vector<std::string>& metrics) { const std::vector<std::string>& metrics) {
screen->Emit(name, display, metrics); screen->Emit(name, display, metrics);

View file

@ -53,7 +53,7 @@ class EventEmitter : public Wrappable<T> {
// this.emit(name, event, args...); // this.emit(name, event, args...);
template <typename... Args> template <typename... Args>
bool EmitCustomEvent(const base::StringPiece& name, bool EmitCustomEvent(base::StringPiece name,
v8::Local<v8::Object> event, v8::Local<v8::Object> event,
Args&&... args) { Args&&... args) {
return EmitWithEvent( return EmitWithEvent(
@ -63,7 +63,7 @@ class EventEmitter : public Wrappable<T> {
// this.emit(name, new Event(flags), args...); // this.emit(name, new Event(flags), args...);
template <typename... Args> template <typename... Args>
bool EmitWithFlags(const base::StringPiece& name, int flags, Args&&... args) { bool EmitWithFlags(base::StringPiece name, int flags, Args&&... args) {
return EmitCustomEvent(name, return EmitCustomEvent(name,
internal::CreateEventFromFlags(isolate(), flags), internal::CreateEventFromFlags(isolate(), flags),
std::forward<Args>(args)...); std::forward<Args>(args)...);
@ -71,7 +71,7 @@ class EventEmitter : public Wrappable<T> {
// this.emit(name, new Event(), args...); // this.emit(name, new Event(), args...);
template <typename... Args> template <typename... Args>
bool Emit(const base::StringPiece& name, Args&&... args) { bool Emit(base::StringPiece name, Args&&... args) {
return EmitWithSender(name, nullptr, base::nullopt, return EmitWithSender(name, nullptr, base::nullopt,
std::forward<Args>(args)...); std::forward<Args>(args)...);
} }
@ -79,7 +79,7 @@ class EventEmitter : public Wrappable<T> {
// this.emit(name, new Event(sender, message), args...); // this.emit(name, new Event(sender, message), args...);
template <typename... Args> template <typename... Args>
bool EmitWithSender( bool EmitWithSender(
const base::StringPiece& name, base::StringPiece name,
content::RenderFrameHost* sender, content::RenderFrameHost* sender,
base::Optional<electron::mojom::ElectronBrowser::MessageSyncCallback> base::Optional<electron::mojom::ElectronBrowser::MessageSyncCallback>
callback, callback,
@ -101,7 +101,7 @@ class EventEmitter : public Wrappable<T> {
private: private:
// this.emit(name, event, args...); // this.emit(name, event, args...);
template <typename... Args> template <typename... Args>
bool EmitWithEvent(const base::StringPiece& name, bool EmitWithEvent(base::StringPiece name,
v8::Local<v8::Object> event, v8::Local<v8::Object> event,
Args&&... args) { Args&&... args) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK_CURRENTLY_ON(content::BrowserThread::UI);

View file

@ -33,7 +33,7 @@
namespace { namespace {
bool GetAsString(const base::Value* val, bool GetAsString(const base::Value* val,
const base::StringPiece& path, base::StringPiece path,
std::string* out) { std::string* out) {
if (val) { if (val) {
auto* found = val->FindKeyOfType(path, base::Value::Type::STRING); 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, bool GetAsString(const base::Value* val,
const base::StringPiece& path, base::StringPiece path,
base::string16* out) { base::string16* out) {
if (val) { if (val) {
auto* found = val->FindKeyOfType(path, base::Value::Type::STRING); auto* found = val->FindKeyOfType(path, base::Value::Type::STRING);
@ -58,9 +58,7 @@ bool GetAsString(const base::Value* val,
return false; return false;
} }
bool GetAsInteger(const base::Value* val, bool GetAsInteger(const base::Value* val, base::StringPiece path, int* out) {
const base::StringPiece& path,
int* out) {
if (val) { if (val) {
auto* found = val->FindKey(path); auto* found = val->FindKey(path);
if (found && found->is_int()) { if (found && found->is_int()) {
@ -74,7 +72,7 @@ bool GetAsInteger(const base::Value* val,
} }
bool GetAsAutoplayPolicy(const base::Value* val, bool GetAsAutoplayPolicy(const base::Value* val,
const base::StringPiece& path, base::StringPiece path,
content::AutoplayPolicy* out) { content::AutoplayPolicy* out) {
std::string policy_str; std::string policy_str;
if (GetAsString(val, path, &policy_str)) { if (GetAsString(val, path, &policy_str)) {
@ -180,8 +178,7 @@ void WebContentsPreferences::SetDefaults() {
last_preference_ = preference_.Clone(); last_preference_ = preference_.Clone();
} }
bool WebContentsPreferences::SetDefaultBoolIfUndefined( bool WebContentsPreferences::SetDefaultBoolIfUndefined(base::StringPiece key,
const base::StringPiece& key,
bool val) { bool val) {
auto* current_value = auto* current_value =
preference_.FindKeyOfType(key, base::Value::Type::BOOLEAN); preference_.FindKeyOfType(key, base::Value::Type::BOOLEAN);
@ -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)); preference_.SetKey(key, base::Value(value));
} }
bool WebContentsPreferences::IsEnabled(const base::StringPiece& name, bool WebContentsPreferences::IsEnabled(base::StringPiece name,
bool default_value) const { bool default_value) const {
auto* current_value = auto* current_value =
preference_.FindKeyOfType(name, base::Value::Type::BOOLEAN); preference_.FindKeyOfType(name, base::Value::Type::BOOLEAN);
@ -218,7 +215,7 @@ void WebContentsPreferences::Clear() {
static_cast<base::DictionaryValue*>(&preference_)->Clear(); static_cast<base::DictionaryValue*>(&preference_)->Clear();
} }
bool WebContentsPreferences::GetPreference(const base::StringPiece& name, bool WebContentsPreferences::GetPreference(base::StringPiece name,
std::string* value) const { std::string* value) const {
return GetAsString(&preference_, name, value); return GetAsString(&preference_, name, value);
} }

View file

@ -40,8 +40,7 @@ class WebContentsPreferences
void SetDefaults(); void SetDefaults();
// A simple way to know whether a Boolean property is enabled. // A simple way to know whether a Boolean property is enabled.
bool IsEnabled(const base::StringPiece& name, bool IsEnabled(base::StringPiece name, bool default_value = false) const;
bool default_value = false) const;
// $.extend(|web_preferences|, |new_web_preferences|). // $.extend(|web_preferences|, |new_web_preferences|).
void Merge(const base::DictionaryValue& new_web_preferences); void Merge(const base::DictionaryValue& new_web_preferences);
@ -57,7 +56,7 @@ class WebContentsPreferences
void Clear(); void Clear();
// Return true if the particular preference value exists. // 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 // Whether to enable the remote module
bool IsRemoteModuleEnabled() const; bool IsRemoteModuleEnabled() const;
@ -77,10 +76,10 @@ class WebContentsPreferences
static content::WebContents* GetWebContentsFromProcessID(int process_id); static content::WebContents* GetWebContentsFromProcessID(int process_id);
// Set preference value to given bool if user did not provide value // 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 // Set preference value to given bool
void SetBool(const base::StringPiece& key, bool value); void SetBool(base::StringPiece key, bool value);
static std::vector<WebContentsPreferences*> instances_; static std::vector<WebContentsPreferences*> instances_;

View file

@ -34,8 +34,8 @@ void CrashReporterCrashpad::SetUploadToServer(const bool upload_to_server) {
} }
} }
void CrashReporterCrashpad::SetCrashKeyValue(const base::StringPiece& key, void CrashReporterCrashpad::SetCrashKeyValue(base::StringPiece key,
const base::StringPiece& value) { base::StringPiece value) {
simple_string_dictionary_->SetKeyValue(key.data(), value.data()); simple_string_dictionary_->SetKeyValue(key.data(), value.data());
} }

View file

@ -32,8 +32,7 @@ class CrashReporterCrashpad : public CrashReporter {
~CrashReporterCrashpad() override; ~CrashReporterCrashpad() override;
void SetUploadsEnabled(bool enable_uploads); void SetUploadsEnabled(bool enable_uploads);
void SetCrashKeyValue(const base::StringPiece& key, void SetCrashKeyValue(base::StringPiece key, base::StringPiece value);
const base::StringPiece& value);
void SetInitialCrashKeyValues(); void SetInitialCrashKeyValues();
std::vector<UploadReportResult> GetUploadedReports( std::vector<UploadReportResult> GetUploadedReports(

View file

@ -12,7 +12,7 @@ namespace gin_util {
template <typename T> template <typename T>
bool SetMethod(v8::Local<v8::Object> recv, bool SetMethod(v8::Local<v8::Object> recv,
const base::StringPiece& key, base::StringPiece key,
const T& callback) { const T& callback) {
v8::Isolate* isolate = v8::Isolate::GetCurrent(); v8::Isolate* isolate = v8::Isolate::GetCurrent();
auto context = isolate->GetCurrentContext(); auto context = isolate->GetCurrentContext();