Skip to content

Commit

Permalink
IDL: Fix wrong usages of [TreatNullAs]
Browse files Browse the repository at this point in the history
[TreatNullAs] is an extended attribute which is applicable to
DOMString type.
This CL fixes following wrong use cases of it in Blink,
but does not change renderer behaviors.

TestDictionary: applied to ByteString
CSSStyleDeclaration: applied to attribute, spec conformance
Window: applied to an union type, and spec does not have it on |url| parameter.


Bug: 819112
Change-Id: I8769248ee48f7c1fad1562ac5c017a49e79cbb1f
Reviewed-on: https://chromium-review.googlesource.com/c/1298813
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Commit-Queue: Hitoshi Yoshida <peria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602629}
  • Loading branch information
peria authored and Commit Bot committed Oct 25, 2018
1 parent 9972f9d commit fdd9924
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ dictionary TestDictionary {
long longMember = 1;
DOMString stringMember;
TestInterface testInterfaceMember;
[TreatNullAs=EmptyString] ByteString byteStringMember;
[TreatNullAs=EmptyString] DOMString domStringTreatNullAsEmptyStringMember;
USVString? usvStringOrNullMember = null;
double? doubleOrNullMember = null;
double restrictedDoubleMember = 3.14;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,6 @@ class CORE_EXPORT TestDictionary : public IDLDictionaryBase {
}
inline void setBooleanMember(bool);

bool hasByteStringMember() const { return !byte_string_member_.IsNull(); }
const String& byteStringMember() const {
return byte_string_member_;
}
inline void setByteStringMember(const String&);

bool hasCallbackFunctionMember() const { return callback_function_member_; }
V8VoidCallbackFunction* callbackFunctionMember() const {
return callback_function_member_;
Expand All @@ -106,6 +100,12 @@ class CORE_EXPORT TestDictionary : public IDLDictionaryBase {
}
void setDictionaryMember(Dictionary);

bool hasDomStringTreatNullAsEmptyStringMember() const { return !dom_string_treat_null_as_empty_string_member_.IsNull(); }
const String& domStringTreatNullAsEmptyStringMember() const {
return dom_string_treat_null_as_empty_string_member_;
}
inline void setDomStringTreatNullAsEmptyStringMember(const String&);

bool hasDoubleOrNullMember() const { return has_double_or_null_member_; }
double doubleOrNullMember() const {
DCHECK(has_double_or_null_member_);
Expand Down Expand Up @@ -485,10 +485,10 @@ class CORE_EXPORT TestDictionary : public IDLDictionaryBase {
int32_t applicable_to_type_long_member_;
String applicable_to_type_string_member_;
bool boolean_member_;
String byte_string_member_;
TraceWrapperMember<V8VoidCallbackFunction> callback_function_member_;
bool create_member_;
Dictionary dictionary_member_;
String dom_string_treat_null_as_empty_string_member_;
double double_or_null_member_;
DoubleOrDoubleOrNullSequence double_or_null_or_double_or_null_sequence_member_;
Vector<std::pair<String, base::Optional<double>>> double_or_null_record_member_;
Expand Down Expand Up @@ -557,15 +557,15 @@ void TestDictionary::setBooleanMember(bool value) {
has_boolean_member_ = true;
}

void TestDictionary::setByteStringMember(const String& value) {
byte_string_member_ = value;
}

void TestDictionary::setCreateMember(bool value) {
create_member_ = value;
has_create_member_ = true;
}

void TestDictionary::setDomStringTreatNullAsEmptyStringMember(const String& value) {
dom_string_treat_null_as_empty_string_member_ = value;
}

void TestDictionary::setDoubleOrNullMember(double value) {
double_or_null_member_ = value;
has_double_or_null_member_ = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@ static const v8::Eternal<v8::Name>* eternalV8TestDictionaryKeys(v8::Isolate* iso
"applicableToTypeLongMember",
"applicableToTypeStringMember",
"booleanMember",
"byteStringMember",
"callbackFunctionMember",
"create",
"deprecatedCreateMember",
"dictionaryMember",
"domStringTreatNullAsEmptyStringMember",
"doubleOrNullMember",
"doubleOrNullOrDoubleOrNullSequenceMember",
"doubleOrNullRecordMember",
Expand Down Expand Up @@ -185,22 +185,8 @@ void V8TestDictionary::ToImpl(v8::Isolate* isolate, v8::Local<v8::Value> v8Value
impl.setBooleanMember(boolean_member_cpp_value);
}

v8::Local<v8::Value> byte_string_member_value;
if (!v8Object->Get(context, keys[5].Get(isolate)).ToLocal(&byte_string_member_value)) {
exceptionState.RethrowV8Exception(block.Exception());
return;
}
if (byte_string_member_value.IsEmpty() || byte_string_member_value->IsUndefined()) {
// Do nothing.
} else {
V8StringResource<kTreatNullAsEmptyString> byte_string_member_cpp_value = NativeValueTraits<IDLByteStringBase<kTreatNullAsEmptyString>>::NativeValue(isolate, byte_string_member_value, exceptionState);
if (exceptionState.HadException())
return;
impl.setByteStringMember(byte_string_member_cpp_value);
}

v8::Local<v8::Value> callback_function_member_value;
if (!v8Object->Get(context, keys[6].Get(isolate)).ToLocal(&callback_function_member_value)) {
if (!v8Object->Get(context, keys[5].Get(isolate)).ToLocal(&callback_function_member_value)) {
exceptionState.RethrowV8Exception(block.Exception());
return;
}
Expand All @@ -212,7 +198,7 @@ void V8TestDictionary::ToImpl(v8::Isolate* isolate, v8::Local<v8::Value> v8Value
}

v8::Local<v8::Value> create_value;
if (!v8Object->Get(context, keys[7].Get(isolate)).ToLocal(&create_value)) {
if (!v8Object->Get(context, keys[6].Get(isolate)).ToLocal(&create_value)) {
exceptionState.RethrowV8Exception(block.Exception());
return;
}
Expand All @@ -226,7 +212,7 @@ void V8TestDictionary::ToImpl(v8::Isolate* isolate, v8::Local<v8::Value> v8Value
}

v8::Local<v8::Value> deprecated_create_member_value;
if (!v8Object->Get(context, keys[8].Get(isolate)).ToLocal(&deprecated_create_member_value)) {
if (!v8Object->Get(context, keys[7].Get(isolate)).ToLocal(&deprecated_create_member_value)) {
exceptionState.RethrowV8Exception(block.Exception());
return;
}
Expand All @@ -241,7 +227,7 @@ void V8TestDictionary::ToImpl(v8::Isolate* isolate, v8::Local<v8::Value> v8Value
}

v8::Local<v8::Value> dictionary_member_value;
if (!v8Object->Get(context, keys[9].Get(isolate)).ToLocal(&dictionary_member_value)) {
if (!v8Object->Get(context, keys[8].Get(isolate)).ToLocal(&dictionary_member_value)) {
exceptionState.RethrowV8Exception(block.Exception());
return;
}
Expand All @@ -258,6 +244,20 @@ void V8TestDictionary::ToImpl(v8::Isolate* isolate, v8::Local<v8::Value> v8Value
impl.setDictionaryMember(dictionary_member_cpp_value);
}

v8::Local<v8::Value> dom_string_treat_null_as_empty_string_member_value;
if (!v8Object->Get(context, keys[9].Get(isolate)).ToLocal(&dom_string_treat_null_as_empty_string_member_value)) {
exceptionState.RethrowV8Exception(block.Exception());
return;
}
if (dom_string_treat_null_as_empty_string_member_value.IsEmpty() || dom_string_treat_null_as_empty_string_member_value->IsUndefined()) {
// Do nothing.
} else {
V8StringResource<kTreatNullAsEmptyString> dom_string_treat_null_as_empty_string_member_cpp_value = dom_string_treat_null_as_empty_string_member_value;
if (!dom_string_treat_null_as_empty_string_member_cpp_value.Prepare(exceptionState))
return;
impl.setDomStringTreatNullAsEmptyStringMember(dom_string_treat_null_as_empty_string_member_cpp_value);
}

v8::Local<v8::Value> double_or_null_member_value;
if (!v8Object->Get(context, keys[10].Get(isolate)).ToLocal(&double_or_null_member_value)) {
exceptionState.RethrowV8Exception(block.Exception());
Expand Down Expand Up @@ -1104,25 +1104,14 @@ bool toV8TestDictionary(const TestDictionary& impl, v8::Local<v8::Object> dictio
return false;
}

v8::Local<v8::Value> byte_string_member_value;
bool byte_string_member_has_value_or_default = false;
if (impl.hasByteStringMember()) {
byte_string_member_value = V8String(isolate, impl.byteStringMember());
byte_string_member_has_value_or_default = true;
}
if (byte_string_member_has_value_or_default &&
!create_property(5, byte_string_member_value)) {
return false;
}

v8::Local<v8::Value> callback_function_member_value;
bool callback_function_member_has_value_or_default = false;
if (impl.hasCallbackFunctionMember()) {
callback_function_member_value = ToV8(impl.callbackFunctionMember(), creationContext, isolate);
callback_function_member_has_value_or_default = true;
}
if (callback_function_member_has_value_or_default &&
!create_property(6, callback_function_member_value)) {
!create_property(5, callback_function_member_value)) {
return false;
}

Expand All @@ -1133,7 +1122,7 @@ bool toV8TestDictionary(const TestDictionary& impl, v8::Local<v8::Object> dictio
create_has_value_or_default = true;
}
if (create_has_value_or_default &&
!create_property(7, create_value)) {
!create_property(6, create_value)) {
return false;
}

Expand All @@ -1144,7 +1133,7 @@ bool toV8TestDictionary(const TestDictionary& impl, v8::Local<v8::Object> dictio
deprecated_create_member_has_value_or_default = true;
}
if (deprecated_create_member_has_value_or_default &&
!create_property(8, deprecated_create_member_value)) {
!create_property(7, deprecated_create_member_value)) {
return false;
}

Expand All @@ -1156,7 +1145,18 @@ bool toV8TestDictionary(const TestDictionary& impl, v8::Local<v8::Object> dictio
dictionary_member_has_value_or_default = true;
}
if (dictionary_member_has_value_or_default &&
!create_property(9, dictionary_member_value)) {
!create_property(8, dictionary_member_value)) {
return false;
}

v8::Local<v8::Value> dom_string_treat_null_as_empty_string_member_value;
bool dom_string_treat_null_as_empty_string_member_has_value_or_default = false;
if (impl.hasDomStringTreatNullAsEmptyStringMember()) {
dom_string_treat_null_as_empty_string_member_value = V8String(isolate, impl.domStringTreatNullAsEmptyStringMember());
dom_string_treat_null_as_empty_string_member_has_value_or_default = true;
}
if (dom_string_treat_null_as_empty_string_member_has_value_or_default &&
!create_property(9, dom_string_treat_null_as_empty_string_member_value)) {
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
// void setPropertyPriority(DOMString property, [TreatNullAs=EmptyString] DOMString priority);
[CEReactions, RaisesException] DOMString removeProperty(DOMString property);
readonly attribute CSSRule? parentRule;
[CEReactions, SetterCallWith=ExecutionContext, RaisesException=Setter, TreatNullAs=EmptyString] attribute DOMString cssFloat;
[CEReactions, SetterCallWith=ExecutionContext, RaisesException=Setter] attribute [TreatNullAs=EmptyString] DOMString cssFloat;

// The camel-cased and dashed attribute getters have custom bindings.
// https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-camel-cased-attribute
Expand Down
2 changes: 1 addition & 1 deletion third_party/blink/renderer/core/frame/window.idl
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
[CrossOrigin, Custom=Setter] attribute Window opener;
[Replaceable, CrossOrigin] readonly attribute Window? parent;
[CheckSecurity=ReturnValue, Custom=Getter] readonly attribute Element? frameElement;
[CallWith=(ExecutionContext,CurrentWindow,EnteredWindow), RaisesException] Window? open(optional [TreatNullAs=EmptyString] URLString url="", optional DOMString target = "_blank", optional [TreatNullAs=EmptyString] DOMString features = "");
[CallWith=(ExecutionContext,CurrentWindow,EnteredWindow), RaisesException] Window? open(optional URLString url="", optional DOMString target = "_blank", optional [TreatNullAs=EmptyString] DOMString features = "");

// indexed properties
// https://html.spec.whatwg.org/C/browsers.html#windowproxy-getownproperty
Expand Down

0 comments on commit fdd9924

Please sign in to comment.