Bug 1584630. Implement missing property use counters for HTMLDocument. r=peterv
authorBoris Zbarsky <bzbarsky@mit.edu>
Fri, 11 Oct 2019 16:56:36 +0000
changeset 497295 31d32668afb5e427b8d50ffca875ee7c7faa4a41
parent 497294 c5e6477c3a245a97d4c3cdd5d3e406f8abaf94ad
child 497296 daf34421eaa9757d83259fa98db64e475fa9949e
push id36682
push userncsoregi@mozilla.com
push dateSat, 12 Oct 2019 09:52:03 +0000
treeherdermozilla-central@06ea2371f897 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv
bugs1584630
milestone71.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 1584630. Implement missing property use counters for HTMLDocument. r=peterv Differential Revision: https://phabricator.services.mozilla.com/D47504
dom/base/UseCounters.conf
dom/bindings/BindingUtils.h
dom/bindings/Codegen.py
dom/bindings/Configuration.py
dom/bindings/parser/WebIDL.py
dom/webidl/HTMLDocument.webidl
modules/libpref/init/StaticPrefList.yaml
--- a/dom/base/UseCounters.conf
+++ b/dom/base/UseCounters.conf
@@ -130,8 +130,44 @@ method MediaDevices.getUserMedia
 method Navigator.mozGetUserMedia
 custom GetUserMediaUnfocused calls MediaDevices.getUserMedia from an unfocused document
 custom GetUserMediaInsec calls MediaDevices.getUserMedia from an insecure context
 custom MozGetUserMediaInsec calls Navigator.mozGetUserMedia from an insecure context
 custom GetUserMediaXOrigin calls MediaDevices.getUserMedia from a cross origin context
 custom MozGetUserMediaXOrigin calls Navigator.mozGetUserMedia from a cross origin context
 method MediaDevices.getDisplayMedia
 custom GetDisplayMediaXOrigin calls MediaDevices.getDisplayMedia from a cross origin context
+
+// Missing-property use counters.  We claim these are "method" use
+// counters, because we don't need a separate description string for
+// them and we only need one use counter, not a getter/setter pair.
+method HTMLDocument.adoptedStyleSheets
+method HTMLDocument.caretRangeFromPoint
+method HTMLDocument.clear
+method HTMLDocument.exitPictureInPicture
+method HTMLDocument.featurePolicy
+method HTMLDocument.onbeforecopy
+method HTMLDocument.onbeforecut
+method HTMLDocument.onbeforepaste
+method HTMLDocument.oncancel
+method HTMLDocument.onfreeze
+method HTMLDocument.onmousewheel
+method HTMLDocument.onresume
+method HTMLDocument.onsearch
+method HTMLDocument.onsecuritypolicyviolation
+method HTMLDocument.onwebkitfullscreenchange
+method HTMLDocument.onwebkitfullscreenerror
+method HTMLDocument.pictureInPictureElement
+method HTMLDocument.pictureInPictureEnabled
+method HTMLDocument.registerElement
+method HTMLDocument.wasDiscarded
+method HTMLDocument.webkitCancelFullScreen
+method HTMLDocument.webkitCurrentFullScreenElement
+method HTMLDocument.webkitExitFullscreen
+method HTMLDocument.webkitFullscreenElement
+method HTMLDocument.webkitFullscreenEnabled
+method HTMLDocument.webkitHidden
+method HTMLDocument.webkitIsFullScreen
+method HTMLDocument.webkitVisibilityState
+method HTMLDocument.xmlEncoding
+method HTMLDocument.xmlStandalone
+method HTMLDocument.xmlVersion
+
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -3129,14 +3129,51 @@ bool HTMLConstructor(JSContext* aCx, uns
                      CreateInterfaceObjectsMethod aCreator);
 
 // A method to test whether an attribute with the given JSJitGetterOp getter is
 // enabled in the given set of prefable proeprty specs.  For use for toJSON
 // conversions.  aObj is the object that would be used as the "this" value.
 bool IsGetterEnabled(JSContext* aCx, JS::Handle<JSObject*> aObj,
                      JSJitGetterOp aGetter,
                      const Prefable<const JSPropertySpec>* aAttributes);
+
+// A class that can be used to examine the chars of a linear string.
+class StringIdChars {
+ public:
+  // Require a non-const ref to an AutoRequireNoGC to prevent callers
+  // from passing temporaries.
+  StringIdChars(JS::AutoRequireNoGC& nogc, JSLinearString* str) {
+    mIsLatin1 = js::LinearStringHasLatin1Chars(str);
+    if (mIsLatin1) {
+      mLatin1Chars = js::GetLatin1LinearStringChars(nogc, str);
+    } else {
+      mTwoByteChars = js::GetTwoByteLinearStringChars(nogc, str);
+    }
+#ifdef DEBUG
+    mLength = js::GetLinearStringLength(str);
+#endif  // DEBUG
+  }
+
+  MOZ_ALWAYS_INLINE char16_t operator[](size_t index) {
+    MOZ_ASSERT(index < mLength);
+    if (mIsLatin1) {
+      return mLatin1Chars[index];
+    }
+    return mTwoByteChars[index];
+  }
+
+ private:
+  bool mIsLatin1;
+  union {
+    const JS::Latin1Char* mLatin1Chars;
+    const char16_t* mTwoByteChars;
+  };
+#ifdef DEBUG
+  size_t mLength;
+#endif  // DEBUG
+};
+
 }  // namespace binding_detail
 
 }  // namespace dom
 }  // namespace mozilla
 
 #endif /* mozilla_dom_BindingUtils_h__ */
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -11771,16 +11771,139 @@ class CGProxyUnwrap(CGAbstractMethod):
               MOZ_ASSERT(xpc::WrapperFactory::IsXrayWrapper(obj));
               obj = js::UncheckedUnwrap(obj);
             }
             MOZ_ASSERT(IsProxy(obj));
             return static_cast<${type}*>(js::GetProxyReservedSlot(obj, DOM_OBJECT_SLOT).toPrivate());
             """,
             type=self.descriptor.nativeType)
 
+MISSING_PROP_PREF = "dom.missing_prop_counters.enabled"
+
+def missingPropUseCountersForDescriptor(desc):
+    instrumentedProps = desc.instrumentedProps
+    if not instrumentedProps:
+        return ""
+
+    def gen_switch(switchDecriptor):
+        """
+        Generate a switch from the switch descriptor.  The descriptor
+        dictionary must have the following properties:
+
+        1) A "precondition" property that contains code to run before the
+           switch statement.  Its value ie a string.
+        2) A "condition" property for the condition.  Its value is a string.
+        3) A "cases" property.  Its value is an object that has property names
+           corresponding to the case labels.  The values of those properties
+           are either new switch descriptor dictionaries (which will then
+           generate nested switches) or strings to use for case bodies.
+        """
+        cases = []
+        for label, body in sorted(switchDecriptor['cases'].iteritems()):
+            if isinstance(body, dict):
+                body = gen_switch(body)
+            cases.append(fill(
+                """
+                case ${label}: {
+                  $*{body}
+                  break;
+                }
+                """,
+                label=label,
+                body=body))
+        return fill(
+            """
+            $*{precondition}
+            switch (${condition}) {
+              $*{cases}
+            }
+            """,
+            precondition=switchDecriptor['precondition'],
+            condition=switchDecriptor['condition'],
+            cases="".join(cases))
+
+    def charSwitch(props, charIndex):
+        """
+        Create a switch for the given props, based on the first char where
+        they start to differ at index charIndex or more.  Each prop is a tuple
+        containing interface name and prop name.
+
+        Incoming props should be a sorted list.
+        """
+        if len(props) == 1:
+            # We're down to one string: just check whether we match it.
+            return fill(
+                """
+                if (JS_FlatStringEqualsLiteral(str, "${name}")) {
+                  counter.emplace(eUseCounter_${iface}_${name});
+                }
+                """,
+                iface=props[0][0],
+                name=props[0][1])
+
+        switch = dict()
+        if charIndex == 0:
+            switch['precondition'] = \
+                'StringIdChars chars(nogc, js::FlatStringToLinearString(str));\n'
+        else:
+            switch['precondition'] = ""
+
+        # Find the first place where we might actually have a difference.
+        while all(prop[1][charIndex] == props[0][1][charIndex]
+                  for prop in props):
+            charIndex += 1
+
+        switch['condition'] = 'chars[%d]' % charIndex
+        switch['cases'] = dict()
+        current_props = None
+        curChar = None
+        idx = 0
+        while idx < len(props):
+            nextChar = "'%s'" % props[idx][1][charIndex]
+            if nextChar != curChar:
+                if curChar:
+                    switch['cases'][curChar] = charSwitch(current_props,
+                                                          charIndex + 1)
+                current_props = []
+                curChar = nextChar
+            current_props.append(props[idx])
+            idx += 1
+        switch['cases'][curChar] = charSwitch(current_props, charIndex + 1)
+        return switch
+
+    lengths = set(len(prop[1]) for prop in instrumentedProps)
+    switchDesc = { 'condition': 'js::GetFlatStringLength(str)',
+                   'precondition': '' }
+    switchDesc['cases'] = dict()
+    for length in sorted(lengths):
+        switchDesc['cases'][str(length)] = charSwitch(
+            list(sorted(prop for prop in instrumentedProps
+                        if len(prop[1]) == length)),
+            0)
+
+    return fill(
+        """
+        if (StaticPrefs::${pref}() && JSID_IS_ATOM(id)) {
+          Maybe<UseCounter> counter;
+          {
+            // Scope for our no-GC section, so we don't need to rely on SetUseCounter not GCing.
+            JS::AutoCheckCannotGC nogc;
+            JSFlatString* str = js::AtomToFlatString(JSID_TO_ATOM(id));
+            // Don't waste time fetching the chars until we've done the length switch.
+            $*{switch}
+          }
+          if (counter) {
+            SetUseCounter(proxy, *counter);
+          }
+        }
+
+        """,
+        pref=prefIdentifier(MISSING_PROP_PREF),
+        switch=gen_switch(switchDesc))
+
 
 class CGDOMJSProxyHandler_getOwnPropDescriptor(ClassMethod):
     def __init__(self, descriptor):
         args = [Argument('JSContext*', 'cx'),
                 Argument('JS::Handle<JSObject*>', 'proxy'),
                 Argument('JS::Handle<jsid>', 'id'),
                 Argument('bool', 'ignoreNamedProps'),
                 Argument('JS::MutableHandle<JS::PropertyDescriptor>', 'desc')]
@@ -11823,16 +11946,18 @@ class CGDOMJSProxyHandler_getOwnPropDesc
                   $*{callGetter}
                 }
 
                 """,
                 callGetter=CGProxyIndexedGetter(self.descriptor, templateValues).define())
         else:
             getIndexed = ""
 
+        missingPropUseCounters = missingPropUseCountersForDescriptor(self.descriptor)
+
         if self.descriptor.supportsNamedProperties():
             operations = self.descriptor.operations
             readonly = toStringBool(operations['NamedSetter'] is None)
             fillDescriptor = (
                 "FillPropertyDescriptor(desc, proxy, %s, %s);\n"
                 "return true;\n" %
                 (readonly,
                  toStringBool(self.descriptor.namedPropertiesEnumerable)))
@@ -11878,16 +12003,17 @@ class CGDOMJSProxyHandler_getOwnPropDesc
             namedGet += "\n"
         else:
             namedGet = ""
 
         return fill(
             """
             $*{xrayDecl}
             $*{getIndexed}
+            $*{missingPropUseCounters}
             JS::Rooted<JSObject*> expando(cx);
             if (${xrayCheck}(expando = GetExpandoObject(proxy))) {
               if (!JS_GetOwnPropertyDescriptorById(cx, expando, id, desc)) {
                 return false;
               }
               if (desc.object()) {
                 // Pretend the property lives on the wrapper.
                 desc.object().set(proxy);
@@ -11897,16 +12023,17 @@ class CGDOMJSProxyHandler_getOwnPropDesc
 
             $*{namedGet}
             desc.object().set(nullptr);
             return true;
             """,
             xrayDecl=xrayDecl,
             xrayCheck=xrayCheck,
             getIndexed=getIndexed,
+            missingPropUseCounters=missingPropUseCounters,
             namedGet=namedGet)
 
 
 class CGDOMJSProxyHandler_defineProperty(ClassMethod):
     def __init__(self, descriptor):
         # The usual convention is to name the ObjectOpResult out-parameter
         # `result`, but that name is a bit overloaded around here.
         args = [Argument('JSContext*', 'cx'),
@@ -12387,38 +12514,42 @@ class CGDOMJSProxyHandler_hasOwn(ClassMe
                     """,
                     protoLacksProperty=named)
                 named += "*bp = false;\n"
             else:
                 named += "\n"
         else:
             named = "*bp = false;\n"
 
+        missingPropUseCounters = missingPropUseCountersForDescriptor(self.descriptor)
+
         return fill(
             """
             MOZ_ASSERT(!xpc::WrapperFactory::IsXrayWrapper(proxy),
                       "Should not have a XrayWrapper here");
             $*{maybeCrossOrigin}
             $*{indexed}
+            $*{missingPropUseCounters}
 
             JS::Rooted<JSObject*> expando(cx, GetExpandoObject(proxy));
             if (expando) {
               bool b = true;
               bool ok = JS_HasPropertyById(cx, expando, id, &b);
               *bp = !!b;
               if (!ok || *bp) {
                 return ok;
               }
             }
 
             $*{named}
             return true;
             """,
             maybeCrossOrigin=maybeCrossOrigin,
             indexed=indexed,
+            missingPropUseCounters=missingPropUseCounters,
             named=named)
 
 
 class CGDOMJSProxyHandler_get(ClassMethod):
     def __init__(self, descriptor):
         args = [Argument('JSContext*', 'cx'),
                 Argument('JS::Handle<JSObject*>', 'proxy'),
                 Argument('JS::Handle<JS::Value>', 'receiver'),
@@ -12435,16 +12566,18 @@ class CGDOMJSProxyHandler_get(ClassMetho
                 if (!IsPlatformObjectSameOrigin(cx, proxy)) {
                   return CrossOriginGet(cx, proxy, receiver, id, vp);
                 }
 
                 """)
         else:
             maybeCrossOriginGet = ""
 
+        missingPropUseCounters = missingPropUseCountersForDescriptor(self.descriptor)
+
         getUnforgeableOrExpando = dedent("""
             { // Scope for expando
               JS::Rooted<JSObject*> expando(cx, DOMProxyHandler::GetExpandoObject(proxy));
               if (expando) {
                 if (!JS_HasPropertyById(cx, expando, id, &expandoHasProp)) {
                   return false;
                 }
 
@@ -12573,23 +12706,25 @@ class CGDOMJSProxyHandler_get(ClassMetho
         return fill(
             """
             MOZ_ASSERT(!xpc::WrapperFactory::IsXrayWrapper(proxy),
                         "Should not have a XrayWrapper here");
 
             $*{maybeCrossOriginGet}
             JS::Rooted<JS::Value> rootedReceiver(cx, receiver);
 
+            $*{missingPropUseCounters}
             $*{indexedOrExpando}
 
             $*{named}
             vp.setUndefined();
             return true;
             """,
             maybeCrossOriginGet=maybeCrossOriginGet,
+            missingPropUseCounters=missingPropUseCounters,
             indexedOrExpando=getIndexedOrExpando,
             named=getNamed)
 
 
 class CGDOMJSProxyHandler_setCustom(ClassMethod):
     def __init__(self, descriptor):
         args = [Argument('JSContext*', 'cx'),
                 Argument('JS::Handle<JSObject*>', 'proxy'),
@@ -14819,22 +14954,30 @@ class CGBindingRoot(CGThing):
         bindingHeaders["mozilla/dom/DOMJSClass.h"] = descriptors
         bindingHeaders["mozilla/dom/ScriptSettings.h"] = dictionaries  # AutoJSAPI
         # Ensure we see our enums in the generated .cpp file, for the ToJSValue
         # method body.  Also ensure that we see jsapi.h.
         if enums:
             bindingHeaders[CGHeaders.getDeclarationFilename(enums[0])] = True
             bindingHeaders["jsapi.h"] = True
 
-        # For things that have [UseCounter]
-        def descriptorRequiresTelemetry(desc):
-            iface = desc.interface
-            return any(m.getExtendedAttribute("UseCounter") for m in iface.members)
-        bindingHeaders["mozilla/UseCounter.h"] = any(
-            descriptorRequiresTelemetry(d) for d in descriptors)
+        # For things that have [UseCounter] or [InstrumentedProps]
+        descriptorsHaveUseCounters = any(
+            m.getExtendedAttribute("UseCounter")
+            for d in descriptors
+            for m in d.interface.members)
+        descriptorsHaveInstrumentedProps = any(
+            d.instrumentedProps for d in descriptors if d.concrete)
+
+        bindingHeaders["mozilla/UseCounter.h"] = (
+            descriptorsHaveUseCounters or descriptorsHaveInstrumentedProps)
+        # Make sure to not overwrite existing pref header bits!
+        bindingHeaders[prefHeader(MISSING_PROP_PREF)] = (
+            bindingHeaders.get(prefHeader(MISSING_PROP_PREF)) or
+            descriptorsHaveInstrumentedProps)
         bindingHeaders["mozilla/dom/SimpleGlobalObject.h"] = any(
             CGDictionary.dictionarySafeToJSONify(d) for d in dictionaries)
         bindingHeaders["XrayWrapper.h"] = any(
             d.wantsXrays and d.wantsXrayExpandoClass for d in descriptors)
         bindingHeaders["mozilla/dom/XrayExpandoClass.h"] = any(
             d.wantsXrays for d in descriptors)
         bindingHeaders["mozilla/dom/StructuredCloneTags.h"] = any(
             d.interface.isSerializable() for d in descriptors)
--- a/dom/bindings/Configuration.py
+++ b/dom/bindings/Configuration.py
@@ -428,29 +428,37 @@ class Descriptor(DescriptorProvider):
             for m in self.interface.members:
                 if m.isMethod() and m.isStringifier():
                     addOperation('Stringifier', m)
                 if m.isMethod() and m.isDefaultToJSON():
                     self.hasDefaultToJSON = True
 
         if self.concrete:
             self.proxy = False
+            self.instrumentedProps = []
             iface = self.interface
             for m in iface.members:
                 # Don't worry about inheriting legacycallers either: in
                 # practice these are on most-derived prototypes.
                 if m.isMethod() and m.isLegacycaller():
                     if not m.isIdentifierLess():
                         raise TypeError("We don't support legacycaller with "
                                         "identifier.\n%s" % m.location)
                     if len(m.signatures()) != 1:
                         raise TypeError("We don't support overloaded "
                                         "legacycaller.\n%s" % m.location)
                     addOperation('LegacyCaller', m)
             while iface:
+                instrumentedProps = iface.getExtendedAttribute("InstrumentedProps")
+                if instrumentedProps:
+                    # It's actually a one-element list, with the list
+                    # we want as the only element.
+                    for prop in instrumentedProps[0]:
+                        self.instrumentedProps.append((iface.identifier.name,
+                                                       prop))
                 for m in iface.members:
                     if not m.isMethod():
                         continue
 
                     def addIndexedOrNamedOperation(operation, m):
                         if m.isIndexed():
                             operation = 'Indexed' + operation
                         else:
@@ -467,16 +475,31 @@ class Descriptor(DescriptorProvider):
                     if m.isLegacycaller() and iface != self.interface:
                         raise TypeError("We don't support legacycaller on "
                                         "non-leaf interface %s.\n%s" %
                                         (iface, iface.location))
 
                 iface.setUserData('hasConcreteDescendant', True)
                 iface = iface.parent
 
+            # Check that we don't have duplicated instrumented props.
+            uniqueInstrumentedProps = set(prop[1] for prop in self.instrumentedProps)
+            if len(uniqueInstrumentedProps) != len(self.instrumentedProps):
+                for prop in self.instrumentedProps:
+                    name = prop[1]
+                    if name in uniqueInstrumentedProps:
+                        uniqueInstrumentedProps.remove(name)
+                    else:
+                        ifaces = list(
+                            entry[0] for entry in self.instrumentedProps if
+                            entry[1] == name)
+                        raise TypeError(
+                            "Duplicated instrumented property '%s' defined on "
+                            "these interfaces: %s." % (name, str(ifaces)))
+
             self.proxy = (self.supportsIndexedProperties() or
                           (self.supportsNamedProperties() and
                            not self.hasNamedPropertiesObject) or
                           self.isMaybeCrossOriginObject())
 
             if self.proxy:
                 if (self.isMaybeCrossOriginObject() and
                     (self.supportsIndexedProperties() or
--- a/dom/bindings/parser/WebIDL.py
+++ b/dom/bindings/parser/WebIDL.py
@@ -1770,16 +1770,21 @@ class IDLInterface(IDLInterfaceOrNamespa
                   identifier == "JSImplementation" or
                   identifier == "HeaderFile" or
                   identifier == "Func" or
                   identifier == "Deprecated"):
                 # Known extended attributes that take a string value
                 if not attr.hasValue():
                     raise WebIDLError("[%s] must have a value" % identifier,
                                       [attr.location])
+            elif identifier == "InstrumentedProps":
+                # Known extended attributes that take a list
+                if not attr.hasArgs():
+                    raise WebIDLError("[%s] must have arguments" % identifier,
+                                      [attr.location])
             else:
                 raise WebIDLError("Unknown extended attribute %s on interface" % identifier,
                                   [attr.location])
 
             attrlist = attr.listValue()
             self._extendedAttrDict[identifier] = attrlist if len(attrlist) else True
 
     def validate(self):
--- a/dom/webidl/HTMLDocument.webidl
+++ b/dom/webidl/HTMLDocument.webidl
@@ -1,13 +1,44 @@
 /* -*- 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/.
  */
 
 [OverrideBuiltins,
- Exposed=Window]
+ Exposed=Window,
+ InstrumentedProps=(adoptedStyleSheets,
+                    caretRangeFromPoint,
+                    clear,
+                    exitPictureInPicture,
+                    featurePolicy,
+                    onbeforecopy,
+                    onbeforecut,
+                    onbeforepaste,
+                    oncancel,
+                    onfreeze,
+                    onmousewheel,
+                    onresume,
+                    onsearch,
+                    onsecuritypolicyviolation,
+                    onwebkitfullscreenchange,
+                    onwebkitfullscreenerror,
+                    pictureInPictureElement,
+                    pictureInPictureEnabled,
+                    registerElement,
+                    wasDiscarded,
+                    webkitCancelFullScreen,
+                    webkitCurrentFullScreenElement,
+                    webkitExitFullscreen,
+                    webkitFullscreenElement,
+                    webkitFullscreenEnabled,
+                    webkitHidden,
+                    webkitIsFullScreen,
+                    webkitVisibilityState,
+                    xmlEncoding,
+                    xmlStandalone,
+                    xmlVersion)]
 interface HTMLDocument : Document {
   // DOM tree accessors
   [Throws]
   getter object (DOMString name);
 };
--- a/modules/libpref/init/StaticPrefList.yaml
+++ b/modules/libpref/init/StaticPrefList.yaml
@@ -1766,16 +1766,22 @@
   mirror: always
 
 # Timeout clamp in ms for background windows.
 - name: dom.min_background_timeout_value
   type: int32_t
   value: 1000
   mirror: always
 
+# Are missing-property use counters for certain DOM attributes enabled?
+- name: dom.missing_prop_counters.enabled
+  type: bool
+  value: false
+  mirror: always
+
 # Is support for module scripts (<script type="module">) enabled for content?
 - name: dom.moduleScripts.enabled
   type: bool
   value: true
   mirror: always
 
 # Is support for mozBrowser frames enabled?
 - name: dom.mozBrowserFramesEnabled