Commit 7f8965f2 authored by foolip's avatar foolip Committed by Commit bot

Enable TypeError for dictionary members of non-nullable interface type

In dictionary_v8.cpp, the extra null check was wrong or redudant
depending on nullabilty:

 * If the member isn't nullable, then passing null should throw
   TypeError.

 * If the member is nullable, then a null check is already emitted
   earlier, and so that part of the condition is always true.

The affected dictionaries were identified by looking at the resulting
changes in the generated bindings. Any affected member was made nullable
so that this change in itself is not observable, but the right behavior
is achieved if those changes are individually reverted.

A few cases that were not given TODOs:

 * MessageEventInit, because the existing TODO is appropriate.

 * MediaStreamEventInit, because it has no spec and MediaStreamEvent's
   corresponding attribute is already nullable.

 * SpeechRecognitionEventInit, because it has no spec.
   SpeechRecognitionEvent's attributes were made nullable because they
   can now in fact be null.

A number of spec issues were discovered, linked with the TODOs.

BUG=647693

Review-Url: https://codereview.chromium.org/2342393002
Cr-Commit-Position: refs/heads/master@{#419344}
parent 410a599d
...@@ -68,7 +68,7 @@ void {{v8_class}}::toImpl(v8::Isolate* isolate, v8::Local<v8::Value> v8Value, {{ ...@@ -68,7 +68,7 @@ void {{v8_class}}::toImpl(v8::Isolate* isolate, v8::Local<v8::Value> v8Value, {{
{% endif %} {% endif %}
{{v8_value_to_local_cpp_value(member) | indent(8)}} {{v8_value_to_local_cpp_value(member) | indent(8)}}
{% if member.is_interface_type %} {% if member.is_interface_type %}
if (!{{member.name}} && !{{member.name}}Value->IsNull()) { if (!{{member.name}}) {
exceptionState.throwTypeError("member {{member.name}} is not of type {{member.idl_type}}."); exceptionState.throwTypeError("member {{member.name}} is not of type {{member.idl_type}}.");
return; return;
} }
......
...@@ -170,7 +170,7 @@ void V8TestDictionary::toImpl(v8::Isolate* isolate, v8::Local<v8::Value> v8Value ...@@ -170,7 +170,7 @@ void V8TestDictionary::toImpl(v8::Isolate* isolate, v8::Local<v8::Value> v8Value
impl.setElementOrNullMemberToNull(); impl.setElementOrNullMemberToNull();
} else { } else {
Element* elementOrNullMember = V8Element::toImplWithTypeCheck(isolate, elementOrNullMemberValue); Element* elementOrNullMember = V8Element::toImplWithTypeCheck(isolate, elementOrNullMemberValue);
if (!elementOrNullMember && !elementOrNullMemberValue->IsNull()) { if (!elementOrNullMember) {
exceptionState.throwTypeError("member elementOrNullMember is not of type Element."); exceptionState.throwTypeError("member elementOrNullMember is not of type Element.");
return; return;
} }
...@@ -230,7 +230,7 @@ void V8TestDictionary::toImpl(v8::Isolate* isolate, v8::Local<v8::Value> v8Value ...@@ -230,7 +230,7 @@ void V8TestDictionary::toImpl(v8::Isolate* isolate, v8::Local<v8::Value> v8Value
// Do nothing. // Do nothing.
} else { } else {
EventTarget* eventTargetMember = toEventTarget(isolate, eventTargetMemberValue); EventTarget* eventTargetMember = toEventTarget(isolate, eventTargetMemberValue);
if (!eventTargetMember && !eventTargetMemberValue->IsNull()) { if (!eventTargetMember) {
exceptionState.throwTypeError("member eventTargetMember is not of type EventTarget."); exceptionState.throwTypeError("member eventTargetMember is not of type EventTarget.");
return; return;
} }
...@@ -426,7 +426,7 @@ void V8TestDictionary::toImpl(v8::Isolate* isolate, v8::Local<v8::Value> v8Value ...@@ -426,7 +426,7 @@ void V8TestDictionary::toImpl(v8::Isolate* isolate, v8::Local<v8::Value> v8Value
// Do nothing. // Do nothing.
} else { } else {
TestInterfaceGarbageCollected* testInterfaceGarbageCollectedMember = V8TestInterfaceGarbageCollected::toImplWithTypeCheck(isolate, testInterfaceGarbageCollectedMemberValue); TestInterfaceGarbageCollected* testInterfaceGarbageCollectedMember = V8TestInterfaceGarbageCollected::toImplWithTypeCheck(isolate, testInterfaceGarbageCollectedMemberValue);
if (!testInterfaceGarbageCollectedMember && !testInterfaceGarbageCollectedMemberValue->IsNull()) { if (!testInterfaceGarbageCollectedMember) {
exceptionState.throwTypeError("member testInterfaceGarbageCollectedMember is not of type TestInterfaceGarbageCollected."); exceptionState.throwTypeError("member testInterfaceGarbageCollectedMember is not of type TestInterfaceGarbageCollected.");
return; return;
} }
...@@ -444,7 +444,7 @@ void V8TestDictionary::toImpl(v8::Isolate* isolate, v8::Local<v8::Value> v8Value ...@@ -444,7 +444,7 @@ void V8TestDictionary::toImpl(v8::Isolate* isolate, v8::Local<v8::Value> v8Value
impl.setTestInterfaceGarbageCollectedOrNullMemberToNull(); impl.setTestInterfaceGarbageCollectedOrNullMemberToNull();
} else { } else {
TestInterfaceGarbageCollected* testInterfaceGarbageCollectedOrNullMember = V8TestInterfaceGarbageCollected::toImplWithTypeCheck(isolate, testInterfaceGarbageCollectedOrNullMemberValue); TestInterfaceGarbageCollected* testInterfaceGarbageCollectedOrNullMember = V8TestInterfaceGarbageCollected::toImplWithTypeCheck(isolate, testInterfaceGarbageCollectedOrNullMemberValue);
if (!testInterfaceGarbageCollectedOrNullMember && !testInterfaceGarbageCollectedOrNullMemberValue->IsNull()) { if (!testInterfaceGarbageCollectedOrNullMember) {
exceptionState.throwTypeError("member testInterfaceGarbageCollectedOrNullMember is not of type TestInterfaceGarbageCollected."); exceptionState.throwTypeError("member testInterfaceGarbageCollectedOrNullMember is not of type TestInterfaceGarbageCollected.");
return; return;
} }
...@@ -474,7 +474,7 @@ void V8TestDictionary::toImpl(v8::Isolate* isolate, v8::Local<v8::Value> v8Value ...@@ -474,7 +474,7 @@ void V8TestDictionary::toImpl(v8::Isolate* isolate, v8::Local<v8::Value> v8Value
// Do nothing. // Do nothing.
} else { } else {
TestInterfaceImplementation* testInterfaceMember = V8TestInterface::toImplWithTypeCheck(isolate, testInterfaceMemberValue); TestInterfaceImplementation* testInterfaceMember = V8TestInterface::toImplWithTypeCheck(isolate, testInterfaceMemberValue);
if (!testInterfaceMember && !testInterfaceMemberValue->IsNull()) { if (!testInterfaceMember) {
exceptionState.throwTypeError("member testInterfaceMember is not of type TestInterface."); exceptionState.throwTypeError("member testInterfaceMember is not of type TestInterface.");
return; return;
} }
...@@ -492,7 +492,7 @@ void V8TestDictionary::toImpl(v8::Isolate* isolate, v8::Local<v8::Value> v8Value ...@@ -492,7 +492,7 @@ void V8TestDictionary::toImpl(v8::Isolate* isolate, v8::Local<v8::Value> v8Value
impl.setTestInterfaceOrNullMemberToNull(); impl.setTestInterfaceOrNullMemberToNull();
} else { } else {
TestInterfaceImplementation* testInterfaceOrNullMember = V8TestInterface::toImplWithTypeCheck(isolate, testInterfaceOrNullMemberValue); TestInterfaceImplementation* testInterfaceOrNullMember = V8TestInterface::toImplWithTypeCheck(isolate, testInterfaceOrNullMemberValue);
if (!testInterfaceOrNullMember && !testInterfaceOrNullMemberValue->IsNull()) { if (!testInterfaceOrNullMember) {
exceptionState.throwTypeError("member testInterfaceOrNullMember is not of type TestInterface."); exceptionState.throwTypeError("member testInterfaceOrNullMember is not of type TestInterface.");
return; return;
} }
...@@ -522,7 +522,7 @@ void V8TestDictionary::toImpl(v8::Isolate* isolate, v8::Local<v8::Value> v8Value ...@@ -522,7 +522,7 @@ void V8TestDictionary::toImpl(v8::Isolate* isolate, v8::Local<v8::Value> v8Value
// Do nothing. // Do nothing.
} else { } else {
DOMUint8Array* uint8ArrayMember = uint8ArrayMemberValue->IsUint8Array() ? V8Uint8Array::toImpl(v8::Local<v8::Uint8Array>::Cast(uint8ArrayMemberValue)) : 0; DOMUint8Array* uint8ArrayMember = uint8ArrayMemberValue->IsUint8Array() ? V8Uint8Array::toImpl(v8::Local<v8::Uint8Array>::Cast(uint8ArrayMemberValue)) : 0;
if (!uint8ArrayMember && !uint8ArrayMemberValue->IsNull()) { if (!uint8ArrayMember) {
exceptionState.throwTypeError("member uint8ArrayMember is not of type Uint8Array."); exceptionState.throwTypeError("member uint8ArrayMember is not of type Uint8Array.");
return; return;
} }
......
...@@ -6,7 +6,8 @@ ...@@ -6,7 +6,8 @@
dictionary TouchInit { dictionary TouchInit {
required long identifier; required long identifier;
required EventTarget target; // TODO(foolip): |target| should not be nullable. https://crbug.com/647693
required EventTarget? target;
double clientX = 0; double clientX = 0;
double clientY = 0; double clientY = 0;
double screenX = 0; double screenX = 0;
......
...@@ -9,7 +9,7 @@ dictionary MessageEventInit : EventInit { ...@@ -9,7 +9,7 @@ dictionary MessageEventInit : EventInit {
DOMString origin; DOMString origin;
DOMString lastEventId; DOMString lastEventId;
// TODO(bashi): |source| should be (WindowProxy or MessagePort)? // TODO(bashi): |source| should be (WindowProxy or MessagePort)?
EventTarget source; EventTarget? source;
// Per spec, |ports| isn't nullable, but it seems it should be. // Per spec, |ports| isn't nullable, but it seems it should be.
// https://www.w3.org/Bugs/Public/show_bug.cgi?id=23176 // https://www.w3.org/Bugs/Public/show_bug.cgi?id=23176
sequence<MessagePort>? ports; sequence<MessagePort>? ports;
......
...@@ -6,5 +6,8 @@ ...@@ -6,5 +6,8 @@
dictionary MediaKeyMessageEventInit : EventInit { dictionary MediaKeyMessageEventInit : EventInit {
MediaKeyMessageType messageType = "license-request"; MediaKeyMessageType messageType = "license-request";
ArrayBuffer message; // TODO(foolip): |message| should be required and not nullable.
// https://crbug.com/647693
// https://github.com/w3c/encrypted-media/issues/329
ArrayBuffer? message;
}; };
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
// https://dvcs.w3.org/hg/gamepad/raw-file/default/gamepad.html#gamepadevent-interface // https://w3c.github.io/gamepad/#gamepadevent-interface
[ [
Constructor(DOMString type, optional GamepadEventInit eventInitDict), Constructor(DOMString type, optional GamepadEventInit eventInitDict),
......
...@@ -2,8 +2,11 @@ ...@@ -2,8 +2,11 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
// https://dvcs.w3.org/hg/gamepad/raw-file/default/gamepad.html#gamepadevent-interface // https://w3c.github.io/gamepad/#gamepadevent-interface
dictionary GamepadEventInit : EventInit { dictionary GamepadEventInit : EventInit {
Gamepad gamepad; // TODO(foolip): |gamepad| should be required and not nullable.
// https://crbug.com/647693
// https://github.com/w3c/gamepad/issues/35
Gamepad? gamepad;
}; };
...@@ -5,5 +5,6 @@ ...@@ -5,5 +5,6 @@
// https://w3c.github.io/mediacapture-record/MediaRecorder.html#blobeventinit // https://w3c.github.io/mediacapture-record/MediaRecorder.html#blobeventinit
dictionary BlobEventInit : EventInit { dictionary BlobEventInit : EventInit {
required Blob data; // TODO(foolip): |data| should not be nullable. https://crbug.com/647693
required Blob? data;
}; };
...@@ -5,5 +5,5 @@ ...@@ -5,5 +5,5 @@
// http://www.w3.org/TR/webrtc/#mediastreamevent // http://www.w3.org/TR/webrtc/#mediastreamevent
dictionary MediaStreamEventInit : EventInit { dictionary MediaStreamEventInit : EventInit {
MediaStream stream; MediaStream? stream;
}; };
...@@ -5,6 +5,8 @@ ...@@ -5,6 +5,8 @@
// https://notifications.spec.whatwg.org/#notificationevent // https://notifications.spec.whatwg.org/#notificationevent
dictionary NotificationEventInit : ExtendableEventInit { dictionary NotificationEventInit : ExtendableEventInit {
required Notification notification; // TODO(foolip): |notification| should not be nullable.
// https://crbug.com/647693
required Notification? notification;
DOMString action = ""; DOMString action = "";
}; };
...@@ -5,5 +5,7 @@ ...@@ -5,5 +5,7 @@
// https://w3c.github.io/presentation-api/#idl-def-presentationconnectionavailableeventinit // https://w3c.github.io/presentation-api/#idl-def-presentationconnectionavailableeventinit
dictionary PresentationConnectionAvailableEventInit : EventInit { dictionary PresentationConnectionAvailableEventInit : EventInit {
required PresentationConnection connection; // TODO(foolip): |connection| should not be nullable.
// https://crbug.com/647693
required PresentationConnection? connection;
}; };
...@@ -6,5 +6,8 @@ ...@@ -6,5 +6,8 @@
// https://w3c.github.io/sensors/#dictdef-sensorreadingeventinit // https://w3c.github.io/sensors/#dictdef-sensorreadingeventinit
dictionary SensorReadingEventInit : EventInit { dictionary SensorReadingEventInit : EventInit {
SensorReading reading; // TODO(foolip): |reading| should be required and not nullable.
// https://crbug.com/647693
// https://github.com/w3c/sensors/issues/129
SensorReading? reading;
}; };
...@@ -2,10 +2,11 @@ ...@@ -2,10 +2,11 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
// https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#fetch-event-interface // https://w3c.github.io/ServiceWorker/#fetch-event-interface
dictionary FetchEventInit : ExtendableEventInit { dictionary FetchEventInit : ExtendableEventInit {
required Request request; // TODO(foolip): |request| should not be nullable. https://crbug.com/647693
required Request? request;
DOMString? clientId = null; DOMString? clientId = null;
boolean isReload = false; boolean isReload = false;
}; };
...@@ -2,9 +2,10 @@ ...@@ -2,9 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
// https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#dictdef-foreignfetchevent-foreignfetcheventinit // https://w3c.github.io/ServiceWorker/#dictdef-foreignfetchevent-foreignfetcheventinit
dictionary ForeignFetchEventInit : ExtendableEventInit { dictionary ForeignFetchEventInit : ExtendableEventInit {
required Request request; // TODO(foolip): |request| should not be nullable. https://crbug.com/647693
required Request? request;
USVString origin = "null"; USVString origin = "null";
}; };
...@@ -2,10 +2,11 @@ ...@@ -2,10 +2,11 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
// https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#dictdef-foreignfetchevent-foreignfetchresponse // https://w3c.github.io/ServiceWorker/#dictdef-foreignfetchevent-foreignfetchresponse
dictionary ForeignFetchResponse { dictionary ForeignFetchResponse {
required Response response; // TODO(foolip): |response| should not be nullable. https://crbug.com/647693
required Response? response;
USVString origin; USVString origin;
sequence<ByteString> headers; sequence<ByteString> headers;
}; };
...@@ -28,8 +28,8 @@ ...@@ -28,8 +28,8 @@
Constructor(DOMString type, optional SpeechRecognitionEventInit initDict), Constructor(DOMString type, optional SpeechRecognitionEventInit initDict),
] interface SpeechRecognitionEvent : Event { ] interface SpeechRecognitionEvent : Event {
readonly attribute unsigned long resultIndex; readonly attribute unsigned long resultIndex;
readonly attribute SpeechRecognitionResultList results; readonly attribute SpeechRecognitionResultList? results;
readonly attribute Document interpretation; readonly attribute Document? interpretation;
readonly attribute Document emma; readonly attribute Document? emma;
}; };
...@@ -6,5 +6,5 @@ ...@@ -6,5 +6,5 @@
dictionary SpeechRecognitionEventInit : EventInit { dictionary SpeechRecognitionEventInit : EventInit {
unsigned long resultIndex; unsigned long resultIndex;
SpeechRecognitionResultList results; SpeechRecognitionResultList? results;
}; };
...@@ -6,6 +6,9 @@ ...@@ -6,6 +6,9 @@
[ [
RuntimeEnabled=WebVR RuntimeEnabled=WebVR
] dictionary VRDisplayEventInit : EventInit { ] dictionary VRDisplayEventInit : EventInit {
VRDisplay display; // TODO(foolip): |display| should be required and not nullable.
// https://crbug.com/647693
// https://github.com/w3c/webvr/issues/83
VRDisplay? display;
VRDisplayEventReason reason; VRDisplayEventReason reason;
}; };
...@@ -5,5 +5,8 @@ ...@@ -5,5 +5,8 @@
// http://webaudio.github.io/web-midi-api/#idl-def-MIDIConnectionEventInit // http://webaudio.github.io/web-midi-api/#idl-def-MIDIConnectionEventInit
dictionary MIDIConnectionEventInit : EventInit { dictionary MIDIConnectionEventInit : EventInit {
MIDIPort port; // TODO(foolip): |port| should be required and not nullable.
// https://crbug.com/647693
// https://github.com/WebAudio/web-midi-api/issues/168
MIDIPort? port;
}; };
...@@ -6,5 +6,8 @@ ...@@ -6,5 +6,8 @@
dictionary MIDIMessageEventInit : EventInit { dictionary MIDIMessageEventInit : EventInit {
double receivedTime; double receivedTime;
Uint8Array data; // TODO(foolip): |data| should be required and not nullable.
// https://crbug.com/647693
// https://github.com/WebAudio/web-midi-api/issues/168
Uint8Array? data;
}; };
...@@ -5,5 +5,8 @@ ...@@ -5,5 +5,8 @@
// http://wicg.github.io/webusb/#events // http://wicg.github.io/webusb/#events
dictionary USBConnectionEventInit : EventInit { dictionary USBConnectionEventInit : EventInit {
USBDevice device; // TODO(foolip): |device| should be required and not nullable.
// https://crbug.com/647693
// https://github.com/WICG/webusb/issues/63
USBDevice? device;
}; };
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