Commit b17e66da authored by Devlin Cronin's avatar Devlin Cronin Committed by Commit Bot

[Extensions Cleanup] Remove v8_helpers::SetProperty()

v8_helpers::SetProperty() just wraps Object::DefineOwnProperty().
However, with it, we lose the WARN_UNUSED_RESULT on the method, which
can lead to dangerous assumptions or not handling edge cases. The method
also doesn't really save us anything (the call to DefineOwnProperty is
pretty concise).

Remove v8_helpers::SetProperty(), and just use DefineOwnProperty
directly.

Bug: None
Change-Id: I261ef1dc4f59f9541067f9fb87a115e49b32dc8d
Reviewed-on: https://chromium-review.googlesource.com/896372
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: default avatarJeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533763}
parent 7a393164
...@@ -468,19 +468,22 @@ void ModuleSystem::LazyFieldGetterInner( ...@@ -468,19 +468,22 @@ void ModuleSystem::LazyFieldGetterInner(
v8::Local<v8::Value> val = info.This(); v8::Local<v8::Value> val = info.This();
if (val->IsObject()) { if (val->IsObject()) {
v8::Local<v8::Object> object = v8::Local<v8::Object>::Cast(val); v8::Local<v8::Object> object = v8::Local<v8::Object>::Cast(val);
auto maybe = object->Delete(context, property); auto maybe_deleted = object->Delete(context, property);
if (!maybe.IsJust()) { if (!maybe_deleted.IsJust()) {
// In theory, deletion should never result in throwing an error. But // In theory, deletion should never result in throwing an error. But
// crazier things have happened. // crazier things have happened.
NOTREACHED(); NOTREACHED();
return; return;
} }
if (!maybe.FromJust()) { if (!maybe_deleted.FromJust()) {
// Deletion can *fail* in certain cases, such as when the script does // Deletion can *fail* in certain cases, such as when the script does
// Object.freeze(chrome). // Object.freeze(chrome).
return; return;
} }
SetProperty(context, object, property, new_field); auto maybe_set = object->CreateDataProperty(context, property, new_field);
// Setting a new value can fail in multiple scenarios. Bail out if it does.
if (!maybe_set.IsJust() || !maybe_set.FromJust())
return;
} else { } else {
NOTREACHED(); NOTREACHED();
} }
......
...@@ -53,18 +53,6 @@ inline bool IsTrue(v8::Maybe<bool> maybe) { ...@@ -53,18 +53,6 @@ inline bool IsTrue(v8::Maybe<bool> maybe) {
return maybe.IsJust() && maybe.FromJust(); return maybe.IsJust() && maybe.FromJust();
} }
// SetProperty() family wraps V8::Object::DefineOwnProperty().
// Returns true on success.
// NOTE: Think about whether you want this or SetPrivateProperty() below.
// TODO(devlin): Sort through more of the callers of this and see if we can
// convert more to be private.
inline bool SetProperty(v8::Local<v8::Context> context,
v8::Local<v8::Object> object,
v8::Local<v8::String> key,
v8::Local<v8::Value> value) {
return IsTrue(object->DefineOwnProperty(context, key, value));
}
// Wraps v8::Object::SetPrivate(). When possible, prefer this to SetProperty(). // Wraps v8::Object::SetPrivate(). When possible, prefer this to SetProperty().
inline bool SetPrivateProperty(v8::Local<v8::Context> context, inline bool SetPrivateProperty(v8::Local<v8::Context> context,
v8::Local<v8::Object> object, v8::Local<v8::Object> object,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment