Bug 1368949. Stop automatically giving dictionary-typed members of dictionaries a default value of null. r=qdot
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 25 Sep 2018 18:09:30 -0400
changeset 496735 13ff5a2606eb0cf2e6484d97c5e4f8da30a3a81d
parent 496734 ea3768a7ecb170cbb584fce574ffdd63bdcd3540
child 496736 32fb340597628c8eb3cea9b507a073a63fb6b81e
child 496777 a03a61d6d724503c3b7c5e31fe32ced1f5d1c219
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersqdot
bugs1368949
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1368949. Stop automatically giving dictionary-typed members of dictionaries a default value of null. r=qdot
dom/bindings/Codegen.py
dom/bindings/parser/WebIDL.py
dom/webidl/CSPReport.webidl
dom/webidl/CredentialManagement.webidl
dom/webidl/DeviceMotionEvent.webidl
dom/webidl/MediaCapabilities.webidl
dom/webidl/MediaStreamTrack.webidl
dom/webidl/PaymentRequest.webidl
dom/webidl/PushSubscription.webidl
dom/webidl/WebAuthentication.webidl
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -5964,22 +5964,23 @@ def getJSToNativeConversionInfo(type, de
                                         declType=CGGeneric(declType),
                                         declArgs=declArgs)
 
     if type.isObject():
         assert not isEnforceRange and not isClamp
         return handleJSObjectType(type, isMember, failureCode, exceptionCode, sourceDescription)
 
     if type.isDictionary():
-        # There are no nullable dictionary arguments or dictionary members
+        # There are no nullable dictionary-typed arguments or dictionary-typed
+        # dictionary members.
         assert(not type.nullable() or isCallbackReturnValue or
                (isMember and isMember != "Dictionary"))
-        # All optional dictionaries always have default values, so we
-        # should be able to assume not isOptional here.
-        assert not isOptional
+        # All optional dictionary-typed arguments always have default values,
+        # but dictionary-typed dictionary members can be optional.
+        assert not isOptional or isMember == "Dictionary"
         # In the callback return value case we never have to worry
         # about a default value; we always have a value.
         assert not isCallbackReturnValue or defaultValue is None
 
         typeName = CGDictionary.makeDictionaryName(type.unroll().inner)
         if not isMember and not isCallbackReturnValue:
             # Since we're not a member and not nullable or optional, no one will
             # see our real type, so we can do the fast version of the dictionary
@@ -6050,17 +6051,18 @@ def getJSToNativeConversionInfo(type, de
             declArgs = "aRetVal"
         elif not isMember and typeNeedsRooting(type):
             declType = CGTemplatedType("RootedDictionary", declType)
             declArgs = "cx"
         else:
             declArgs = None
 
         return JSToNativeConversionInfo(template, declType=declType,
-                                        declArgs=declArgs)
+                                        declArgs=declArgs,
+                                        dealWithOptional=isOptional)
 
     if type.isVoid():
         assert not isOptional
         # This one only happens for return values, and its easy: Just
         # ignore the jsval.
         return JSToNativeConversionInfo("")
 
     if type.isDate():
--- a/dom/bindings/parser/WebIDL.py
+++ b/dom/bindings/parser/WebIDL.py
@@ -4521,18 +4521,19 @@ class IDLArgument(IDLObjectWithIdentifie
             type = self.type.complete(scope)
             assert not isinstance(type, IDLUnresolvedType)
             assert not isinstance(type, IDLTypedefType)
             assert not isinstance(type.name, IDLUnresolvedIdentifier)
             self.type = type
 
         if ((self.type.isDictionary() or
              self.type.isUnion() and self.type.unroll().hasDictionaryType()) and
-            self.optional and not self.defaultValue and not self.variadic):
-            # Default optional non-variadic dictionaries to null,
+            self.optional and not self.defaultValue and not self.variadic and
+            not self.dictionaryMember):
+            # Default optional non-variadic dictionary arguments to null,
             # for simplicity, so the codegen doesn't have to special-case this.
             self.defaultValue = IDLNullValue(self.location)
         elif self.type.isAny():
             assert (self.defaultValue is None or
                     isinstance(self.defaultValue, IDLNullValue))
             # optional 'any' values always have a default value
             if self.optional and not self.defaultValue and not self.variadic:
                 # Set the default value to undefined, for simplicity, so the
--- a/dom/webidl/CSPReport.webidl
+++ b/dom/webidl/CSPReport.webidl
@@ -15,10 +15,12 @@ dictionary CSPReportProperties {
   DOMString original-policy= "";
   DOMString source-file;
   DOMString script-sample;
   long line-number;
   long column-number;
 };
 
 dictionary CSPReport {
-  CSPReportProperties csp-report;
+  // We always want to have a "csp-report" property, so just pre-initialize it
+  // to an empty dictionary..
+  CSPReportProperties csp-report = null;
 };
--- a/dom/webidl/CredentialManagement.webidl
+++ b/dom/webidl/CredentialManagement.webidl
@@ -21,16 +21,18 @@ interface CredentialsContainer {
   Promise<Credential?> create(optional CredentialCreationOptions options);
   [Throws]
   Promise<Credential> store(Credential credential);
   [Throws]
   Promise<void> preventSilentAccess();
 };
 
 dictionary CredentialRequestOptions {
-  PublicKeyCredentialRequestOptions publicKey;
+  // FIXME: bug 1493860: should this "= null" be here?
+  PublicKeyCredentialRequestOptions publicKey = null;
   AbortSignal signal;
 };
 
 dictionary CredentialCreationOptions {
-  PublicKeyCredentialCreationOptions publicKey;
+  // FIXME: bug 1493860: should this "= null" be here?
+  PublicKeyCredentialCreationOptions publicKey = null;
   AbortSignal signal;
 };
--- a/dom/webidl/DeviceMotionEvent.webidl
+++ b/dom/webidl/DeviceMotionEvent.webidl
@@ -1,12 +1,14 @@
 /* -*- Mode: IDL; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/.
+ *
+ * https://w3c.github.io/deviceorientation/
  */
 
 [NoInterfaceObject]
 interface DeviceAcceleration {
   readonly attribute double? x;
   readonly attribute double? y;
   readonly attribute double? z;
 };
@@ -34,19 +36,22 @@ dictionary DeviceAccelerationInit {
 
 dictionary DeviceRotationRateInit {
   double? alpha = null;
   double? beta = null;
   double? gamma = null;
 };
 
 dictionary DeviceMotionEventInit : EventInit {
-  DeviceAccelerationInit acceleration;
-  DeviceAccelerationInit accelerationIncludingGravity;
-  DeviceRotationRateInit rotationRate;
+  // FIXME: bug 1493860: should this "= null" be here?
+  DeviceAccelerationInit acceleration = null;
+  // FIXME: bug 1493860: should this "= null" be here?
+  DeviceAccelerationInit accelerationIncludingGravity = null;
+  // FIXME: bug 1493860: should this "= null" be here?
+  DeviceRotationRateInit rotationRate = null;
   double? interval = null;
 };
 
 // Mozilla extensions.
 partial interface DeviceMotionEvent {
   void initDeviceMotionEvent(DOMString type,
                              optional boolean canBubble = false,
                              optional boolean cancelable = false,
--- a/dom/webidl/MediaCapabilities.webidl
+++ b/dom/webidl/MediaCapabilities.webidl
@@ -5,18 +5,20 @@
  *
  * The origin of this IDL file is
  * https://wicg.github.io/media-capabilities/
  *
  * Copyright © 2018 the Contributors to the Media Capabilities Specification
  */
 
 dictionary MediaConfiguration {
-  VideoConfiguration video;
-  AudioConfiguration audio;
+  // Bug 1493798: This should actually be optional and its members required.
+  VideoConfiguration video = null;
+  // Bug 1493798: This should actually be optional and its members required.
+  AudioConfiguration audio = null;
 };
 
 dictionary MediaDecodingConfiguration : MediaConfiguration {
   required MediaDecodingType type;
 };
 
 dictionary MediaEncodingConfiguration : MediaConfiguration {
   required MediaEncodingType type;
--- a/dom/webidl/MediaStreamTrack.webidl
+++ b/dom/webidl/MediaStreamTrack.webidl
@@ -37,36 +37,51 @@ typedef (long or ConstrainLongRange) Con
 typedef (double or ConstrainDoubleRange) ConstrainDouble;
 typedef (boolean or ConstrainBooleanParameters) ConstrainBoolean;
 typedef (DOMString or sequence<DOMString> or ConstrainDOMStringParameters) ConstrainDOMString;
 
 // Note: When adding new constraints, remember to update the SelectSettings()
 // function in MediaManager.cpp to make OverconstrainedError's constraint work!
 
 dictionary MediaTrackConstraintSet {
-    ConstrainLong width;
-    ConstrainLong height;
-    ConstrainDouble frameRate;
-    ConstrainDOMString facingMode;
+    // FIXME: bug 1493860 or bug 1493798: should this "= null" be here?
+    ConstrainLong width = null;
+    // FIXME: bug 1493860 or bug 1493798: should this "= null" be here?
+    ConstrainLong height = null;
+    // FIXME: bug 1493860 or bug 1493798: should this "= null" be here?
+    ConstrainDouble frameRate = null;
+    // FIXME: bug 1493860 or bug 1493798: should this "= null" be here?
+    ConstrainDOMString facingMode = null;
     DOMString mediaSource = "camera";
     long long browserWindow;
     boolean scrollWithPage;
-    ConstrainDOMString deviceId;
-    ConstrainLong viewportOffsetX;
-    ConstrainLong viewportOffsetY;
-    ConstrainLong viewportWidth;
-    ConstrainLong viewportHeight;
-    ConstrainBoolean echoCancellation;
-    ConstrainBoolean noiseSuppression;
-    ConstrainBoolean autoGainControl;
-    ConstrainLong channelCount;
+    // FIXME: bug 1493860 or bug 1493798: should this "= null" be here?
+    ConstrainDOMString deviceId = null;
+    // FIXME: bug 1493860 or bug 1493798: should this "= null" be here?
+    ConstrainLong viewportOffsetX = null;
+    // FIXME: bug 1493860 or bug 1493798: should this "= null" be here?
+    ConstrainLong viewportOffsetY = null;
+    // FIXME: bug 1493860 or bug 1493798: should this "= null" be here?
+    ConstrainLong viewportWidth = null;
+    // FIXME: bug 1493860 or bug 1493798: should this "= null" be here?
+    ConstrainLong viewportHeight = null;
+    // FIXME: bug 1493860 or bug 1493798: should this "= null" be here?
+    ConstrainBoolean echoCancellation = null;
+    // FIXME: bug 1493860 or bug 1493798: should this "= null" be here?
+    ConstrainBoolean noiseSuppression = null;
+    // FIXME: bug 1493860 or bug 1493798: should this "= null" be here?
+    ConstrainBoolean autoGainControl = null;
+    // FIXME: bug 1493860 or bug 1493798: should this "= null" be here?
+    ConstrainLong channelCount = null;
 
     // Deprecated with warnings:
-    ConstrainBoolean mozNoiseSuppression;
-    ConstrainBoolean mozAutoGainControl;
+    // FIXME: bug 1493860 or bug 1493798: should this "= null" be here?
+    ConstrainBoolean mozNoiseSuppression = null;
+    // FIXME: bug 1493860 or bug 1493798: should this "= null" be here?
+    ConstrainBoolean mozAutoGainControl = null;
 };
 
 dictionary MediaTrackConstraints : MediaTrackConstraintSet {
     sequence<MediaTrackConstraintSet> advanced;
 };
 
 enum MediaStreamTrackState {
     "live",
--- a/dom/webidl/PaymentRequest.webidl
+++ b/dom/webidl/PaymentRequest.webidl
@@ -36,17 +36,18 @@ dictionary PaymentShippingOption {
   required DOMString             id;
   required DOMString             label;
   required PaymentCurrencyAmount amount;
            boolean               selected = false;
 };
 
 dictionary PaymentDetailsModifier {
   required DOMString             supportedMethods;
-           PaymentItem           total;
+  // FIXME: bug 1493860: should this "= null" be here?
+           PaymentItem           total = null;
            sequence<PaymentItem> additionalDisplayItems;
            object                data;
 };
 
 dictionary PaymentDetailsBase {
   sequence<PaymentItem>            displayItems;
   sequence<PaymentShippingOption>  shippingOptions;
   sequence<PaymentDetailsModifier> modifiers;
@@ -67,34 +68,39 @@ dictionary AddressErrors {
   DOMString postalCode;
   DOMString recipient;
   DOMString region;
   DOMString regionCode;
   DOMString sortingCode;
 };
 
 dictionary PaymentValidationErrors {
-  PayerErrorFields payer;
-  AddressErrors shippingAddress;
+  // FIXME: bug 1493860: should this "= null" be here?
+  PayerErrorFields payer = null;
+  // FIXME: bug 1493860: should this "= null" be here?
+  AddressErrors shippingAddress = null;
   DOMString error;
   object paymentMethod;
 };
 
 dictionary PayerErrorFields {
   DOMString email;
   DOMString name;
   DOMString phone;
 };
 
 dictionary PaymentDetailsUpdate : PaymentDetailsBase {
   DOMString     error;
-  AddressErrors shippingAddressErrors;
-  PayerErrorFields payerErrors;
+  // FIXME: bug 1493860: should this "= null" be here?
+  AddressErrors shippingAddressErrors = null;
+  // FIXME: bug 1493860: should this "= null" be here?
+  PayerErrorFields payerErrors = null;
   object paymentMethodErrors;
-  PaymentItem   total;
+  // FIXME: bug 1493860: should this "= null" be here?
+  PaymentItem   total = null;
 };
 
 enum PaymentShippingType {
   "shipping",
   "delivery",
   "pickup"
 };
 
--- a/dom/webidl/PushSubscription.webidl
+++ b/dom/webidl/PushSubscription.webidl
@@ -19,17 +19,20 @@ dictionary PushSubscriptionKeys
 {
   ByteString p256dh;
   ByteString auth;
 };
 
 dictionary PushSubscriptionJSON
 {
   USVString endpoint;
-  PushSubscriptionKeys keys;
+  // FIXME: bug 1493860: should this "= null" be here?  For that matter, this
+  // PushSubscriptionKeys thing is not even in the spec; "keys" is a record
+  // there.
+  PushSubscriptionKeys keys = null;
 };
 
 dictionary PushSubscriptionInit
 {
   required USVString endpoint;
   required USVString scope;
   ArrayBuffer? p256dhKey;
   ArrayBuffer? authSecret;
--- a/dom/webidl/WebAuthentication.webidl
+++ b/dom/webidl/WebAuthentication.webidl
@@ -47,19 +47,21 @@ dictionary PublicKeyCredentialCreationOp
     required PublicKeyCredentialRpEntity   rp;
     required PublicKeyCredentialUserEntity user;
 
     required BufferSource                            challenge;
     required sequence<PublicKeyCredentialParameters> pubKeyCredParams;
 
     unsigned long                                timeout;
     sequence<PublicKeyCredentialDescriptor>      excludeCredentials = [];
-    AuthenticatorSelectionCriteria               authenticatorSelection;
+    // FIXME: bug 1493860: should this "= null" be here?
+    AuthenticatorSelectionCriteria               authenticatorSelection = null;
     AttestationConveyancePreference              attestation = "none";
-    AuthenticationExtensionsClientInputs         extensions;
+    // FIXME: bug 1493860: should this "= null" be here?
+    AuthenticationExtensionsClientInputs         extensions = null;
 };
 
 dictionary PublicKeyCredentialEntity {
     required DOMString    name;
     USVString             icon;
 };
 
 dictionary PublicKeyCredentialRpEntity : PublicKeyCredentialEntity {
@@ -95,17 +97,18 @@ enum UserVerificationRequirement {
 };
 
 dictionary PublicKeyCredentialRequestOptions {
     required BufferSource                challenge;
     unsigned long                        timeout;
     USVString                            rpId;
     sequence<PublicKeyCredentialDescriptor> allowCredentials = [];
     UserVerificationRequirement          userVerification = "preferred";
-    AuthenticationExtensionsClientInputs extensions;
+    // FIXME: bug 1493860: should this "= null" be here?
+    AuthenticationExtensionsClientInputs extensions = null;
 };
 
 // TODO - Use partial dictionaries when bug 1436329 is fixed.
 dictionary AuthenticationExtensionsClientInputs {
     // FIDO AppID Extension (appid)
     // <https://w3c.github.io/webauthn/#sctn-appid-extension>
     USVString appid;
 };
@@ -120,17 +123,18 @@ dictionary AuthenticationExtensionsClien
 typedef record<DOMString, DOMString> AuthenticationExtensionsAuthenticatorInputs;
 
 dictionary CollectedClientData {
     required DOMString           type;
     required DOMString           challenge;
     required DOMString           origin;
     required DOMString           hashAlgorithm;
     DOMString                    tokenBindingId;
-    AuthenticationExtensionsClientInputs clientExtensions;
+    // FIXME: bug 1493860: should this "= null" be here?
+    AuthenticationExtensionsClientInputs clientExtensions = null;
     AuthenticationExtensionsAuthenticatorInputs authenticatorExtensions;
 };
 
 enum PublicKeyCredentialType {
     "public-key"
 };
 
 dictionary PublicKeyCredentialDescriptor {