From 293303603102d8b168be966c6a76dcf60ea22f8d Mon Sep 17 00:00:00 2001 From: Jon Coppeard Date: Wed, 22 May 2019 17:54:07 +0100 Subject: [PATCH 01/14] Bug 1395509 - Track malloc memory associated with JSObject elements r=jandem This adds memory tracking for object elements. I had to swap the association in JSObject::swap. I also moved NativeObject::shrinkCapacityToInitializedLength out of line so as not to have to pull Zone.h into NativeObject.h. I don't know how performance sensitive this is - if it is I could look at this again. Differential Revision: https://phabricator.services.mozilla.com/D32171 --- js/src/gc/GCEnum.h | 3 +- js/src/gc/Marking.cpp | 14 ++++++--- js/src/gc/Scheduling.h | 17 +++++++---- js/src/gc/Zone.cpp | 31 ++++++++++++++++++++ js/src/gc/Zone.h | 3 ++ js/src/vm/JSObject-inl.h | 6 ++-- js/src/vm/JSObject.cpp | 5 +++- js/src/vm/NativeObject.cpp | 58 ++++++++++++++++++++++++++++++++++++-- js/src/vm/NativeObject.h | 21 +------------- 9 files changed, 123 insertions(+), 35 deletions(-) diff --git a/js/src/gc/GCEnum.h b/js/src/gc/GCEnum.h index ae5dec12e9b2..4f1d8cda7b2b 100644 --- a/js/src/gc/GCEnum.h +++ b/js/src/gc/GCEnum.h @@ -93,7 +93,8 @@ enum class ZealMode { #define JS_FOR_EACH_INTERNAL_MEMORY_USE(_) \ _(ArrayBufferContents) \ - _(StringContents) + _(StringContents) \ + _(ObjectElements) #define JS_FOR_EACH_MEMORY_USE(_) \ JS_FOR_EACH_PUBLIC_MEMORY_USE(_) \ diff --git a/js/src/gc/Marking.cpp b/js/src/gc/Marking.cpp index 4209b7f30026..5ab7989cf93f 100644 --- a/js/src/gc/Marking.cpp +++ b/js/src/gc/Marking.cpp @@ -2997,20 +2997,25 @@ size_t js::TenuringTracer::moveElementsToTenured(NativeObject* dst, return 0; } + Zone* zone = src->zone(); + + ObjectElements* srcHeader = src->getElementsHeader(); + size_t nslots = srcHeader->numAllocatedElements(); + void* srcAllocatedHeader = src->getUnshiftedElementsHeader(); /* TODO Bug 874151: Prefer to put element data inline if we have space. */ if (!nursery().isInside(srcAllocatedHeader)) { MOZ_ASSERT(src->elements_ == dst->elements_); nursery().removeMallocedBuffer(srcAllocatedHeader); + + AddCellMemory(dst, nslots * sizeof(HeapSlot), MemoryUse::ObjectElements); + return 0; } - ObjectElements* srcHeader = src->getElementsHeader(); - // Shifted elements are copied too. uint32_t numShifted = srcHeader->numShiftedElements(); - size_t nslots = srcHeader->numAllocatedElements(); /* Unlike other objects, Arrays can have fixed elements. */ if (src->is() && nslots <= GetGCKindSlots(dstKind)) { @@ -3025,7 +3030,6 @@ size_t js::TenuringTracer::moveElementsToTenured(NativeObject* dst, MOZ_ASSERT(nslots >= 2); - Zone* zone = src->zone(); ObjectElements* dstHeader; { AutoEnterOOMUnsafeRegion oomUnsafe; @@ -3037,6 +3041,8 @@ size_t js::TenuringTracer::moveElementsToTenured(NativeObject* dst, } } + AddCellMemory(dst, nslots * sizeof(HeapSlot), MemoryUse::ObjectElements); + js_memcpy(dstHeader, srcAllocatedHeader, nslots * sizeof(HeapSlot)); dst->elements_ = dstHeader->elements() + numShifted; nursery().setElementsForwardingPointer(srcHeader, dst->getElementsHeader(), diff --git a/js/src/gc/Scheduling.h b/js/src/gc/Scheduling.h index bbf934d521f0..f4f03d0b2d3d 100644 --- a/js/src/gc/Scheduling.h +++ b/js/src/gc/Scheduling.h @@ -681,6 +681,11 @@ class MemoryTracker { untrackMemory(cell, nbytes, use); #endif } + void swapMemory(Cell* a, Cell* b, MemoryUse use) { +#ifdef DEBUG + swapTrackedMemory(a, b, use); +#endif + } size_t bytes() const { return bytes_; } @@ -692,9 +697,6 @@ class MemoryTracker { bytes_; #ifdef DEBUG - void trackMemory(Cell* cell, size_t nbytes, MemoryUse use); - void untrackMemory(Cell* cell, size_t nbytes, MemoryUse use); - struct Key { Key(Cell* cell, MemoryUse use); Cell* cell() const; @@ -718,9 +720,14 @@ class MemoryTracker { static void rekey(Key& k, const Key& newKey); }; - Mutex mutex; - using Map = HashMap; + + void trackMemory(Cell* cell, size_t nbytes, MemoryUse use); + void untrackMemory(Cell* cell, size_t nbytes, MemoryUse use); + void swapTrackedMemory(Cell* a, Cell* b, MemoryUse use); + size_t getAndRemoveEntry(const Key& key, LockGuard& lock); + + Mutex mutex; Map map; #endif }; diff --git a/js/src/gc/Zone.cpp b/js/src/gc/Zone.cpp index 773b0424c54a..1fd187bbf0aa 100644 --- a/js/src/gc/Zone.cpp +++ b/js/src/gc/Zone.cpp @@ -595,6 +595,37 @@ void MemoryTracker::untrackMemory(Cell* cell, size_t nbytes, MemoryUse use) { map.remove(ptr); } +void MemoryTracker::swapTrackedMemory(Cell* a, Cell* b, MemoryUse use) { + MOZ_ASSERT(a->isTenured()); + MOZ_ASSERT(b->isTenured()); + + Key ka{a, use}; + Key kb{b, use}; + + LockGuard lock(mutex); + + size_t sa = getAndRemoveEntry(ka, lock); + size_t sb = getAndRemoveEntry(kb, lock); + + AutoEnterOOMUnsafeRegion oomUnsafe; + + if ((sa && !map.put(kb, sa)) || + (sb && !map.put(ka, sb))) { + oomUnsafe.crash("MemoryTracker::swapTrackedMemory"); + } +} + +size_t MemoryTracker::getAndRemoveEntry(const Key& key, LockGuard& lock) { + auto ptr = map.lookup(key); + if (!ptr) { + return 0; + } + + size_t size = ptr->value(); + map.remove(ptr); + return size; +} + void MemoryTracker::fixupAfterMovingGC() { // Update the table after we move GC things. We don't use MovableCellHasher // because that would create a difference between debug and release builds. diff --git a/js/src/gc/Zone.h b/js/src/gc/Zone.h index a38621a9c00b..95ae295e403b 100644 --- a/js/src/gc/Zone.h +++ b/js/src/gc/Zone.h @@ -508,6 +508,9 @@ class Zone : public JS::shadow::Zone, void removeCellMemory(js::gc::Cell* cell, size_t nbytes, js::MemoryUse use) { gcMallocSize.removeMemory(cell, nbytes, use); } + void swapCellMemory(js::gc::Cell* a, js::gc::Cell* b, js::MemoryUse use) { + gcMallocSize.swapMemory(a, b, use); + } size_t totalBytes() const { return zoneSize.gcBytes() + gcMallocSize.bytes(); diff --git a/js/src/vm/JSObject-inl.h b/js/src/vm/JSObject-inl.h index 5147a2549a07..6e957839cab9 100644 --- a/js/src/vm/JSObject-inl.h +++ b/js/src/vm/JSObject-inl.h @@ -50,16 +50,18 @@ inline void JSObject::finalize(js::FreeOp* fop) { if (nobj->hasDynamicElements()) { js::ObjectElements* elements = nobj->getElementsHeader(); + size_t size = elements->numAllocatedElements() * sizeof(js::HeapSlot); if (elements->isCopyOnWrite()) { if (elements->ownerObject() == this) { // Don't free the elements until object finalization finishes, // so that other objects can access these elements while they // are themselves finalized. MOZ_ASSERT(elements->numShiftedElements() == 0); - fop->freeLater(elements); + fop->freeLater(this, elements, size, js::MemoryUse::ObjectElements); } } else { - fop->free_(nobj->getUnshiftedElementsHeader()); + fop->free_(this, nobj->getUnshiftedElementsHeader(), size, + js::MemoryUse::ObjectElements); } } diff --git a/js/src/vm/JSObject.cpp b/js/src/vm/JSObject.cpp index de47f6e6c662..85af12f78331 100644 --- a/js/src/vm/JSObject.cpp +++ b/js/src/vm/JSObject.cpp @@ -1866,6 +1866,10 @@ void JSObject::swap(JSContext* cx, HandleObject a, HandleObject b) { bool bIsProxyWithInlineValues = b->is() && b->as().usingInlineValueArray(); + // Swap element associations. + Zone* zone = a->zone(); + zone->swapCellMemory(a, b, MemoryUse::ObjectElements); + if (a->tenuredSizeOfThis() == b->tenuredSizeOfThis()) { // When both objects are the same size, just do a plain swap of their // contents. @@ -1985,7 +1989,6 @@ void JSObject::swap(JSContext* cx, HandleObject a, HandleObject b) { * Normally write barriers happen before the write. However, that's not * necessary here because nothing is being destroyed. We're just swapping. */ - JS::Zone* zone = a->zone(); if (zone->needsIncrementalBarrier()) { a->traceChildren(zone->barrierTracer()); b->traceChildren(zone->barrierTracer()); diff --git a/js/src/vm/NativeObject.cpp b/js/src/vm/NativeObject.cpp index 5bb5d0a99670..17a90e933328 100644 --- a/js/src/vm/NativeObject.cpp +++ b/js/src/vm/NativeObject.cpp @@ -905,10 +905,10 @@ bool NativeObject::growElements(JSContext* cx, uint32_t reqCapacity) { HeapSlot* oldHeaderSlots = reinterpret_cast(getUnshiftedElementsHeader()); HeapSlot* newHeaderSlots; + uint32_t oldAllocated = 0; if (hasDynamicElements()) { MOZ_ASSERT(oldCapacity <= MAX_DENSE_ELEMENTS_COUNT); - uint32_t oldAllocated = - oldCapacity + ObjectElements::VALUES_PER_HEADER + numShifted; + oldAllocated = oldCapacity + ObjectElements::VALUES_PER_HEADER + numShifted; newHeaderSlots = ReallocateObjectBuffer( cx, this, oldHeaderSlots, oldAllocated, newAllocated); @@ -924,12 +924,20 @@ bool NativeObject::growElements(JSContext* cx, uint32_t reqCapacity) { ObjectElements::VALUES_PER_HEADER + initlen + numShifted); } + if (oldAllocated) { + RemoveCellMemory(this, oldAllocated * sizeof(HeapSlot), + MemoryUse::ObjectElements); + } + ObjectElements* newheader = reinterpret_cast(newHeaderSlots); elements_ = newheader->elements() + numShifted; getElementsHeader()->capacity = newCapacity; Debug_SetSlotRangeToCrashOnTouch(elements_ + initlen, newCapacity - initlen); + AddCellMemory(this, newAllocated * sizeof(HeapSlot), + MemoryUse::ObjectElements); + return true; } @@ -980,9 +988,52 @@ void NativeObject::shrinkElements(JSContext* cx, uint32_t reqCapacity) { return; // Leave elements at its old size. } + RemoveCellMemory(this, oldAllocated * sizeof(HeapSlot), + MemoryUse::ObjectElements); + ObjectElements* newheader = reinterpret_cast(newHeaderSlots); elements_ = newheader->elements() + numShifted; getElementsHeader()->capacity = newCapacity; + + AddCellMemory(this, newAllocated * sizeof(HeapSlot), + MemoryUse::ObjectElements); +} + +void NativeObject::shrinkCapacityToInitializedLength(JSContext* cx) { + // When an array's length becomes non-writable, writes to indexes greater + // greater than or equal to the length don't change the array. We handle this + // with a check for non-writable length in most places. But in JIT code every + // check counts -- so we piggyback the check on the already-required range + // check for |index < capacity| by making capacity of arrays with non-writable + // length never exceed the length. This mechanism is also used when an object + // becomes non-extensible. + + if (getElementsHeader()->numShiftedElements() > 0) { + moveShiftedElements(); + } + + ObjectElements* header = getElementsHeader(); + uint32_t len = header->initializedLength; + MOZ_ASSERT(header->capacity >= len); + if (header->capacity == len) { + return; + } + + shrinkElements(cx, len); + + header = getElementsHeader(); + uint32_t oldAllocated = header->numAllocatedElements(); + header->capacity = len; + + // The size of the memory allocation hasn't changed but we lose the actual + // capacity information. Make the associated size match the updated capacity. + if (!hasFixedElements()) { + uint32_t newAllocated = header->numAllocatedElements(); + RemoveCellMemory(this, oldAllocated * sizeof(HeapSlot), + MemoryUse::ObjectElements); + AddCellMemory(this, newAllocated * sizeof(HeapSlot), + MemoryUse::ObjectElements); + } } /* static */ @@ -1022,6 +1073,9 @@ bool NativeObject::CopyElementsForWrite(JSContext* cx, NativeObject* obj) { Debug_SetSlotRangeToCrashOnTouch(obj->elements_ + initlen, newCapacity - initlen); + AddCellMemory(obj, newAllocated * sizeof(HeapSlot), + MemoryUse::ObjectElements); + return true; } diff --git a/js/src/vm/NativeObject.h b/js/src/vm/NativeObject.h index 9e1782cd5e65..bfb003380b13 100644 --- a/js/src/vm/NativeObject.h +++ b/js/src/vm/NativeObject.h @@ -1215,26 +1215,7 @@ class NativeObject : public JSObject { inline void elementsRangeWriteBarrierPost(uint32_t start, uint32_t count); public: - // When an array's length becomes non-writable, writes to indexes greater - // greater than or equal to the length don't change the array. We handle - // this with a check for non-writable length in most places. But in JIT code - // every check counts -- so we piggyback the check on the already-required - // range check for |index < capacity| by making capacity of arrays with - // non-writable length never exceed the length. This mechanism is also used - // when an object becomes non-extensible. - void shrinkCapacityToInitializedLength(JSContext* cx) { - if (getElementsHeader()->numShiftedElements() > 0) { - moveShiftedElements(); - } - - ObjectElements* header = getElementsHeader(); - uint32_t len = header->initializedLength; - if (header->capacity > len) { - shrinkElements(cx, len); - header = getElementsHeader(); - header->capacity = len; - } - } + void shrinkCapacityToInitializedLength(JSContext* cx); private: void setDenseInitializedLengthInternal(uint32_t length) { From 068fe93affd1156c58a28ed0fdffb11cd0e0ed33 Mon Sep 17 00:00:00 2001 From: Jon Coppeard Date: Wed, 22 May 2019 17:55:17 +0100 Subject: [PATCH 02/14] Bug 1395509 - Track malloc memory associated with JSObject slots r=jandem Differential Revision: https://phabricator.services.mozilla.com/D32172 --- js/src/gc/Allocator.cpp | 2 ++ js/src/gc/GCEnum.h | 3 ++- js/src/gc/Marking.cpp | 9 ++++++--- js/src/vm/JSObject-inl.h | 32 +++++++++++++++++++++++++++++++- js/src/vm/JSObject.cpp | 19 ++++++++++++++++--- js/src/vm/NativeObject-inl.h | 29 ----------------------------- js/src/vm/NativeObject.cpp | 29 +++++++++++------------------ js/src/vm/NativeObject.h | 6 +----- 8 files changed, 69 insertions(+), 60 deletions(-) diff --git a/js/src/gc/Allocator.cpp b/js/src/gc/Allocator.cpp index 52c19c32f3ae..62c4a6eb2384 100644 --- a/js/src/gc/Allocator.cpp +++ b/js/src/gc/Allocator.cpp @@ -140,6 +140,8 @@ JSObject* GCRuntime::tryNewTenuredObject(JSContext* cx, AllocKind kind, if (obj) { if (nDynamicSlots) { static_cast(obj)->initSlots(slots); + AddCellMemory(obj, nDynamicSlots * sizeof(HeapSlot), + MemoryUse::ObjectSlots); } } else { js_free(slots); diff --git a/js/src/gc/GCEnum.h b/js/src/gc/GCEnum.h index 4f1d8cda7b2b..dd756b33ca1f 100644 --- a/js/src/gc/GCEnum.h +++ b/js/src/gc/GCEnum.h @@ -94,7 +94,8 @@ enum class ZealMode { #define JS_FOR_EACH_INTERNAL_MEMORY_USE(_) \ _(ArrayBufferContents) \ _(StringContents) \ - _(ObjectElements) + _(ObjectElements) \ + _(ObjectSlots) #define JS_FOR_EACH_MEMORY_USE(_) \ JS_FOR_EACH_PUBLIC_MEMORY_USE(_) \ diff --git a/js/src/gc/Marking.cpp b/js/src/gc/Marking.cpp index 5ab7989cf93f..e4c820be0c16 100644 --- a/js/src/gc/Marking.cpp +++ b/js/src/gc/Marking.cpp @@ -2968,14 +2968,15 @@ size_t js::TenuringTracer::moveSlotsToTenured(NativeObject* dst, return 0; } + Zone* zone = src->zone(); + size_t count = src->numDynamicSlots(); + if (!nursery().isInside(src->slots_)) { + AddCellMemory(dst, count * sizeof(HeapSlot), MemoryUse::ObjectSlots); nursery().removeMallocedBuffer(src->slots_); return 0; } - Zone* zone = src->zone(); - size_t count = src->numDynamicSlots(); - { AutoEnterOOMUnsafeRegion oomUnsafe; dst->slots_ = zone->pod_malloc(count); @@ -2985,6 +2986,8 @@ size_t js::TenuringTracer::moveSlotsToTenured(NativeObject* dst, } } + AddCellMemory(dst, count * sizeof(HeapSlot), MemoryUse::ObjectSlots); + PodCopy(dst->slots_, src->slots_, count); nursery().setSlotsForwardingPointer(src->slots_, dst->slots_, count); return count * sizeof(HeapSlot); diff --git a/js/src/vm/JSObject-inl.h b/js/src/vm/JSObject-inl.h index 6e957839cab9..9ebb67ebd6c9 100644 --- a/js/src/vm/JSObject-inl.h +++ b/js/src/vm/JSObject-inl.h @@ -20,6 +20,35 @@ #include "vm/ObjectOperations-inl.h" // js::MaybeHasInterestingSymbolProperty #include "vm/Realm-inl.h" +MOZ_ALWAYS_INLINE uint32_t js::NativeObject::numDynamicSlots() const { + return dynamicSlotsCount(numFixedSlots(), slotSpan(), getClass()); +} + +/* static */ MOZ_ALWAYS_INLINE uint32_t js::NativeObject::dynamicSlotsCount( + uint32_t nfixed, uint32_t span, const Class* clasp) { + if (span <= nfixed) { + return 0; + } + span -= nfixed; + + // Increase the slots to SLOT_CAPACITY_MIN to decrease the likelihood + // the dynamic slots need to get increased again. ArrayObjects ignore + // this because slots are uncommon in that case. + if (clasp != &ArrayObject::class_ && span <= SLOT_CAPACITY_MIN) { + return SLOT_CAPACITY_MIN; + } + + uint32_t slots = mozilla::RoundUpPow2(span); + MOZ_ASSERT(slots >= span); + return slots; +} + +/* static */ MOZ_ALWAYS_INLINE uint32_t +js::NativeObject::dynamicSlotsCount(Shape* shape) { + return dynamicSlotsCount(shape->numFixedSlots(), shape->slotSpan(), + shape->getObjectClass()); +} + inline void JSObject::finalize(js::FreeOp* fop) { js::probes::FinalizeObject(this); @@ -45,7 +74,8 @@ inline void JSObject::finalize(js::FreeOp* fop) { } if (nobj->hasDynamicSlots()) { - fop->free_(nobj->slots_); + size_t size = nobj->numDynamicSlots() * sizeof(js::HeapSlot); + fop->free_(this, nobj->slots_, size, js::MemoryUse::ObjectSlots); } if (nobj->hasDynamicElements()) { diff --git a/js/src/vm/JSObject.cpp b/js/src/vm/JSObject.cpp index 85af12f78331..409f22ec012d 100644 --- a/js/src/vm/JSObject.cpp +++ b/js/src/vm/JSObject.cpp @@ -1703,11 +1703,15 @@ template XDRResult js::XDRObjectLiteral(XDRState* xdr, /* static */ bool NativeObject::fillInAfterSwap(JSContext* cx, HandleNativeObject obj, - HandleValueVector values, void* priv) { + NativeObject* old, HandleValueVector values, + void* priv) { // This object has just been swapped with some other object, and its shape // no longer reflects its allocated size. Correct this information and // fill the slots in with the specified values. MOZ_ASSERT(obj->slotSpan() == values.length()); + MOZ_ASSERT(!IsInsideNursery(obj)); + + size_t oldSlotCount = obj->numDynamicSlots(); // Make sure the shape's numFixedSlots() is correct. size_t nfixed = @@ -1725,7 +1729,10 @@ bool NativeObject::fillInAfterSwap(JSContext* cx, HandleNativeObject obj, MOZ_ASSERT(!priv); } + Zone* zone = obj->zone(); if (obj->slots_) { + size_t size = oldSlotCount * sizeof(HeapSlot); + zone->removeCellMemory(old, size, MemoryUse::ObjectSlots); js_free(obj->slots_); obj->slots_ = nullptr; } @@ -1736,6 +1743,8 @@ bool NativeObject::fillInAfterSwap(JSContext* cx, HandleNativeObject obj, if (!obj->slots_) { return false; } + size_t size = ndynamic * sizeof(HeapSlot); + zone->addCellMemory(obj, size, MemoryUse::ObjectSlots); Debug_SetSlotRangeToCrashOnTouch(obj->slots_, ndynamic); } @@ -1873,6 +1882,10 @@ void JSObject::swap(JSContext* cx, HandleObject a, HandleObject b) { if (a->tenuredSizeOfThis() == b->tenuredSizeOfThis()) { // When both objects are the same size, just do a plain swap of their // contents. + + // Swap slot associations. + zone->swapCellMemory(a, b, MemoryUse::ObjectSlots); + size_t size = a->tenuredSizeOfThis(); char tmp[mozilla::tl::MaxfixDictionaryShapeAfterSwap(); if (na) { - if (!NativeObject::fillInAfterSwap(cx, b.as(), avals, + if (!NativeObject::fillInAfterSwap(cx, b.as(), na, avals, apriv)) { oomUnsafe.crash("fillInAfterSwap"); } } if (nb) { - if (!NativeObject::fillInAfterSwap(cx, a.as(), bvals, + if (!NativeObject::fillInAfterSwap(cx, a.as(), nb, bvals, bpriv)) { oomUnsafe.crash("fillInAfterSwap"); } diff --git a/js/src/vm/NativeObject-inl.h b/js/src/vm/NativeObject-inl.h index 1cab804bd940..8cc36491ecd9 100644 --- a/js/src/vm/NativeObject-inl.h +++ b/js/src/vm/NativeObject-inl.h @@ -534,35 +534,6 @@ NativeObject::createWithTemplate(JSContext* cx, HandleObject templateObject) { return create(cx, kind, heap, shape, group); } -MOZ_ALWAYS_INLINE uint32_t NativeObject::numDynamicSlots() const { - return dynamicSlotsCount(numFixedSlots(), slotSpan(), getClass()); -} - -/* static */ MOZ_ALWAYS_INLINE uint32_t NativeObject::dynamicSlotsCount( - uint32_t nfixed, uint32_t span, const Class* clasp) { - if (span <= nfixed) { - return 0; - } - span -= nfixed; - - // Increase the slots to SLOT_CAPACITY_MIN to decrease the likelihood - // the dynamic slots need to get increased again. ArrayObjects ignore - // this because slots are uncommon in that case. - if (clasp != &ArrayObject::class_ && span <= SLOT_CAPACITY_MIN) { - return SLOT_CAPACITY_MIN; - } - - uint32_t slots = mozilla::RoundUpPow2(span); - MOZ_ASSERT(slots >= span); - return slots; -} - -/* static */ MOZ_ALWAYS_INLINE uint32_t -NativeObject::dynamicSlotsCount(Shape* shape) { - return dynamicSlotsCount(shape->numFixedSlots(), shape->slotSpan(), - shape->getObjectClass()); -} - MOZ_ALWAYS_INLINE bool NativeObject::updateSlotsForSpan(JSContext* cx, size_t oldSpan, size_t newSpan) { diff --git a/js/src/vm/NativeObject.cpp b/js/src/vm/NativeObject.cpp index 17a90e933328..c4d643bd3865 100644 --- a/js/src/vm/NativeObject.cpp +++ b/js/src/vm/NativeObject.cpp @@ -321,24 +321,6 @@ void NativeObject::setLastPropertyShrinkFixedSlots(Shape* shape) { setShape(shape); } -void NativeObject::setLastPropertyMakeNonNative(Shape* shape) { - MOZ_ASSERT(!inDictionaryMode()); - MOZ_ASSERT(!shape->getObjectClass()->isNative()); - MOZ_ASSERT(shape->zone() == zone()); - MOZ_ASSERT(shape->slotSpan() == 0); - MOZ_ASSERT(shape->numFixedSlots() == 0); - - if (hasDynamicElements()) { - js_free(getUnshiftedElementsHeader()); - } - if (hasDynamicSlots()) { - js_free(slots_); - slots_ = nullptr; - } - - setShape(shape); -} - bool NativeObject::setSlotSpan(JSContext* cx, uint32_t span) { MOZ_ASSERT(inDictionaryMode()); @@ -375,6 +357,9 @@ bool NativeObject::growSlots(JSContext* cx, uint32_t oldCount, return false; } Debug_SetSlotRangeToCrashOnTouch(slots_, newCount); + + AddCellMemory(this, newCount * sizeof(HeapSlot), MemoryUse::ObjectSlots); + return true; } @@ -384,6 +369,9 @@ bool NativeObject::growSlots(JSContext* cx, uint32_t oldCount, return false; /* Leave slots at its old size. */ } + RemoveCellMemory(this, oldCount * sizeof(HeapSlot), MemoryUse::ObjectSlots); + AddCellMemory(this, newCount * sizeof(HeapSlot), MemoryUse::ObjectSlots); + slots_ = newslots; Debug_SetSlotRangeToCrashOnTouch(slots_ + oldCount, newCount - oldCount); @@ -443,6 +431,8 @@ void NativeObject::shrinkSlots(JSContext* cx, uint32_t oldCount, MOZ_ASSERT(newCount < oldCount); if (newCount == 0) { + RemoveCellMemory(this, numDynamicSlots() * sizeof(HeapSlot), + MemoryUse::ObjectSlots); FreeSlots(cx, slots_); slots_ = nullptr; return; @@ -457,6 +447,9 @@ void NativeObject::shrinkSlots(JSContext* cx, uint32_t oldCount, return; /* Leave slots at its old size. */ } + RemoveCellMemory(this, oldCount * sizeof(HeapSlot), MemoryUse::ObjectSlots); + AddCellMemory(this, newCount * sizeof(HeapSlot), MemoryUse::ObjectSlots); + slots_ = newslots; } diff --git a/js/src/vm/NativeObject.h b/js/src/vm/NativeObject.h index bfb003380b13..e2e49f171258 100644 --- a/js/src/vm/NativeObject.h +++ b/js/src/vm/NativeObject.h @@ -538,11 +538,6 @@ class NativeObject : public JSObject { // the new properties. void setLastPropertyShrinkFixedSlots(Shape* shape); - // As for setLastProperty(), but changes the class associated with the - // object to a non-native one. This leaves the object with a type and shape - // that are (temporarily) inconsistent. - void setLastPropertyMakeNonNative(Shape* shape); - // Newly-created TypedArrays that map a SharedArrayBuffer are // marked as shared by giving them an ObjectElements that has the // ObjectElements::SHARED_MEMORY flag set. @@ -937,6 +932,7 @@ class NativeObject : public JSObject { static MOZ_MUST_USE bool fillInAfterSwap(JSContext* cx, HandleNativeObject obj, + NativeObject* old, HandleValueVector values, void* priv); From d0fbeca679c5662f9673eb83c152cd6c00c217dc Mon Sep 17 00:00:00 2001 From: Jon Coppeard Date: Fri, 24 May 2019 15:45:33 +0100 Subject: [PATCH 03/14] Bug 1395509 - Track malloc memory associated with JSScripts r=tcampbell Differential Revision: https://phabricator.services.mozilla.com/D32500 --- js/src/gc/GCEnum.h | 4 +++- js/src/vm/JSScript.cpp | 23 ++++++++++++++++++++--- js/src/vm/JSScript.h | 1 + 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/js/src/gc/GCEnum.h b/js/src/gc/GCEnum.h index dd756b33ca1f..8329fe6aff7f 100644 --- a/js/src/gc/GCEnum.h +++ b/js/src/gc/GCEnum.h @@ -95,7 +95,9 @@ enum class ZealMode { _(ArrayBufferContents) \ _(StringContents) \ _(ObjectElements) \ - _(ObjectSlots) + _(ObjectSlots) \ + _(ScriptPrivateData) \ + _(LazyScriptData) #define JS_FOR_EACH_MEMORY_USE(_) \ JS_FOR_EACH_PUBLIC_MEMORY_USE(_) \ diff --git a/js/src/vm/JSScript.cpp b/js/src/vm/JSScript.cpp index e6145dd81061..e5ab49a9627c 100644 --- a/js/src/vm/JSScript.cpp +++ b/js/src/vm/JSScript.cpp @@ -3760,6 +3760,7 @@ bool JSScript::createPrivateScriptData(JSContext* cx, HandleScript script, uint32_t nscopenotes, uint32_t nresumeoffsets) { cx->check(script); + MOZ_ASSERT(!script->data_); uint32_t dataSize; @@ -3772,6 +3773,8 @@ bool JSScript::createPrivateScriptData(JSContext* cx, HandleScript script, script->data_ = data; script->dataSize_ = dataSize; + AddCellMemory(script, dataSize, MemoryUse::ScriptPrivateData); + return true; } @@ -4068,8 +4071,9 @@ void JSScript::finalize(FreeOp* fop) { #endif if (data_) { - AlwaysPoison(data_, 0xdb, computedSizeOfData(), MemCheckKind::MakeNoAccess); - fop->free_(data_); + size_t size = computedSizeOfData(); + AlwaysPoison(data_, 0xdb, size, MemCheckKind::MakeNoAccess); + fop->free_(this, data_, size, MemoryUse::ScriptPrivateData); } freeScriptData(); @@ -4927,7 +4931,12 @@ void JSScript::traceChildren(JSTracer* trc) { } } -void LazyScript::finalize(FreeOp* fop) { fop->free_(lazyData_); } +void LazyScript::finalize(FreeOp* fop) { + if (lazyData_) { + fop->free_(this, lazyData_, lazyData_->allocationSize(), + MemoryUse::LazyScriptData); + } +} size_t JSScript::calculateLiveFixed(jsbytecode* pc) { size_t nlivefixed = numAlwaysLiveFixedSlots(); @@ -5161,6 +5170,10 @@ bool JSScript::formalLivesInArgumentsObject(unsigned argSlot) { return size; } +inline size_t LazyScriptData::allocationSize() const { + return AllocationSize(numClosedOverBindings_, numInnerFunctions_); +} + // Placement-new elements of an array. This should optimize away for types with // trivial default initiation. template @@ -5256,6 +5269,10 @@ LazyScript::LazyScript(JSFunction* fun, ScriptSourceObject& sourceObject, MOZ_ASSERT(function_->compartment() == sourceObject_->compartment()); MOZ_ASSERT(sourceStart <= sourceEnd); MOZ_ASSERT(toStringStart <= sourceStart); + + if (data) { + AddCellMemory(this, data->allocationSize(), MemoryUse::LazyScriptData); + } } void LazyScript::initScript(JSScript* script) { diff --git a/js/src/vm/JSScript.h b/js/src/vm/JSScript.h index 3209a72d138e..2890059a8c14 100644 --- a/js/src/vm/JSScript.h +++ b/js/src/vm/JSScript.h @@ -2987,6 +2987,7 @@ class alignas(uintptr_t) LazyScriptData final { // Size to allocate static size_t AllocationSize(uint32_t numClosedOverBindings, uint32_t numInnerFunctions); + size_t allocationSize() const; // Translate an offset into a concrete pointer. template From 2571a90a44dd0688f3056a44b107993dd021d23b Mon Sep 17 00:00:00 2001 From: Andrew Osmond Date: Sun, 26 May 2019 17:43:34 -0400 Subject: [PATCH 04/14] Bug 1481405 - Force disable the GPU process if using Wayland. r=kats,stransky Wayland does not support remote drawing for widgets from another process at this time. As such, it is best to force disable the GPU process, so that users will be able to get WebRender with Wayland. Differential Revision: https://phabricator.services.mozilla.com/D32640 --- gfx/thebes/gfxPlatform.cpp | 2 ++ gfx/thebes/gfxPlatform.h | 1 + gfx/thebes/gfxPlatformGtk.cpp | 11 +++++++++++ gfx/thebes/gfxPlatformGtk.h | 1 + 4 files changed, 15 insertions(+) diff --git a/gfx/thebes/gfxPlatform.cpp b/gfx/thebes/gfxPlatform.cpp index 9521895b3f06..9bb8b0011df2 100644 --- a/gfx/thebes/gfxPlatform.cpp +++ b/gfx/thebes/gfxPlatform.cpp @@ -2467,6 +2467,8 @@ void gfxPlatform::InitGPUProcessPrefs() { NS_LITERAL_CSTRING("FEATURE_FAILURE_LAYERSCOPE")); return; } + + InitPlatformGPUProcessPrefs(); } void gfxPlatform::InitCompositorAccelerationPrefs() { diff --git a/gfx/thebes/gfxPlatform.h b/gfx/thebes/gfxPlatform.h index e76eeab0521c..94deaddb6423 100644 --- a/gfx/thebes/gfxPlatform.h +++ b/gfx/thebes/gfxPlatform.h @@ -883,6 +883,7 @@ class gfxPlatform : public mozilla::layers::MemoryPressureListener { void InitCompositorAccelerationPrefs(); void InitGPUProcessPrefs(); + virtual void InitPlatformGPUProcessPrefs() {} void InitOMTPConfig(); static bool IsDXInterop2Blocked(); diff --git a/gfx/thebes/gfxPlatformGtk.cpp b/gfx/thebes/gfxPlatformGtk.cpp index 207a3894b62c..89b3018f3f24 100644 --- a/gfx/thebes/gfxPlatformGtk.cpp +++ b/gfx/thebes/gfxPlatformGtk.cpp @@ -121,6 +121,17 @@ void gfxPlatformGtk::FlushContentDrawing() { } } +void gfxPlatformGtk::InitPlatformGPUProcessPrefs() { +#ifdef MOZ_WAYLAND + if (!GDK_IS_X11_DISPLAY(gdk_display_get_default())) { + FeatureState& gpuProc = gfxConfig::GetFeature(Feature::GPU_PROCESS); + gpuProc.ForceDisable(FeatureStatus::Blocked, + "Wayland does not work in the GPU process", + NS_LITERAL_CSTRING("FEATURE_FAILURE_WAYLAND")); + } +#endif +} + already_AddRefed gfxPlatformGtk::CreateOffscreenSurface( const IntSize& aSize, gfxImageFormat aFormat) { if (!Factory::AllowedSurfaceSize(aSize)) { diff --git a/gfx/thebes/gfxPlatformGtk.h b/gfx/thebes/gfxPlatformGtk.h index 173b1ae08514..36054d3b1d54 100644 --- a/gfx/thebes/gfxPlatformGtk.h +++ b/gfx/thebes/gfxPlatformGtk.h @@ -106,6 +106,7 @@ class gfxPlatformGtk : public gfxPlatform { #endif protected: + void InitPlatformGPUProcessPrefs() override; bool CheckVariationFontSupport() override; int8_t mMaxGenericSubstitutions; From fd0fefd13a5dd5e18705602edf2a149dad334db9 Mon Sep 17 00:00:00 2001 From: Andrew Osmond Date: Sun, 26 May 2019 20:02:32 -0400 Subject: [PATCH 05/14] Bug 1554540 - Expose window protocol (X11, Wayland) in nsIGfxInfo and about:support. r=kats,stransky,flod Differential Revision: https://phabricator.services.mozilla.com/D32651 --- toolkit/content/aboutSupport.js | 1 + toolkit/crashreporter/CrashAnnotations.yaml | 5 +++++ .../en-US/toolkit/about/aboutSupport.ftl | 2 ++ toolkit/modules/Troubleshoot.jsm | 1 + widget/GfxInfoX11.cpp | 17 +++++++++++++++++ widget/GfxInfoX11.h | 2 ++ widget/android/GfxInfo.cpp | 5 +++++ widget/android/GfxInfo.h | 1 + widget/cocoa/GfxInfo.h | 1 + widget/cocoa/GfxInfo.mm | 4 ++++ widget/moz.build | 4 +--- widget/nsIGfxInfo.idl | 5 +++++ widget/windows/GfxInfo.cpp | 5 +++++ widget/windows/GfxInfo.h | 1 + 14 files changed, 51 insertions(+), 3 deletions(-) diff --git a/toolkit/content/aboutSupport.js b/toolkit/content/aboutSupport.js index 79762f87a0b6..b3b6a8f7fe78 100644 --- a/toolkit/content/aboutSupport.js +++ b/toolkit/content/aboutSupport.js @@ -479,6 +479,7 @@ var snapshotFormatters = { "webgl2Extensions", ["supportsHardwareH264", "hardware-h264"], ["direct2DEnabled", "#Direct2D"], + ["windowProtocol", "graphics-window-protocol"], "usesTiling", "contentUsesTiling", "offMainThreadPaintEnabled", diff --git a/toolkit/crashreporter/CrashAnnotations.yaml b/toolkit/crashreporter/CrashAnnotations.yaml index cb062a50957a..4b637ebea4c9 100644 --- a/toolkit/crashreporter/CrashAnnotations.yaml +++ b/toolkit/crashreporter/CrashAnnotations.yaml @@ -468,6 +468,11 @@ IsGarbageCollecting: type: boolean ping: true +IsWayland: + description: > + If true then the Wayland windowing system was in use. + type: boolean + JavaStackTrace: description: > Java stack trace, only present on Firefox for Android if we encounter an diff --git a/toolkit/locales/en-US/toolkit/about/aboutSupport.ftl b/toolkit/locales/en-US/toolkit/about/aboutSupport.ftl index f55ac2650029..99049d876a6b 100644 --- a/toolkit/locales/en-US/toolkit/about/aboutSupport.ftl +++ b/toolkit/locales/en-US/toolkit/about/aboutSupport.ftl @@ -82,6 +82,8 @@ graphics-gpu2-title = GPU #2 graphics-decision-log-title = Decision Log graphics-crash-guards-title = Crash Guard Disabled Features graphics-workarounds-title = Workarounds +# Windowing system in use on Linux (e.g. X11, Wayland). +graphics-window-protocol = Window Protocol place-database-title = Places Database place-database-integrity = Integrity place-database-verify-integrity = Verify Integrity diff --git a/toolkit/modules/Troubleshoot.jsm b/toolkit/modules/Troubleshoot.jsm index 2895dc77b688..0d6b01289262 100644 --- a/toolkit/modules/Troubleshoot.jsm +++ b/toolkit/modules/Troubleshoot.jsm @@ -467,6 +467,7 @@ var dataProviders = { OffMainThreadPaintEnabled: "offMainThreadPaintEnabled", OffMainThreadPaintWorkerCount: "offMainThreadPaintWorkerCount", TargetFrameRate: "targetFrameRate", + windowProtocol: null, }; for (let prop in gfxInfoProps) { diff --git a/widget/GfxInfoX11.cpp b/widget/GfxInfoX11.cpp index f9eee009919e..0792ebda5092 100644 --- a/widget/GfxInfoX11.cpp +++ b/widget/GfxInfoX11.cpp @@ -19,6 +19,8 @@ #include "GfxInfoX11.h" +#include + #ifdef DEBUG bool fire_glxtest_process(); #endif @@ -40,6 +42,7 @@ nsresult GfxInfo::Init() { mHasTextureFromPixmap = false; mIsMesa = false; mIsAccelerated = true; + mIsWayland = false; return GfxInfoBase::Init(); } @@ -52,6 +55,8 @@ void GfxInfo::AddCrashReportAnnotations() { CrashReporter::Annotation::AdapterDriverVendor, mDriverVendor); CrashReporter::AnnotateCrashReport( CrashReporter::Annotation::AdapterDriverVersion, mDriverVersion); + CrashReporter::AnnotateCrashReport( + CrashReporter::Annotation::IsWayland, mIsWayland); } void GfxInfo::GetData() { @@ -286,6 +291,7 @@ void GfxInfo::GetData() { } mAdapterDescription.Assign(glRenderer); + mIsWayland = !GDK_IS_X11_DISPLAY(gdk_display_get_default()); AddCrashReportAnnotations(); } @@ -440,6 +446,17 @@ GfxInfo::GetCleartypeParameters(nsAString& aCleartypeParams) { return NS_ERROR_FAILURE; } +NS_IMETHODIMP +GfxInfo::GetWindowProtocol(nsAString& aWindowProtocol) { + if (mIsWayland) { + aWindowProtocol.AssignLiteral("wayland"); + return NS_OK; + } + + aWindowProtocol.AssignLiteral("x11"); + return NS_OK; +} + NS_IMETHODIMP GfxInfo::GetAdapterDescription(nsAString& aAdapterDescription) { GetData(); diff --git a/widget/GfxInfoX11.h b/widget/GfxInfoX11.h index 67e423694002..5ec9b5285d3f 100644 --- a/widget/GfxInfoX11.h +++ b/widget/GfxInfoX11.h @@ -22,6 +22,7 @@ class GfxInfo final : public GfxInfoBase { NS_IMETHOD GetDWriteEnabled(bool* aDWriteEnabled) override; NS_IMETHOD GetDWriteVersion(nsAString& aDwriteVersion) override; NS_IMETHOD GetCleartypeParameters(nsAString& aCleartypeParams) override; + NS_IMETHOD GetWindowProtocol(nsAString& aWindowProtocol) override; NS_IMETHOD GetAdapterDescription(nsAString& aAdapterDescription) override; NS_IMETHOD GetAdapterDriver(nsAString& aAdapterDriver) override; NS_IMETHOD GetAdapterVendorID(nsAString& aAdapterVendorID) override; @@ -79,6 +80,7 @@ class GfxInfo final : public GfxInfoBase { unsigned int mGLMajorVersion, mGLMinorVersion; bool mIsMesa; bool mIsAccelerated; + bool mIsWayland; void AddCrashReportAnnotations(); }; diff --git a/widget/android/GfxInfo.cpp b/widget/android/GfxInfo.cpp index 68a75a55cc83..860670f711c2 100644 --- a/widget/android/GfxInfo.cpp +++ b/widget/android/GfxInfo.cpp @@ -139,6 +139,11 @@ GfxInfo::GetCleartypeParameters(nsAString& aCleartypeParams) { return NS_ERROR_FAILURE; } +NS_IMETHODIMP +GfxInfo::GetWindowProtocol(nsAString& aWindowProtocol) { + return NS_ERROR_FAILURE; +} + void GfxInfo::EnsureInitialized() { if (mInitialized) return; diff --git a/widget/android/GfxInfo.h b/widget/android/GfxInfo.h index 6566823308cd..55f9a8488aad 100644 --- a/widget/android/GfxInfo.h +++ b/widget/android/GfxInfo.h @@ -31,6 +31,7 @@ class GfxInfo : public GfxInfoBase { NS_IMETHOD GetDWriteEnabled(bool* aDWriteEnabled) override; NS_IMETHOD GetDWriteVersion(nsAString& aDwriteVersion) override; NS_IMETHOD GetCleartypeParameters(nsAString& aCleartypeParams) override; + NS_IMETHOD GetWindowProtocol(nsAString& aWindowProtocol) override; NS_IMETHOD GetAdapterDescription(nsAString& aAdapterDescription) override; NS_IMETHOD GetAdapterDriver(nsAString& aAdapterDriver) override; NS_IMETHOD GetAdapterVendorID(nsAString& aAdapterVendorID) override; diff --git a/widget/cocoa/GfxInfo.h b/widget/cocoa/GfxInfo.h index fbf49bb1d32f..2ce8ec510f4d 100644 --- a/widget/cocoa/GfxInfo.h +++ b/widget/cocoa/GfxInfo.h @@ -23,6 +23,7 @@ class GfxInfo : public GfxInfoBase { NS_IMETHOD GetD2DEnabled(bool* aD2DEnabled) override; NS_IMETHOD GetDWriteEnabled(bool* aDWriteEnabled) override; NS_IMETHOD GetDWriteVersion(nsAString& aDwriteVersion) override; + NS_IMETHOD GetWindowProtocol(nsAString& aWindowProtocol) override; NS_IMETHOD GetCleartypeParameters(nsAString& aCleartypeParams) override; NS_IMETHOD GetAdapterDescription(nsAString& aAdapterDescription) override; NS_IMETHOD GetAdapterDriver(nsAString& aAdapterDriver) override; diff --git a/widget/cocoa/GfxInfo.mm b/widget/cocoa/GfxInfo.mm index bc8329a831a8..523fe8f71417 100644 --- a/widget/cocoa/GfxInfo.mm +++ b/widget/cocoa/GfxInfo.mm @@ -118,6 +118,10 @@ GfxInfo::GetDWriteVersion(nsAString& aDwriteVersion) { return NS_ERROR_FAILURE; NS_IMETHODIMP GfxInfo::GetCleartypeParameters(nsAString& aCleartypeParams) { return NS_ERROR_FAILURE; } +/* readonly attribute DOMString windowProtocol; */ +NS_IMETHODIMP +GfxInfo::GetWindowProtocol(nsAString& aWindowProtocol) { return NS_ERROR_FAILURE; } + /* readonly attribute DOMString adapterDescription; */ NS_IMETHODIMP GfxInfo::GetAdapterDescription(nsAString& aAdapterDescription) { diff --git a/widget/moz.build b/widget/moz.build index 7e48746edb4a..fb9d0ab5884a 100644 --- a/widget/moz.build +++ b/widget/moz.build @@ -250,10 +250,8 @@ EXPORTS.ipc = ['nsGUIEventIPC.h'] if CONFIG['MOZ_X11']: DIRS += ['x11'] - UNIFIED_SOURCES += [ - 'GfxInfoX11.cpp' - ] SOURCES += [ + 'GfxInfoX11.cpp', 'nsShmImage.cpp', 'WindowSurfaceX11SHM.cpp', ] diff --git a/widget/nsIGfxInfo.idl b/widget/nsIGfxInfo.idl index 5c7aae756305..a953b8c268d8 100644 --- a/widget/nsIGfxInfo.idl +++ b/widget/nsIGfxInfo.idl @@ -19,6 +19,11 @@ interface nsIGfxInfo : nsISupports readonly attribute AString DWriteVersion; readonly attribute AString cleartypeParameters; + /* + * These are non-Android linux-specific + */ + readonly attribute AString windowProtocol; + /* * These are valid across all platforms. */ diff --git a/widget/windows/GfxInfo.cpp b/widget/windows/GfxInfo.cpp index 55101c128674..5599b7d76851 100644 --- a/widget/windows/GfxInfo.cpp +++ b/widget/windows/GfxInfo.cpp @@ -126,6 +126,11 @@ GfxInfo::GetCleartypeParameters(nsAString& aCleartypeParams) { return NS_ERROR_FAILURE; } +NS_IMETHODIMP +GfxInfo::GetWindowProtocol(nsAString& aWindowProtocol) { + return NS_ERROR_FAILURE; +} + static nsresult GetKeyValue(const WCHAR* keyLocation, const WCHAR* keyName, nsAString& destString, int type) { HKEY key; diff --git a/widget/windows/GfxInfo.h b/widget/windows/GfxInfo.h index 75c56c23b9a3..13f3d17689da 100644 --- a/widget/windows/GfxInfo.h +++ b/widget/windows/GfxInfo.h @@ -25,6 +25,7 @@ class GfxInfo : public GfxInfoBase { NS_IMETHOD GetDWriteEnabled(bool* aDWriteEnabled) override; NS_IMETHOD GetDWriteVersion(nsAString& aDwriteVersion) override; NS_IMETHOD GetCleartypeParameters(nsAString& aCleartypeParams) override; + NS_IMETHOD GetWindowProtocol(nsAString& aWindowProtocol) override; NS_IMETHOD GetAdapterDescription(nsAString& aAdapterDescription) override; NS_IMETHOD GetAdapterDriver(nsAString& aAdapterDriver) override; NS_IMETHOD GetAdapterVendorID(nsAString& aAdapterVendorID) override; From ae738653936245895287e8e6add82611e1c4c9a4 Mon Sep 17 00:00:00 2001 From: Andrew Osmond Date: Tue, 28 May 2019 08:02:01 -0400 Subject: [PATCH 06/14] Bug 1554540 - Follow up to fix broken browser chrome tests. r=aosmond --- toolkit/modules/tests/browser/browser_Troubleshoot.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/toolkit/modules/tests/browser/browser_Troubleshoot.js b/toolkit/modules/tests/browser/browser_Troubleshoot.js index 063cd623778e..d9aef81066f9 100644 --- a/toolkit/modules/tests/browser/browser_Troubleshoot.js +++ b/toolkit/modules/tests/browser/browser_Troubleshoot.js @@ -463,6 +463,9 @@ const SNAPSHOT_SCHEMA = { targetFrameRate: { type: "number", }, + windowProtocol: { + type: "string", + }, }, }, media: { From a445258270c7b6282f982259ac48c5fc57be1a24 Mon Sep 17 00:00:00 2001 From: Andrew Osmond Date: Thu, 4 Apr 2019 11:44:12 -0400 Subject: [PATCH 07/14] Bug 1255106 - Part 1. Move color transform state to image decoder base class. r=tnikkel Differential Revision: https://phabricator.services.mozilla.com/D26369 --- image/Decoder.cpp | 12 +++++++++++- image/Decoder.h | 7 +++++++ image/decoders/nsJPEGDecoder.cpp | 6 ------ image/decoders/nsJPEGDecoder.h | 4 ---- image/decoders/nsPNGDecoder.cpp | 10 ---------- image/decoders/nsPNGDecoder.h | 3 --- image/decoders/nsWebPDecoder.cpp | 11 +---------- image/decoders/nsWebPDecoder.h | 6 ------ 8 files changed, 19 insertions(+), 40 deletions(-) diff --git a/image/Decoder.cpp b/image/Decoder.cpp index 8f065908f240..af72265aac0c 100644 --- a/image/Decoder.cpp +++ b/image/Decoder.cpp @@ -44,7 +44,9 @@ class MOZ_STACK_CLASS AutoRecordDecoderTelemetry final { }; Decoder::Decoder(RasterImage* aImage) - : mImageData(nullptr), + : mInProfile(nullptr), + mTransform(nullptr), + mImageData(nullptr), mImageDataLength(0), mImage(aImage), mFrameRecycler(nullptr), @@ -72,6 +74,14 @@ Decoder::~Decoder() { "Destroying Decoder without taking all its invalidations"); mInitialized = false; + if (mInProfile) { + // mTransform belongs to us only if mInProfile is non-null + if (mTransform) { + qcms_transform_release(mTransform); + } + qcms_profile_release(mInProfile); + } + if (mImage && !NS_IsMainThread()) { // Dispatch mImage to main thread to prevent it from being destructed by the // decode thread. diff --git a/image/Decoder.h b/image/Decoder.h index 5e673cc082e3..74d19c123411 100644 --- a/image/Decoder.h +++ b/image/Decoder.h @@ -19,6 +19,7 @@ #include "SourceBuffer.h" #include "StreamingLexer.h" #include "SurfaceFlags.h" +#include "qcms.h" namespace mozilla { @@ -559,6 +560,12 @@ class Decoder { protected: Maybe mDownscaler; + /// Color management profile from the ICCP chunk in the image. + qcms_profile* mInProfile; + + /// Color management transform to apply to image data. + qcms_transform* mTransform; + uint8_t* mImageData; // Pointer to image data in BGRA/X uint32_t mImageDataLength; diff --git a/image/decoders/nsJPEGDecoder.cpp b/image/decoders/nsJPEGDecoder.cpp index 0a9796fc97ab..33dc1cf28977 100644 --- a/image/decoders/nsJPEGDecoder.cpp +++ b/image/decoders/nsJPEGDecoder.cpp @@ -124,12 +124,6 @@ nsJPEGDecoder::~nsJPEGDecoder() { free(mBackBuffer); mBackBuffer = nullptr; - if (mTransform) { - qcms_transform_release(mTransform); - } - if (mInProfile) { - qcms_profile_release(mInProfile); - } MOZ_LOG(sJPEGDecoderAccountingLog, LogLevel::Debug, ("nsJPEGDecoder::~nsJPEGDecoder: Destroying JPEG decoder %p", this)); diff --git a/image/decoders/nsJPEGDecoder.h b/image/decoders/nsJPEGDecoder.h index 7f934c1f615e..82f99ff6df23 100644 --- a/image/decoders/nsJPEGDecoder.h +++ b/image/decoders/nsJPEGDecoder.h @@ -17,7 +17,6 @@ #include "nsIInputStream.h" #include "nsIPipe.h" -#include "qcms.h" extern "C" { #include "jpeglib.h" @@ -100,9 +99,6 @@ class nsJPEGDecoder : public Decoder { JOCTET* mProfile; uint32_t mProfileLength; - qcms_profile* mInProfile; - qcms_transform* mTransform; - bool mReading; const Decoder::DecodeStyle mDecodeStyle; diff --git a/image/decoders/nsPNGDecoder.cpp b/image/decoders/nsPNGDecoder.cpp index dbb700fc4e5e..785c1d43e96d 100644 --- a/image/decoders/nsPNGDecoder.cpp +++ b/image/decoders/nsPNGDecoder.cpp @@ -109,8 +109,6 @@ nsPNGDecoder::nsPNGDecoder(RasterImage* aImage) mInfo(nullptr), mCMSLine(nullptr), interlacebuf(nullptr), - mInProfile(nullptr), - mTransform(nullptr), mFormat(SurfaceFormat::UNKNOWN), mCMSMode(0), mChannels(0), @@ -130,14 +128,6 @@ nsPNGDecoder::~nsPNGDecoder() { if (interlacebuf) { free(interlacebuf); } - if (mInProfile) { - qcms_profile_release(mInProfile); - - // mTransform belongs to us only if mInProfile is non-null - if (mTransform) { - qcms_transform_release(mTransform); - } - } } nsPNGDecoder::TransparencyType nsPNGDecoder::GetTransparencyType( diff --git a/image/decoders/nsPNGDecoder.h b/image/decoders/nsPNGDecoder.h index 4cbc97a307e5..4ed454fea2b3 100644 --- a/image/decoders/nsPNGDecoder.h +++ b/image/decoders/nsPNGDecoder.h @@ -9,7 +9,6 @@ #include "Decoder.h" #include "png.h" -#include "qcms.h" #include "StreamingLexer.h" #include "SurfacePipe.h" @@ -92,8 +91,6 @@ class nsPNGDecoder : public Decoder { nsIntRect mFrameRect; uint8_t* mCMSLine; uint8_t* interlacebuf; - qcms_profile* mInProfile; - qcms_transform* mTransform; gfx::SurfaceFormat mFormat; // whether CMS or premultiplied alpha are forced off diff --git a/image/decoders/nsWebPDecoder.cpp b/image/decoders/nsWebPDecoder.cpp index ebf496ead9d7..3c5c459478e6 100644 --- a/image/decoders/nsWebPDecoder.cpp +++ b/image/decoders/nsWebPDecoder.cpp @@ -30,9 +30,7 @@ nsWebPDecoder::nsWebPDecoder(RasterImage* aImage) mLength(0), mIteratorComplete(false), mNeedDemuxer(true), - mGotColorProfile(false), - mInProfile(nullptr), - mTransform(nullptr) { + mGotColorProfile(false) { MOZ_LOG(sWebPLog, LogLevel::Debug, ("[this=%p] nsWebPDecoder::nsWebPDecoder", this)); } @@ -44,13 +42,6 @@ nsWebPDecoder::~nsWebPDecoder() { WebPIDelete(mDecoder); WebPFreeDecBuffer(&mBuffer); } - if (mInProfile) { - // mTransform belongs to us only if mInProfile is non-null - if (mTransform) { - qcms_transform_release(mTransform); - } - qcms_profile_release(mInProfile); - } } LexerResult nsWebPDecoder::ReadData() { diff --git a/image/decoders/nsWebPDecoder.h b/image/decoders/nsWebPDecoder.h index 87784b14a756..b8b86b31992a 100644 --- a/image/decoders/nsWebPDecoder.h +++ b/image/decoders/nsWebPDecoder.h @@ -97,12 +97,6 @@ class nsWebPDecoder final : public Decoder { /// True if we have setup the color profile for the image. bool mGotColorProfile; - - /// Color management profile from the ICCP chunk in the image. - qcms_profile* mInProfile; - - /// Color management transform to apply to image data. - qcms_transform* mTransform; }; } // namespace image From 3c28440f4362c16316cbfad6ec0d8e9356a48800 Mon Sep 17 00:00:00 2001 From: Andrew Osmond Date: Thu, 4 Apr 2019 11:44:35 -0400 Subject: [PATCH 08/14] Bug 1255106 - Part 2. Implement color management filter for SurfacePipe. r=tnikkel Differential Revision: https://phabricator.services.mozilla.com/D26370 --- image/SurfaceFilters.h | 62 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/image/SurfaceFilters.h b/image/SurfaceFilters.h index de0453a57b5d..4018bba75730 100644 --- a/image/SurfaceFilters.h +++ b/image/SurfaceFilters.h @@ -29,6 +29,68 @@ namespace mozilla { namespace image { +////////////////////////////////////////////////////////////////////////////// +// ColorManagementFilter +////////////////////////////////////////////////////////////////////////////// + +template +class ColorManagementFilter; + +/** + * A configuration struct for ColorManagementFilter. + */ +struct ColorManagementConfig { + template + using Filter = ColorManagementFilter; + qcms_transform* mTransform; +}; + +/** + * ColorManagementFilter performs color transforms with qcms on rows written + * to it. + * + * The 'Next' template parameter specifies the next filter in the chain. + */ +template +class ColorManagementFilter final : public SurfaceFilter { + public: + ColorManagementFilter() : mTransform(nullptr) {} + + template + nsresult Configure(const ColorManagementConfig& aConfig, + const Rest&... aRest) { + nsresult rv = mNext.Configure(aRest...); + if (NS_FAILED(rv)) { + return rv; + } + + if (!aConfig.mTransform) { + return NS_ERROR_INVALID_ARG; + } + + mTransform = aConfig.mTransform; + ConfigureFilter(mNext.InputSize(), sizeof(uint32_t)); + return NS_OK; + } + + Maybe TakeInvalidRect() override { + return mNext.TakeInvalidRect(); + } + + protected: + uint8_t* DoResetToFirstRow() override { return mNext.ResetToFirstRow(); } + + uint8_t* DoAdvanceRow() override { + uint8_t* rowPtr = mNext.CurrentRowPointer(); + qcms_transform_data(mTransform, rowPtr, rowPtr, mNext.InputSize().width); + return mNext.AdvanceRow(); + } + + Next mNext; /// The next SurfaceFilter in the chain. + + qcms_transform* mTransform; +}; + ////////////////////////////////////////////////////////////////////////////// // DeinterlacingFilter ////////////////////////////////////////////////////////////////////////////// From 741028ad6b90bfe73869f04142c1cc02adab503e Mon Sep 17 00:00:00 2001 From: Andrew Osmond Date: Thu, 4 Apr 2019 13:22:59 -0400 Subject: [PATCH 09/14] Bug 1255106 - Part 3. Use color management filter with decoders using SurfacePipe. r=tnikkel Differential Revision: https://phabricator.services.mozilla.com/D26371 --- image/SurfacePipeFactory.h | 165 ++++++++++++++++++++++--------- image/decoders/nsGIFDecoder2.cpp | 3 +- image/decoders/nsIconDecoder.cpp | 2 +- image/decoders/nsPNGDecoder.cpp | 88 +++++++---------- image/decoders/nsPNGDecoder.h | 1 + image/decoders/nsWebPDecoder.cpp | 12 +-- 6 files changed, 159 insertions(+), 112 deletions(-) diff --git a/image/SurfacePipeFactory.h b/image/SurfacePipeFactory.h index 5069e827c8dc..52fc650d8251 100644 --- a/image/SurfacePipeFactory.h +++ b/image/SurfacePipeFactory.h @@ -89,7 +89,7 @@ class SurfacePipeFactory { Decoder* aDecoder, const nsIntSize& aInputSize, const nsIntSize& aOutputSize, const nsIntRect& aFrameRect, gfx::SurfaceFormat aFormat, const Maybe& aAnimParams, - SurfacePipeFlags aFlags) { + qcms_transform* aTransform, SurfacePipeFlags aFlags) { const bool deinterlace = bool(aFlags & SurfacePipeFlags::DEINTERLACE); const bool flipVertically = bool(aFlags & SurfacePipeFlags::FLIP_VERTICALLY); @@ -99,6 +99,7 @@ class SurfacePipeFactory { const bool removeFrameRect = !aFrameRect.IsEqualEdges( nsIntRect(0, 0, aInputSize.width, aInputSize.height)); const bool blendAnimation = aAnimParams.isSome(); + const bool colorManagement = aTransform != nullptr; // Don't interpolate if we're sure we won't show this surface to the user // until it's completely decoded. The final pass of an ADAM7 image doesn't @@ -124,63 +125,129 @@ class SurfacePipeFactory { RemoveFrameRectConfig removeFrameRectConfig{aFrameRect}; BlendAnimationConfig blendAnimationConfig{aDecoder}; DownscalingConfig downscalingConfig{aInputSize, aFormat}; + ColorManagementConfig colorManagementConfig{aTransform}; SurfaceConfig surfaceConfig{aDecoder, aOutputSize, aFormat, flipVertically, aAnimParams}; Maybe pipe; - if (downscale) { - MOZ_ASSERT(!blendAnimation); - if (removeFrameRect) { - if (deinterlace) { - pipe = MakePipe(deinterlacingConfig, removeFrameRectConfig, - downscalingConfig, surfaceConfig); - } else if (adam7Interpolate) { - pipe = MakePipe(interpolatingConfig, removeFrameRectConfig, - downscalingConfig, surfaceConfig); - } else { // (deinterlace and adam7Interpolate are false) - pipe = - MakePipe(removeFrameRectConfig, downscalingConfig, surfaceConfig); + if (colorManagement) { + if (downscale) { + MOZ_ASSERT(!blendAnimation); + if (removeFrameRect) { + if (deinterlace) { + pipe = MakePipe(deinterlacingConfig, removeFrameRectConfig, + downscalingConfig, colorManagementConfig, + surfaceConfig); + } else if (adam7Interpolate) { + pipe = MakePipe(interpolatingConfig, removeFrameRectConfig, + downscalingConfig, colorManagementConfig, + surfaceConfig); + } else { // (deinterlace and adam7Interpolate are false) + pipe = MakePipe(removeFrameRectConfig, downscalingConfig, + colorManagementConfig, surfaceConfig); + } + } else { // (removeFrameRect is false) + if (deinterlace) { + pipe = MakePipe(deinterlacingConfig, downscalingConfig, + colorManagementConfig, surfaceConfig); + } else if (adam7Interpolate) { + pipe = MakePipe(interpolatingConfig, downscalingConfig, + colorManagementConfig, surfaceConfig); + } else { // (deinterlace and adam7Interpolate are false) + pipe = MakePipe(downscalingConfig, colorManagementConfig, + surfaceConfig); + } } - } else { // (removeFrameRect is false) - if (deinterlace) { - pipe = - MakePipe(deinterlacingConfig, downscalingConfig, surfaceConfig); - } else if (adam7Interpolate) { - pipe = - MakePipe(interpolatingConfig, downscalingConfig, surfaceConfig); - } else { // (deinterlace and adam7Interpolate are false) - pipe = MakePipe(downscalingConfig, surfaceConfig); + } else { // (downscale is false) + if (blendAnimation) { + if (deinterlace) { + pipe = MakePipe(deinterlacingConfig, colorManagementConfig, + blendAnimationConfig, surfaceConfig); + } else if (adam7Interpolate) { + pipe = MakePipe(interpolatingConfig, colorManagementConfig, + blendAnimationConfig, surfaceConfig); + } else { // (deinterlace and adam7Interpolate are false) + pipe = MakePipe(colorManagementConfig, blendAnimationConfig, + surfaceConfig); + } + } else if (removeFrameRect) { + if (deinterlace) { + pipe = MakePipe(deinterlacingConfig, colorManagementConfig, + removeFrameRectConfig, surfaceConfig); + } else if (adam7Interpolate) { + pipe = MakePipe(interpolatingConfig, colorManagementConfig, + removeFrameRectConfig, surfaceConfig); + } else { // (deinterlace and adam7Interpolate are false) + pipe = MakePipe(colorManagementConfig, removeFrameRectConfig, + surfaceConfig); + } + } else { // (blendAnimation and removeFrameRect is false) + if (deinterlace) { + pipe = MakePipe(deinterlacingConfig, colorManagementConfig, + surfaceConfig); + } else if (adam7Interpolate) { + pipe = MakePipe(interpolatingConfig, colorManagementConfig, + surfaceConfig); + } else { // (deinterlace and adam7Interpolate are false) + pipe = MakePipe(colorManagementConfig, surfaceConfig); + } } } - } else { // (downscale is false) - if (blendAnimation) { - if (deinterlace) { - pipe = MakePipe(deinterlacingConfig, blendAnimationConfig, - surfaceConfig); - } else if (adam7Interpolate) { - pipe = MakePipe(interpolatingConfig, blendAnimationConfig, - surfaceConfig); - } else { // (deinterlace and adam7Interpolate are false) - pipe = MakePipe(blendAnimationConfig, surfaceConfig); + } else { // (colorManagement is false) + if (downscale) { + MOZ_ASSERT(!blendAnimation); + if (removeFrameRect) { + if (deinterlace) { + pipe = MakePipe(deinterlacingConfig, removeFrameRectConfig, + downscalingConfig, surfaceConfig); + } else if (adam7Interpolate) { + pipe = MakePipe(interpolatingConfig, removeFrameRectConfig, + downscalingConfig, surfaceConfig); + } else { // (deinterlace and adam7Interpolate are false) + pipe = MakePipe(removeFrameRectConfig, downscalingConfig, + surfaceConfig); + } + } else { // (removeFrameRect is false) + if (deinterlace) { + pipe = + MakePipe(deinterlacingConfig, downscalingConfig, surfaceConfig); + } else if (adam7Interpolate) { + pipe = + MakePipe(interpolatingConfig, downscalingConfig, surfaceConfig); + } else { // (deinterlace and adam7Interpolate are false) + pipe = MakePipe(downscalingConfig, surfaceConfig); + } } - } else if (removeFrameRect) { - if (deinterlace) { - pipe = MakePipe(deinterlacingConfig, removeFrameRectConfig, - surfaceConfig); - } else if (adam7Interpolate) { - pipe = MakePipe(interpolatingConfig, removeFrameRectConfig, - surfaceConfig); - } else { // (deinterlace and adam7Interpolate are false) - pipe = MakePipe(removeFrameRectConfig, surfaceConfig); - } - } else { // (blendAnimation and removeFrameRect is false) - if (deinterlace) { - pipe = MakePipe(deinterlacingConfig, surfaceConfig); - } else if (adam7Interpolate) { - pipe = MakePipe(interpolatingConfig, surfaceConfig); - } else { // (deinterlace and adam7Interpolate are false) - pipe = MakePipe(surfaceConfig); + } else { // (downscale is false) + if (blendAnimation) { + if (deinterlace) { + pipe = MakePipe(deinterlacingConfig, blendAnimationConfig, + surfaceConfig); + } else if (adam7Interpolate) { + pipe = MakePipe(interpolatingConfig, blendAnimationConfig, + surfaceConfig); + } else { // (deinterlace and adam7Interpolate are false) + pipe = MakePipe(blendAnimationConfig, surfaceConfig); + } + } else if (removeFrameRect) { + if (deinterlace) { + pipe = MakePipe(deinterlacingConfig, removeFrameRectConfig, + surfaceConfig); + } else if (adam7Interpolate) { + pipe = MakePipe(interpolatingConfig, removeFrameRectConfig, + surfaceConfig); + } else { // (deinterlace and adam7Interpolate are false) + pipe = MakePipe(removeFrameRectConfig, surfaceConfig); + } + } else { // (blendAnimation and removeFrameRect is false) + if (deinterlace) { + pipe = MakePipe(deinterlacingConfig, surfaceConfig); + } else if (adam7Interpolate) { + pipe = MakePipe(interpolatingConfig, surfaceConfig); + } else { // (deinterlace and adam7Interpolate are false) + pipe = MakePipe(surfaceConfig); + } } } } diff --git a/image/decoders/nsGIFDecoder2.cpp b/image/decoders/nsGIFDecoder2.cpp index ec36f3099631..6e761164cc88 100644 --- a/image/decoders/nsGIFDecoder2.cpp +++ b/image/decoders/nsGIFDecoder2.cpp @@ -193,7 +193,8 @@ nsresult nsGIFDecoder2::BeginImageFrame(const IntRect& aFrameRect, } Maybe pipe = SurfacePipeFactory::CreateSurfacePipe( - this, Size(), OutputSize(), aFrameRect, format, animParams, pipeFlags); + this, Size(), OutputSize(), aFrameRect, format, animParams, mTransform, + pipeFlags); mCurrentFrameIndex = mGIFStruct.images_decoded; if (!pipe) { diff --git a/image/decoders/nsIconDecoder.cpp b/image/decoders/nsIconDecoder.cpp index f97cf94b317f..5e3bb24a039e 100644 --- a/image/decoders/nsIconDecoder.cpp +++ b/image/decoders/nsIconDecoder.cpp @@ -68,7 +68,7 @@ LexerTransition nsIconDecoder::ReadHeader( MOZ_ASSERT(!mImageData, "Already have a buffer allocated?"); Maybe pipe = SurfacePipeFactory::CreateSurfacePipe( this, Size(), OutputSize(), FullFrame(), SurfaceFormat::B8G8R8A8, - /* aAnimParams */ Nothing(), SurfacePipeFlags()); + /* aAnimParams */ Nothing(), mTransform, SurfacePipeFlags()); if (!pipe) { return Transition::TerminateFailure(); } diff --git a/image/decoders/nsPNGDecoder.cpp b/image/decoders/nsPNGDecoder.cpp index 785c1d43e96d..df9dddafe82f 100644 --- a/image/decoders/nsPNGDecoder.cpp +++ b/image/decoders/nsPNGDecoder.cpp @@ -116,6 +116,7 @@ nsPNGDecoder::nsPNGDecoder(RasterImage* aImage) mFrameIsHidden(false), mDisablePremultipliedAlpha(false), mGotInfoCallback(false), + mUsePipeTransform(false), mNumFrames(0) {} nsPNGDecoder::~nsPNGDecoder() { @@ -212,9 +213,10 @@ nsresult nsPNGDecoder::CreateFrame(const FrameInfo& aFrameInfo) { pipeFlags |= SurfacePipeFlags::PROGRESSIVE_DISPLAY; } + qcms_transform* pipeTransform = mUsePipeTransform ? mTransform : nullptr; Maybe pipe = SurfacePipeFactory::CreateSurfacePipe( this, Size(), OutputSize(), aFrameInfo.mFrameRect, mFormat, animParams, - pipeFlags); + pipeTransform, pipeFlags); if (!pipe) { mPipe = SurfacePipe(); @@ -413,8 +415,7 @@ static void PNGDoGammaCorrection(png_structp png_ptr, png_infop info_ptr) { // Adapted from http://www.littlecms.com/pngchrm.c example code static qcms_profile* PNGGetColorProfile(png_structp png_ptr, png_infop info_ptr, - int color_type, qcms_data_type* inType, - uint32_t* intent) { + int color_type, uint32_t* intent) { qcms_profile* profile = nullptr; *intent = QCMS_INTENT_PERCEPTUAL; // Our default @@ -492,24 +493,6 @@ static qcms_profile* PNGGetColorProfile(png_structp png_ptr, png_infop info_ptr, } } - if (profile) { - uint32_t profileSpace = qcms_profile_get_color_space(profile); - if (profileSpace == icSigGrayData) { - if (color_type & PNG_COLOR_MASK_ALPHA) { - *inType = QCMS_DATA_GRAYA_8; - } else { - *inType = QCMS_DATA_GRAY_8; - } - } else { - if (color_type & PNG_COLOR_MASK_ALPHA || - png_get_valid(png_ptr, info_ptr, PNG_INFO_tRNS)) { - *inType = QCMS_DATA_RGBA_8; - } else { - *inType = QCMS_DATA_RGB_8; - } - } - } - return profile; } @@ -587,25 +570,41 @@ void nsPNGDecoder::info_callback(png_structp png_ptr, png_infop info_ptr) { png_set_scale_16(png_ptr); } - qcms_data_type inType = QCMS_DATA_RGBA_8; + // Let libpng expand interlaced images. + const bool isInterlaced = interlace_type == PNG_INTERLACE_ADAM7; + if (isInterlaced) { + png_set_interlace_handling(png_ptr); + } + uint32_t intent = -1; uint32_t pIntent; if (decoder->mCMSMode != eCMSMode_Off) { intent = gfxPlatform::GetRenderingIntent(); decoder->mInProfile = - PNGGetColorProfile(png_ptr, info_ptr, color_type, &inType, &pIntent); + PNGGetColorProfile(png_ptr, info_ptr, color_type, &pIntent); // If we're not mandating an intent, use the one from the image. if (intent == uint32_t(-1)) { intent = pIntent; } } if (decoder->mInProfile && gfxPlatform::GetCMSOutputProfile()) { + qcms_data_type inType; qcms_data_type outType; - if (color_type & PNG_COLOR_MASK_ALPHA || num_trans) { - outType = QCMS_DATA_RGBA_8; + uint32_t profileSpace = qcms_profile_get_color_space(decoder->mInProfile); + decoder->mUsePipeTransform = profileSpace != icSigGrayData; + if (decoder->mUsePipeTransform) { + // If the transform happens with SurfacePipe, it will always be in BGRA. + inType = QCMS_DATA_BGRA_8; + outType = QCMS_DATA_BGRA_8; } else { - outType = QCMS_DATA_RGB_8; + if (color_type & PNG_COLOR_MASK_ALPHA) { + inType = QCMS_DATA_GRAYA_8; + outType = QCMS_DATA_RGBA_8; + } else { + inType = QCMS_DATA_GRAY_8; + outType = QCMS_DATA_RGB_8; + } } decoder->mTransform = qcms_transform_create( @@ -620,20 +619,11 @@ void nsPNGDecoder::info_callback(png_structp png_ptr, png_infop info_ptr) { } if (decoder->mCMSMode == eCMSMode_All) { - if (color_type & PNG_COLOR_MASK_ALPHA || num_trans) { - decoder->mTransform = gfxPlatform::GetCMSRGBATransform(); - } else { - decoder->mTransform = gfxPlatform::GetCMSRGBTransform(); - } + decoder->mTransform = gfxPlatform::GetCMSBGRATransform(); + decoder->mUsePipeTransform = true; } } - // Let libpng expand interlaced images. - const bool isInterlaced = interlace_type == PNG_INTERLACE_ADAM7; - if (isInterlaced) { - png_set_interlace_handling(png_ptr); - } - // now all of those things we set above are used to update various struct // members and whatnot, after which we can get channels, rowbytes, etc. png_read_update_info(png_ptr, info_ptr); @@ -698,8 +688,8 @@ void nsPNGDecoder::info_callback(png_structp png_ptr, png_infop info_ptr) { } #endif - if (decoder->mTransform && (channels <= 2 || isInterlaced)) { - uint32_t bpp[] = {0, 3, 4, 3, 4}; + if (decoder->mTransform && !decoder->mUsePipeTransform) { + uint32_t bpp[] = {0, 3, 4}; decoder->mCMSLine = static_cast(malloc(bpp[channels] * frameRect.Width())); if (!decoder->mCMSLine) { @@ -840,21 +830,11 @@ void nsPNGDecoder::WriteRow(uint8_t* aRow) { uint32_t width = uint32_t(mFrameRect.Width()); // Apply color management to the row, if necessary, before writing it out. - if (mTransform) { - if (mCMSLine) { - qcms_transform_data(mTransform, rowToWrite, mCMSLine, width); - - // Copy alpha over. - if (HasAlphaChannel()) { - for (uint32_t i = 0; i < width; ++i) { - mCMSLine[4 * i + 3] = rowToWrite[mChannels * i + mChannels - 1]; - } - } - - rowToWrite = mCMSLine; - } else { - qcms_transform_data(mTransform, rowToWrite, rowToWrite, width); - } + // This is only needed for grayscale images. + if (mTransform && !mUsePipeTransform) { + MOZ_ASSERT(mCMSLine); + qcms_transform_data(mTransform, rowToWrite, mCMSLine, width); + rowToWrite = mCMSLine; } // Write this row to the SurfacePipe. diff --git a/image/decoders/nsPNGDecoder.h b/image/decoders/nsPNGDecoder.h index 4ed454fea2b3..ea2326694a82 100644 --- a/image/decoders/nsPNGDecoder.h +++ b/image/decoders/nsPNGDecoder.h @@ -101,6 +101,7 @@ class nsPNGDecoder : public Decoder { bool mFrameIsHidden; bool mDisablePremultipliedAlpha; bool mGotInfoCallback; + bool mUsePipeTransform; struct AnimFrameInfo { AnimFrameInfo(); diff --git a/image/decoders/nsWebPDecoder.cpp b/image/decoders/nsWebPDecoder.cpp index 3c5c459478e6..2d6bd0de76fa 100644 --- a/image/decoders/nsWebPDecoder.cpp +++ b/image/decoders/nsWebPDecoder.cpp @@ -228,7 +228,8 @@ nsresult nsWebPDecoder::CreateFrame(const nsIntRect& aFrameRect) { } Maybe pipe = SurfacePipeFactory::CreateSurfacePipe( - this, Size(), OutputSize(), aFrameRect, mFormat, animParams, pipeFlags); + this, Size(), OutputSize(), aFrameRect, mFormat, animParams, mTransform, + pipeFlags); if (!pipe) { MOZ_LOG(sWebPLog, LogLevel::Error, ("[this=%p] nsWebPDecoder::CreateFrame -- no pipe\n", this)); @@ -280,7 +281,7 @@ void nsWebPDecoder::ApplyColorProfile(const char* aProfile, size_t aLength) { "output " "profile , use sRGB transform\n", this)); - mTransform = gfxPlatform::GetCMSRGBATransform(); + mTransform = gfxPlatform::GetCMSBGRATransform(); return; } @@ -310,9 +311,9 @@ void nsWebPDecoder::ApplyColorProfile(const char* aProfile, size_t aLength) { } // Create the color management transform. - mTransform = qcms_transform_create(mInProfile, QCMS_DATA_RGBA_8, + mTransform = qcms_transform_create(mInProfile, QCMS_DATA_BGRA_8, gfxPlatform::GetCMSOutputProfile(), - QCMS_DATA_RGBA_8, (qcms_intent)intent); + QCMS_DATA_BGRA_8, (qcms_intent)intent); MOZ_LOG(sWebPLog, LogLevel::Debug, ("[this=%p] nsWebPDecoder::ApplyColorProfile -- use tagged " "transform\n", @@ -462,9 +463,6 @@ LexerResult nsWebPDecoder::ReadSingle(const uint8_t* aData, size_t aLength, for (int row = mLastRow; row < lastRow; row++) { uint8_t* src = rowStart + row * stride; - if (mTransform) { - qcms_transform_data(mTransform, src, src, width); - } WriteState result; if (mFormat == SurfaceFormat::B8G8R8A8) { From 1f049d27872b9fa311cd70011864d254a0941db8 Mon Sep 17 00:00:00 2001 From: Andrew Osmond Date: Thu, 4 Apr 2019 16:30:10 -0400 Subject: [PATCH 10/14] Bug 1255106 - Part 4. Ensure we don't apply CMS transforms in cases we missed. r=tnikkel GIF images should not apply CMS transforms if disabled by caller via SurfaceFlags::NO_COLORSPACE_CONVERSION. WebP images should reject any non-RGB ICC profiles, not just grayscale profiles. Differential Revision: https://phabricator.services.mozilla.com/D26372 --- image/decoders/nsGIFDecoder2.cpp | 5 +++-- image/decoders/nsGIFDecoder2.h | 3 +++ image/decoders/nsWebPDecoder.cpp | 14 +++++++------- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/image/decoders/nsGIFDecoder2.cpp b/image/decoders/nsGIFDecoder2.cpp index 6e761164cc88..079b9d71c979 100644 --- a/image/decoders/nsGIFDecoder2.cpp +++ b/image/decoders/nsGIFDecoder2.cpp @@ -392,9 +392,10 @@ Tuple> nsGIFDecoder2::YieldPixels( /// Expand the colormap from RGB to Packed ARGB as needed by Cairo. /// And apply any LCMS transformation. -static void ConvertColormap(uint32_t* aColormap, uint32_t aColors) { +void nsGIFDecoder2::ConvertColormap(uint32_t* aColormap, uint32_t aColors) { // Apply CMS transformation if enabled and available - if (gfxPlatform::GetCMSMode() == eCMSMode_All) { + if (!(GetSurfaceFlags() & SurfaceFlags::NO_COLORSPACE_CONVERSION) && + gfxPlatform::GetCMSMode() == eCMSMode_All) { qcms_transform* transform = gfxPlatform::GetCMSRGBTransform(); if (transform) { qcms_transform_data(transform, aColormap, aColormap, aColors); diff --git a/image/decoders/nsGIFDecoder2.h b/image/decoders/nsGIFDecoder2.h index 61a4248b274a..599deaf11a88 100644 --- a/image/decoders/nsGIFDecoder2.h +++ b/image/decoders/nsGIFDecoder2.h @@ -58,6 +58,9 @@ class nsGIFDecoder2 : public Decoder { /// Called when we finish decoding the entire image. void FlushImageData(); + /// Convert color map to BGRA, applying any necessary CMS tranforms. + void ConvertColormap(uint32_t* aColormap, uint32_t aColors); + /// Transforms a palette index into a pixel. template PixelSize ColormapIndexToPixel(uint8_t aIndex); diff --git a/image/decoders/nsWebPDecoder.cpp b/image/decoders/nsWebPDecoder.cpp index 2d6bd0de76fa..f2c5bcdb721c 100644 --- a/image/decoders/nsWebPDecoder.cpp +++ b/image/decoders/nsWebPDecoder.cpp @@ -271,15 +271,15 @@ void nsWebPDecoder::ApplyColorProfile(const char* aProfile, size_t aLength) { } auto mode = gfxPlatform::GetCMSMode(); - if (mode == eCMSMode_Off || (mode == eCMSMode_TaggedOnly && !aProfile)) { + if (mode == eCMSMode_Off || (mode == eCMSMode_TaggedOnly && !aProfile) || + !gfxPlatform::GetCMSOutputProfile()) { return; } - if (!aProfile || !gfxPlatform::GetCMSOutputProfile()) { + if (!aProfile) { MOZ_LOG(sWebPLog, LogLevel::Debug, - ("[this=%p] nsWebPDecoder::ApplyColorProfile -- not tagged or no " - "output " - "profile , use sRGB transform\n", + ("[this=%p] nsWebPDecoder::ApplyColorProfile -- not tagged, use " + "sRGB transform\n", this)); mTransform = gfxPlatform::GetCMSBGRATransform(); return; @@ -295,10 +295,10 @@ void nsWebPDecoder::ApplyColorProfile(const char* aProfile, size_t aLength) { } uint32_t profileSpace = qcms_profile_get_color_space(mInProfile); - if (profileSpace == icSigGrayData) { + if (profileSpace != icSigRgbData) { // WebP doesn't produce grayscale data, this must be corrupt. MOZ_LOG(sWebPLog, LogLevel::Error, - ("[this=%p] nsWebPDecoder::ApplyColorProfile -- ignoring grayscale " + ("[this=%p] nsWebPDecoder::ApplyColorProfile -- ignoring non-rgb " "color profile\n", this)); return; From ee7a7e73d56f89a094d901981cd62736753c7028 Mon Sep 17 00:00:00 2001 From: Andrew Osmond Date: Thu, 4 Apr 2019 15:38:52 -0400 Subject: [PATCH 11/14] Bug 1255108 - Make JPEG decoder use SurfacePipe. r=tnikkel Differential Revision: https://phabricator.services.mozilla.com/D26373 --- image/decoders/nsJPEGDecoder.cpp | 475 +++++++++++++------------------ image/decoders/nsJPEGDecoder.h | 11 +- 2 files changed, 199 insertions(+), 287 deletions(-) diff --git a/image/decoders/nsJPEGDecoder.cpp b/image/decoders/nsJPEGDecoder.cpp index 33dc1cf28977..8462ee81ed5e 100644 --- a/image/decoders/nsJPEGDecoder.cpp +++ b/image/decoders/nsJPEGDecoder.cpp @@ -13,8 +13,7 @@ #include "imgFrame.h" #include "Orientation.h" #include "EXIF.h" - -#include "nsIInputStream.h" +#include "SurfacePipeFactory.h" #include "nspr.h" #include "nsCRT.h" @@ -37,7 +36,8 @@ extern "C" { # define MOZ_JCS_EXT_NATIVE_ENDIAN_XRGB JCS_EXT_BGRX #endif -static void cmyk_convert_rgb(JSAMPROW row, JDIMENSION width); +static void cmyk_convert_bgra(uint32_t* aInput, uint32_t* aOutput, + int32_t aWidth); using mozilla::gfx::SurfaceFormat; @@ -79,6 +79,7 @@ nsJPEGDecoder::nsJPEGDecoder(RasterImage* aImage, Transition::TerminateSuccess()), mProfile(nullptr), mProfileLength(0), + mCMSLine(nullptr), mDecodeStyle(aDecodeStyle) { this->mErr.pub.error_exit = nullptr; this->mErr.pub.emit_message = nullptr; @@ -108,9 +109,6 @@ nsJPEGDecoder::nsJPEGDecoder(RasterImage* aImage, mBackBuffer = nullptr; mBackBufferLen = mBackBufferSize = mBackBufferUnreadLen = 0; - mInProfile = nullptr; - mTransform = nullptr; - mCMSMode = 0; MOZ_LOG(sJPEGDecoderAccountingLog, LogLevel::Debug, @@ -125,6 +123,8 @@ nsJPEGDecoder::~nsJPEGDecoder() { free(mBackBuffer); mBackBuffer = nullptr; + delete[] mCMSLine; + MOZ_LOG(sJPEGDecoderAccountingLog, LogLevel::Debug, ("nsJPEGDecoder::~nsJPEGDecoder: Destroying JPEG decoder %p", this)); } @@ -257,74 +257,60 @@ LexerTransition nsJPEGDecoder::ReadJPEGData( } // We're doing a full decode. - if (mCMSMode != eCMSMode_Off && - (mInProfile = GetICCProfile(mInfo)) != nullptr) { - uint32_t profileSpace = qcms_profile_get_color_space(mInProfile); - bool mismatch = false; + switch (mInfo.jpeg_color_space) { + case JCS_GRAYSCALE: + case JCS_RGB: + case JCS_YCbCr: + // By default, we will output directly to BGRA. If we need to apply + // special color transforms, this may change. + mInfo.out_color_space = MOZ_JCS_EXT_NATIVE_ENDIAN_XRGB; + break; + case JCS_CMYK: + case JCS_YCCK: + // libjpeg can convert from YCCK to CMYK, but not to XRGB. + mInfo.out_color_space = JCS_CMYK; + break; + default: + mState = JPEG_ERROR; + MOZ_LOG(sJPEGDecoderAccountingLog, LogLevel::Debug, + ("} (unknown colorpsace (3))")); + return Transition::TerminateFailure(); + } + + if (mCMSMode != eCMSMode_Off) { + if ((mInProfile = GetICCProfile(mInfo)) != nullptr && + gfxPlatform::GetCMSOutputProfile()) { + uint32_t profileSpace = qcms_profile_get_color_space(mInProfile); #ifdef DEBUG_tor - fprintf(stderr, "JPEG profileSpace: 0x%08X\n", profileSpace); + fprintf(stderr, "JPEG profileSpace: 0x%08X\n", profileSpace); #endif - switch (mInfo.jpeg_color_space) { - case JCS_GRAYSCALE: - if (profileSpace == icSigRgbData) { - mInfo.out_color_space = JCS_RGB; - } else if (profileSpace != icSigGrayData) { - mismatch = true; - } - break; - case JCS_RGB: - if (profileSpace != icSigRgbData) { - mismatch = true; - } - break; - case JCS_YCbCr: - if (profileSpace == icSigRgbData) { - mInfo.out_color_space = JCS_RGB; - } else { - // qcms doesn't support ycbcr - mismatch = true; - } - break; - case JCS_CMYK: - case JCS_YCCK: - // qcms doesn't support cmyk - mismatch = true; - break; - default: - mState = JPEG_ERROR; - MOZ_LOG(sJPEGDecoderAccountingLog, LogLevel::Debug, - ("} (unknown colorpsace (1))")); - return Transition::TerminateFailure(); - } - - if (!mismatch) { - qcms_data_type type; - switch (mInfo.out_color_space) { - case JCS_GRAYSCALE: - type = QCMS_DATA_GRAY_8; - break; - case JCS_RGB: - type = QCMS_DATA_RGB_8; - break; - default: - mState = JPEG_ERROR; - MOZ_LOG(sJPEGDecoderAccountingLog, LogLevel::Debug, - ("} (unknown colorpsace (2))")); - return Transition::TerminateFailure(); + Maybe type; + if (profileSpace == icSigRgbData) { + // We can always color manage RGB profiles since it happens at the + // end of the pipeline. + type.emplace(QCMS_DATA_BGRA_8); + } else if (profileSpace == icSigGrayData && + mInfo.jpeg_color_space == JCS_GRAYSCALE) { + // We can only color manage gray profiles if the original color + // space is grayscale. This means we must downscale after color + // management since the downscaler assumes BGRA. + mInfo.out_color_space = JCS_GRAYSCALE; + type.emplace(QCMS_DATA_GRAY_8); } -#if 0 - // We don't currently support CMYK profiles. The following - // code dealt with lcms types. Add something like this - // back when we gain support for CMYK. - // Adobe Photoshop writes YCCK/CMYK files with inverted data - if (mInfo.out_color_space == JCS_CMYK) { - type |= FLAVOR_SH(mInfo.saw_Adobe_marker ? 1 : 0); - } +#if 0 + // We don't currently support CMYK profiles. The following + // code dealt with lcms types. Add something like this + // back when we gain support for CMYK. + + // Adobe Photoshop writes YCCK/CMYK files with inverted data + if (mInfo.out_color_space == JCS_CMYK) { + type |= FLAVOR_SH(mInfo.saw_Adobe_marker ? 1 : 0); + } #endif - if (gfxPlatform::GetCMSOutputProfile()) { + if (type) { // Calculate rendering intent. int intent = gfxPlatform::GetRenderingIntent(); if (intent == -1) { @@ -333,40 +319,24 @@ LexerTransition nsJPEGDecoder::ReadJPEGData( // Create the color management transform. mTransform = qcms_transform_create( - mInProfile, type, gfxPlatform::GetCMSOutputProfile(), - QCMS_DATA_RGB_8, (qcms_intent)intent); + mInProfile, *type, gfxPlatform::GetCMSOutputProfile(), + QCMS_DATA_BGRA_8, (qcms_intent)intent); } - } else { -#ifdef DEBUG_tor - fprintf(stderr, "ICM profile colorspace mismatch\n"); -#endif + } else if (mCMSMode == eCMSMode_All) { + mTransform = gfxPlatform::GetCMSBGRATransform(); } } - if (!mTransform) { - switch (mInfo.jpeg_color_space) { - case JCS_GRAYSCALE: - case JCS_RGB: - case JCS_YCbCr: - // if we're not color managing we can decode directly to - // MOZ_JCS_EXT_NATIVE_ENDIAN_XRGB - if (mCMSMode != eCMSMode_All) { - mInfo.out_color_space = MOZ_JCS_EXT_NATIVE_ENDIAN_XRGB; - mInfo.out_color_components = 4; - } else { - mInfo.out_color_space = JCS_RGB; - } - break; - case JCS_CMYK: - case JCS_YCCK: - // libjpeg can convert from YCCK to CMYK, but not to RGB - mInfo.out_color_space = JCS_CMYK; - break; - default: - mState = JPEG_ERROR; - MOZ_LOG(sJPEGDecoderAccountingLog, LogLevel::Debug, - ("} (unknown colorpsace (3))")); - return Transition::TerminateFailure(); + // We don't want to use the pipe buffers directly because we don't want + // any reads on non-BGRA formatted data. + if (mInfo.out_color_space == JCS_GRAYSCALE || + mInfo.out_color_space == JCS_CMYK) { + mCMSLine = new (std::nothrow) uint32_t[mInfo.image_width]; + if (!mCMSLine) { + mState = JPEG_ERROR; + MOZ_LOG(sJPEGDecoderAccountingLog, LogLevel::Debug, + ("} (could allocate buffer for color conversion)")); + return Transition::TerminateFailure(); } } @@ -378,25 +348,24 @@ LexerTransition nsJPEGDecoder::ReadJPEGData( /* Used to set up image size so arrays can be allocated */ jpeg_calc_output_dimensions(&mInfo); - MOZ_ASSERT(!mImageData, "Already have a buffer allocated?"); - nsresult rv = AllocateFrame(OutputSize(), SurfaceFormat::B8G8R8X8); - if (NS_FAILED(rv)) { + // We handle the transform outside the pipeline if we are outputting in + // grayscale, because the pipeline wants BGRA pixels, particularly the + // downscaling filter, so we can't handle it after downscaling as would + // be optimal. + qcms_transform* pipeTransform = + mInfo.out_color_space != JCS_GRAYSCALE ? mTransform : nullptr; + + Maybe pipe = SurfacePipeFactory::CreateSurfacePipe( + this, Size(), OutputSize(), FullFrame(), SurfaceFormat::B8G8R8X8, + Nothing(), pipeTransform, SurfacePipeFlags()); + if (!pipe) { mState = JPEG_ERROR; MOZ_LOG(sJPEGDecoderAccountingLog, LogLevel::Debug, - ("} (could not initialize image frame)")); + ("} (could not initialize surface pipe)")); return Transition::TerminateFailure(); } - MOZ_ASSERT(mImageData, "Should have a buffer now"); - - if (mDownscaler) { - nsresult rv = mDownscaler->BeginFrame(Size(), Nothing(), mImageData, - /* aHasAlpha = */ false); - if (NS_FAILED(rv)) { - mState = JPEG_ERROR; - return Transition::TerminateFailure(); - } - } + mPipe = std::move(*pipe); MOZ_LOG(sJPEGDecoderAccountingLog, LogLevel::Debug, (" JPEGDecoderAccounting: nsJPEGDecoder::" @@ -442,20 +411,24 @@ LexerTransition nsJPEGDecoder::ReadJPEGData( "nsJPEGDecoder::Write -- " "JPEG_DECOMPRESS_SEQUENTIAL case"); - bool suspend; - OutputScanlines(&suspend); - - if (suspend) { - MOZ_LOG(sJPEGDecoderAccountingLog, LogLevel::Debug, - ("} (I/O suspension after OutputScanlines() - SEQUENTIAL)")); - return Transition::ContinueUnbuffered( - State::JPEG_DATA); // I/O suspension + switch (OutputScanlines()) { + case WriteState::NEED_MORE_DATA: + MOZ_LOG( + sJPEGDecoderAccountingLog, LogLevel::Debug, + ("} (I/O suspension after OutputScanlines() - SEQUENTIAL)")); + return Transition::ContinueUnbuffered( + State::JPEG_DATA); // I/O suspension + case WriteState::FINISHED: + NS_ASSERTION(mInfo.output_scanline == mInfo.output_height, + "We didn't process all of the data!"); + mState = JPEG_DONE; + break; + case WriteState::FAILURE: + mState = JPEG_ERROR; + MOZ_LOG(sJPEGDecoderAccountingLog, LogLevel::Debug, + ("} (Error in pipeline from OutputScalines())")); + return Transition::TerminateFailure(); } - - // If we've completed image output ... - NS_ASSERTION(mInfo.output_scanline == mInfo.output_height, - "We didn't process all of the data!"); - mState = JPEG_DONE; } MOZ_FALLTHROUGH; // to decompress progressive JPEG. } @@ -470,7 +443,7 @@ LexerTransition nsJPEGDecoder::ReadJPEGData( status = jpeg_consume_input(&mInfo); } while ((status != JPEG_SUSPENDED) && (status != JPEG_REACHED_EOI)); - for (;;) { + while (mState != JPEG_DONE) { if (mInfo.output_scanline == 0) { int scan = mInfo.input_scan_number; @@ -494,43 +467,45 @@ LexerTransition nsJPEGDecoder::ReadJPEGData( mInfo.output_scanline = 0; } - bool suspend; - OutputScanlines(&suspend); - - if (suspend) { - if (mInfo.output_scanline == 0) { - // didn't manage to read any lines - flag so we don't call - // jpeg_start_output() multiple times for the same scan - mInfo.output_scanline = 0xffffff; - } - MOZ_LOG( - sJPEGDecoderAccountingLog, LogLevel::Debug, - ("} (I/O suspension after OutputScanlines() - PROGRESSIVE)")); - return Transition::ContinueUnbuffered( - State::JPEG_DATA); // I/O suspension - } - - if (mInfo.output_scanline == mInfo.output_height) { - if (!jpeg_finish_output(&mInfo)) { + switch (OutputScanlines()) { + case WriteState::NEED_MORE_DATA: + if (mInfo.output_scanline == 0) { + // didn't manage to read any lines - flag so we don't call + // jpeg_start_output() multiple times for the same scan + mInfo.output_scanline = 0xffffff; + } MOZ_LOG(sJPEGDecoderAccountingLog, LogLevel::Debug, - ("} (I/O suspension after jpeg_finish_output() -" - " PROGRESSIVE)")); + ("} (I/O suspension after OutputScanlines() - " + "PROGRESSIVE)")); return Transition::ContinueUnbuffered( State::JPEG_DATA); // I/O suspension - } + case WriteState::FINISHED: + NS_ASSERTION(mInfo.output_scanline == mInfo.output_height, + "We didn't process all of the data!"); - if (jpeg_input_complete(&mInfo) && - (mInfo.input_scan_number == mInfo.output_scan_number)) + if (!jpeg_finish_output(&mInfo)) { + MOZ_LOG(sJPEGDecoderAccountingLog, LogLevel::Debug, + ("} (I/O suspension after jpeg_finish_output() -" + " PROGRESSIVE)")); + return Transition::ContinueUnbuffered( + State::JPEG_DATA); // I/O suspension + } + + if (jpeg_input_complete(&mInfo) && + (mInfo.input_scan_number == mInfo.output_scan_number)) { + mState = JPEG_DONE; + } else { + mInfo.output_scanline = 0; + mPipe.ResetToFirstRow(); + } break; - - mInfo.output_scanline = 0; - if (mDownscaler) { - mDownscaler->ResetForNextProgressivePass(); - } + case WriteState::FAILURE: + mState = JPEG_ERROR; + MOZ_LOG(sJPEGDecoderAccountingLog, LogLevel::Debug, + ("} (Error in pipeline from OutputScalines())")); + return Transition::TerminateFailure(); } } - - mState = JPEG_DONE; } MOZ_FALLTHROUGH; // to finish decompressing. } @@ -577,7 +552,7 @@ LexerTransition nsJPEGDecoder::ReadJPEGData( MOZ_ASSERT_UNREACHABLE("Escaped the JPEG decoder state machine"); return Transition::TerminateFailure(); -} +} // namespace image LexerTransition nsJPEGDecoder::FinishedJPEGData() { // Since we set up an unbuffered read for SIZE_MAX bytes, if we actually read @@ -612,120 +587,45 @@ void nsJPEGDecoder::NotifyDone() { PostDecodeDone(); } -void nsJPEGDecoder::FinishRow(uint32_t aLastSourceRow) { - if (mDownscaler) { - mDownscaler->CommitRow(); - if (mDownscaler->HasInvalidation()) { - DownscalerInvalidRect invalidRect = mDownscaler->TakeInvalidRect(); - PostInvalidation(invalidRect.mOriginalSizeRect, - Some(invalidRect.mTargetSizeRect)); - MOZ_ASSERT(!mDownscaler->HasInvalidation()); - } - } else if (aLastSourceRow != mInfo.output_scanline) { - PostInvalidation(nsIntRect(0, aLastSourceRow, mInfo.output_width, - mInfo.output_scanline - aLastSourceRow)); - } -} - -void nsJPEGDecoder::OutputScanlines(bool* suspend) { - *suspend = false; - - while ((mInfo.output_scanline < mInfo.output_height)) { - const uint32_t top = mInfo.output_scanline; - uint32_t* imageRow = nullptr; - if (mDownscaler) { - imageRow = reinterpret_cast(mDownscaler->RowBuffer()); - } else { - imageRow = reinterpret_cast(mImageData) + - (mInfo.output_scanline * mInfo.output_width); - } - - MOZ_ASSERT(imageRow, "Should have a row buffer here"); - - if (mInfo.out_color_space == MOZ_JCS_EXT_NATIVE_ENDIAN_XRGB) { - // Special case: scanline will be directly converted into packed ARGB - if (jpeg_read_scanlines(&mInfo, (JSAMPARRAY)&imageRow, 1) != 1) { - *suspend = true; // suspend - break; - } - FinishRow(top); - continue; // all done for this row! - } - - JSAMPROW sampleRow = (JSAMPROW)imageRow; - if (mInfo.output_components == 3) { - // Put the pixels at end of row to enable in-place expansion - sampleRow += mInfo.output_width; - } - - // Request one scanline. Returns 0 or 1 scanlines. - if (jpeg_read_scanlines(&mInfo, &sampleRow, 1) != 1) { - *suspend = true; // suspend - break; - } - - if (mTransform) { - JSAMPROW source = sampleRow; - if (mInfo.out_color_space == JCS_GRAYSCALE) { - // Convert from the 1byte grey pixels at begin of row - // to the 3byte RGB byte pixels at 'end' of row - sampleRow += mInfo.output_width; - } - qcms_transform_data(mTransform, source, sampleRow, mInfo.output_width); - // Move 3byte RGB data to end of row - if (mInfo.out_color_space == JCS_CMYK) { - memmove(sampleRow + mInfo.output_width, sampleRow, - 3 * mInfo.output_width); - sampleRow += mInfo.output_width; - } - } else { - if (mInfo.out_color_space == JCS_CMYK) { - // Convert from CMYK to RGB - // We cannot convert directly to Cairo, as the CMSRGBTransform - // may wants to do a RGB transform... - // Would be better to have platform CMSenabled transformation - // from CMYK to (A)RGB... - cmyk_convert_rgb((JSAMPROW)imageRow, mInfo.output_width); - sampleRow += mInfo.output_width; - } - if (mCMSMode == eCMSMode_All) { - // No embedded ICC profile - treat as sRGB - qcms_transform* transform = gfxPlatform::GetCMSRGBTransform(); - if (transform) { - qcms_transform_data(transform, sampleRow, sampleRow, - mInfo.output_width); +WriteState nsJPEGDecoder::OutputScanlines() { + auto result = mPipe.WritePixelBlocks( + [&](uint32_t* aPixelBlock, int32_t aBlockSize) { + JSAMPROW sampleRow = (JSAMPROW)(mCMSLine ? mCMSLine : aPixelBlock); + if (jpeg_read_scanlines(&mInfo, &sampleRow, 1) != 1) { + return MakeTuple(/* aWritten */ 0, Some(WriteState::NEED_MORE_DATA)); } - } - } - // counter for while() loops below - uint32_t idx = mInfo.output_width; + switch (mInfo.out_color_space) { + default: + // Already outputted directly to aPixelBlock as BGRA. + MOZ_ASSERT(!mCMSLine); + break; + case JCS_GRAYSCALE: + // The transform here does both color management, and converts the + // pixels from grayscale to BGRA. This is why we do it here, instead + // of using ColorManagementFilter in the SurfacePipe, because the + // other filters (e.g. DownscalingFilter) require BGRA pixels. + MOZ_ASSERT(mCMSLine); + qcms_transform_data(mTransform, mCMSLine, aPixelBlock, + mInfo.output_width); + break; + case JCS_CMYK: + // Convert from CMYK to BGRA + MOZ_ASSERT(mCMSLine); + cmyk_convert_bgra(mCMSLine, aPixelBlock, aBlockSize); + break; + } - // copy as bytes until source pointer is 32-bit-aligned - for (; (NS_PTR_TO_UINT32(sampleRow) & 0x3) && idx; --idx) { - *imageRow++ = - gfxPackedPixel(0xFF, sampleRow[0], sampleRow[1], sampleRow[2]); - sampleRow += 3; - } + return MakeTuple(aBlockSize, Maybe()); + }); - // copy pixels in blocks of 4 - while (idx >= 4) { - GFX_BLOCK_RGB_TO_FRGB(sampleRow, imageRow); - idx -= 4; - sampleRow += 12; - imageRow += 4; - } - - // copy remaining pixel(s) - while (idx--) { - // 32-bit read of final pixel will exceed buffer, so read bytes - *imageRow++ = - gfxPackedPixel(0xFF, sampleRow[0], sampleRow[1], sampleRow[2]); - sampleRow += 3; - } - - FinishRow(top); + Maybe invalidRect = mPipe.TakeInvalidRect(); + if (invalidRect) { + PostInvalidation(invalidRect->mInputSpaceRect, + Some(invalidRect->mOutputSpaceRect)); } + + return result; } // Override the standard error method in the IJG JPEG decoder code. @@ -954,18 +854,16 @@ term_source(j_decompress_ptr jd) { ///*************** Inverted CMYK -> RGB conversion ************************* /// Input is (Inverted) CMYK stored as 4 bytes per pixel. /// Output is RGB stored as 3 bytes per pixel. -/// @param row Points to row buffer containing the CMYK bytes for each pixel -/// in the row. -/// @param width Number of pixels in the row. -static void cmyk_convert_rgb(JSAMPROW row, JDIMENSION width) { - // Work from end to front to shrink from 4 bytes per pixel to 3 - JSAMPROW in = row + width * 4; - JSAMPROW out = in; - - for (uint32_t i = width; i > 0; i--) { - in -= 4; - out -= 3; +/// @param aInput Points to row buffer containing the CMYK bytes for each pixel +/// in the row. +/// @param aOutput Points to row buffer to write BGRA to. +/// @param aWidth Number of pixels in the row. +static void cmyk_convert_bgra(uint32_t* aInput, uint32_t* aOutput, + int32_t aWidth) { + uint8_t* input = reinterpret_cast(aInput); + uint8_t* output = reinterpret_cast(aOutput); + for (int32_t i = 0; i < aWidth; ++i) { // Source is 'Inverted CMYK', output is RGB. // See: http://www.easyrgb.com/math.php?MATH=M12#text12 // Or: http://www.ilkeratalay.com/colorspacesfaq.php#rgb @@ -985,12 +883,23 @@ static void cmyk_convert_rgb(JSAMPROW row, JDIMENSION width) { // B = 1 - Y => 1 - (1 - iY*iK) => iY*iK // Convert from Inverted CMYK (0..255) to RGB (0..255) - const uint32_t iC = in[0]; - const uint32_t iM = in[1]; - const uint32_t iY = in[2]; - const uint32_t iK = in[3]; - out[0] = iC * iK / 255; // Red - out[1] = iM * iK / 255; // Green - out[2] = iY * iK / 255; // Blue + const uint32_t iC = input[0]; + const uint32_t iM = input[1]; + const uint32_t iY = input[2]; + const uint32_t iK = input[3]; +#if MOZ_BIG_ENDIAN + output[0] = 0xFF; // Alpha + output[1] = iC * iK / 255; // Red + output[2] = iM * iK / 255; // Green + output[3] = iY * iK / 255; // Blue +#else + output[0] = iY * iK / 255; // Blue + output[1] = iM * iK / 255; // Green + output[2] = iC * iK / 255; // Red + output[3] = 0xFF; // Alpha +#endif + + input += 4; + output += 4; } } diff --git a/image/decoders/nsJPEGDecoder.h b/image/decoders/nsJPEGDecoder.h index 82f99ff6df23..7502a9ffbe6a 100644 --- a/image/decoders/nsJPEGDecoder.h +++ b/image/decoders/nsJPEGDecoder.h @@ -8,6 +8,8 @@ #define mozilla_image_decoders_nsJPEGDecoder_h #include "RasterImage.h" +#include "SurfacePipe.h" + // On Windows systems, RasterImage.h brings in 'windows.h', which defines INT32. // But the jpeg decoder has its own definition of INT32. To avoid build issues, // we need to undefine the version from 'windows.h'. @@ -15,9 +17,6 @@ #include "Decoder.h" -#include "nsIInputStream.h" -#include "nsIPipe.h" - extern "C" { #include "jpeglib.h" } @@ -64,7 +63,7 @@ class nsJPEGDecoder : public Decoder { protected: Orientation ReadOrientationFromEXIF(); - void OutputScanlines(bool* suspend); + WriteState OutputScanlines(); private: friend class DecoderFactory; @@ -99,11 +98,15 @@ class nsJPEGDecoder : public Decoder { JOCTET* mProfile; uint32_t mProfileLength; + uint32_t* mCMSLine; + bool mReading; const Decoder::DecodeStyle mDecodeStyle; uint32_t mCMSMode; + + SurfacePipe mPipe; }; } // namespace image From 680528af692da402b617fa262668849a11e67254 Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Wed, 22 May 2019 14:30:40 -0700 Subject: [PATCH 12/14] Bug 1551183: Part 1 - Add SCOPE_APPLICATION to enabledScopes for Fennec. r=aswan This scope now controls add-ons bundled in omni.ja as well as those in the app directory, so we need to enable it on Fennec in order for the default theme to work. Differential Revision: https://phabricator.services.mozilla.com/D32223 --HG-- extra : rebase_source : 3dd7ddec5b73e41dc385e60f2e8f8ce36db5be1b --- mobile/android/app/mobile.js | 2 +- mobile/android/chrome/content/browser.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/mobile/android/app/mobile.js b/mobile/android/app/mobile.js index 75d242c2d5c7..7036b3a49215 100644 --- a/mobile/android/app/mobile.js +++ b/mobile/android/app/mobile.js @@ -191,7 +191,7 @@ pref("xpinstall.signatures.required", true); // constants in AddonManager.jsm for values to use here, and Bug 1405528 for a rationale). pref("extensions.autoDisableScopes", 15); -pref("extensions.enabledScopes", 1); +pref("extensions.enabledScopes", 5); pref("extensions.autoupdate.enabled", true); pref("extensions.autoupdate.interval", 86400); pref("extensions.update.enabled", true); diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js index 8568e1e4ffcf..2a330d3b517c 100644 --- a/mobile/android/chrome/content/browser.js +++ b/mobile/android/chrome/content/browser.js @@ -466,7 +466,7 @@ var BrowserApp = { if (!ParentalControls.isAllowed(ParentalControls.INSTALL_EXTENSION)) { // Disable extension installs - Services.prefs.setIntPref("extensions.enabledScopes", 1); + Services.prefs.setIntPref("extensions.enabledScopes", 5); Services.prefs.setIntPref("extensions.autoDisableScopes", 1); Services.prefs.setBoolPref("xpinstall.enabled", false); } else if (ParentalControls.parentalControlsEnabled) { From d3aac04243fed41b998216ca8a6d60f0e39fcffc Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Wed, 22 May 2019 14:34:07 -0700 Subject: [PATCH 13/14] Bug 1551183: Part 2 - Support enumerating directories in extensions in nested JARs. r=aswan On Android, omni.ja is bundled inside an API, and therefore loaded as a nested JAR. That means that its resource URIs resolve to something resembling "jar:jar:...!/omni.ja!/...". Our current enumeration code assumes no nesting of jar: URIs, and therefore can't handle this. This patch changes our enumeration helpers to accept URIs rather than nsIFile instances, and to correctly handle resolving the ZipReaders for those nested JARs. It also moves the path filter generation into the native helper to make things easier for other callers which may need this behavior. Differential Revision: https://phabricator.services.mozilla.com/D32224 --HG-- extra : rebase_source : 2a4f236f6066ddf65cf7ee78af87496dd3b4f882 --- toolkit/components/extensions/Extension.jsm | 20 +-- .../extensions/AddonManagerStartup.cpp | 130 +++++++++++++----- .../extensions/amIAddonManagerStartup.idl | 24 +++- 3 files changed, 121 insertions(+), 53 deletions(-) diff --git a/toolkit/components/extensions/Extension.jsm b/toolkit/components/extensions/Extension.jsm index 27bfbeef2919..fc8536fabefd 100644 --- a/toolkit/components/extensions/Extension.jsm +++ b/toolkit/components/extensions/Extension.jsm @@ -428,28 +428,22 @@ class ExtensionData { } let uri = this.rootURI.QueryInterface(Ci.nsIJARURI); - let file = uri.JARFile.QueryInterface(Ci.nsIFileURL).file; - // Normalize the directory path. - path = `${uri.JAREntry}/${path}`; - path = path.replace(/\/\/+/g, "/").replace(/^\/|\/$/g, "") + "/"; - if (path === "/") { - path = ""; - } - - // Escape pattern metacharacters. - let pattern = path.replace(/[[\]()?*~|$\\]/g, "\\$&") + "*"; + // Append the sub-directory path to the base JAR URI and normalize the + // result. + let entry = `${uri.JAREntry}/${path}/`.replace(/\/\/+/g, "/").replace(/^\//, ""); + uri = Services.io.newURI(`jar:${uri.JARFile.spec}!/${entry}`); let results = []; - for (let name of aomStartup.enumerateZipFile(file, pattern)) { - if (!name.startsWith(path)) { + for (let name of aomStartup.enumerateJARSubtree(uri)) { + if (!name.startsWith(entry)) { throw new Error("Unexpected ZipReader entry"); } // The enumerator returns the full path of all entries. // Trim off the leading path, and filter out entries from // subdirectories. - name = name.slice(path.length); + name = name.slice(entry.length); if (name && !/\/./.test(name)) { results.push({ name: name.replace("/", ""), diff --git a/toolkit/mozapps/extensions/AddonManagerStartup.cpp b/toolkit/mozapps/extensions/AddonManagerStartup.cpp index e5763a6c9253..65498f443cdf 100644 --- a/toolkit/mozapps/extensions/AddonManagerStartup.cpp +++ b/toolkit/mozapps/extensions/AddonManagerStartup.cpp @@ -75,6 +75,44 @@ nsIFile* AddonManagerStartup::ProfileDir() { NS_IMPL_ISUPPORTS(AddonManagerStartup, amIAddonManagerStartup, nsIObserver) +/***************************************************************************** + * URI utils + *****************************************************************************/ + +static nsresult ParseJARURI(nsIJARURI* uri, nsIURI** jarFile, + nsCString& entry) { + MOZ_TRY(uri->GetJARFile(jarFile)); + MOZ_TRY(uri->GetJAREntry(entry)); + + // The entry portion of a jar: URI is required to begin with a '/', but for + // nested JAR URIs, the leading / of the outer entry is currently stripped. + // This is a bug which should be fixed in the JAR URI code, but... + if (entry.IsEmpty() || entry[0] != '/') { + entry.Insert('/', 0); + } + return NS_OK; +} + +static nsresult ParseJARURI(nsIURI* uri, nsIURI** jarFile, nsCString& entry) { + nsresult rv; + nsCOMPtr jarURI = do_QueryInterface(uri, &rv); + MOZ_TRY(rv); + + return ParseJARURI(jarURI, jarFile, entry); +} + +static Result, nsresult> GetFile(nsIURI* uri) { + nsresult rv; + nsCOMPtr fileURL = do_QueryInterface(uri, &rv); + MOZ_TRY(rv); + + nsCOMPtr file; + MOZ_TRY(fileURL->GetFile(getter_AddRefs(file))); + MOZ_ASSERT(file); + + return std::move(file); +} + /***************************************************************************** * File utils *****************************************************************************/ @@ -210,19 +248,11 @@ static Result GetFileLocation(nsIURI* uri) { MOZ_TRY(fileURL->GetFile(getter_AddRefs(file))); location.Init(file); } else { - nsCOMPtr jarURI = do_QueryInterface(uri); - NS_ENSURE_TRUE(jarURI, Err(NS_ERROR_INVALID_ARG)); - nsCOMPtr fileURI; - MOZ_TRY(jarURI->GetJARFile(getter_AddRefs(fileURI))); - - fileURL = do_QueryInterface(fileURI); - NS_ENSURE_TRUE(fileURL, Err(NS_ERROR_INVALID_ARG)); - - MOZ_TRY(fileURL->GetFile(getter_AddRefs(file))); - nsCString entry; - MOZ_TRY(jarURI->GetJAREntry(entry)); + MOZ_TRY(ParseJARURI(uri, getter_AddRefs(fileURI), entry)); + + MOZ_TRY_VAR(file, GetFile(fileURI)); location.Init(file, entry.get()); } @@ -562,24 +592,11 @@ nsresult AddonManagerStartup::DecodeBlob(JS::HandleValue value, JSContext* cx, ; } -nsresult AddonManagerStartup::EnumerateZipFile(nsIFile* file, - const nsACString& pattern, - uint32_t* countOut, - char16_t*** entriesOut) { - NS_ENSURE_ARG_POINTER(file); - NS_ENSURE_ARG_POINTER(countOut); - NS_ENSURE_ARG_POINTER(entriesOut); - - nsCOMPtr zipCache; - MOZ_TRY_VAR(zipCache, GetJarCache()); - - nsCOMPtr zip; - MOZ_TRY(zipCache->GetZip(file, getter_AddRefs(zip))); - +static nsresult EnumerateZip(nsIZipReader* zip, const nsACString& pattern, + nsTArray& results) { nsCOMPtr entries; MOZ_TRY(zip->FindEntries(pattern, getter_AddRefs(entries))); - nsTArray results; bool hasMore; while (NS_SUCCEEDED(entries->HasMore(&hasMore)) && hasMore) { nsAutoCString name; @@ -588,17 +605,62 @@ nsresult AddonManagerStartup::EnumerateZipFile(nsIFile* file, results.AppendElement(NS_ConvertUTF8toUTF16(name)); } - auto strResults = MakeUnique(results.Length()); - for (uint32_t i = 0; i < results.Length(); i++) { - strResults[i] = ToNewUnicode(results[i]); - } - - *countOut = results.Length(); - *entriesOut = strResults.release(); - return NS_OK; } +nsresult AddonManagerStartup::EnumerateJAR(nsIURI* uri, + const nsACString& pattern, + nsTArray& results) { + nsCOMPtr zipCache; + MOZ_TRY_VAR(zipCache, GetJarCache()); + + nsCOMPtr zip; + nsCOMPtr file; + if (nsCOMPtr jarURI = do_QueryInterface(uri)) { + nsCOMPtr fileURI; + nsCString entry; + MOZ_TRY(ParseJARURI(jarURI, getter_AddRefs(fileURI), entry)); + + MOZ_TRY_VAR(file, GetFile(fileURI)); + MOZ_TRY(zipCache->GetInnerZip(file, Substring(entry, 1), + getter_AddRefs(zip))); + } else { + MOZ_TRY_VAR(file, GetFile(uri)); + MOZ_TRY(zipCache->GetZip(file, getter_AddRefs(zip))); + } + MOZ_ASSERT(zip); + + return EnumerateZip(zip, pattern, results); +} + +nsresult AddonManagerStartup::EnumerateJARSubtree(nsIURI* uri, + nsTArray& results) { + nsCOMPtr fileURI; + nsCString entry; + MOZ_TRY(ParseJARURI(uri, getter_AddRefs(fileURI), entry)); + + // Mangle the path into a pattern to match all child entries by escaping any + // existing pattern matching metacharacters it contains and appending "/*". + NS_NAMED_LITERAL_CSTRING(metaChars, "[]()?*~|$\\"); + + nsCString pattern; + pattern.SetCapacity(entry.Length()); + + // The first character of the entry name is "/", which we want to skip. + for (auto chr : MakeSpan(Substring(entry, 1))) { + if (metaChars.FindChar(chr) >= 0) { + pattern.Append('\\'); + } + pattern.Append(chr); + } + if (!pattern.IsEmpty() && !StringEndsWith(pattern, NS_LITERAL_CSTRING("/"))) { + pattern.Append('/'); + } + pattern.Append('*'); + + return EnumerateJAR(fileURI, pattern, results); +} + nsresult AddonManagerStartup::InitializeURLPreloader() { MOZ_RELEASE_ASSERT(xpc::IsInAutomation()); diff --git a/toolkit/mozapps/extensions/amIAddonManagerStartup.idl b/toolkit/mozapps/extensions/amIAddonManagerStartup.idl index d4717a5c60f4..779b9f4cfefd 100644 --- a/toolkit/mozapps/extensions/amIAddonManagerStartup.idl +++ b/toolkit/mozapps/extensions/amIAddonManagerStartup.idl @@ -45,20 +45,32 @@ interface amIAddonManagerStartup : nsISupports jsval decodeBlob(in jsval value); /** - * Enumerates over all entries in the given zip file matching the given - * pattern, and returns an array of their paths. + * Enumerates over all entries in the JAR file at the given URI, and returns + * an array of entry paths which match the given pattern. The URI may be + * either a file: URL pointing directly to a zip file, or a jar: URI + * pointing to a zip file nested within another zip file. Only one level of + * nesting is supported. * * This should be used in preference to manually opening or retrieving a * ZipReader from the zip cache, since the former causes main thread IO and * the latter can lead to file locking issues due to unpredictable GC behavior * keeping the cached ZipReader alive after the cache is flushed. * - * @param file The zip file to enumerate. + * @param uri The URI of the zip file to enumerate. * @param pattern The pattern to match, as passed to nsIZipReader.findEntries. */ - void enumerateZipFile(in nsIFile file, in AUTF8String pattern, - [optional] out unsigned long count, - [retval, array, size_is(count)] out wstring entries); + Array enumerateJAR(in nsIURI uri, in AUTF8String pattern); + + /** + * Similar to |enumerateJAR| above, but accepts the URI of a directory + * within a JAR file, and returns a list of all entries below it. + * + * The given URI must be a jar: URI, and its JAR file must point either to a + * file: URI, or to a singly-nested JAR within another JAR file (i.e., + * "jar:file:///thing.jar!/" or "jar:jar:file:///thing.jar!/stuff.jar!/"). + * Multiple levels of nesting are not supported. + */ + Array enumerateJARSubtree(in nsIURI uri); /** * Initializes the URL Preloader. From f8bc88a3ebc871dfd55b0523c5b0d707d4cf4d05 Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Wed, 22 May 2019 14:34:44 -0700 Subject: [PATCH 14/14] Bug 1551183: Part 3 - Add a test to sanity check the default theme on all platforms. r=aswan Differential Revision: https://phabricator.services.mozilla.com/D32225 --HG-- extra : rebase_source : 7577e30e91c478ae60fa82ad4f8459527bd83024 --- .../extensions/internal/XPIDatabase.jsm | 2 +- .../extensions/test/mochitest/chrome.ini | 2 ++ .../test/mochitest/test_default_theme.html | 28 +++++++++++++++++++ toolkit/mozapps/extensions/test/moz.build | 1 + 4 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 toolkit/mozapps/extensions/test/mochitest/chrome.ini create mode 100644 toolkit/mozapps/extensions/test/mochitest/test_default_theme.html diff --git a/toolkit/mozapps/extensions/internal/XPIDatabase.jsm b/toolkit/mozapps/extensions/internal/XPIDatabase.jsm index a9a55de12013..8d517d9716ed 100644 --- a/toolkit/mozapps/extensions/internal/XPIDatabase.jsm +++ b/toolkit/mozapps/extensions/internal/XPIDatabase.jsm @@ -396,7 +396,7 @@ class AddonInternal { } get hidden() { - return this.location.hidden || (this._hidden && this.isPrivileged); + return this.location.hidden || (this._hidden && this.isPrivileged) || false; } set hidden(val) { diff --git a/toolkit/mozapps/extensions/test/mochitest/chrome.ini b/toolkit/mozapps/extensions/test/mochitest/chrome.ini new file mode 100644 index 000000000000..a8a8d49e3c56 --- /dev/null +++ b/toolkit/mozapps/extensions/test/mochitest/chrome.ini @@ -0,0 +1,2 @@ + +[test_default_theme.html] diff --git a/toolkit/mozapps/extensions/test/mochitest/test_default_theme.html b/toolkit/mozapps/extensions/test/mochitest/test_default_theme.html new file mode 100644 index 000000000000..84f91be50dcd --- /dev/null +++ b/toolkit/mozapps/extensions/test/mochitest/test_default_theme.html @@ -0,0 +1,28 @@ + + + + + Test for correct installation of default theme + + + + + + + + + diff --git a/toolkit/mozapps/extensions/test/moz.build b/toolkit/mozapps/extensions/test/moz.build index cdcd666c312d..ac88768486e0 100644 --- a/toolkit/mozapps/extensions/test/moz.build +++ b/toolkit/mozapps/extensions/test/moz.build @@ -8,6 +8,7 @@ DIRS += ['browser'] BROWSER_CHROME_MANIFESTS += ['xpinstall/browser.ini'] MOCHITEST_MANIFESTS += ['mochitest/mochitest.ini'] +MOCHITEST_CHROME_MANIFESTS += ['mochitest/chrome.ini'] XPCSHELL_TESTS_MANIFESTS += [ 'xpcshell/rs-blocklist/xpcshell.ini',