diff --git a/dom/svg/crashtests/1531578-1.html b/dom/svg/crashtests/1531578-1.html new file mode 100644 index 000000000000..6fb1077c8f51 --- /dev/null +++ b/dom/svg/crashtests/1531578-1.html @@ -0,0 +1,17 @@ + + + + + + + + diff --git a/dom/svg/crashtests/crashtests.list b/dom/svg/crashtests/crashtests.list index 5c83f59a8f52..0fd9de524383 100644 --- a/dom/svg/crashtests/crashtests.list +++ b/dom/svg/crashtests/crashtests.list @@ -91,4 +91,5 @@ load 1477853.html load 1486488.html load 1493447.html skip-if(Android) load 1507961-1.html # times out on Android due to the test size +load 1531578-1.html load test_nested_svg.html diff --git a/js/src/jit-test/tests/basic/bug1403679.js b/js/src/jit-test/tests/basic/bug1403679.js new file mode 100644 index 000000000000..72bb0462032d --- /dev/null +++ b/js/src/jit-test/tests/basic/bug1403679.js @@ -0,0 +1,195 @@ +load(libdir + "asserts.js"); + +const thisGlobal = this; +const otherGlobalSameCompartment = newGlobal({sameCompartmentAs: thisGlobal}); +const otherGlobalNewCompartment = newGlobal({newCompartment: true}); + +const globals = [thisGlobal, otherGlobalSameCompartment, otherGlobalNewCompartment]; + +function testWithOptions(fn, variants = [undefined]) { + for (let variant of variants) { + for (let global of globals) { + for (let options of [ + {}, + {proxy: true}, + {object: new FakeDOMObject()}, + ]) { + fn(options, global, variant); + } + } + } +} + +function testWithGlobals(fn) { + for (let global of globals) { + fn(global); + } +} + +function testBasic(options, global) { + let {object: source, transplant} = transplantableObject(options); + + // Validate that |source| is an object and |transplant| is a function. + assertEq(typeof source, "object"); + assertEq(typeof transplant, "function"); + + // |source| is created in the current global. + assertEq(objectGlobal(source), this); + + // |source|'s prototype is %ObjectPrototype%, unless it's a FakeDOMObject. + let oldPrototype; + if (options.object) { + oldPrototype = FakeDOMObject.prototype; + } else { + oldPrototype = Object.prototype; + } + assertEq(Object.getPrototypeOf(source), oldPrototype); + + // Properties can be created on |source|. + assertEq(source.foo, undefined); + source.foo = 1; + assertEq(source.foo, 1); + + // Calling |transplant| transplants the object and then returns undefined. + assertEq(transplant(global), undefined); + + // |source| was moved into the new global. If the new global is in a + // different compartment, |source| is a now a CCW. + if (global !== otherGlobalNewCompartment) { + assertEq(objectGlobal(source), global); + } else { + assertEq(objectGlobal(source), null); + assertEq(isProxy(source), true); + } + + // The properties are copied over to the swapped object. + assertEq(source.foo, 1); + + // The prototype was changed to %ObjectPrototype% of |global| or the + // FakeDOMObject.prototype. + let newPrototype; + if (options.object) { + newPrototype = global.FakeDOMObject.prototype; + } else { + newPrototype = global.Object.prototype; + } + assertEq(Object.getPrototypeOf(source), newPrototype); +} +testWithOptions(testBasic); + +// Objects can be transplanted multiple times between globals. +function testTransplantMulti(options, global1, global2) { + let {object: source, transplant} = transplantableObject(options); + + transplant(global1); + transplant(global2); +} +testWithOptions(testTransplantMulti, globals); + +// Test the case when the source object already has a wrapper in the target global. +function testHasWrapperInTarget(options, global) { + let {object: source, transplant} = transplantableObject(options); + + // Create a wrapper for |source| in the other global. + global.p = source; + assertEq(global.eval("p"), source); + + if (options.proxy) { + // It's a proxy object either way. + assertEq(global.eval("isProxy(p)"), true); + } else { + if (global === otherGlobalNewCompartment) { + // |isProxy| returns true because |p| is a CCW. + assertEq(global.eval("isProxy(p)"), true); + } else { + // |isProxy| returns false because |p| is not a CCW. + assertEq(global.eval("isProxy(p)"), false); + } + } + + // And now transplant it into that global. + transplant(global); + + assertEq(global.eval("p"), source); + + if (options.proxy) { + // It's a proxy object either way. + assertEq(global.eval("isProxy(p)"), true); + } else { + // The previous CCW was replaced with a same-compartment object. + assertEq(global.eval("isProxy(p)"), false); + } +} +testWithOptions(testHasWrapperInTarget); + +// Test the case when the source object has a wrapper, but in a different compartment. +function testHasWrapperOtherCompartment(options, global) { + let thirdGlobal = newGlobal({newCompartment: true}); + let {object: source, transplant} = transplantableObject(options); + + // Create a wrapper for |source| in the new global. + thirdGlobal.p = source; + assertEq(thirdGlobal.eval("p"), source); + + // And now transplant the object. + transplant(global); + + assertEq(thirdGlobal.eval("p"), source); +} +testWithOptions(testHasWrapperOtherCompartment); + +// Ensure a transplanted object is correctly handled by (weak) collections. +function testCollections(options, global, AnySet) { + let {object, transplant} = transplantableObject(options); + + let set = new AnySet(); + + assertEq(set.has(object), false); + set.add(object); + assertEq(set.has(object), true); + + transplant(global); + + assertEq(set.has(object), true); +} +testWithOptions(testCollections, [Set, WeakSet]); + +// Ensure DOM object slot is correctly transplanted. +function testDOMObjectSlot(global) { + let domObject = new FakeDOMObject(); + let expectedValue = domObject.x; + assertEq(typeof expectedValue, "number"); + + let {object, transplant} = transplantableObject({object: domObject}); + assertEq(object, domObject); + + transplant(global); + + assertEq(object, domObject); + assertEq(domObject.x, expectedValue); +} +testWithGlobals(testDOMObjectSlot); + +function testArgumentValidation() { + // Throws an error if too many arguments are present. + assertThrowsInstanceOf(() => transplantableObject(thisGlobal, {}), Error); + + let {object, transplant} = transplantableObject(); + + // Throws an error if called with no arguments. + assertThrowsInstanceOf(() => transplant(), Error); + + // Throws an error if called with too many arguments. + assertThrowsInstanceOf(() => transplant(thisGlobal, {}), Error); + + // Throws an error if the first argument isn't an object + assertThrowsInstanceOf(() => transplant(null), Error); + + // Throws an error if the argument isn't a global object. + assertThrowsInstanceOf(() => transplant({}), Error); + + // Throws an error if the 'object' option isn't a FakeDOMObject. + assertThrowsInstanceOf(() => transplant({object: null}), Error); + assertThrowsInstanceOf(() => transplant({object: {}}), Error); +} +testArgumentValidation(); diff --git a/js/src/shell/js.cpp b/js/src/shell/js.cpp index 4081a1294248..f0c74c22d523 100644 --- a/js/src/shell/js.cpp +++ b/js/src/shell/js.cpp @@ -98,6 +98,7 @@ #include "js/MemoryFunctions.h" #include "js/Printf.h" #include "js/PropertySpec.h" +#include "js/Realm.h" #include "js/SourceText.h" #include "js/StableStringChars.h" #include "js/StructuredClone.h" @@ -8060,6 +8061,287 @@ static bool WasmLoop(JSContext* cx, unsigned argc, Value* vp) { #endif } +static constexpr uint32_t DOM_OBJECT_SLOT = 0; + +static const JSClass* GetDomClass(); + +static JSObject* GetDOMPrototype(JSObject* global); + +static const JSClass TransplantableDOMObjectClass = { + "TransplantableDOMObject", + JSCLASS_IS_DOMJSCLASS | JSCLASS_HAS_RESERVED_SLOTS(1)}; + +static const Class TransplantableDOMProxyObjectClass = + PROXY_CLASS_DEF("TransplantableDOMProxyObject", + JSCLASS_IS_DOMJSCLASS | JSCLASS_HAS_RESERVED_SLOTS(1)); + +class TransplantableDOMProxyHandler final : public ForwardingProxyHandler { + public: + static const TransplantableDOMProxyHandler singleton; + static const char family; + + constexpr TransplantableDOMProxyHandler() : ForwardingProxyHandler(&family) {} + + // These two proxy traps are called in |js::DeadProxyTargetValue|, which in + // turn is called when nuking proxies. Because this proxy can temporarily be + // without an object in its private slot, see |EnsureExpandoObject|, the + // default implementation inherited from ForwardingProxyHandler can't be used, + // since it tries to derive the callable/constructible value from the target. + bool isCallable(JSObject* obj) const override { return false; } + bool isConstructor(JSObject* obj) const override { return false; } + + // Simplified implementation of |DOMProxyHandler::GetAndClearExpandoObject|. + static JSObject* GetAndClearExpandoObject(JSObject* obj) { + const Value& v = GetProxyPrivate(obj); + if (v.isUndefined()) { + return nullptr; + } + + JSObject* expandoObject = &v.toObject(); + SetProxyPrivate(obj, UndefinedValue()); + return expandoObject; + } + + // Simplified implementation of |DOMProxyHandler::EnsureExpandoObject|. + static JSObject* EnsureExpandoObject(JSContext* cx, JS::HandleObject obj) { + const Value& v = GetProxyPrivate(obj); + if (v.isObject()) { + return &v.toObject(); + } + MOZ_ASSERT(v.isUndefined()); + + JSObject* expando = JS_NewObjectWithGivenProto(cx, nullptr, nullptr); + if (!expando) { + return nullptr; + } + SetProxyPrivate(obj, ObjectValue(*expando)); + return expando; + } +}; + +const TransplantableDOMProxyHandler TransplantableDOMProxyHandler::singleton; +const char TransplantableDOMProxyHandler::family = 0; + +enum TransplantObjectSlots { + TransplantSourceObject = 0, +}; + +static bool TransplantObject(JSContext* cx, unsigned argc, Value* vp) { + CallArgs args = CallArgsFromVp(argc, vp); + RootedFunction callee(cx, &args.callee().as()); + + if (args.length() != 1 || !args[0].isObject()) { + JS_ReportErrorASCII(cx, "transplant() must be called with an object"); + return false; + } + + // |newGlobal| needs to be a GlobalObject. + RootedObject newGlobal(cx, CheckedUnwrapDynamic(&args[0].toObject(), cx)); + if (!newGlobal) { + ReportAccessDenied(cx); + return false; + } + if (!JS_IsGlobalObject(newGlobal)) { + JS_ReportErrorNumberASCII( + cx, GetErrorMessage, nullptr, JSMSG_UNEXPECTED_TYPE, + "\"global\" passed to transplant()", "not a global object"); + return false; + } + + const Value& reserved = + GetFunctionNativeReserved(callee, TransplantSourceObject); + RootedObject source(cx, CheckedUnwrapStatic(&reserved.toObject())); + if (!source) { + ReportAccessDenied(cx); + return false; + } + MOZ_ASSERT(source->getClass()->isDOMClass()); + + // The following steps aim to replicate the behavior of UpdateReflectorGlobal + // in dom/bindings/BindingUtils.cpp. In detail: + // 1. Check the recursion depth using CheckRecursionLimitConservative. + // 2. Enter the target compartment. + // 3. Clone the source object using JS_CloneObject. + // 4. Copy all properties from source to a temporary holder object. + // 5. Actually transplant the object. + // 6. And finally copy the properties back to the source object. + // + // As an extension to the algorithm in UpdateReflectorGlobal, we also allow + // to transplant an object into the same compartment as the source object to + // cover all operations supported by JS_TransplantObject. + + if (!CheckRecursionLimitConservative(cx)) { + return false; + } + + bool isProxy = IsProxy(source); + RootedObject expandoObject(cx); + if (isProxy) { + expandoObject = + TransplantableDOMProxyHandler::GetAndClearExpandoObject(source); + } + + JSAutoRealm ar(cx, newGlobal); + + RootedObject proto(cx); + if (JS_GetClass(source) == GetDomClass()) { + proto = GetDOMPrototype(newGlobal); + } else { + proto = JS::GetRealmObjectPrototype(cx); + if (!proto) { + return false; + } + } + + RootedObject target(cx, JS_CloneObject(cx, source, proto)); + if (!target) { + return false; + } + + RootedObject copyFrom(cx, isProxy ? expandoObject : source); + RootedObject propertyHolder(cx, + JS_NewObjectWithGivenProto(cx, nullptr, nullptr)); + if (!propertyHolder) { + return false; + } + + if (!JS_CopyPropertiesFrom(cx, propertyHolder, copyFrom)) { + return false; + } + + SetReservedSlot(target, DOM_OBJECT_SLOT, + GetReservedSlot(source, DOM_OBJECT_SLOT)); + SetReservedSlot(source, DOM_OBJECT_SLOT, JS::PrivateValue(nullptr)); + + source = JS_TransplantObject(cx, source, target); + if (!source) { + return false; + } + + RootedObject copyTo(cx); + if (isProxy) { + copyTo = TransplantableDOMProxyHandler::EnsureExpandoObject(cx, source); + if (!copyTo) { + return false; + } + } else { + copyTo = source; + } + if (!JS_CopyPropertiesFrom(cx, copyTo, propertyHolder)) { + return false; + } + + args.rval().setUndefined(); + return true; +} + +static bool TransplantableObject(JSContext* cx, unsigned argc, Value* vp) { + CallArgs args = CallArgsFromVp(argc, vp); + RootedObject callee(cx, &args.callee()); + + if (args.length() > 1) { + ReportUsageErrorASCII(cx, callee, "Wrong number of arguments"); + return false; + } + + bool createProxy = false; + RootedObject source(cx); + if (args.length() == 1 && !args[0].isUndefined()) { + if (!args[0].isObject()) { + ReportUsageErrorASCII(cx, callee, "Argument must be an object"); + return false; + } + + RootedObject options(cx, &args[0].toObject()); + RootedValue value(cx); + + if (!JS_GetProperty(cx, options, "proxy", &value)) { + return false; + } + createProxy = JS::ToBoolean(value); + + if (!JS_GetProperty(cx, options, "object", &value)) { + return false; + } + if (!value.isUndefined()) { + if (!value.isObject()) { + ReportUsageErrorASCII(cx, callee, "'object' option must be an object"); + return false; + } + + source = &value.toObject(); + if (JS_GetClass(source) != GetDomClass()) { + ReportUsageErrorASCII(cx, callee, "Object not a FakeDOMObject"); + return false; + } + + // |source| must be a tenured object to be transplantable. + if (gc::IsInsideNursery(source)) { + JS_GC(cx); + + MOZ_ASSERT(!gc::IsInsideNursery(source), + "Live objects should be tenured after one GC, because " + "the nursery has only a single generation"); + } + } + } + + if (!source) { + if (!createProxy) { + source = NewBuiltinClassInstance( + cx, Valueify(&TransplantableDOMObjectClass), TenuredObject); + if (!source) { + return false; + } + + SetReservedSlot(source, DOM_OBJECT_SLOT, JS::PrivateValue(nullptr)); + } else { + JSObject* expando = JS_NewPlainObject(cx); + if (!expando) { + return false; + } + RootedValue expandoVal(cx, ObjectValue(*expando)); + + ProxyOptions options; + options.setClass(&TransplantableDOMProxyObjectClass); + options.setLazyProto(true); + + source = NewProxyObject(cx, &TransplantableDOMProxyHandler::singleton, + expandoVal, nullptr, options); + if (!source) { + return false; + } + + SetProxyReservedSlot(source, DOM_OBJECT_SLOT, JS::PrivateValue(nullptr)); + } + } + + jsid emptyId = NameToId(cx->names().empty); + RootedObject transplant( + cx, NewFunctionByIdWithReserved(cx, TransplantObject, 0, 0, emptyId)); + if (!transplant) { + return false; + } + + SetFunctionNativeReserved(transplant, TransplantSourceObject, + ObjectValue(*source)); + + RootedObject result(cx, JS_NewPlainObject(cx)); + if (!result) { + return false; + } + + RootedValue sourceVal(cx, ObjectValue(*source)); + RootedValue transplantVal(cx, ObjectValue(*transplant)); + if (!JS_DefineProperty(cx, result, "object", sourceVal, 0) || + !JS_DefineProperty(cx, result, "transplant", transplantVal, 0)) { + return false; + } + + args.rval().setObject(*result); + return true; +} + // clang-format off static const JSFunctionSpecWithHelp shell_functions[] = { JS_FN_HELP("clone", Clone, 1, 0, @@ -8705,6 +8987,19 @@ JS_FN_HELP("parseBin", BinParse, 1, 0, " wasm::Module into bytes, and deserialize those bytes in the current\n" " process, returning the resulting WebAssembly.Module."), + JS_FN_HELP("transplantableObject", TransplantableObject, 0, 0, +"transplantableObject([options])", +" Returns the pair {object, transplant}. |object| is an object which can be\n" +" transplanted into a new object when the |transplant| function, which must\n" +" be invoked with a global object, is called.\n" +" |object| is swapped with a cross-compartment wrapper if the global object\n" +" is in a different compartment.\n" +"\n" +" If options is given, it may have any of the following properties:\n" +" proxy: Create a DOM Proxy object instead of a plain DOM object.\n" +" object: Don't create a new DOM object, but instead use the supplied\n" +" FakeDOMObject."), + JS_FS_HELP_END }; // clang-format on @@ -9221,15 +9516,22 @@ static const JSClassOps global_classOps = {nullptr, nullptr, JS_GlobalObjectTraceHook}; -static const JSClass global_class = {"global", JSCLASS_GLOBAL_FLAGS, - &global_classOps}; +static constexpr uint32_t DOM_PROTOTYPE_SLOT = JSCLASS_GLOBAL_SLOT_COUNT; +static constexpr uint32_t DOM_GLOBAL_SLOTS = 1; + +static const JSClass global_class = { + "global", + JSCLASS_GLOBAL_FLAGS | JSCLASS_GLOBAL_FLAGS_WITH_SLOTS(DOM_GLOBAL_SLOTS), + &global_classOps}; /* * Define a FakeDOMObject constructor. It returns an object with a getter, * setter and method with attached JitInfo. This object can be used to test * IonMonkey DOM optimizations in the shell. */ -static const uint32_t DOM_OBJECT_SLOT = 0; + +/* Fow now just use to a constant we can check. */ +static const void* DOM_PRIVATE_VALUE = (void*)0x1234; static bool dom_genericGetter(JSContext* cx, unsigned argc, JS::Value* vp); @@ -9237,14 +9539,10 @@ static bool dom_genericSetter(JSContext* cx, unsigned argc, JS::Value* vp); static bool dom_genericMethod(JSContext* cx, unsigned argc, JS::Value* vp); -#ifdef DEBUG -static const JSClass* GetDomClass(); -#endif - static bool dom_get_x(JSContext* cx, HandleObject obj, void* self, JSJitGetterCallArgs args) { MOZ_ASSERT(JS_GetClass(obj) == GetDomClass()); - MOZ_ASSERT(self == (void*)0x1234); + MOZ_ASSERT(self == DOM_PRIVATE_VALUE); args.rval().set(JS_NumberValue(double(3.14))); return true; } @@ -9252,14 +9550,14 @@ static bool dom_get_x(JSContext* cx, HandleObject obj, void* self, static bool dom_set_x(JSContext* cx, HandleObject obj, void* self, JSJitSetterCallArgs args) { MOZ_ASSERT(JS_GetClass(obj) == GetDomClass()); - MOZ_ASSERT(self == (void*)0x1234); + MOZ_ASSERT(self == DOM_PRIVATE_VALUE); return true; } static bool dom_get_global(JSContext* cx, HandleObject obj, void* self, JSJitGetterCallArgs args) { MOZ_ASSERT(JS_GetClass(obj) == GetDomClass()); - MOZ_ASSERT(self == (void*)0x1234); + MOZ_ASSERT(self == DOM_PRIVATE_VALUE); // Return the current global (instead of obj->global()) to test cx->realm // switching in the JIT. @@ -9271,7 +9569,7 @@ static bool dom_get_global(JSContext* cx, HandleObject obj, void* self, static bool dom_set_global(JSContext* cx, HandleObject obj, void* self, JSJitSetterCallArgs args) { MOZ_ASSERT(JS_GetClass(obj) == GetDomClass()); - MOZ_ASSERT(self == (void*)0x1234); + MOZ_ASSERT(self == DOM_PRIVATE_VALUE); // Throw an exception if our argument is not the current global. This lets // us test cx->realm switching. @@ -9287,7 +9585,7 @@ static bool dom_set_global(JSContext* cx, HandleObject obj, void* self, static bool dom_doFoo(JSContext* cx, HandleObject obj, void* self, const JSJitMethodCallArgs& args) { MOZ_ASSERT(JS_GetClass(obj) == GetDomClass()); - MOZ_ASSERT(self == (void*)0x1234); + MOZ_ASSERT(self == DOM_PRIVATE_VALUE); MOZ_ASSERT(cx->realm() == args.callee().as().realm()); /* Just return args.length(). */ @@ -9401,9 +9699,7 @@ static const JSFunctionSpec dom_methods[] = { static const JSClass dom_class = { "FakeDOMObject", JSCLASS_IS_DOMJSCLASS | JSCLASS_HAS_RESERVED_SLOTS(2)}; -#ifdef DEBUG static const JSClass* GetDomClass() { return &dom_class; } -#endif static bool dom_genericGetter(JSContext* cx, unsigned argc, JS::Value* vp) { CallArgs args = CallArgsFromVp(argc, vp); @@ -9476,8 +9772,14 @@ static bool dom_genericMethod(JSContext* cx, unsigned argc, JS::Value* vp) { } static void InitDOMObject(HandleObject obj) { - /* Fow now just initialize to a constant we can check. */ - SetReservedSlot(obj, DOM_OBJECT_SLOT, PrivateValue((void*)0x1234)); + SetReservedSlot(obj, DOM_OBJECT_SLOT, + PrivateValue(const_cast(DOM_PRIVATE_VALUE))); +} + +static JSObject* GetDOMPrototype(JSObject* global) { + MOZ_ASSERT(JS_IsGlobalObject(global)); + MOZ_ASSERT(GetReservedSlot(global, DOM_PROTOTYPE_SLOT).isObject()); + return &GetReservedSlot(global, DOM_PROTOTYPE_SLOT).toObject(); } static bool dom_constructor(JSContext* cx, unsigned argc, JS::Value* vp) { @@ -9509,9 +9811,8 @@ static bool dom_constructor(JSContext* cx, unsigned argc, JS::Value* vp) { static bool InstanceClassHasProtoAtDepth(const Class* clasp, uint32_t protoID, uint32_t depth) { - // There's only a single (fake) DOM object in the shell, so just return - // true. - return true; + // Only the (fake) DOM object supports any JIT optimizations. + return clasp == Valueify(GetDomClass()); } static bool ShellBuildId(JS::BuildIdCharVector* buildId) { @@ -9640,6 +9941,11 @@ static JSObject* NewGlobalObject(JSContext* cx, JS::RealmOptions& options, return nullptr; } + // FakeDOMObject.prototype is the only DOM object which needs to retrieved + // in the shell; store it directly instead of creating a separate layer + // (ProtoAndIfaceCache) as done in the browser. + SetReservedSlot(glob, DOM_PROTOTYPE_SLOT, ObjectValue(*domProto)); + /* Initialize FakeDOMObject.prototype */ InitDOMObject(domProto); diff --git a/js/src/vm/JSObject.cpp b/js/src/vm/JSObject.cpp index 7cbda59fddcf..88b74454a61a 100644 --- a/js/src/vm/JSObject.cpp +++ b/js/src/vm/JSObject.cpp @@ -1357,7 +1357,10 @@ JSObject* js::CloneObject(JSContext* cx, HandleObject obj, RootedObject clone(cx); if (obj->isNative()) { - clone = NewObjectWithGivenTaggedProto(cx, obj->getClass(), proto); + // CloneObject is used to create the target object for JSObject::swap() and + // swap() requires its arguments are tenured, so ensure tenure allocation. + clone = NewObjectWithGivenTaggedProto(cx, obj->getClass(), proto, + NewObjectKind::TenuredObject); if (!clone) { return nullptr; } @@ -1377,8 +1380,17 @@ JSObject* js::CloneObject(JSContext* cx, HandleObject obj, ProxyOptions options; options.setClass(obj->getClass()); - clone = ProxyObject::New(cx, GetProxyHandler(obj), JS::NullHandleValue, - proto, options); + auto* handler = GetProxyHandler(obj); + + // Same as above, require tenure allocation of the clone. This means for + // proxy objects we need to reject nursery allocatable proxies. + if (handler->canNurseryAllocate()) { + JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, + JSMSG_CANT_CLONE_OBJECT); + return nullptr; + } + + clone = ProxyObject::New(cx, handler, JS::NullHandleValue, proto, options); if (!clone) { return nullptr; } diff --git a/layout/base/RestyleManager.cpp b/layout/base/RestyleManager.cpp index b272b509df7c..9ea3eab05a5b 100644 --- a/layout/base/RestyleManager.cpp +++ b/layout/base/RestyleManager.cpp @@ -1618,7 +1618,8 @@ void RestyleManager::ProcessRestyledFrames(nsStyleChangeList& aChangeList) { hint &= ~(nsChangeHint_UpdateOverflow | nsChangeHint_ChildrenOnlyTransform | nsChangeHint_UpdatePostTransformOverflow | - nsChangeHint_UpdateParentOverflow); + nsChangeHint_UpdateParentOverflow | + nsChangeHint_UpdateSubtreeOverflow); } if (primaryFrame && diff --git a/layout/style/test/property_database.js b/layout/style/test/property_database.js index 321f246c70ce..d915764d37b8 100644 --- a/layout/style/test/property_database.js +++ b/layout/style/test/property_database.js @@ -5594,8 +5594,8 @@ var gCSSProperties = { inherited: true, type: CSS_TYPE_LONGHAND, initial_values: [ "4" ], - other_values: [ "1", "7", "5000", "1.1" ], - invalid_values: [ "0.9", "0", "-1", "3px", "-0.3" ] + other_values: [ "0", "0.9", "1", "7", "5000", "1.1" ], + invalid_values: [ "-1", "3px", "-0.3" ] }, "stroke-opacity": { domProp: "strokeOpacity", diff --git a/layout/style/test/test_transitions_per_property.html b/layout/style/test/test_transitions_per_property.html index a8585bc5902f..c1b7c2fdec56 100644 --- a/layout/style/test/test_transitions_per_property.html +++ b/layout/style/test/test_transitions_per_property.html @@ -263,8 +263,9 @@ var supported_properties = { "stroke-dasharray": [ test_dasharray_transition ], "stroke-dashoffset": [ test_length_transition, test_percent_transition, test_length_unclamped, test_percent_unclamped, ], - "stroke-miterlimit": [ test_float_aboveOne_transition, - test_float_aboveOne_clamped ], + "stroke-miterlimit": [ test_float_zeroToOne_transition, + test_float_aboveOne_transition, + test_float_aboveZero_clamped ], "stroke-opacity" : [ test_float_zeroToOne_transition, // opacity is clamped in computed style // (not parsing/interpolation) @@ -1350,7 +1351,7 @@ function test_float_zeroToOne_clamped_or_unclamped(prop, is_clamped) { div.style.setProperty("transition-timing-function", "linear", ""); } -// Test using float values in the range [1, infinity) (e.g. stroke-miterlimit) +// Test using float values in the range [1, infinity) function test_float_aboveOne_transition(prop) { div.style.setProperty("transition-property", "none", ""); div.style.setProperty(prop, "1", ""); @@ -1363,15 +1364,15 @@ function test_float_aboveOne_transition(prop) { check_distance(prop, "1", "1.275", "2.1"); } -function test_float_aboveOne_clamped(prop) { +function test_float_aboveZero_clamped(prop) { div.style.setProperty("transition-timing-function", FUNC_NEGATIVE, ""); div.style.setProperty("transition-property", "none", ""); - div.style.setProperty(prop, "1", ""); - is(cs.getPropertyValue(prop), "1", + div.style.setProperty(prop, "0", ""); + is(cs.getPropertyValue(prop), "0", "float-valued property " + prop + ": flush before clamping test"); div.style.setProperty("transition-property", prop, ""); div.style.setProperty(prop, "5", ""); - is(cs.getPropertyValue(prop), "1", + is(cs.getPropertyValue(prop), "0", "float-valued property " + prop + ": clamping of negatives"); div.style.setProperty("transition-timing-function", "linear", ""); } diff --git a/layout/svg/SVGTextFrame.cpp b/layout/svg/SVGTextFrame.cpp index ea4677e95b9d..3e84d50a4ad2 100644 --- a/layout/svg/SVGTextFrame.cpp +++ b/layout/svg/SVGTextFrame.cpp @@ -3017,8 +3017,7 @@ void SVGTextFrame::ReflowSVGNonDisplayText() { // element is within a , say, the element referencing the will // be updated, which will then cause this SVGTextFrame to be painted and // in doing so cause the anonymous block frame to be reflowed. - nsLayoutUtils::PostRestyleEvent(mContent->AsElement(), nsRestyleHint(0), - nsChangeHint_InvalidateRenderingObservers); + SVGObserverUtils::InvalidateRenderingObservers(this); // Finally, we need to actually reflow the anonymous block frame and update // mPositions, in case we are being reflowed immediately after a DOM diff --git a/layout/svg/nsSVGUtils.cpp b/layout/svg/nsSVGUtils.cpp index b8f044d5b87e..ab9224db3622 100644 --- a/layout/svg/nsSVGUtils.cpp +++ b/layout/svg/nsSVGUtils.cpp @@ -1295,6 +1295,8 @@ gfxRect nsSVGUtils::PathExtentsToMaxStrokeExtents(const gfxRect& aPathExtents, // The stroke can extend even further for paths that can be affected by // stroke-miterlimit. + // We only need to do this if the limit is greater than 1, but it's probably + // not worth optimizing for that. bool affectedByMiterlimit = aFrame->GetContent()->IsAnyOfSVGElements( nsGkAtoms::path, nsGkAtoms::polyline, nsGkAtoms::polygon); diff --git a/servo/components/style/properties/longhands/inherited_svg.mako.rs b/servo/components/style/properties/longhands/inherited_svg.mako.rs index 35d0a02aab25..2bbdc2794c20 100644 --- a/servo/components/style/properties/longhands/inherited_svg.mako.rs +++ b/servo/components/style/properties/longhands/inherited_svg.mako.rs @@ -110,11 +110,11 @@ ${helpers.single_keyword( ${helpers.predefined_type( "stroke-miterlimit", - "GreaterThanOrEqualToOneNumber", + "NonNegativeNumber", "From::from(4.0)", products="gecko", - animation_value_type="crate::values::computed::GreaterThanOrEqualToOneNumber", - spec="https://www.w3.org/TR/SVG11/painting.html#StrokeMiterlimitProperty", + animation_value_type="crate::values::computed::NonNegativeNumber", + spec="https://www.w3.org/TR/SVG2/painting.html#StrokeMiterlimitProperty", )} ${helpers.predefined_type( diff --git a/testing/web-platform/meta/svg/painting/parsing/stroke-miterlimit-computed.svg.ini b/testing/web-platform/meta/svg/painting/parsing/stroke-miterlimit-computed.svg.ini deleted file mode 100644 index 42dd5f40d246..000000000000 --- a/testing/web-platform/meta/svg/painting/parsing/stroke-miterlimit-computed.svg.ini +++ /dev/null @@ -1,7 +0,0 @@ -[stroke-miterlimit-computed.svg] - [Property stroke-miterlimit value '0.5' computes to '0.5'] - expected: FAIL - - [Property stroke-miterlimit value '0' computes to '0'] - expected: FAIL - diff --git a/testing/web-platform/meta/svg/painting/parsing/stroke-miterlimit-valid.svg.ini b/testing/web-platform/meta/svg/painting/parsing/stroke-miterlimit-valid.svg.ini deleted file mode 100644 index 159a8e0ec3e0..000000000000 --- a/testing/web-platform/meta/svg/painting/parsing/stroke-miterlimit-valid.svg.ini +++ /dev/null @@ -1,7 +0,0 @@ -[stroke-miterlimit-valid.svg] - [e.style['stroke-miterlimit'\] = "0.5" should set the property value] - expected: FAIL - - [e.style['stroke-miterlimit'\] = "0" should set the property value] - expected: FAIL -