Skip to content
Permalink
Browse files

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."
  • Loading branch information...
ckerr committed Jul 25, 2019
1 parent 539078f commit f6fb877de915f93d352c2eb732a3998c1425ee1d
@@ -138,9 +138,8 @@ v8::Local<v8::Value> Converter<const char*>::ToV8(v8::Isolate* isolate,
.ToLocalChecked();
}

v8::Local<v8::Value> Converter<base::StringPiece>::ToV8(
v8::Isolate* isolate,
const base::StringPiece& val) {
v8::Local<v8::Value> Converter<base::StringPiece>::ToV8(v8::Isolate* isolate,
base::StringPiece val) {
return v8::String::NewFromUtf8(isolate, val.data(),
v8::NewStringType::kNormal,
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,
const base::StringPiece& val) {
base::StringPiece val) {
return v8::String::NewFromUtf8(isolate, val.data(),
v8::NewStringType::kInternalized,
static_cast<uint32_t>(val.length()))
@@ -123,8 +123,7 @@ struct Converter<const char*> {

template <>
struct Converter<base::StringPiece> {
static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,
const base::StringPiece& val);
static v8::Local<v8::Value> ToV8(v8::Isolate* isolate, base::StringPiece val);
// 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,
const base::StringPiece& input);
base::StringPiece input);

template <>
struct Converter<v8::Local<v8::Function>> {
@@ -343,7 +342,7 @@ bool ConvertFromV8(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>();
}

@@ -41,7 +41,7 @@ class Dictionary {
static Dictionary CreateEmpty(v8::Isolate* isolate);

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
// returns true when T == v8::Local<v8::Value>.
v8::Local<v8::Context> context = isolate_->GetCurrentContext();
@@ -56,7 +56,7 @@ class Dictionary {
}

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::Private> privateKey =
v8::Private::ForApi(isolate_, StringToV8(isolate_, key));
@@ -69,7 +69,7 @@ class Dictionary {
}

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;
if (!TryConvertToV8(isolate_, val, &v8_value))
return false;
@@ -79,7 +79,7 @@ class Dictionary {
}

template <typename T>
bool SetHidden(const base::StringPiece& key, T val) {
bool SetHidden(base::StringPiece key, T val) {
v8::Local<v8::Value> v8_value;
if (!TryConvertToV8(isolate_, val, &v8_value))
return false;
@@ -92,7 +92,7 @@ class Dictionary {
}

template <typename T>
bool SetReadOnly(const base::StringPiece& key, T val) {
bool SetReadOnly(base::StringPiece key, T val) {
v8::Local<v8::Value> v8_value;
if (!TryConvertToV8(isolate_, val, &v8_value))
return false;
@@ -103,7 +103,7 @@ class Dictionary {
}

template <typename T>
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<T>::CreateTemplate(isolate_, callback)
@@ -112,7 +112,7 @@ class Dictionary {
.ToChecked();
}

bool Delete(const base::StringPiece& key) {
bool Delete(base::StringPiece key) {
v8::Maybe<bool> result = GetHandle()->Delete(isolate_->GetCurrentContext(),
StringToV8(isolate_, key));
return !result.IsNothing() && result.FromJust();
@@ -13,15 +13,14 @@ ObjectTemplateBuilder::ObjectTemplateBuilder(

ObjectTemplateBuilder::~ObjectTemplateBuilder() {}

ObjectTemplateBuilder& ObjectTemplateBuilder::SetImpl(
const base::StringPiece& name,
v8::Local<v8::Data> val) {
ObjectTemplateBuilder& ObjectTemplateBuilder::SetImpl(base::StringPiece name,
v8::Local<v8::Data> val) {
template_->Set(StringToSymbol(isolate_, name), val);
return *this;
}

ObjectTemplateBuilder& ObjectTemplateBuilder::SetPropertyImpl(
const base::StringPiece& name,
base::StringPiece name,
v8::Local<v8::FunctionTemplate> getter,
v8::Local<v8::FunctionTemplate> setter) {
template_->SetAccessorProperty(StringToSymbol(isolate_, name), getter,
@@ -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 <typename T>
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 <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));
}
template <typename T>
ObjectTemplateBuilder& SetProperty(const base::StringPiece& name, T getter) {
ObjectTemplateBuilder& SetProperty(base::StringPiece name, T getter) {
return SetPropertyImpl(name,
CallbackTraits<T>::CreateTemplate(isolate_, getter),
v8::Local<v8::FunctionTemplate>());
}
template <typename T, typename U>
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<v8::ObjectTemplate> Build();

private:
ObjectTemplateBuilder& SetImpl(const base::StringPiece& name,
ObjectTemplateBuilder& SetImpl(base::StringPiece name,
v8::Local<v8::Data> val);
ObjectTemplateBuilder& SetPropertyImpl(
const base::StringPiece& name,
base::StringPiece name,
v8::Local<v8::FunctionTemplate> getter,
v8::Local<v8::FunctionTemplate> setter);

@@ -52,13 +52,13 @@ std::vector<std::string> 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<std::string>& metrics) {
screen->Emit(name, display, metrics);
@@ -53,7 +53,7 @@ class EventEmitter : public Wrappable<T> {

// this.emit(name, event, args...);
template <typename... Args>
bool EmitCustomEvent(const base::StringPiece& name,
bool EmitCustomEvent(base::StringPiece name,
v8::Local<v8::Object> event,
Args&&... args) {
return EmitWithEvent(
@@ -63,23 +63,23 @@ class EventEmitter : public Wrappable<T> {

// this.emit(name, new Event(flags), 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,
internal::CreateEventFromFlags(isolate(), flags),
std::forward<Args>(args)...);
}

// this.emit(name, new Event(), 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,
std::forward<Args>(args)...);
}

// this.emit(name, new Event(sender, message), args...);
template <typename... Args>
bool EmitWithSender(
const base::StringPiece& name,
base::StringPiece name,
content::RenderFrameHost* sender,
base::Optional<electron::mojom::ElectronBrowser::MessageSyncCallback>
callback,
@@ -101,7 +101,7 @@ class EventEmitter : public Wrappable<T> {
private:
// this.emit(name, event, args...);
template <typename... Args>
bool EmitWithEvent(const base::StringPiece& name,
bool EmitWithEvent(base::StringPiece name,
v8::Local<v8::Object> event,
Args&&... args) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
@@ -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<base::DictionaryValue*>(&preference_)->Clear();
}

bool WebContentsPreferences::GetPreference(const base::StringPiece& name,
bool WebContentsPreferences::GetPreference(base::StringPiece name,
std::string* value) const {
return GetAsString(&preference_, name, value);
}
@@ -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<WebContentsPreferences*> instances_;

@@ -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());
}

@@ -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<UploadReportResult> GetUploadedReports(
@@ -12,7 +12,7 @@ namespace gin_util {

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

0 comments on commit f6fb877

Please sign in to comment.
You can’t perform that action at this time.