Bug 1375829 part 2. The default binding toJSON should skip over attributes that are not exposed in the current global. r=qdot
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 17 May 2018 23:39:52 -0400
changeset 418796 5a9cdc8de26a
parent 418795 db208ef2f3cb
child 418797 3db98d12d700
push id34013
push userdluca@mozilla.com
push date2018-05-18 09:56 +0000
treeherdermozilla-central@11ee70f24ea5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersqdot
bugs1375829
milestone62.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 1375829 part 2. The default binding toJSON should skip over attributes that are not exposed in the current global. r=qdot Without this, we will start including mozMemory in performance.toJSON() even if the pref for it is not set, once 'object' becomes a JSON type. This changes behavior in the following observable ways: 1) We stop exposing PerformanceResourceTiming's .serverTiming in the JSON serialization in insecure contexts. 2) We stop exposing PerformanceTiming's timeToNonBlankPaint and timeToDOMContentFlushed in the JSON serialization unless the relevant preferences are turned on.
dom/bindings/BindingUtils.cpp
dom/bindings/BindingUtils.h
dom/bindings/Codegen.py
dom/performance/tests/mochitest.ini
dom/performance/tests/test_performance_server_timing_plain_http.html
dom/performance/tests/test_performance_timing_json.html
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -4091,10 +4091,45 @@ GetPerInterfaceObjectHandle(JSContext* a
    */
 
   const JS::Heap<JSObject*>& entrySlot =
     protoAndIfaceCache.EntrySlotMustExist(aSlotId);
   MOZ_ASSERT(JS::ObjectIsNotGray(entrySlot));
   return JS::Handle<JSObject*>::fromMarkedLocation(entrySlot.address());
 }
 
+namespace binding_detail {
+bool
+IsGetterEnabled(JSContext* aCx, JS::Handle<JSObject*> aObj,
+                JSJitGetterOp aGetter,
+                const Prefable<const JSPropertySpec>* aAttributes)
+{
+  MOZ_ASSERT(aAttributes);
+  MOZ_ASSERT(aAttributes->specs);
+  do {
+    if (aAttributes->isEnabled(aCx, aObj)) {
+      const JSPropertySpec* specs = aAttributes->specs;
+      do {
+        MOZ_ASSERT(specs->isAccessor());
+        if (specs->isSelfHosted()) {
+          // It won't have a JSJitGetterOp.
+          continue;
+        }
+        const JSJitInfo* info = specs->accessors.getter.native.info;
+        if (!info) {
+          continue;
+        }
+        MOZ_ASSERT(info->type() == JSJitInfo::OpType::Getter);
+        if (info->getter == aGetter) {
+          return true;
+        }
+      } while ((++specs)->name);
+    }
+  } while ((++aAttributes)->specs);
+
+  // Didn't find it.
+  return false;
+}
+
+} // namespace binding_detail
+
 } // namespace dom
 } // namespace mozilla
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -3485,14 +3485,22 @@ namespace binding_detail {
 JSObject* UnprivilegedJunkScopeOrWorkerGlobal();
 
 // Implementation of the [HTMLConstructor] extended attribute.
 bool
 HTMLConstructor(JSContext* aCx, unsigned aArgc, JS::Value* aVp,
                 constructors::id::ID aConstructorId,
                 prototypes::id::ID aProtoId,
                 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);
 } // namespace binding_detail
 
 } // namespace dom
 } // namespace mozilla
 
 #endif /* mozilla_dom_BindingUtils_h__ */
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -2877,42 +2877,68 @@ class CGNativeProperties(CGList):
     def define(self):
         return CGList.define(self)
 
 
 class CGJsonifyAttributesMethod(CGAbstractMethod):
     """
     Generate the JsonifyAttributes method for an interface descriptor
     """
-    def __init__(self, descriptor):
+    def __init__(self, descriptor, jsonifierMethod):
         args = [Argument('JSContext*', 'aCx'),
                 Argument('JS::Handle<JSObject*>', 'obj'),
                 Argument('%s*' % descriptor.nativeType, 'self'),
                 Argument('JS::Rooted<JSObject*>&', 'aResult')]
         CGAbstractMethod.__init__(self, descriptor, 'JsonifyAttributes',
                                   'bool', args, canRunScript=True)
+        self.jsonifierMethod = jsonifierMethod
 
     def definition_body(self):
         ret = ''
         interface = self.descriptor.interface
+        jsonifierCondition = PropertyDefiner.getControllingCondition(self.jsonifierMethod,
+                                                                     self.descriptor)
         for m in interface.members:
             if m.isAttr() and not m.isStatic() and m.type.isJSONType():
-                ret += fill(
+                getAndDefine = fill(
                     """
-                    { // scope for "temp"
-                      JS::Rooted<JS::Value> temp(aCx);
-                      if (!get_${name}(aCx, obj, self, JSJitGetterCallArgs(&temp))) {
-                        return false;
-                      }
-                      if (!JS_DefineProperty(aCx, aResult, "${name}", temp, JSPROP_ENUMERATE)) {
-                        return false;
-                      }
+                    JS::Rooted<JS::Value> temp(aCx);
+                    if (!get_${name}(aCx, obj, self, JSJitGetterCallArgs(&temp))) {
+                      return false;
+                    }
+                    if (!JS_DefineProperty(aCx, aResult, "${name}", temp, JSPROP_ENUMERATE)) {
+                      return false;
                     }
                     """,
                     name=IDLToCIdentifier(m.identifier.name))
+                # Make sure we don't include things which are supposed to be
+                # disabled.  Things that either don't have disablers or whose
+                # disablers match the disablers for our jsonifier method can't
+                # possibly be disabled, but other things might be.
+                condition = PropertyDefiner.getControllingCondition(m, self.descriptor)
+                if condition.hasDisablers() and condition != jsonifierCondition:
+                    ret += fill(
+                        """
+                        // This is unfortunately a linear scan through sAttributes, but we
+                        // only do it for things which _might_ be disabled, which should
+                        // help keep the performance problems down.
+                        if (IsGetterEnabled(aCx, obj, (JSJitGetterOp)get_${name}, sAttributes)) {
+                          $*{getAndDefine}
+                        }
+                        """,
+                        name=IDLToCIdentifier(m.identifier.name),
+                        getAndDefine=getAndDefine)
+                else:
+                    ret += fill(
+                        """
+                        { // scope for "temp"
+                          $*{getAndDefine}
+                        }
+                        """,
+                        getAndDefine=getAndDefine)
         ret += 'return true;\n'
         return ret
 
 
 class CGCreateInterfaceObjectsMethod(CGAbstractMethod):
     """
     Generate the CreateInterfaceObjects method for an interface descriptor.
 
@@ -12360,17 +12386,16 @@ class CGDescriptor(CGThing):
                     cgThings.append(CGSpecializedLenientSetter(descriptor, m))
                 if (not m.isStatic() and
                     descriptor.interface.hasInterfacePrototypeObject()):
                     cgThings.append(CGMemberJITInfo(descriptor, m))
             if m.isConst() and m.type.isPrimitive():
                 cgThings.append(CGConstDefinition(m))
 
         if jsonifierMethod:
-            cgThings.append(CGJsonifyAttributesMethod(descriptor))
             cgThings.append(CGJsonifierMethod(descriptor, jsonifierMethod))
             cgThings.append(CGMemberJITInfo(descriptor, jsonifierMethod))
         if descriptor.interface.isNavigatorProperty():
             cgThings.append(CGConstructNavigatorObject(descriptor))
 
         if descriptor.concrete and not descriptor.proxy:
             if wantsAddProperty(descriptor):
                 cgThings.append(CGAddPropertyHook(descriptor))
@@ -12386,16 +12411,21 @@ class CGDescriptor(CGThing):
         if descriptor.interface.isJSImplemented():
             for m in clearableCachedAttrs(descriptor):
                 cgThings.append(CGJSImplClearCachedValueMethod(descriptor, m))
 
         properties = PropertyArrays(descriptor)
         cgThings.append(CGGeneric(define=str(properties)))
         cgThings.append(CGNativeProperties(descriptor, properties))
 
+        if jsonifierMethod:
+            # Now that we know about our property arrays, we can
+            # output our "jsonify attributes" method, which uses those.
+            cgThings.append(CGJsonifyAttributesMethod(descriptor, jsonifierMethod))
+
         if descriptor.interface.hasInterfaceObject():
             cgThings.append(CGClassConstructor(descriptor,
                                                descriptor.interface.ctor()))
             cgThings.append(CGInterfaceObjectJSClass(descriptor, properties))
             cgThings.append(CGNamedConstructors(descriptor))
 
         cgThings.append(CGLegacyCallHook(descriptor))
         if descriptor.interface.getExtendedAttribute("NeedResolve"):
--- a/dom/performance/tests/mochitest.ini
+++ b/dom/performance/tests/mochitest.ini
@@ -14,11 +14,12 @@ support-files =
 [test_performance_observer.html]
 [test_performance_user_timing.html]
 [test_worker_user_timing.html]
 [test_worker_observer.html]
 [test_sharedWorker_performance_user_timing.html]
 [test_worker_performance_now.html]
 [test_timeOrigin.html]
 [test_worker_performance_entries.html]
+[test_performance_timing_json.html]
 [test_performance_server_timing.html]
 scheme = https
 [test_performance_server_timing_plain_http.html]
--- a/dom/performance/tests/test_performance_server_timing_plain_http.html
+++ b/dom/performance/tests/test_performance_server_timing_plain_http.html
@@ -28,13 +28,15 @@ promise_test(t => {
     t.add_cleanup(() => observer.disconnect());
   });
 
   makeXHR("serverTiming.sjs");
 
   return promise.then(list => {
     assert_equals(list.getEntries().length, 1);
     assert_equals(list.getEntries()[0].serverTiming, undefined);
+    assert_equals(list.getEntries()[0].toJSON().serverTiming, undefined,
+		  "toJSON should not pick up properties that aren't on the object");
   });
 }, "server-timing test");
 
 </script>
 </body>
new file mode 100644
--- /dev/null
+++ b/dom/performance/tests/test_performance_timing_json.html
@@ -0,0 +1,32 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1375829
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 1375829</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+  <script type="application/javascript">
+
+  /** Test for Bug 1375829 **/
+  var json = performance.timing.toJSON();
+
+  // Ensure it doesn't have any attributes that performance.timing doesn't have
+  for (let key of Object.keys(json)) {
+    ok(key in performance.timing, key + " should be a property of performance.timing");
+  }
+
+  </script>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1375829">Mozilla Bug 1375829</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+
+</div>
+<pre id="test">
+</pre>
+</body>
+</html>