From 6ba1b50abd8dbe83c41310aacfa2ec2fd478b6e8 Mon Sep 17 00:00:00 2001 From: longsonr Date: Wed, 10 Apr 2019 05:21:44 +0100 Subject: [PATCH 01/22] Bug 1542646 Part 1 - Correct namespace of forward declaration of DOMSVGAnimatedPreserveAspectRatio r=dholbert --- dom/svg/SVGViewportElement.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dom/svg/SVGViewportElement.h b/dom/svg/SVGViewportElement.h index c8b31b9c3c5a..b6c0fc8f5852 100644 --- a/dom/svg/SVGViewportElement.h +++ b/dom/svg/SVGViewportElement.h @@ -25,9 +25,9 @@ class nsSVGViewportFrame; namespace mozilla { class AutoPreserveAspectRatioOverride; -class DOMSVGAnimatedPreserveAspectRatio; namespace dom { +class DOMSVGAnimatedPreserveAspectRatio; class SVGAnimatedRect; class SVGViewElement; class SVGViewportElement; From f2c2d74da8dbc9b9b997e8fd4e4425afbee093b9 Mon Sep 17 00:00:00 2001 From: longsonr Date: Wed, 10 Apr 2019 05:34:35 +0100 Subject: [PATCH 02/22] Bug 1542646 Part 2 - Use default keyword instead of empty constructors and destructors r=dholbert --- dom/svg/SVGAnimatedLength.h | 2 +- dom/svg/SVGAnimatedLengthList.h | 2 +- dom/svg/SVGAnimatedPathSegList.h | 2 +- dom/svg/SVGAnimatedPointList.h | 2 +- dom/svg/SVGComponentTransferFunctionElement.h | 2 +- dom/svg/SVGFilters.h | 4 ++-- dom/svg/SVGIRect.h | 2 +- dom/svg/SVGIntegerPairSMILType.h | 2 +- dom/svg/SVGLengthList.h | 4 ++-- dom/svg/SVGLengthListSMILType.h | 2 +- dom/svg/SVGMatrix.h | 4 ++-- dom/svg/SVGMotionSMILType.h | 2 +- dom/svg/SVGNumberList.h | 4 ++-- dom/svg/SVGNumberListSMILType.h | 2 +- dom/svg/SVGNumberPairSMILType.h | 2 +- dom/svg/SVGOrientSMILType.h | 2 +- dom/svg/SVGPathData.h | 4 ++-- dom/svg/SVGPathSegListSMILType.h | 2 +- dom/svg/SVGPathSegUtils.h | 2 +- dom/svg/SVGPointList.h | 4 ++-- dom/svg/SVGPointListSMILType.h | 2 +- dom/svg/SVGRect.h | 2 +- dom/svg/SVGSVGElement.h | 2 +- dom/svg/SVGStringList.h | 2 +- dom/svg/SVGTests.h | 2 +- dom/svg/SVGTransformList.h | 4 ++-- dom/svg/SVGTransformListSMILType.h | 2 +- dom/svg/SVGTransformableElement.h | 2 +- dom/svg/SVGViewBoxSMILType.h | 2 +- 29 files changed, 36 insertions(+), 36 deletions(-) diff --git a/dom/svg/SVGAnimatedLength.h b/dom/svg/SVGAnimatedLength.h index 01ee773cef28..580675c2f8cc 100644 --- a/dom/svg/SVGAnimatedLength.h +++ b/dom/svg/SVGAnimatedLength.h @@ -33,7 +33,7 @@ class SVGViewportElement; class UserSpaceMetrics { public: - virtual ~UserSpaceMetrics() {} + virtual ~UserSpaceMetrics() = default; virtual float GetEmLength() const = 0; virtual float GetExLength() const = 0; diff --git a/dom/svg/SVGAnimatedLengthList.h b/dom/svg/SVGAnimatedLengthList.h index 1454155ebd76..4cd5f4693cf5 100644 --- a/dom/svg/SVGAnimatedLengthList.h +++ b/dom/svg/SVGAnimatedLengthList.h @@ -42,7 +42,7 @@ class SVGAnimatedLengthList { friend class dom::DOMSVGLengthList; public: - SVGAnimatedLengthList() {} + SVGAnimatedLengthList() = default; /** * Because it's so important that mBaseVal and its DOMSVGLengthList wrapper diff --git a/dom/svg/SVGAnimatedPathSegList.h b/dom/svg/SVGAnimatedPathSegList.h index 3bd4e1d04fc9..0b369d5ceec1 100644 --- a/dom/svg/SVGAnimatedPathSegList.h +++ b/dom/svg/SVGAnimatedPathSegList.h @@ -44,7 +44,7 @@ class SVGAnimatedPathSegList final { friend class DOMSVGPathSegList; public: - SVGAnimatedPathSegList() {} + SVGAnimatedPathSegList() = default; /** * Because it's so important that mBaseVal and its DOMSVGPathSegList wrapper diff --git a/dom/svg/SVGAnimatedPointList.h b/dom/svg/SVGAnimatedPointList.h index cf7d67c339ab..157ecf3709be 100644 --- a/dom/svg/SVGAnimatedPointList.h +++ b/dom/svg/SVGAnimatedPointList.h @@ -43,7 +43,7 @@ class SVGAnimatedPointList { friend class DOMSVGPointList; public: - SVGAnimatedPointList() {} + SVGAnimatedPointList() = default; /** * Because it's so important that mBaseVal and its DOMSVGPointList wrapper diff --git a/dom/svg/SVGComponentTransferFunctionElement.h b/dom/svg/SVGComponentTransferFunctionElement.h index ae7fff6a2d25..cf3ed14cc444 100644 --- a/dom/svg/SVGComponentTransferFunctionElement.h +++ b/dom/svg/SVGComponentTransferFunctionElement.h @@ -34,7 +34,7 @@ class SVGComponentTransferFunctionElement already_AddRefed&& aNodeInfo) : SVGComponentTransferFunctionElementBase(std::move(aNodeInfo)) {} - virtual ~SVGComponentTransferFunctionElement() {} + virtual ~SVGComponentTransferFunctionElement() = default; public: typedef gfx::ComponentTransferAttributes ComponentTransferAttributes; diff --git a/dom/svg/SVGFilters.h b/dom/svg/SVGFilters.h index 7960a9cbc2dd..f594e7b0bc2f 100644 --- a/dom/svg/SVGFilters.h +++ b/dom/svg/SVGFilters.h @@ -55,7 +55,7 @@ class SVGFE : public SVGFEBase { explicit SVGFE(already_AddRefed&& aNodeInfo) : SVGFEBase(std::move(aNodeInfo)) {} - virtual ~SVGFE() {} + virtual ~SVGFE() = default; public: typedef mozilla::gfx::PrimitiveAttributes PrimitiveAttributes; @@ -183,7 +183,7 @@ class SVGFELightingElement : public SVGFELightingElementBase { already_AddRefed&& aNodeInfo) : SVGFELightingElementBase(std::move(aNodeInfo)) {} - virtual ~SVGFELightingElement() {} + virtual ~SVGFELightingElement() = default; public: // interfaces: diff --git a/dom/svg/SVGIRect.h b/dom/svg/SVGIRect.h index 01147f2e5b04..62055d5309e8 100644 --- a/dom/svg/SVGIRect.h +++ b/dom/svg/SVGIRect.h @@ -20,7 +20,7 @@ namespace dom { class SVGIRect : public nsISupports, public nsWrapperCache { public: - virtual ~SVGIRect() {} + virtual ~SVGIRect() = default; JSObject* WrapObject(JSContext* aCx, JS::Handle aGivenProto) override { diff --git a/dom/svg/SVGIntegerPairSMILType.h b/dom/svg/SVGIntegerPairSMILType.h index 4b15efa083a5..2a6708abcc26 100644 --- a/dom/svg/SVGIntegerPairSMILType.h +++ b/dom/svg/SVGIntegerPairSMILType.h @@ -41,7 +41,7 @@ class SVGIntegerPairSMILType : public SMILType { private: // Private constructor: prevent instances beyond my singleton. - constexpr SVGIntegerPairSMILType() {} + constexpr SVGIntegerPairSMILType() = default; }; } // namespace mozilla diff --git a/dom/svg/SVGLengthList.h b/dom/svg/SVGLengthList.h index dcd1d12f8f95..24c255ee06cc 100644 --- a/dom/svg/SVGLengthList.h +++ b/dom/svg/SVGLengthList.h @@ -39,8 +39,8 @@ class SVGLengthList { friend class SVGAnimatedLengthList; public: - SVGLengthList() {} - ~SVGLengthList() {} + SVGLengthList() = default; + ~SVGLengthList() = default; // Only methods that don't make/permit modification to this list are public. // Only our friend classes can access methods that may change us. diff --git a/dom/svg/SVGLengthListSMILType.h b/dom/svg/SVGLengthListSMILType.h index f4be976a9110..d1f1d8e19ca9 100644 --- a/dom/svg/SVGLengthListSMILType.h +++ b/dom/svg/SVGLengthListSMILType.h @@ -91,7 +91,7 @@ class SVGLengthListSMILType : public SMILType { private: // Private constructor: prevent instances beyond my singleton. - constexpr SVGLengthListSMILType() {} + constexpr SVGLengthListSMILType() = default; }; } // namespace mozilla diff --git a/dom/svg/SVGMatrix.h b/dom/svg/SVGMatrix.h index 85b8ac6f473a..23d8f794068f 100644 --- a/dom/svg/SVGMatrix.h +++ b/dom/svg/SVGMatrix.h @@ -63,7 +63,7 @@ class SVGMatrix final : public nsWrapperCache { * Ctors for SVGMatrix objects created independently of a DOMSVGTransform. */ // Default ctor for gfxMatrix will produce identity mx - SVGMatrix() {} + SVGMatrix() = default; explicit SVGMatrix(const gfxMatrix& aMatrix) : mMatrix(aMatrix) {} @@ -103,7 +103,7 @@ class SVGMatrix final : public nsWrapperCache { already_AddRefed SkewY(float angle, ErrorResult& rv); private: - ~SVGMatrix() {} + ~SVGMatrix() = default; void SetMatrix(const gfxMatrix& aMatrix) { if (mTransform) { diff --git a/dom/svg/SVGMotionSMILType.h b/dom/svg/SVGMotionSMILType.h index 963b155e06f1..8cf0508128fb 100644 --- a/dom/svg/SVGMotionSMILType.h +++ b/dom/svg/SVGMotionSMILType.h @@ -71,7 +71,7 @@ class SVGMotionSMILType : public SMILType { private: // Private constructor: prevent instances beyond my singleton. - constexpr SVGMotionSMILType() {} + constexpr SVGMotionSMILType() = default; }; } // namespace mozilla diff --git a/dom/svg/SVGNumberList.h b/dom/svg/SVGNumberList.h index bc1642eb5166..9a37e7e8f919 100644 --- a/dom/svg/SVGNumberList.h +++ b/dom/svg/SVGNumberList.h @@ -37,8 +37,8 @@ class SVGNumberList { friend class SVGAnimatedNumberList; public: - SVGNumberList() {} - ~SVGNumberList() {} + SVGNumberList() = default; + ~SVGNumberList() = default; // Only methods that don't make/permit modification to this list are public. // Only our friend classes can access methods that may change us. diff --git a/dom/svg/SVGNumberListSMILType.h b/dom/svg/SVGNumberListSMILType.h index bdb88ecdf3d3..4353932390ff 100644 --- a/dom/svg/SVGNumberListSMILType.h +++ b/dom/svg/SVGNumberListSMILType.h @@ -45,7 +45,7 @@ class SVGNumberListSMILType : public SMILType { private: // Private constructor: prevent instances beyond my singleton. - constexpr SVGNumberListSMILType() {} + constexpr SVGNumberListSMILType() = default; }; } // namespace mozilla diff --git a/dom/svg/SVGNumberPairSMILType.h b/dom/svg/SVGNumberPairSMILType.h index 63140330f5a6..e5857de186b1 100644 --- a/dom/svg/SVGNumberPairSMILType.h +++ b/dom/svg/SVGNumberPairSMILType.h @@ -38,7 +38,7 @@ class SVGNumberPairSMILType : public SMILType { private: // Private constructor: prevent instances beyond my singleton. - constexpr SVGNumberPairSMILType() {} + constexpr SVGNumberPairSMILType() = default; }; } // namespace mozilla diff --git a/dom/svg/SVGOrientSMILType.h b/dom/svg/SVGOrientSMILType.h index 16bb7e15fa40..5e959f9392b3 100644 --- a/dom/svg/SVGOrientSMILType.h +++ b/dom/svg/SVGOrientSMILType.h @@ -58,7 +58,7 @@ class SVGOrientSMILType : public SMILType { private: // Private constructor: prevent instances beyond my singleton. - constexpr SVGOrientSMILType() {} + constexpr SVGOrientSMILType() = default; }; } // namespace mozilla diff --git a/dom/svg/SVGPathData.h b/dom/svg/SVGPathData.h index 593015870ba5..f59833343591 100644 --- a/dom/svg/SVGPathData.h +++ b/dom/svg/SVGPathData.h @@ -91,8 +91,8 @@ class SVGPathData { public: typedef const float* const_iterator; - SVGPathData() {} - ~SVGPathData() {} + SVGPathData() = default; + ~SVGPathData() = default; // Only methods that don't make/permit modification to this list are public. // Only our friend classes can access methods that may change us. diff --git a/dom/svg/SVGPathSegListSMILType.h b/dom/svg/SVGPathSegListSMILType.h index b5a08fe9dc4d..b712cd373f21 100644 --- a/dom/svg/SVGPathSegListSMILType.h +++ b/dom/svg/SVGPathSegListSMILType.h @@ -48,7 +48,7 @@ class SVGPathSegListSMILType : public SMILType { private: // Private constructor: prevent instances beyond my singleton. - constexpr SVGPathSegListSMILType() {} + constexpr SVGPathSegListSMILType() = default; }; } // namespace mozilla diff --git a/dom/svg/SVGPathSegUtils.h b/dom/svg/SVGPathSegUtils.h index 26654a8a34c9..53d05b04441f 100644 --- a/dom/svg/SVGPathSegUtils.h +++ b/dom/svg/SVGPathSegUtils.h @@ -73,7 +73,7 @@ struct SVGPathTraversalState { */ class SVGPathSegUtils { private: - SVGPathSegUtils() {} // private to prevent instances + SVGPathSegUtils() = default; // private to prevent instances public: static void GetValueAsString(const float* aSeg, nsAString& aValue); diff --git a/dom/svg/SVGPointList.h b/dom/svg/SVGPointList.h index 78f7320642ed..7d12d64069de 100644 --- a/dom/svg/SVGPointList.h +++ b/dom/svg/SVGPointList.h @@ -37,8 +37,8 @@ class SVGPointList { friend class DOMSVGPoint; public: - SVGPointList() {} - ~SVGPointList() {} + SVGPointList() = default; + ~SVGPointList() = default; // Only methods that don't make/permit modification to this list are public. // Only our friend classes can access methods that may change us. diff --git a/dom/svg/SVGPointListSMILType.h b/dom/svg/SVGPointListSMILType.h index 8400ef8fd58d..88f79038cb72 100644 --- a/dom/svg/SVGPointListSMILType.h +++ b/dom/svg/SVGPointListSMILType.h @@ -45,7 +45,7 @@ class SVGPointListSMILType : public SMILType { private: // Private constructor: prevent instances beyond my singleton. - constexpr SVGPointListSMILType() {} + constexpr SVGPointListSMILType() = default; }; } // namespace mozilla diff --git a/dom/svg/SVGRect.h b/dom/svg/SVGRect.h index 934795e08a29..6474aefa3783 100644 --- a/dom/svg/SVGRect.h +++ b/dom/svg/SVGRect.h @@ -46,7 +46,7 @@ class SVGRect final : public SVGIRect { virtual nsIContent* GetParentObject() const override { return mParent; } protected: - ~SVGRect() {} + ~SVGRect() = default; nsCOMPtr mParent; float mX, mY, mWidth, mHeight; diff --git a/dom/svg/SVGSVGElement.h b/dom/svg/SVGSVGElement.h index e4c471dc264d..550807b8e28d 100644 --- a/dom/svg/SVGSVGElement.h +++ b/dom/svg/SVGSVGElement.h @@ -65,7 +65,7 @@ class DOMSVGTranslatePoint final : public nsISVGPoint { RefPtr mElement; private: - ~DOMSVGTranslatePoint() {} + ~DOMSVGTranslatePoint() = default; }; typedef SVGViewportElement SVGSVGElementBase; diff --git a/dom/svg/SVGStringList.h b/dom/svg/SVGStringList.h index e53c81a6c90d..2b9c604b9a70 100644 --- a/dom/svg/SVGStringList.h +++ b/dom/svg/SVGStringList.h @@ -22,7 +22,7 @@ class SVGStringList { public: SVGStringList() : mIsSet(false), mIsCommaSeparated(false) {} - ~SVGStringList() {} + ~SVGStringList() = default; void SetIsCommaSeparated(bool aIsCommaSeparated) { mIsCommaSeparated = aIsCommaSeparated; diff --git a/dom/svg/SVGTests.h b/dom/svg/SVGTests.h index 743e91622622..500af1c58fc0 100644 --- a/dom/svg/SVGTests.h +++ b/dom/svg/SVGTests.h @@ -105,7 +105,7 @@ class SVGTests : public nsISupports { } protected: - virtual ~SVGTests() {} + virtual ~SVGTests() = default; private: enum { FEATURES, EXTENSIONS, LANGUAGE }; diff --git a/dom/svg/SVGTransformList.h b/dom/svg/SVGTransformList.h index 536056153b3a..adbb1cc8fc39 100644 --- a/dom/svg/SVGTransformList.h +++ b/dom/svg/SVGTransformList.h @@ -32,8 +32,8 @@ class SVGTransformList { friend class dom::DOMSVGTransform; public: - SVGTransformList() {} - ~SVGTransformList() {} + SVGTransformList() = default; + ~SVGTransformList() = default; // Only methods that don't make/permit modification to this list are public. // Only our friend classes can access methods that may change us. diff --git a/dom/svg/SVGTransformListSMILType.h b/dom/svg/SVGTransformListSMILType.h index 0b15b006966e..bc7833e04bf0 100644 --- a/dom/svg/SVGTransformListSMILType.h +++ b/dom/svg/SVGTransformListSMILType.h @@ -116,7 +116,7 @@ class SVGTransformListSMILType : public SMILType { private: // Private constructor: prevent instances beyond my singleton. - constexpr SVGTransformListSMILType() {} + constexpr SVGTransformListSMILType() = default; }; } // end namespace mozilla diff --git a/dom/svg/SVGTransformableElement.h b/dom/svg/SVGTransformableElement.h index 204e807ab8bf..2a3adce9a44f 100644 --- a/dom/svg/SVGTransformableElement.h +++ b/dom/svg/SVGTransformableElement.h @@ -27,7 +27,7 @@ class SVGTransformableElement : public SVGElement { public: explicit SVGTransformableElement(already_AddRefed&& aNodeInfo) : SVGElement(std::move(aNodeInfo)) {} - virtual ~SVGTransformableElement() {} + virtual ~SVGTransformableElement() = default; virtual nsresult Clone(dom::NodeInfo*, nsINode** aResult) const override = 0; diff --git a/dom/svg/SVGViewBoxSMILType.h b/dom/svg/SVGViewBoxSMILType.h index 996697aac62f..943831247fba 100644 --- a/dom/svg/SVGViewBoxSMILType.h +++ b/dom/svg/SVGViewBoxSMILType.h @@ -38,7 +38,7 @@ class SVGViewBoxSMILType : public SMILType { private: // Private constructor: prevent instances beyond my singleton. - constexpr SVGViewBoxSMILType() {} + constexpr SVGViewBoxSMILType() = default; }; } // namespace mozilla From a63f47f4d58a735e1643233c167fb366424c8213 Mon Sep 17 00:00:00 2001 From: Tooru Fujisawa Date: Tue, 2 Apr 2019 18:15:55 +0900 Subject: [PATCH 03/22] Bug 1535994 - Part 1: Add BytecodeSection class with BytecodeVector. r=jorendorff Differential Revision: https://phabricator.services.mozilla.com/D25731 --- js/src/frontend/BytecodeControlStructures.cpp | 7 +- js/src/frontend/BytecodeEmitter.cpp | 100 ++++++++++-------- js/src/frontend/BytecodeEmitter.h | 41 +++++-- js/src/frontend/CForEmitter.cpp | 4 +- js/src/frontend/EmitterScope.cpp | 7 +- js/src/frontend/ForInEmitter.cpp | 4 +- js/src/frontend/ForOfEmitter.cpp | 2 +- js/src/frontend/ForOfLoopControl.cpp | 6 +- js/src/frontend/LabelEmitter.cpp | 4 +- js/src/frontend/ObjectEmitter.cpp | 2 +- js/src/frontend/SwitchEmitter.cpp | 18 ++-- js/src/frontend/TryEmitter.cpp | 7 +- js/src/vm/JSScript.cpp | 4 +- 13 files changed, 118 insertions(+), 88 deletions(-) diff --git a/js/src/frontend/BytecodeControlStructures.cpp b/js/src/frontend/BytecodeControlStructures.cpp index eb1007251b7f..7c6df46e70e1 100644 --- a/js/src/frontend/BytecodeControlStructures.cpp +++ b/js/src/frontend/BytecodeControlStructures.cpp @@ -109,7 +109,7 @@ bool LoopControl::emitLoopHead(BytecodeEmitter* bce, } } - head_ = {bce->offset()}; + head_ = {bce->bytecodeSection().offset()}; ptrdiff_t off; if (!bce->emitJumpTargetOp(JSOP_LOOPHEAD, &off)) { return false; @@ -126,7 +126,7 @@ bool LoopControl::emitLoopEntry(BytecodeEmitter* bce, } } - JumpTarget entry = {bce->offset()}; + JumpTarget entry = {bce->bytecodeSection().offset()}; bce->patchJumpsToTarget(entryJump_, entry); MOZ_ASSERT(loopDepth_ > 0); @@ -135,7 +135,8 @@ bool LoopControl::emitLoopEntry(BytecodeEmitter* bce, if (!bce->emitJumpTargetOp(JSOP_LOOPENTRY, &off)) { return false; } - SetLoopEntryDepthHintAndFlags(bce->code(off), loopDepth_, canIonOsr_); + SetLoopEntryDepthHintAndFlags(bce->bytecodeSection().code(off), loopDepth_, + canIonOsr_); return true; } diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index 0c2bab81ff09..cb8267106703 100644 --- a/js/src/frontend/BytecodeEmitter.cpp +++ b/js/src/frontend/BytecodeEmitter.cpp @@ -91,6 +91,8 @@ static bool ParseNodeRequiresSpecialLineNumberNotes(ParseNode* pn) { kind == ParseNodeKind::Function; } +BytecodeEmitter::BytecodeSection::BytecodeSection(JSContext* cx) : code_(cx) {} + BytecodeEmitter::BytecodeEmitter( BytecodeEmitter* parent, SharedContext* sc, HandleScript script, Handle lazyScript, uint32_t lineNum, EmitterMode emitterMode, @@ -100,7 +102,7 @@ BytecodeEmitter::BytecodeEmitter( parent(parent), script(cx, script), lazyScript(cx, lazyScript), - code_(cx), + bytecodeSection_(cx), notes_(cx), currentLine_(lineNum), fieldInitializers_(fieldInitializers), @@ -194,7 +196,7 @@ bool BytecodeEmitter::markStepBreakpoint() { // We track the location of the most recent separator for use in // markSimpleBreakpoint. Note that this means that the position must already // be set before markStepBreakpoint is called. - lastSeparatorOffet_ = code().length(); + lastSeparatorOffet_ = bytecodeSection().code().length(); lastSeparatorLine_ = currentLine_; lastSeparatorColumn_ = lastColumn_; @@ -223,9 +225,9 @@ bool BytecodeEmitter::markSimpleBreakpoint() { } bool BytecodeEmitter::emitCheck(JSOp op, ptrdiff_t delta, ptrdiff_t* offset) { - *offset = code().length(); + *offset = bytecodeSection().code().length(); - if (!code().growByUninitialized(delta)) { + if (!bytecodeSection().code().growByUninitialized(delta)) { ReportOutOfMemory(cx); return false; } @@ -246,7 +248,7 @@ bool BytecodeEmitter::emitCheck(JSOp op, ptrdiff_t delta, ptrdiff_t* offset) { } void BytecodeEmitter::updateDepth(ptrdiff_t target) { - jsbytecode* pc = code(target); + jsbytecode* pc = bytecodeSection().code(target); int nuses = StackUses(pc); int ndefs = StackDefs(pc); @@ -280,7 +282,7 @@ bool BytecodeEmitter::emit1(JSOp op) { return false; } - jsbytecode* code = this->code(offset); + jsbytecode* code = bytecodeSection().code(offset); code[0] = jsbytecode(op); updateDepth(offset); return true; @@ -294,7 +296,7 @@ bool BytecodeEmitter::emit2(JSOp op, uint8_t op1) { return false; } - jsbytecode* code = this->code(offset); + jsbytecode* code = bytecodeSection().code(offset); code[0] = jsbytecode(op); code[1] = jsbytecode(op1); updateDepth(offset); @@ -313,7 +315,7 @@ bool BytecodeEmitter::emit3(JSOp op, jsbytecode op1, jsbytecode op2) { return false; } - jsbytecode* code = this->code(offset); + jsbytecode* code = bytecodeSection().code(offset); code[0] = jsbytecode(op); code[1] = op1; code[2] = op2; @@ -330,7 +332,7 @@ bool BytecodeEmitter::emitN(JSOp op, size_t extra, ptrdiff_t* offset) { return false; } - jsbytecode* code = this->code(off); + jsbytecode* code = bytecodeSection().code(off); code[0] = jsbytecode(op); /* The remaining |extra| bytes are set by the caller */ @@ -361,12 +363,12 @@ bool BytecodeEmitter::emitJumpTargetOp(JSOp op, ptrdiff_t* off) { return false; } - SET_ICINDEX(code(*off), numEntries); + SET_ICINDEX(bytecodeSection().code(*off), numEntries); return true; } bool BytecodeEmitter::emitJumpTarget(JumpTarget* target) { - ptrdiff_t off = offset(); + ptrdiff_t off = bytecodeSection().offset(); // Alias consecutive jump targets. if (off == lastTarget.offset + ptrdiff_t(JSOP_JUMPTARGET_LENGTH)) { @@ -387,10 +389,10 @@ bool BytecodeEmitter::emitJumpNoFallthrough(JSOp op, JumpList* jump) { return false; } - jsbytecode* code = this->code(offset); + jsbytecode* code = bytecodeSection().code(offset); code[0] = jsbytecode(op); MOZ_ASSERT(-1 <= jump->offset && jump->offset < offset); - jump->push(this->code(0), offset); + jump->push(bytecodeSection().code(0), offset); updateDepth(offset); return true; } @@ -425,11 +427,12 @@ bool BytecodeEmitter::emitBackwardJump(JSOp op, JumpTarget target, } void BytecodeEmitter::patchJumpsToTarget(JumpList jump, JumpTarget target) { - MOZ_ASSERT(-1 <= jump.offset && jump.offset <= offset()); - MOZ_ASSERT(0 <= target.offset && target.offset <= offset()); - MOZ_ASSERT_IF(jump.offset != -1 && target.offset + 4 <= offset(), - BytecodeIsJumpTarget(JSOp(*code(target.offset)))); - jump.patchAll(code(0), target); + MOZ_ASSERT(-1 <= jump.offset && jump.offset <= bytecodeSection().offset()); + MOZ_ASSERT(0 <= target.offset && target.offset <= bytecodeSection().offset()); + MOZ_ASSERT_IF( + jump.offset != -1 && target.offset + 4 <= bytecodeSection().offset(), + BytecodeIsJumpTarget(JSOp(*bytecodeSection().code(target.offset)))); + jump.patchAll(bytecodeSection().code(0), target); } bool BytecodeEmitter::emitJumpTargetAndPatch(JumpList jump) { @@ -475,7 +478,7 @@ bool BytecodeEmitter::emitDupAt(unsigned slotFromTop) { return false; } - jsbytecode* pc = code(off); + jsbytecode* pc = bytecodeSection().code(off); SET_UINT24(pc, slotFromTop); return true; } @@ -587,7 +590,8 @@ bool BytecodeEmitter::updateSourceCoordNotes(uint32_t offset) { /* Updates the last separator position, if present */ void BytecodeEmitter::updateSeparatorPosition() { - if (!inPrologue() && lastSeparatorOffet_ == code().length()) { + if (!inPrologue() && + lastSeparatorOffet_ == bytecodeSection().code().length()) { lastSeparatorLine_ = currentLine_; lastSeparatorColumn_ = lastColumn_; } @@ -626,7 +630,7 @@ bool BytecodeEmitter::emitUint32Operand(JSOp op, uint32_t operand) { if (!emitN(op, 4, &off)) { return false; } - SET_UINT32(code(off), operand); + SET_UINT32(bytecodeSection().code(off), operand); return true; } @@ -671,7 +675,7 @@ class NonLocalExitControl { ~NonLocalExitControl() { for (uint32_t n = savedScopeNoteIndex_; n < bce_->scopeNoteList.length(); n++) { - bce_->scopeNoteList.recordEnd(n, bce_->offset()); + bce_->scopeNoteList.recordEnd(n, bce_->bytecodeSection().offset()); } bce_->stackDepth = savedDepth_; } @@ -695,7 +699,8 @@ bool NonLocalExitControl::leaveScope(EmitterScope* es) { if (es->enclosingInFrame()) { enclosingScopeIndex = es->enclosingInFrame()->index(); } - if (!bce_->scopeNoteList.append(enclosingScopeIndex, bce_->offset(), + if (!bce_->scopeNoteList.append(enclosingScopeIndex, + bce_->bytecodeSection().offset(), openScopeNoteIndex_)) { return false; } @@ -838,7 +843,7 @@ bool NonLocalExitControl::prepareForNonLocalJump(NestableControl* target) { } // Close FOR_OF_ITERCLOSE trynotes. - ptrdiff_t end = bce_->offset(); + ptrdiff_t end = bce_->bytecodeSection().offset(); for (ptrdiff_t start : forOfIterCloseScopeStarts) { if (!bce_->addTryNote(JSTRY_FOR_OF_ITERCLOSE, 0, start, end)) { return false; @@ -884,7 +889,7 @@ bool BytecodeEmitter::emitIndex32(JSOp op, uint32_t index) { return false; } - jsbytecode* code = this->code(offset); + jsbytecode* code = bytecodeSection().code(offset); code[0] = jsbytecode(op); SET_UINT32_INDEX(code, index); updateDepth(offset); @@ -902,7 +907,7 @@ bool BytecodeEmitter::emitIndexOp(JSOp op, uint32_t index) { return false; } - jsbytecode* code = this->code(offset); + jsbytecode* code = bytecodeSection().code(offset); code[0] = jsbytecode(op); SET_UINT32_INDEX(code, index); updateDepth(offset); @@ -974,7 +979,7 @@ bool BytecodeEmitter::emitLocalOp(JSOp op, uint32_t slot) { return false; } - SET_LOCALNO(code(off), slot); + SET_LOCALNO(bytecodeSection().code(off), slot); return true; } @@ -985,7 +990,7 @@ bool BytecodeEmitter::emitArgOp(JSOp op, uint16_t slot) { return false; } - SET_ARGNO(code(off), slot); + SET_ARGNO(bytecodeSection().code(off), slot); return true; } @@ -1000,7 +1005,7 @@ bool BytecodeEmitter::emitEnvCoordOp(JSOp op, EnvironmentCoordinate ec) { return false; } - jsbytecode* pc = code(off); + jsbytecode* pc = bytecodeSection().code(off); SET_ENVCOORD_HOPS(pc, ec.hops()); pc += ENVCOORD_HOPS_LEN; SET_ENVCOORD_SLOT(pc, ec.slot()); @@ -1680,7 +1685,7 @@ bool BytecodeEmitter::emitNewInit() { return false; } - jsbytecode* code = this->code(offset); + jsbytecode* code = bytecodeSection().code(offset); code[0] = JSOP_NEWINIT; code[1] = 0; code[2] = 0; @@ -2017,7 +2022,7 @@ bool BytecodeEmitter::emitDouble(double d) { return false; } - jsbytecode* code = this->code(offset); + jsbytecode* code = bytecodeSection().code(offset); code[0] = jsbytecode(JSOP_DOUBLE); SET_INLINE_VALUE(code, DoubleValue(d)); updateDepth(offset); @@ -2047,13 +2052,13 @@ bool BytecodeEmitter::emitNumberOp(double dval) { if (!emitN(JSOP_UINT24, 3, &off)) { return false; } - SET_UINT24(code(off), u); + SET_UINT24(bytecodeSection().code(off), u); } else { ptrdiff_t off; if (!emitN(JSOP_INT32, 4, &off)) { return false; } - SET_INT32(code(off), ival); + SET_INT32(bytecodeSection().code(off), ival); } return true; } @@ -2297,11 +2302,11 @@ bool BytecodeEmitter::emitYieldOp(JSOp op) { } uint32_t resumeIndex; - if (!allocateResumeIndex(offset(), &resumeIndex)) { + if (!allocateResumeIndex(bytecodeSection().offset(), &resumeIndex)) { return false; } - SET_RESUMEINDEX(code(off), resumeIndex); + SET_RESUMEINDEX(bytecodeSection().code(off), resumeIndex); return emit1(JSOP_DEBUGAFTERYIELD); } @@ -3092,11 +3097,11 @@ bool BytecodeEmitter::wrapWithDestructuringTryNote(int32_t iterDepth, return false; } - ptrdiff_t start = offset(); + ptrdiff_t start = bytecodeSection().offset(); if (!emitter(this)) { return false; } - ptrdiff_t end = offset(); + ptrdiff_t end = bytecodeSection().offset(); if (start != end) { return addTryNote(JSTRY_DESTRUCTURING, iterDepth, start, end); } @@ -3815,7 +3820,7 @@ bool BytecodeEmitter::emitDestructuringObjRestExclusionSet(ListNode* pattern) { MOZ_ASSERT(pattern->isKind(ParseNodeKind::ObjectExpr)); MOZ_ASSERT(pattern->last()->isKind(ParseNodeKind::Spread)); - ptrdiff_t offset = this->offset(); + ptrdiff_t offset = bytecodeSection().offset(); if (!emitNewInit()) { return false; } @@ -4690,11 +4695,11 @@ MOZ_MUST_USE bool BytecodeEmitter::emitGoSub(JumpList* jump) { } uint32_t resumeIndex; - if (!allocateResumeIndex(offset(), &resumeIndex)) { + if (!allocateResumeIndex(bytecodeSection().offset(), &resumeIndex)) { return false; } - SET_RESUMEINDEX(code(off), resumeIndex); + SET_RESUMEINDEX(bytecodeSection().code(off), resumeIndex); return true; } @@ -5967,7 +5972,7 @@ bool BytecodeEmitter::emitReturn(UnaryNode* returnNode) { * In this case we mutate JSOP_RETURN into JSOP_SETRVAL and add an * extra JSOP_RETRVAL after the fixups. */ - ptrdiff_t top = offset(); + ptrdiff_t top = bytecodeSection().offset(); bool needsFinalYield = sc->isFunctionBox() && sc->asFunctionBox()->needsFinalYield(); @@ -6028,12 +6033,13 @@ bool BytecodeEmitter::emitReturn(UnaryNode* returnNode) { return false; } } else if (isDerivedClassConstructor) { - MOZ_ASSERT(code()[top] == JSOP_SETRVAL); + MOZ_ASSERT(bytecodeSection().code()[top] == JSOP_SETRVAL); if (!emit1(JSOP_RETRVAL)) { return false; } - } else if (top + static_cast(JSOP_RETURN_LENGTH) != offset()) { - code()[top] = JSOP_SETRVAL; + } else if (top + static_cast(JSOP_RETURN_LENGTH) != + bytecodeSection().offset()) { + bytecodeSection().code()[top] = JSOP_SETRVAL; if (!emit1(JSOP_RETRVAL)) { return false; } @@ -6707,7 +6713,7 @@ bool BytecodeEmitter::emitExpressionStatement(UnaryNode* exprStmt) { if (innermostNestableControl && innermostNestableControl->is() && innermostNestableControl->as().startOffset() >= - offset()) { + bytecodeSection().offset()) { useful = true; } } @@ -8183,7 +8189,7 @@ bool BytecodeEmitter::replaceNewInitWithNewObject(JSObject* obj, "newinit and newobject must have equal length to edit in-place"); uint32_t index = objectList.add(objbox); - jsbytecode* code = this->code(offset); + jsbytecode* code = bytecodeSection().code(offset); MOZ_ASSERT(code[0] == JSOP_NEWINIT); code[0] = JSOP_NEWOBJECT; @@ -9348,7 +9354,7 @@ bool BytecodeEmitter::newSrcNote(SrcNoteType type, unsigned* indexp) { * Compute delta from the last annotated bytecode's offset. If it's too * big to fit in sn, allocate one or more xdelta notes and reset sn. */ - ptrdiff_t offset = this->offset(); + ptrdiff_t offset = bytecodeSection().offset(); ptrdiff_t delta = offset - lastNoteOffset(); lastNoteOffset_ = offset; if (delta >= SN_DELTA_LIMIT) { diff --git a/js/src/frontend/BytecodeEmitter.h b/js/src/frontend/BytecodeEmitter.h index 7d4c9e93d2f9..9866064defd1 100644 --- a/js/src/frontend/BytecodeEmitter.h +++ b/js/src/frontend/BytecodeEmitter.h @@ -124,7 +124,32 @@ struct MOZ_STACK_CLASS BytecodeEmitter { Rooted lazyScript; private: - BytecodeVector code_; /* bytecode */ + // Bytecode and all data directly associated with specific opcode/index inside + // bytecode is stored in this class. + class BytecodeSection { + public: + explicit BytecodeSection(JSContext* cx); + + BytecodeVector& code() { return code_; } + const BytecodeVector& code() const { return code_; } + + jsbytecode* code(ptrdiff_t offset) { return code_.begin() + offset; } + ptrdiff_t offset() const { return code_.end() - code_.begin(); } + + private: + // ---- Bytecode ---- + + // Bytecode. + BytecodeVector code_; + }; + + BytecodeSection bytecodeSection_; + + public: + BytecodeSection& bytecodeSection() { return bytecodeSection_; } + const BytecodeSection& bytecodeSection() const { return bytecodeSection_; } + + private: SrcNotesVector notes_; /* source notes, see below */ // Code offset for last source note @@ -400,19 +425,13 @@ struct MOZ_STACK_CLASS BytecodeEmitter { void tellDebuggerAboutCompiledScript(JSContext* cx); - BytecodeVector& code() { return code_; } - const BytecodeVector& code() const { return code_; } - - jsbytecode* code(ptrdiff_t offset) { return code_.begin() + offset; } - ptrdiff_t offset() const { return code_.end() - code_.begin(); } - uint32_t mainOffset() const { return *mainOffset_; } bool inPrologue() const { return mainOffset_.isNothing(); } void switchToMain() { MOZ_ASSERT(inPrologue()); - mainOffset_.emplace(code_.length()); + mainOffset_.emplace(bytecodeSection().code().length()); } SrcNotesVector& notes() { @@ -430,7 +449,8 @@ struct MOZ_STACK_CLASS BytecodeEmitter { // Check if the last emitted opcode is a jump target. bool lastOpcodeIsJumpTarget() const { - return offset() - lastTarget.offset == ptrdiff_t(JSOP_JUMPTARGET_LENGTH); + return bytecodeSection().offset() - lastTarget.offset == + ptrdiff_t(JSOP_JUMPTARGET_LENGTH); } // JumpTarget should not be part of the emitted statement, as they can be @@ -438,7 +458,8 @@ struct MOZ_STACK_CLASS BytecodeEmitter { // the statement we might have issues where the enclosing statement might // not contain all the opcodes of the enclosed statements. ptrdiff_t lastNonJumpTargetOffset() const { - return lastOpcodeIsJumpTarget() ? lastTarget.offset : offset(); + return lastOpcodeIsJumpTarget() ? lastTarget.offset + : bytecodeSection().offset(); } void setFunctionBodyEndPos(uint32_t pos) { diff --git a/js/src/frontend/CForEmitter.cpp b/js/src/frontend/CForEmitter.cpp index e2be22fa3a86..4a8b6a63321f 100644 --- a/js/src/frontend/CForEmitter.cpp +++ b/js/src/frontend/CForEmitter.cpp @@ -75,7 +75,7 @@ bool CForEmitter::emitBody(Cond cond, const Maybe& bodyPos) { return false; } - biasedTop_ = bce_->offset(); + biasedTop_ = bce_->bytecodeSection().offset(); if (cond_ == Cond::Present) { // Goto the loop condition, which branches back to iterate. @@ -176,7 +176,7 @@ bool CForEmitter::emitCond(const Maybe& forPos, tdzCache_.reset(); } - condOffset_ = bce_->offset(); + condOffset_ = bce_->bytecodeSection().offset(); if (cond_ == Cond::Present) { if (!loopInfo_->emitLoopEntry(bce_, condPos)) { diff --git a/js/src/frontend/EmitterScope.cpp b/js/src/frontend/EmitterScope.cpp index 7c31ab46438e..322720c9411b 100644 --- a/js/src/frontend/EmitterScope.cpp +++ b/js/src/frontend/EmitterScope.cpp @@ -359,7 +359,7 @@ bool EmitterScope::appendScopeNote(BytecodeEmitter* bce) { MOZ_ASSERT(ScopeKindIsInBody(scope(bce)->kind()) && enclosingInFrame(), "Scope notes are not needed for body-level scopes."); noteIndex_ = bce->scopeNoteList.length(); - return bce->scopeNoteList.append(index(), bce->offset(), + return bce->scopeNoteList.append(index(), bce->bytecodeSection().offset(), enclosingInFrame() ? enclosingInFrame()->noteIndex() : ScopeNote::NoScopeNoteIndex); @@ -1057,8 +1057,9 @@ bool EmitterScope::leave(BytecodeEmitter* bce, bool nonLocal) { if (ScopeKindIsInBody(kind)) { // The extra function var scope is never popped once it's pushed, // so its scope note extends until the end of any possible code. - uint32_t offset = - kind == ScopeKind::FunctionBodyVar ? UINT32_MAX : bce->offset(); + uint32_t offset = kind == ScopeKind::FunctionBodyVar + ? UINT32_MAX + : bce->bytecodeSection().offset(); bce->scopeNoteList.recordEnd(noteIndex_, offset); } } diff --git a/js/src/frontend/ForInEmitter.cpp b/js/src/frontend/ForInEmitter.cpp index 9ca13747b9a3..ff1bf21c995e 100644 --- a/js/src/frontend/ForInEmitter.cpp +++ b/js/src/frontend/ForInEmitter.cpp @@ -124,7 +124,7 @@ bool ForInEmitter::emitBody() { bool ForInEmitter::emitEnd(const Maybe& forPos) { MOZ_ASSERT(state_ == State::Body); - loopInfo_->setContinueTarget(bce_->offset()); + loopInfo_->setContinueTarget(bce_->bytecodeSection().offset()); if (forPos) { // Make sure this code is attributed to the "for". @@ -172,7 +172,7 @@ bool ForInEmitter::emitEnd(const Maybe& forPos) { } if (!bce_->addTryNote(JSTRY_FOR_IN, bce_->stackDepth, loopInfo_->headOffset(), - bce_->offset())) { + bce_->bytecodeSection().offset())) { return false; } diff --git a/js/src/frontend/ForOfEmitter.cpp b/js/src/frontend/ForOfEmitter.cpp index 3efabeed38d6..84ab3894aa67 100644 --- a/js/src/frontend/ForOfEmitter.cpp +++ b/js/src/frontend/ForOfEmitter.cpp @@ -225,7 +225,7 @@ bool ForOfEmitter::emitEnd(const Maybe& iteratedPos) { return false; } - loopInfo_->setContinueTarget(bce_->offset()); + loopInfo_->setContinueTarget(bce_->bytecodeSection().offset()); // We use the iterated value's position to attribute JSOP_LOOPENTRY, // which corresponds to the iteration protocol. diff --git a/js/src/frontend/ForOfLoopControl.cpp b/js/src/frontend/ForOfLoopControl.cpp index f8cb520ea1bd..660b9e959453 100644 --- a/js/src/frontend/ForOfLoopControl.cpp +++ b/js/src/frontend/ForOfLoopControl.cpp @@ -135,12 +135,12 @@ bool ForOfLoopControl::emitEndCodeNeedingIteratorClose(BytecodeEmitter* bce) { bool ForOfLoopControl::emitIteratorCloseInInnermostScopeWithTryNote( BytecodeEmitter* bce, CompletionKind completionKind /* = CompletionKind::Normal */) { - ptrdiff_t start = bce->offset(); + ptrdiff_t start = bce->bytecodeSection().offset(); if (!emitIteratorCloseInScope(bce, *bce->innermostEmitterScope(), completionKind)) { return false; } - ptrdiff_t end = bce->offset(); + ptrdiff_t end = bce->bytecodeSection().offset(); return bce->addTryNote(JSTRY_FOR_OF_ITERCLOSE, 0, start, end); } @@ -193,7 +193,7 @@ bool ForOfLoopControl::emitPrepareForNonLocalJumpFromScope( return false; } - *tryNoteStart = bce->offset(); + *tryNoteStart = bce->bytecodeSection().offset(); if (!emitIteratorCloseInScope(bce, currentScope, CompletionKind::Normal)) { // [stack] UNDEF return false; diff --git a/js/src/frontend/LabelEmitter.cpp b/js/src/frontend/LabelEmitter.cpp index 79a62da171c1..c05192b3fd1e 100644 --- a/js/src/frontend/LabelEmitter.cpp +++ b/js/src/frontend/LabelEmitter.cpp @@ -27,7 +27,7 @@ bool LabelEmitter::emitLabel(JSAtom* name) { return false; } - controlInfo_.emplace(bce_, name, bce_->offset()); + controlInfo_.emplace(bce_, name, bce_->bytecodeSection().offset()); #ifdef DEBUG state_ = State::Label; @@ -39,7 +39,7 @@ bool LabelEmitter::emitEnd() { MOZ_ASSERT(state_ == State::Label); // Patch the JSOP_LABEL offset. - jsbytecode* labelpc = bce_->code(top_); + jsbytecode* labelpc = bce_->bytecodeSection().code(top_); int32_t offset = bce_->lastNonJumpTargetOffset() - top_; MOZ_ASSERT(*labelpc == JSOP_LABEL); SET_CODE_OFFSET(labelpc, offset); diff --git a/js/src/frontend/ObjectEmitter.cpp b/js/src/frontend/ObjectEmitter.cpp index b19483eb1639..df42cee8767c 100644 --- a/js/src/frontend/ObjectEmitter.cpp +++ b/js/src/frontend/ObjectEmitter.cpp @@ -412,7 +412,7 @@ bool ObjectEmitter::emitObject(size_t propertyCount) { // Emit code for {p:a, '%q':b, 2:c} that is equivalent to constructing // a new object and defining (in source order) each property on the object // (or mutating the object's [[Prototype]], in the case of __proto__). - top_ = bce_->offset(); + top_ = bce_->bytecodeSection().offset(); if (!bce_->emitNewInit()) { // [stack] OBJ return false; diff --git a/js/src/frontend/SwitchEmitter.cpp b/js/src/frontend/SwitchEmitter.cpp index 9e62613b8491..01de8af56ebd 100644 --- a/js/src/frontend/SwitchEmitter.cpp +++ b/js/src/frontend/SwitchEmitter.cpp @@ -151,7 +151,7 @@ bool SwitchEmitter::emitCond() { // After entering the scope if necessary, push the switch control. controlInfo_.emplace(bce_, StatementKind::Switch); - top_ = bce_->offset(); + top_ = bce_->bytecodeSection().offset(); if (!caseOffsets_.resize(caseCount_)) { ReportOutOfMemory(bce_->cx); @@ -164,7 +164,7 @@ bool SwitchEmitter::emitCond() { return false; } - MOZ_ASSERT(top_ == bce_->offset()); + MOZ_ASSERT(top_ == bce_->bytecodeSection().offset()); if (!bce_->emitN(JSOP_CONDSWITCH, 0)) { return false; } @@ -181,7 +181,7 @@ bool SwitchEmitter::emitTable(const TableGenerator& tableGen) { // After entering the scope if necessary, push the switch control. controlInfo_.emplace(bce_, StatementKind::Switch); - top_ = bce_->offset(); + top_ = bce_->bytecodeSection().offset(); // The note has one offset that tells total switch code length. if (!bce_->newSrcNote2(SRC_TABLESWITCH, 0, ¬eIndex_)) { @@ -193,14 +193,14 @@ bool SwitchEmitter::emitTable(const TableGenerator& tableGen) { return false; } - MOZ_ASSERT(top_ == bce_->offset()); + MOZ_ASSERT(top_ == bce_->bytecodeSection().offset()); if (!bce_->emitN(JSOP_TABLESWITCH, JSOP_TABLESWITCH_LENGTH - sizeof(jsbytecode))) { return false; } // Skip default offset. - jsbytecode* pc = bce_->code(top_ + JUMP_OFFSET_LEN); + jsbytecode* pc = bce_->bytecodeSection().code(top_ + JUMP_OFFSET_LEN); // Fill in switch bounds, which we know fit in 16-bit offsets. SET_JUMP_OFFSET(pc, tableGen.low()); @@ -223,9 +223,9 @@ bool SwitchEmitter::emitCaseOrDefaultJump(uint32_t caseIndex, bool isDefault) { if (caseIndex > 0) { // Link the last JSOP_CASE's SRC_NEXTCASE to current JSOP_CASE for the // benefit of IonBuilder. - if (!bce_->setSrcNoteOffset(caseNoteIndex_, - SrcNote::NextCase::NextCaseOffset, - bce_->offset() - lastCaseOffset_)) { + if (!bce_->setSrcNoteOffset( + caseNoteIndex_, SrcNote::NextCase::NextCaseOffset, + bce_->bytecodeSection().offset() - lastCaseOffset_)) { return false; } } @@ -398,7 +398,7 @@ bool SwitchEmitter::emitEnd() { defaultJumpTargetOffset_); } else { // Fill in the default jump target. - pc = bce_->code(top_); + pc = bce_->bytecodeSection().code(top_); SET_JUMP_OFFSET(pc, defaultJumpTargetOffset_.offset - top_); pc += JUMP_OFFSET_LEN; } diff --git a/js/src/frontend/TryEmitter.cpp b/js/src/frontend/TryEmitter.cpp index 2de77536e139..4c8dbd296300 100644 --- a/js/src/frontend/TryEmitter.cpp +++ b/js/src/frontend/TryEmitter.cpp @@ -62,7 +62,7 @@ bool TryEmitter::emitTry() { if (!bce_->emit1(JSOP_TRY)) { return false; } - tryStart_ = bce_->offset(); + tryStart_ = bce_->bytecodeSection().offset(); #ifdef DEBUG state_ = State::Try; @@ -82,8 +82,9 @@ bool TryEmitter::emitTryEnd() { } // Source note points to the jump at the end of the try block. - if (!bce_->setSrcNoteOffset(noteIndex_, SrcNote::Try::EndOfTryJumpOffset, - bce_->offset() - tryStart_ + JSOP_TRY_LENGTH)) { + if (!bce_->setSrcNoteOffset( + noteIndex_, SrcNote::Try::EndOfTryJumpOffset, + bce_->bytecodeSection().offset() - tryStart_ + JSOP_TRY_LENGTH)) { return false; } diff --git a/js/src/vm/JSScript.cpp b/js/src/vm/JSScript.cpp index 6c02540e09a7..3e05254373c7 100644 --- a/js/src/vm/JSScript.cpp +++ b/js/src/vm/JSScript.cpp @@ -4523,7 +4523,7 @@ bool JSScript::hasBreakpointsAt(jsbytecode* pc) { /* static */ bool SharedScriptData::InitFromEmitter( JSContext* cx, js::HandleScript script, frontend::BytecodeEmitter* bce) { uint32_t natoms = bce->atomIndices->count(); - uint32_t codeLength = bce->code().length(); + uint32_t codeLength = bce->bytecodeSection().code().length(); // The + 1 is to account for the final SN_MAKE_TERMINATOR that is appended // when the notes are copied to their final destination by copySrcNotes. @@ -4537,7 +4537,7 @@ bool JSScript::hasBreakpointsAt(jsbytecode* pc) { js::SharedScriptData* data = script->scriptData_; // Initialize trailing arrays - std::copy_n(bce->code().begin(), codeLength, data->code()); + std::copy_n(bce->bytecodeSection().code().begin(), codeLength, data->code()); bce->copySrcNotes(data->notes(), noteLength); InitAtomMap(*bce->atomIndices, data->atoms()); From d6704c5f6326c62c6a89bed6b8bb5a88ed02ba03 Mon Sep 17 00:00:00 2001 From: Tooru Fujisawa Date: Tue, 2 Apr 2019 18:15:59 +0900 Subject: [PATCH 04/22] Bug 1535994 - Part 2: Move source notes to BytecodeSection class. r=jorendorff Differential Revision: https://phabricator.services.mozilla.com/D25732 --- js/src/frontend/BytecodeEmitter.cpp | 18 ++++++++++-------- js/src/frontend/BytecodeEmitter.h | 29 ++++++++++++++++++----------- js/src/frontend/SwitchEmitter.cpp | 5 +++-- js/src/vm/JSScript.cpp | 2 +- 4 files changed, 32 insertions(+), 22 deletions(-) diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index cb8267106703..9e7b9e1ebcc8 100644 --- a/js/src/frontend/BytecodeEmitter.cpp +++ b/js/src/frontend/BytecodeEmitter.cpp @@ -91,7 +91,8 @@ static bool ParseNodeRequiresSpecialLineNumberNotes(ParseNode* pn) { kind == ParseNodeKind::Function; } -BytecodeEmitter::BytecodeSection::BytecodeSection(JSContext* cx) : code_(cx) {} +BytecodeEmitter::BytecodeSection::BytecodeSection(JSContext* cx) + : code_(cx), notes_(cx) {} BytecodeEmitter::BytecodeEmitter( BytecodeEmitter* parent, SharedContext* sc, HandleScript script, @@ -103,7 +104,6 @@ BytecodeEmitter::BytecodeEmitter( script(cx, script), lazyScript(cx, lazyScript), bytecodeSection_(cx), - notes_(cx), currentLine_(lineNum), fieldInitializers_(fieldInitializers), atomIndices(cx->frontendCollectionPool()), @@ -9344,7 +9344,9 @@ bool BytecodeEmitter::addTryNote(JSTryNoteKind kind, uint32_t stackDepth, } bool BytecodeEmitter::newSrcNote(SrcNoteType type, unsigned* indexp) { - SrcNotesVector& notes = this->notes(); + // Prologue shouldn't have source notes. + MOZ_ASSERT(!inPrologue()); + SrcNotesVector& notes = bytecodeSection().notes(); unsigned index; if (!AllocSrcNote(cx, notes, &index)) { return false; @@ -9355,8 +9357,8 @@ bool BytecodeEmitter::newSrcNote(SrcNoteType type, unsigned* indexp) { * big to fit in sn, allocate one or more xdelta notes and reset sn. */ ptrdiff_t offset = bytecodeSection().offset(); - ptrdiff_t delta = offset - lastNoteOffset(); - lastNoteOffset_ = offset; + ptrdiff_t delta = offset - bytecodeSection().lastNoteOffset(); + bytecodeSection().setLastNoteOffset(offset); if (delta >= SN_DELTA_LIMIT) { do { ptrdiff_t xdelta = Min(delta, SN_XDELTA_MASK); @@ -9426,7 +9428,7 @@ bool BytecodeEmitter::setSrcNoteOffset(unsigned index, unsigned which, return false; } - SrcNotesVector& notes = this->notes(); + SrcNotesVector& notes = bytecodeSection().notes(); /* Find the offset numbered which (i.e., skip exactly which offsets). */ jssrcnote* sn = ¬es[index]; @@ -9464,10 +9466,10 @@ bool BytecodeEmitter::setSrcNoteOffset(unsigned index, unsigned which, } void BytecodeEmitter::copySrcNotes(jssrcnote* destination, uint32_t nsrcnotes) { - unsigned count = notes_.length(); + unsigned count = bytecodeSection().notes().length(); // nsrcnotes includes SN_MAKE_TERMINATOR in addition to the srcnotes. MOZ_ASSERT(nsrcnotes == count + 1); - PodCopy(destination, notes_.begin(), count); + PodCopy(destination, bytecodeSection().notes().begin(), count); SN_MAKE_TERMINATOR(&destination[count]); } diff --git a/js/src/frontend/BytecodeEmitter.h b/js/src/frontend/BytecodeEmitter.h index 9866064defd1..a43f1c815cc0 100644 --- a/js/src/frontend/BytecodeEmitter.h +++ b/js/src/frontend/BytecodeEmitter.h @@ -130,17 +130,35 @@ struct MOZ_STACK_CLASS BytecodeEmitter { public: explicit BytecodeSection(JSContext* cx); + // ---- Bytecode ---- + BytecodeVector& code() { return code_; } const BytecodeVector& code() const { return code_; } jsbytecode* code(ptrdiff_t offset) { return code_.begin() + offset; } ptrdiff_t offset() const { return code_.end() - code_.begin(); } + // ---- Source notes ---- + + SrcNotesVector& notes() { return notes_; } + const SrcNotesVector& notes() const { return notes_; } + + ptrdiff_t lastNoteOffset() const { return lastNoteOffset_; } + void setLastNoteOffset(ptrdiff_t offset) { lastNoteOffset_ = offset; } + private: // ---- Bytecode ---- // Bytecode. BytecodeVector code_; + + // ---- Source notes ---- + + // Source notes + SrcNotesVector notes_; + + // Code offset for last source note + ptrdiff_t lastNoteOffset_ = 0; }; BytecodeSection bytecodeSection_; @@ -150,11 +168,6 @@ struct MOZ_STACK_CLASS BytecodeEmitter { const BytecodeSection& bytecodeSection() const { return bytecodeSection_; } private: - SrcNotesVector notes_; /* source notes, see below */ - - // Code offset for last source note - ptrdiff_t lastNoteOffset_ = 0; - // Line number for srcnotes. // // WARNING: If this becomes out of sync with already-emitted srcnotes, @@ -434,12 +447,6 @@ struct MOZ_STACK_CLASS BytecodeEmitter { mainOffset_.emplace(bytecodeSection().code().length()); } - SrcNotesVector& notes() { - // Prologue shouldn't have source notes. - MOZ_ASSERT(!inPrologue()); - return notes_; - } - ptrdiff_t lastNoteOffset() const { return lastNoteOffset_; } unsigned currentLine() const { return currentLine_; } void setCurrentLine(uint32_t line) { diff --git a/js/src/frontend/SwitchEmitter.cpp b/js/src/frontend/SwitchEmitter.cpp index 01de8af56ebd..1ff0901fbc1d 100644 --- a/js/src/frontend/SwitchEmitter.cpp +++ b/js/src/frontend/SwitchEmitter.cpp @@ -243,11 +243,12 @@ bool SwitchEmitter::emitCaseOrDefaultJump(uint32_t caseIndex, bool isDefault) { if (caseIndex == 0) { // Switch note's second offset is to first JSOP_CASE. - unsigned noteCount = bce_->notes().length(); + unsigned noteCount = bce_->bytecodeSection().notes().length(); if (!bce_->setSrcNoteOffset(noteIndex_, 1, lastCaseOffset_ - top_)) { return false; } - unsigned noteCountDelta = bce_->notes().length() - noteCount; + unsigned noteCountDelta = + bce_->bytecodeSection().notes().length() - noteCount; if (noteCountDelta != 0) { caseNoteIndex_ += noteCountDelta; } diff --git a/js/src/vm/JSScript.cpp b/js/src/vm/JSScript.cpp index 3e05254373c7..35507ef133ba 100644 --- a/js/src/vm/JSScript.cpp +++ b/js/src/vm/JSScript.cpp @@ -4527,7 +4527,7 @@ bool JSScript::hasBreakpointsAt(jsbytecode* pc) { // The + 1 is to account for the final SN_MAKE_TERMINATOR that is appended // when the notes are copied to their final destination by copySrcNotes. - uint32_t noteLength = bce->notes().length() + 1; + uint32_t noteLength = bce->bytecodeSection().notes().length() + 1; // Create and initialize SharedScriptData if (!script->createSharedScriptData(cx, codeLength, noteLength, natoms)) { From d38aa902136fe6a9490d04de8d2a98d332605f6d Mon Sep 17 00:00:00 2001 From: Tooru Fujisawa Date: Tue, 2 Apr 2019 18:16:00 +0900 Subject: [PATCH 05/22] Bug 1535994 - Part 3: Move lastTarget to BytecodeSection class. r=jorendorff Differential Revision: https://phabricator.services.mozilla.com/D25733 --- js/src/frontend/BytecodeEmitter.cpp | 7 ++--- js/src/frontend/BytecodeEmitter.h | 41 ++++++++++++++++------------- js/src/frontend/LabelEmitter.cpp | 2 +- js/src/frontend/SwitchEmitter.cpp | 5 ++-- 4 files changed, 31 insertions(+), 24 deletions(-) diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index 9e7b9e1ebcc8..9186af42eb62 100644 --- a/js/src/frontend/BytecodeEmitter.cpp +++ b/js/src/frontend/BytecodeEmitter.cpp @@ -371,13 +371,14 @@ bool BytecodeEmitter::emitJumpTarget(JumpTarget* target) { ptrdiff_t off = bytecodeSection().offset(); // Alias consecutive jump targets. - if (off == lastTarget.offset + ptrdiff_t(JSOP_JUMPTARGET_LENGTH)) { - target->offset = lastTarget.offset; + if (off == bytecodeSection().lastTargetOffset() + + ptrdiff_t(JSOP_JUMPTARGET_LENGTH)) { + target->offset = bytecodeSection().lastTargetOffset(); return true; } target->offset = off; - lastTarget.offset = off; + bytecodeSection().setLastTargetOffset(off); ptrdiff_t opOff; return emitJumpTargetOp(JSOP_JUMPTARGET, &opOff); diff --git a/js/src/frontend/BytecodeEmitter.h b/js/src/frontend/BytecodeEmitter.h index a43f1c815cc0..10ebc6c7a1c4 100644 --- a/js/src/frontend/BytecodeEmitter.h +++ b/js/src/frontend/BytecodeEmitter.h @@ -146,6 +146,24 @@ struct MOZ_STACK_CLASS BytecodeEmitter { ptrdiff_t lastNoteOffset() const { return lastNoteOffset_; } void setLastNoteOffset(ptrdiff_t offset) { lastNoteOffset_ = offset; } + // ---- Jump ---- + + ptrdiff_t lastTargetOffset() const { return lastTarget_.offset; } + void setLastTargetOffset(ptrdiff_t offset) { lastTarget_.offset = offset; } + + // Check if the last emitted opcode is a jump target. + bool lastOpcodeIsJumpTarget() const { + return offset() - lastTarget_.offset == ptrdiff_t(JSOP_JUMPTARGET_LENGTH); + } + + // JumpTarget should not be part of the emitted statement, as they can be + // aliased by multiple statements. If we included the jump target as part of + // the statement we might have issues where the enclosing statement might + // not contain all the opcodes of the enclosed statements. + ptrdiff_t lastNonJumpTargetOffset() const { + return lastOpcodeIsJumpTarget() ? lastTarget_.offset : offset(); + } + private: // ---- Bytecode ---- @@ -159,6 +177,11 @@ struct MOZ_STACK_CLASS BytecodeEmitter { // Code offset for last source note ptrdiff_t lastNoteOffset_ = 0; + + // ---- Jump ---- + + // Last jump target emitted. + JumpTarget lastTarget_ = {-1 - ptrdiff_t(JSOP_JUMPTARGET_LENGTH)}; }; BytecodeSection bytecodeSection_; @@ -192,9 +215,6 @@ struct MOZ_STACK_CLASS BytecodeEmitter { const FieldInitializers fieldInitializers_; public: - // Last jump target emitted. - JumpTarget lastTarget = {-1 - ptrdiff_t(JSOP_JUMPTARGET_LENGTH)}; - // Private storage for parser wrapper. DO NOT REFERENCE INTERNALLY. May not be // initialized. Use |parser| instead. mozilla::Maybe ep_ = {}; @@ -454,21 +474,6 @@ struct MOZ_STACK_CLASS BytecodeEmitter { lastColumn_ = 0; } - // Check if the last emitted opcode is a jump target. - bool lastOpcodeIsJumpTarget() const { - return bytecodeSection().offset() - lastTarget.offset == - ptrdiff_t(JSOP_JUMPTARGET_LENGTH); - } - - // JumpTarget should not be part of the emitted statement, as they can be - // aliased by multiple statements. If we included the jump target as part of - // the statement we might have issues where the enclosing statement might - // not contain all the opcodes of the enclosed statements. - ptrdiff_t lastNonJumpTargetOffset() const { - return lastOpcodeIsJumpTarget() ? lastTarget.offset - : bytecodeSection().offset(); - } - void setFunctionBodyEndPos(uint32_t pos) { functionBodyEndPos = mozilla::Some(pos); } diff --git a/js/src/frontend/LabelEmitter.cpp b/js/src/frontend/LabelEmitter.cpp index c05192b3fd1e..30d863d6e974 100644 --- a/js/src/frontend/LabelEmitter.cpp +++ b/js/src/frontend/LabelEmitter.cpp @@ -40,7 +40,7 @@ bool LabelEmitter::emitEnd() { // Patch the JSOP_LABEL offset. jsbytecode* labelpc = bce_->bytecodeSection().code(top_); - int32_t offset = bce_->lastNonJumpTargetOffset() - top_; + int32_t offset = bce_->bytecodeSection().lastNonJumpTargetOffset() - top_; MOZ_ASSERT(*labelpc == JSOP_LABEL); SET_CODE_OFFSET(labelpc, offset); diff --git a/js/src/frontend/SwitchEmitter.cpp b/js/src/frontend/SwitchEmitter.cpp index 1ff0901fbc1d..58d17e7c1932 100644 --- a/js/src/frontend/SwitchEmitter.cpp +++ b/js/src/frontend/SwitchEmitter.cpp @@ -409,8 +409,9 @@ bool SwitchEmitter::emitEnd() { static_assert(unsigned(SrcNote::TableSwitch::EndOffset) == unsigned(SrcNote::CondSwitch::EndOffset), "{TableSwitch,CondSwitch}::EndOffset should be same"); - if (!bce_->setSrcNoteOffset(noteIndex_, SrcNote::TableSwitch::EndOffset, - bce_->lastNonJumpTargetOffset() - top_)) { + if (!bce_->setSrcNoteOffset( + noteIndex_, SrcNote::TableSwitch::EndOffset, + bce_->bytecodeSection().lastNonJumpTargetOffset() - top_)) { return false; } From f9f5150fc5c77df6af7d5deeefa64dcee008fed1 Mon Sep 17 00:00:00 2001 From: Tooru Fujisawa Date: Tue, 2 Apr 2019 18:17:02 +0900 Subject: [PATCH 06/22] Bug 1535994 - Part 4: Move stackDepth to BytecodeSection class. r=jorendorff Differential Revision: https://phabricator.services.mozilla.com/D25734 --- js/src/frontend/BytecodeControlStructures.cpp | 4 +- js/src/frontend/BytecodeEmitter.cpp | 100 +++++++++--------- js/src/frontend/BytecodeEmitter.h | 22 +++- js/src/frontend/CForEmitter.cpp | 3 +- js/src/frontend/DoWhileEmitter.cpp | 3 +- .../frontend/ExpressionStatementEmitter.cpp | 4 +- js/src/frontend/ForInEmitter.cpp | 7 +- js/src/frontend/ForOfEmitter.cpp | 13 +-- js/src/frontend/ForOfLoopControl.cpp | 5 +- js/src/frontend/IfEmitter.cpp | 10 +- js/src/frontend/TryEmitter.cpp | 12 +-- js/src/frontend/WhileEmitter.cpp | 3 +- js/src/vm/JSScript.cpp | 3 +- 13 files changed, 104 insertions(+), 85 deletions(-) diff --git a/js/src/frontend/BytecodeControlStructures.cpp b/js/src/frontend/BytecodeControlStructures.cpp index 7c6df46e70e1..727a707fb762 100644 --- a/js/src/frontend/BytecodeControlStructures.cpp +++ b/js/src/frontend/BytecodeControlStructures.cpp @@ -41,7 +41,7 @@ LoopControl::LoopControl(BytecodeEmitter* bce, StatementKind loopKind) LoopControl* enclosingLoop = findNearest(enclosing()); - stackDepth_ = bce->stackDepth; + stackDepth_ = bce->bytecodeSection().stackDepth(); loopDepth_ = enclosingLoop ? enclosingLoop->loopDepth_ + 1 : 1; int loopSlots; @@ -81,7 +81,7 @@ bool LoopControl::emitContinueTarget(BytecodeEmitter* bce) { bool LoopControl::emitSpecialBreakForDone(BytecodeEmitter* bce) { // This doesn't pop stack values, nor handle any other controls. // Should be called on the toplevel of the loop. - MOZ_ASSERT(bce->stackDepth == stackDepth_); + MOZ_ASSERT(bce->bytecodeSection().stackDepth() == stackDepth_); MOZ_ASSERT(bce->innermostNestableControl == this); if (!bce->newSrcNote(SRC_BREAK)) { diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index 9186af42eb62..25c36f292645 100644 --- a/js/src/frontend/BytecodeEmitter.cpp +++ b/js/src/frontend/BytecodeEmitter.cpp @@ -247,18 +247,18 @@ bool BytecodeEmitter::emitCheck(JSOp op, ptrdiff_t delta, ptrdiff_t* offset) { return true; } -void BytecodeEmitter::updateDepth(ptrdiff_t target) { - jsbytecode* pc = bytecodeSection().code(target); +void BytecodeEmitter::BytecodeSection::updateDepth(ptrdiff_t target) { + jsbytecode* pc = code(target); int nuses = StackUses(pc); int ndefs = StackDefs(pc); - stackDepth -= nuses; - MOZ_ASSERT(stackDepth >= 0); - stackDepth += ndefs; + stackDepth_ -= nuses; + MOZ_ASSERT(stackDepth_ >= 0); + stackDepth_ += ndefs; - if ((uint32_t)stackDepth > maxStackDepth) { - maxStackDepth = stackDepth; + if ((uint32_t)stackDepth_ > maxStackDepth_) { + maxStackDepth_ = stackDepth_; } } @@ -284,7 +284,7 @@ bool BytecodeEmitter::emit1(JSOp op) { jsbytecode* code = bytecodeSection().code(offset); code[0] = jsbytecode(op); - updateDepth(offset); + bytecodeSection().updateDepth(offset); return true; } @@ -299,7 +299,7 @@ bool BytecodeEmitter::emit2(JSOp op, uint8_t op1) { jsbytecode* code = bytecodeSection().code(offset); code[0] = jsbytecode(op); code[1] = jsbytecode(op1); - updateDepth(offset); + bytecodeSection().updateDepth(offset); return true; } @@ -319,7 +319,7 @@ bool BytecodeEmitter::emit3(JSOp op, jsbytecode op1, jsbytecode op2) { code[0] = jsbytecode(op); code[1] = op1; code[2] = op2; - updateDepth(offset); + bytecodeSection().updateDepth(offset); return true; } @@ -341,7 +341,7 @@ bool BytecodeEmitter::emitN(JSOp op, size_t extra, ptrdiff_t* offset) { * operand yet to be stored in the extra bytes after op. */ if (CodeSpec[op].nuses >= 0) { - updateDepth(off); + bytecodeSection().updateDepth(off); } if (offset) { @@ -394,7 +394,7 @@ bool BytecodeEmitter::emitJumpNoFallthrough(JSOp op, JumpList* jump) { code[0] = jsbytecode(op); MOZ_ASSERT(-1 <= jump->offset && jump->offset < offset); jump->push(bytecodeSection().code(0), offset); - updateDepth(offset); + bytecodeSection().updateDepth(offset); return true; } @@ -463,7 +463,7 @@ bool BytecodeEmitter::emitCall(JSOp op, uint16_t argc, ParseNode* pn) { } bool BytecodeEmitter::emitDupAt(unsigned slotFromTop) { - MOZ_ASSERT(slotFromTop < unsigned(stackDepth)); + MOZ_ASSERT(slotFromTop < unsigned(bytecodeSection().stackDepth())); if (slotFromTop == 0) { return emit1(JSOP_DUP); @@ -669,7 +669,7 @@ class NonLocalExitControl { NonLocalExitControl(BytecodeEmitter* bce, Kind kind) : bce_(bce), savedScopeNoteIndex_(bce->scopeNoteList.length()), - savedDepth_(bce->stackDepth), + savedDepth_(bce->bytecodeSection().stackDepth()), openScopeNoteIndex_(bce->innermostEmitterScope()->noteIndex()), kind_(kind) {} @@ -678,7 +678,7 @@ class NonLocalExitControl { n++) { bce_->scopeNoteList.recordEnd(n, bce_->bytecodeSection().offset()); } - bce_->stackDepth = savedDepth_; + bce_->bytecodeSection().setStackDepth(savedDepth_); } MOZ_MUST_USE bool prepareForNonLocalJump(NestableControl* target); @@ -893,7 +893,7 @@ bool BytecodeEmitter::emitIndex32(JSOp op, uint32_t index) { jsbytecode* code = bytecodeSection().code(offset); code[0] = jsbytecode(op); SET_UINT32_INDEX(code, index); - updateDepth(offset); + bytecodeSection().updateDepth(offset); return true; } @@ -911,7 +911,7 @@ bool BytecodeEmitter::emitIndexOp(JSOp op, uint32_t index) { jsbytecode* code = bytecodeSection().code(offset); code[0] = jsbytecode(op); SET_UINT32_INDEX(code, index); - updateDepth(offset); + bytecodeSection().updateDepth(offset); return true; } @@ -1692,7 +1692,7 @@ bool BytecodeEmitter::emitNewInit() { code[2] = 0; code[3] = 0; code[4] = 0; - updateDepth(offset); + bytecodeSection().updateDepth(offset); return true; } @@ -2026,7 +2026,7 @@ bool BytecodeEmitter::emitDouble(double d) { jsbytecode* code = bytecodeSection().code(offset); code[0] = jsbytecode(JSOP_DOUBLE); SET_INLINE_VALUE(code, DoubleValue(d)); - updateDepth(offset); + bytecodeSection().updateDepth(offset); return true; } @@ -2575,7 +2575,7 @@ bool BytecodeEmitter::emitDestructuringLHSRef(ParseNode* target, } #ifdef DEBUG - int depth = stackDepth; + int depth = bytecodeSection().stackDepth(); #endif switch (target->getKind()) { @@ -2653,7 +2653,7 @@ bool BytecodeEmitter::emitDestructuringLHSRef(ParseNode* target, MOZ_CRASH("emitDestructuringLHSRef: bad lhs kind"); } - MOZ_ASSERT(stackDepth == depth + int(*emitted)); + MOZ_ASSERT(bytecodeSection().stackDepth() == depth + int(*emitted)); return true; } @@ -2816,7 +2816,7 @@ bool BytecodeEmitter::emitIteratorNext( "can run user-modifiable iteration code"); // [stack] ... NEXT ITER - MOZ_ASSERT(this->stackDepth >= 2); + MOZ_ASSERT(bytecodeSection().stackDepth() >= 2); if (!emitCall(JSOP_CALL, 0, callSourceCoordOffset)) { // [stack] ... RESULT @@ -2839,7 +2839,7 @@ bool BytecodeEmitter::emitIteratorNext( bool BytecodeEmitter::emitPushNotUndefinedOrNull() { // [stack] V - MOZ_ASSERT(this->stackDepth > 0); + MOZ_ASSERT(bytecodeSection().stackDepth() > 0); if (!emit1(JSOP_DUP)) { // [stack] V V @@ -3087,7 +3087,7 @@ bool BytecodeEmitter::emitIteratorCloseInScope( template bool BytecodeEmitter::wrapWithDestructuringTryNote(int32_t iterDepth, InnerEmitter emitter) { - MOZ_ASSERT(this->stackDepth >= iterDepth); + MOZ_ASSERT(bytecodeSection().stackDepth() >= iterDepth); // Pad a nop at the beginning of the bytecode covered by the trynote so // that when unwinding environments, we may unwind to the scope @@ -3208,7 +3208,7 @@ bool BytecodeEmitter::emitInitializer(ParseNode* initializer, bool BytecodeEmitter::emitDestructuringOpsArray(ListNode* pattern, DestructuringFlavor flav) { MOZ_ASSERT(pattern->isKind(ParseNodeKind::ArrayExpr)); - MOZ_ASSERT(this->stackDepth != 0); + MOZ_ASSERT(bytecodeSection().stackDepth() != 0); // Here's pseudo code for |let [a, b, , c=y, ...d] = x;| // @@ -3332,7 +3332,7 @@ bool BytecodeEmitter::emitDestructuringOpsArray(ListNode* pattern, // JSTRY_DESTRUCTURING expects the iterator and the done value // to be the second to top and the top of the stack, respectively. // IteratorClose is called upon exception only if done is false. - int32_t tryNoteDepth = stackDepth; + int32_t tryNoteDepth = bytecodeSection().stackDepth(); for (ParseNode* member : pattern->contents()) { bool isFirst = member == pattern->head(); @@ -3641,7 +3641,7 @@ bool BytecodeEmitter::emitDestructuringOpsObject(ListNode* pattern, MOZ_ASSERT(pattern->isKind(ParseNodeKind::ObjectExpr)); // [stack] ... RHS - MOZ_ASSERT(this->stackDepth > 0); + MOZ_ASSERT(bytecodeSection().stackDepth() > 0); if (!emit1(JSOP_CHECKOBJCOERCIBLE)) { // [stack] ... RHS @@ -4905,7 +4905,7 @@ bool BytecodeEmitter::emitWith(BinaryNode* withNode) { } bool BytecodeEmitter::emitCopyDataProperties(CopyOption option) { - DebugOnly depth = this->stackDepth; + DebugOnly depth = bytecodeSection().stackDepth(); uint32_t argc; if (option == CopyOption::Filtered) { @@ -4960,7 +4960,7 @@ bool BytecodeEmitter::emitCopyDataProperties(CopyOption option) { return false; } - MOZ_ASSERT(depth - int(argc) == this->stackDepth); + MOZ_ASSERT(depth - int(argc) == bytecodeSection().stackDepth()); return true; } @@ -5151,11 +5151,11 @@ bool BytecodeEmitter::emitSpread(bool allowSelfHosted) { // when we reach this point on the loop backedge (if spreading produces at // least one value), we've additionally pushed a RESULT iteration value. // Increment manually to reflect this. - this->stackDepth++; + bytecodeSection().setStackDepth(bytecodeSection().stackDepth() + 1); { #ifdef DEBUG - auto loopDepth = this->stackDepth; + auto loopDepth = bytecodeSection().stackDepth(); #endif // Emit code to assign result.value to the iteration variable. @@ -5168,7 +5168,7 @@ bool BytecodeEmitter::emitSpread(bool allowSelfHosted) { return false; } - MOZ_ASSERT(this->stackDepth == loopDepth - 1); + MOZ_ASSERT(bytecodeSection().stackDepth() == loopDepth - 1); // Spread operations can't contain |continue|, so don't bother setting loop // and enclosing "update" offsets, as we do with for-loops. @@ -5205,7 +5205,7 @@ bool BytecodeEmitter::emitSpread(bool allowSelfHosted) { return false; } - MOZ_ASSERT(this->stackDepth == loopDepth); + MOZ_ASSERT(bytecodeSection().stackDepth() == loopDepth); } // Let Ion know where the closing jump of this loop is. @@ -5218,8 +5218,8 @@ bool BytecodeEmitter::emitSpread(bool allowSelfHosted) { MOZ_ASSERT(loopInfo.breaks.offset == -1); MOZ_ASSERT(loopInfo.continues.offset == -1); - if (!addTryNote(JSTRY_FOR_OF, stackDepth, loopInfo.headOffset(), - loopInfo.breakTargetOffset())) { + if (!addTryNote(JSTRY_FOR_OF, bytecodeSection().stackDepth(), + loopInfo.headOffset(), loopInfo.breakTargetOffset())) { return false; } @@ -5240,7 +5240,7 @@ bool BytecodeEmitter::emitInitializeForInOrOfTarget(TernaryNode* forHead) { MOZ_ASSERT(forHead->isKind(ParseNodeKind::ForIn) || forHead->isKind(ParseNodeKind::ForOf)); - MOZ_ASSERT(this->stackDepth >= 1, + MOZ_ASSERT(bytecodeSection().stackDepth() >= 1, "must have a per-iteration value for initializing"); ParseNode* target = forHead->kid1(); @@ -5288,14 +5288,14 @@ bool BytecodeEmitter::emitInitializeForInOrOfTarget(TernaryNode* forHead) { // iteration value *before* initializing. Thus the initializing // value may be buried under a bind-specific value on the stack. // Swap it to the top of the stack. - MOZ_ASSERT(stackDepth >= 2); + MOZ_ASSERT(bytecodeSection().stackDepth() >= 2); if (!emit1(JSOP_SWAP)) { return false; } } else { // In cases of emitting a frame slot or environment slot, // nothing needs be done. - MOZ_ASSERT(stackDepth >= 1); + MOZ_ASSERT(bytecodeSection().stackDepth() >= 1); } if (!noe.emitAssignment()) { return false; @@ -6220,7 +6220,7 @@ bool BytecodeEmitter::emitYieldStar(ParseNode* iter) { } int32_t savedDepthTemp; - int32_t startDepth = stackDepth; + int32_t startDepth = bytecodeSection().stackDepth(); MOZ_ASSERT(startDepth >= 3); TryEmitter tryCatch(this, TryEmitter::Kind::TryCatchFinally, @@ -6252,7 +6252,7 @@ bool BytecodeEmitter::emitYieldStar(ParseNode* iter) { return false; } - MOZ_ASSERT(this->stackDepth == startDepth); + MOZ_ASSERT(bytecodeSection().stackDepth() == startDepth); // Step 7.a.vi. // Step 7.b.ii.7. @@ -6285,7 +6285,7 @@ bool BytecodeEmitter::emitYieldStar(ParseNode* iter) { return false; } - MOZ_ASSERT(stackDepth == startDepth); + MOZ_ASSERT(bytecodeSection().stackDepth() == startDepth); if (!emit1(JSOP_EXCEPTION)) { // [stack] NEXT ITER RESULT EXCEPTION @@ -6304,7 +6304,7 @@ bool BytecodeEmitter::emitYieldStar(ParseNode* iter) { return false; } - savedDepthTemp = stackDepth; + savedDepthTemp = bytecodeSection().stackDepth(); InternalIfEmitter ifThrowMethodIsNotDefined(this); if (!emitPushNotUndefinedOrNull()) { // [stack] NEXT ITER RESULT EXCEPTION ITER THROW @@ -6355,7 +6355,7 @@ bool BytecodeEmitter::emitYieldStar(ParseNode* iter) { // [stack] NEXT ITER RESULT return false; } - MOZ_ASSERT(this->stackDepth == startDepth); + MOZ_ASSERT(bytecodeSection().stackDepth() == startDepth); JumpList checkResult; // Note that there is no GOSUB to the finally block here. If the iterator has @@ -6366,7 +6366,7 @@ bool BytecodeEmitter::emitYieldStar(ParseNode* iter) { return false; } - stackDepth = savedDepthTemp; + bytecodeSection().setStackDepth(savedDepthTemp); if (!ifThrowMethodIsNotDefined.emitElse()) { // [stack] NEXT ITER RESULT EXCEPTION ITER THROW return false; @@ -6391,12 +6391,12 @@ bool BytecodeEmitter::emitYieldStar(ParseNode* iter) { return false; } - stackDepth = savedDepthTemp; + bytecodeSection().setStackDepth(savedDepthTemp); if (!ifThrowMethodIsNotDefined.emitEnd()) { return false; } - stackDepth = startDepth; + bytecodeSection().setStackDepth(startDepth); if (!tryCatch.emitFinally()) { return false; } @@ -6523,7 +6523,7 @@ bool BytecodeEmitter::emitYieldStar(ParseNode* iter) { // [stack] NEXT ITER OLDRESULT FTYPE FVALUE return false; } - savedDepthTemp = this->stackDepth; + savedDepthTemp = bytecodeSection().stackDepth(); if (!ifReturnDone.emitElse()) { // [stack] NEXT ITER OLDRESULT FTYPE FVALUE RESULT return false; @@ -6545,7 +6545,7 @@ bool BytecodeEmitter::emitYieldStar(ParseNode* iter) { return false; } } - this->stackDepth = savedDepthTemp; + bytecodeSection().setStackDepth(savedDepthTemp); if (!ifReturnDone.emitEnd()) { return false; } @@ -6620,7 +6620,7 @@ bool BytecodeEmitter::emitYieldStar(ParseNode* iter) { // [stack] NEXT ITER RESULT return false; } - MOZ_ASSERT(this->stackDepth == startDepth); + MOZ_ASSERT(bytecodeSection().stackDepth() == startDepth); // Steps 7.a.iv-v. // Steps 7.b.ii.5-6. @@ -6665,7 +6665,7 @@ bool BytecodeEmitter::emitYieldStar(ParseNode* iter) { return false; } - MOZ_ASSERT(this->stackDepth == startDepth - 2); + MOZ_ASSERT(bytecodeSection().stackDepth() == startDepth - 2); return true; } diff --git a/js/src/frontend/BytecodeEmitter.h b/js/src/frontend/BytecodeEmitter.h index 10ebc6c7a1c4..1458457d580d 100644 --- a/js/src/frontend/BytecodeEmitter.h +++ b/js/src/frontend/BytecodeEmitter.h @@ -164,6 +164,15 @@ struct MOZ_STACK_CLASS BytecodeEmitter { return lastOpcodeIsJumpTarget() ? lastTarget_.offset : offset(); } + // ---- Stack ---- + + int32_t stackDepth() const { return stackDepth_; } + void setStackDepth(int32_t depth) { stackDepth_ = depth; } + + uint32_t maxStackDepth() const { return maxStackDepth_; } + + void updateDepth(ptrdiff_t target); + private: // ---- Bytecode ---- @@ -182,6 +191,14 @@ struct MOZ_STACK_CLASS BytecodeEmitter { // Last jump target emitted. JumpTarget lastTarget_ = {-1 - ptrdiff_t(JSOP_JUMPTARGET_LENGTH)}; + + // ---- Stack ---- + + // Maximum number of expression stack slots so far. + uint32_t maxStackDepth_ = 0; + + // Current stack depth in script frame. + int32_t stackDepth_ = 0; }; BytecodeSection bytecodeSection_; @@ -224,10 +241,6 @@ struct MOZ_STACK_CLASS BytecodeEmitter { unsigned firstLine = 0; /* first line, for JSScript::initFromEmitter */ uint32_t maxFixedSlots = 0; /* maximum number of fixed frame slots so far */ - uint32_t maxStackDepth = - 0; /* maximum number of expression stack slots so far */ - - int32_t stackDepth = 0; /* current stack depth in script frame */ uint32_t bodyScopeIndex = UINT32_MAX; /* index into scopeList of the body scope */ @@ -542,7 +555,6 @@ struct MOZ_STACK_CLASS BytecodeEmitter { MOZ_MUST_USE bool emitFunctionScript(FunctionNode* funNode, TopLevelFunction isTopLevel); - void updateDepth(ptrdiff_t target); MOZ_MUST_USE bool markStepBreakpoint(); MOZ_MUST_USE bool markSimpleBreakpoint(); MOZ_MUST_USE bool updateLineNumberNotes(uint32_t offset); diff --git a/js/src/frontend/CForEmitter.cpp b/js/src/frontend/CForEmitter.cpp index 4a8b6a63321f..7a397034c45c 100644 --- a/js/src/frontend/CForEmitter.cpp +++ b/js/src/frontend/CForEmitter.cpp @@ -228,7 +228,8 @@ bool CForEmitter::emitEnd() { return false; } - if (!bce_->addTryNote(JSTRY_LOOP, bce_->stackDepth, loopInfo_->headOffset(), + if (!bce_->addTryNote(JSTRY_LOOP, bce_->bytecodeSection().stackDepth(), + loopInfo_->headOffset(), loopInfo_->breakTargetOffset())) { return false; } diff --git a/js/src/frontend/DoWhileEmitter.cpp b/js/src/frontend/DoWhileEmitter.cpp index 3a57036cf8c0..35d86b312f4d 100644 --- a/js/src/frontend/DoWhileEmitter.cpp +++ b/js/src/frontend/DoWhileEmitter.cpp @@ -75,7 +75,8 @@ bool DoWhileEmitter::emitEnd() { return false; } - if (!bce_->addTryNote(JSTRY_LOOP, bce_->stackDepth, loopInfo_->headOffset(), + if (!bce_->addTryNote(JSTRY_LOOP, bce_->bytecodeSection().stackDepth(), + loopInfo_->headOffset(), loopInfo_->breakTargetOffset())) { return false; } diff --git a/js/src/frontend/ExpressionStatementEmitter.cpp b/js/src/frontend/ExpressionStatementEmitter.cpp index ef70db1beccf..c786f3486091 100644 --- a/js/src/frontend/ExpressionStatementEmitter.cpp +++ b/js/src/frontend/ExpressionStatementEmitter.cpp @@ -29,7 +29,7 @@ bool ExpressionStatementEmitter::prepareForExpr( } #ifdef DEBUG - depth_ = bce_->stackDepth; + depth_ = bce_->bytecodeSection().stackDepth(); state_ = State::Expr; #endif return true; @@ -37,7 +37,7 @@ bool ExpressionStatementEmitter::prepareForExpr( bool ExpressionStatementEmitter::emitEnd() { MOZ_ASSERT(state_ == State::Expr); - MOZ_ASSERT(bce_->stackDepth == depth_ + 1); + MOZ_ASSERT(bce_->bytecodeSection().stackDepth() == depth_ + 1); // [stack] VAL diff --git a/js/src/frontend/ForInEmitter.cpp b/js/src/frontend/ForInEmitter.cpp index ff1bf21c995e..627fa668b5b5 100644 --- a/js/src/frontend/ForInEmitter.cpp +++ b/js/src/frontend/ForInEmitter.cpp @@ -94,7 +94,7 @@ bool ForInEmitter::emitInitialize() { } #ifdef DEBUG - loopDepth_ = bce_->stackDepth; + loopDepth_ = bce_->bytecodeSection().stackDepth(); #endif MOZ_ASSERT(loopDepth_ >= 2); @@ -112,7 +112,7 @@ bool ForInEmitter::emitInitialize() { bool ForInEmitter::emitBody() { MOZ_ASSERT(state_ == State::Initialize); - MOZ_ASSERT(bce_->stackDepth == loopDepth_, + MOZ_ASSERT(bce_->bytecodeSection().stackDepth() == loopDepth_, "iterator and iterval must be left on the stack"); #ifdef DEBUG @@ -171,7 +171,8 @@ bool ForInEmitter::emitEnd(const Maybe& forPos) { return false; } - if (!bce_->addTryNote(JSTRY_FOR_IN, bce_->stackDepth, loopInfo_->headOffset(), + if (!bce_->addTryNote(JSTRY_FOR_IN, bce_->bytecodeSection().stackDepth(), + loopInfo_->headOffset(), bce_->bytecodeSection().offset())) { return false; } diff --git a/js/src/frontend/ForOfEmitter.cpp b/js/src/frontend/ForOfEmitter.cpp index 84ab3894aa67..dac23f9e5ba7 100644 --- a/js/src/frontend/ForOfEmitter.cpp +++ b/js/src/frontend/ForOfEmitter.cpp @@ -58,7 +58,7 @@ bool ForOfEmitter::emitInitialize(const Maybe& forPos) { } } - int32_t iterDepth = bce_->stackDepth; + int32_t iterDepth = bce_->bytecodeSection().stackDepth(); // For-of loops have the iterator next method, the iterator itself, and // the result.value on the stack. @@ -111,7 +111,7 @@ bool ForOfEmitter::emitInitialize(const Maybe& forPos) { } #ifdef DEBUG - loopDepth_ = bce_->stackDepth; + loopDepth_ = bce_->bytecodeSection().stackDepth(); #endif // Make sure this code is attributed to the "for". @@ -195,7 +195,7 @@ bool ForOfEmitter::emitInitialize(const Maybe& forPos) { bool ForOfEmitter::emitBody() { MOZ_ASSERT(state_ == State::Initialize); - MOZ_ASSERT(bce_->stackDepth == loopDepth_, + MOZ_ASSERT(bce_->bytecodeSection().stackDepth() == loopDepth_, "the stack must be balanced around the initializing " "operation"); @@ -218,7 +218,7 @@ bool ForOfEmitter::emitBody() { bool ForOfEmitter::emitEnd(const Maybe& iteratedPos) { MOZ_ASSERT(state_ == State::Body); - MOZ_ASSERT(bce_->stackDepth == loopDepth_, + MOZ_ASSERT(bce_->bytecodeSection().stackDepth() == loopDepth_, "the stack must be balanced around the for-of body"); if (!loopInfo_->emitEndCodeNeedingIteratorClose(bce_)) { @@ -244,7 +244,7 @@ bool ForOfEmitter::emitEnd(const Maybe& iteratedPos) { return false; } - MOZ_ASSERT(bce_->stackDepth == loopDepth_); + MOZ_ASSERT(bce_->bytecodeSection().stackDepth() == loopDepth_); // Let Ion know where the closing jump of this loop is. if (!bce_->setSrcNoteOffset(noteIndex_, SrcNote::ForOf::BackJumpOffset, @@ -256,7 +256,8 @@ bool ForOfEmitter::emitEnd(const Maybe& iteratedPos) { return false; } - if (!bce_->addTryNote(JSTRY_FOR_OF, bce_->stackDepth, loopInfo_->headOffset(), + if (!bce_->addTryNote(JSTRY_FOR_OF, bce_->bytecodeSection().stackDepth(), + loopInfo_->headOffset(), loopInfo_->breakTargetOffset())) { return false; } diff --git a/js/src/frontend/ForOfLoopControl.cpp b/js/src/frontend/ForOfLoopControl.cpp index 660b9e959453..ec8bf910eca9 100644 --- a/js/src/frontend/ForOfLoopControl.cpp +++ b/js/src/frontend/ForOfLoopControl.cpp @@ -45,7 +45,7 @@ bool ForOfLoopControl::emitEndCodeNeedingIteratorClose(BytecodeEmitter* bce) { // [stack] ITER ... EXCEPTION return false; } - unsigned slotFromTop = bce->stackDepth - iterDepth_; + unsigned slotFromTop = bce->bytecodeSection().stackDepth() - iterDepth_; if (!bce->emitDupAt(slotFromTop)) { // [stack] ITER ... EXCEPTION ITER return false; @@ -69,7 +69,8 @@ bool ForOfLoopControl::emitEndCodeNeedingIteratorClose(BytecodeEmitter* bce) { return false; } - MOZ_ASSERT(slotFromTop == unsigned(bce->stackDepth - iterDepth_)); + MOZ_ASSERT(slotFromTop == + unsigned(bce->bytecodeSection().stackDepth() - iterDepth_)); if (!bce->emitDupAt(slotFromTop)) { // [stack] ITER ... EXCEPTION ITER return false; diff --git a/js/src/frontend/IfEmitter.cpp b/js/src/frontend/IfEmitter.cpp index 6925ba96cf40..f9b9f7f656dd 100644 --- a/js/src/frontend/IfEmitter.cpp +++ b/js/src/frontend/IfEmitter.cpp @@ -41,10 +41,10 @@ bool BranchEmitterBase::emitThenInternal(SrcNoteType type) { // To restore stack depth in else part, save depth of the then part. #ifdef DEBUG // If DEBUG, this is also necessary to calculate |pushed_|. - thenDepth_ = bce_->stackDepth; + thenDepth_ = bce_->bytecodeSection().stackDepth(); #else if (type == SRC_COND || type == SRC_IF_ELSE) { - thenDepth_ = bce_->stackDepth; + thenDepth_ = bce_->bytecodeSection().stackDepth(); } #endif @@ -59,10 +59,10 @@ bool BranchEmitterBase::emitThenInternal(SrcNoteType type) { void BranchEmitterBase::calculateOrCheckPushed() { #ifdef DEBUG if (!calculatedPushed_) { - pushed_ = bce_->stackDepth - thenDepth_; + pushed_ = bce_->bytecodeSection().stackDepth() - thenDepth_; calculatedPushed_ = true; } else { - MOZ_ASSERT(pushed_ == bce_->stackDepth - thenDepth_); + MOZ_ASSERT(pushed_ == bce_->bytecodeSection().stackDepth() - thenDepth_); } #endif } @@ -92,7 +92,7 @@ bool BranchEmitterBase::emitElseInternal() { jumpAroundThen_ = JumpList(); // Restore stack depth of the then part. - bce_->stackDepth = thenDepth_; + bce_->bytecodeSection().setStackDepth(thenDepth_); // Enclose else-branch with TDZCheckCache. if (kind_ == Kind::MayContainLexicalAccessInBranch) { diff --git a/js/src/frontend/TryEmitter.cpp b/js/src/frontend/TryEmitter.cpp index 4c8dbd296300..d194c14810ca 100644 --- a/js/src/frontend/TryEmitter.cpp +++ b/js/src/frontend/TryEmitter.cpp @@ -53,7 +53,7 @@ bool TryEmitter::emitTry() { // For that we store in a try note associated with the catch or // finally block the stack depth upon the try entry. The interpreter // uses this depth to properly unwind the stack and the scope chain. - depth_ = bce_->stackDepth; + depth_ = bce_->bytecodeSection().stackDepth(); // Record the try location, then emit the try block. if (!bce_->newSrcNote(SRC_TRY, ¬eIndex_)) { @@ -72,7 +72,7 @@ bool TryEmitter::emitTry() { bool TryEmitter::emitTryEnd() { MOZ_ASSERT(state_ == State::Try); - MOZ_ASSERT(depth_ == bce_->stackDepth); + MOZ_ASSERT(depth_ == bce_->bytecodeSection().stackDepth()); // GOSUB to finally, if present. if (hasFinally() && controlInfo_) { @@ -106,7 +106,7 @@ bool TryEmitter::emitCatch() { return false; } - MOZ_ASSERT(bce_->stackDepth == depth_); + MOZ_ASSERT(bce_->bytecodeSection().stackDepth() == depth_); if (controlKind_ == ControlKind::Syntactic) { // Clear the frame's return value that might have been set by the @@ -139,7 +139,7 @@ bool TryEmitter::emitCatchEnd() { if (!bce_->emitGoSub(&controlInfo_->gosubs)) { return false; } - MOZ_ASSERT(bce_->stackDepth == depth_); + MOZ_ASSERT(bce_->bytecodeSection().stackDepth() == depth_); // Jump over the finally block. if (!bce_->emitJump(JSOP_GOTO, &catchAndFinallyJump_)) { @@ -178,7 +178,7 @@ bool TryEmitter::emitFinally( } } - MOZ_ASSERT(bce_->stackDepth == depth_); + MOZ_ASSERT(bce_->bytecodeSection().stackDepth() == depth_); if (!bce_->emitJumpTarget(&finallyStart_)) { return false; @@ -254,7 +254,7 @@ bool TryEmitter::emitEnd() { } } - MOZ_ASSERT(bce_->stackDepth == depth_); + MOZ_ASSERT(bce_->bytecodeSection().stackDepth() == depth_); // ReconstructPCStack needs a NOP here to mark the end of the last // catch block. diff --git a/js/src/frontend/WhileEmitter.cpp b/js/src/frontend/WhileEmitter.cpp index c73ef4a5da57..f76d4010641b 100644 --- a/js/src/frontend/WhileEmitter.cpp +++ b/js/src/frontend/WhileEmitter.cpp @@ -100,7 +100,8 @@ bool WhileEmitter::emitEnd() { return false; } - if (!bce_->addTryNote(JSTRY_LOOP, bce_->stackDepth, loopInfo_->headOffset(), + if (!bce_->addTryNote(JSTRY_LOOP, bce_->bytecodeSection().stackDepth(), + loopInfo_->headOffset(), loopInfo_->breakTargetOffset())) { return false; } diff --git a/js/src/vm/JSScript.cpp b/js/src/vm/JSScript.cpp index 35507ef133ba..27a5549d7eca 100644 --- a/js/src/vm/JSScript.cpp +++ b/js/src/vm/JSScript.cpp @@ -3569,7 +3569,8 @@ bool JSScript::fullyInitFromEmitter(JSContext* cx, HandleScript script, MOZ_ASSERT(bce->objectList.length <= INDEX_LIMIT); uint64_t nslots = - bce->maxFixedSlots + static_cast(bce->maxStackDepth); + bce->maxFixedSlots + + static_cast(bce->bytecodeSection().maxStackDepth()); if (nslots > UINT32_MAX) { bce->reportError(nullptr, JSMSG_NEED_DIET, js_script_str); return false; From 14daa8678aab5e974e9361e4cb3c31abe1d421c2 Mon Sep 17 00:00:00 2001 From: Tooru Fujisawa Date: Tue, 2 Apr 2019 18:20:42 +0900 Subject: [PATCH 07/22] Bug 1535994 - Part 5: Move tryNodeList to BytecodeSection class. r=jorendorff Differential Revision: https://phabricator.services.mozilla.com/D25735 --- js/src/frontend/BytecodeEmitter.cpp | 5 ++--- js/src/frontend/BytecodeEmitter.h | 11 ++++++++++- js/src/vm/JSScript.cpp | 4 ++-- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index 25c36f292645..4e75f5136b95 100644 --- a/js/src/frontend/BytecodeEmitter.cpp +++ b/js/src/frontend/BytecodeEmitter.cpp @@ -92,7 +92,7 @@ static bool ParseNodeRequiresSpecialLineNumberNotes(ParseNode* pn) { } BytecodeEmitter::BytecodeSection::BytecodeSection(JSContext* cx) - : code_(cx), notes_(cx) {} + : code_(cx), notes_(cx), tryNoteList_(cx) {} BytecodeEmitter::BytecodeEmitter( BytecodeEmitter* parent, SharedContext* sc, HandleScript script, @@ -110,7 +110,6 @@ BytecodeEmitter::BytecodeEmitter( firstLine(lineNum), numberList(cx), scopeList(cx), - tryNoteList(cx), scopeNoteList(cx), resumeOffsetList(cx), emitterMode(emitterMode) { @@ -9341,7 +9340,7 @@ static bool AllocSrcNote(JSContext* cx, SrcNotesVector& notes, bool BytecodeEmitter::addTryNote(JSTryNoteKind kind, uint32_t stackDepth, size_t start, size_t end) { MOZ_ASSERT(!inPrologue()); - return tryNoteList.append(kind, stackDepth, start, end); + return bytecodeSection().tryNoteList().append(kind, stackDepth, start, end); } bool BytecodeEmitter::newSrcNote(SrcNoteType type, unsigned* indexp) { diff --git a/js/src/frontend/BytecodeEmitter.h b/js/src/frontend/BytecodeEmitter.h index 1458457d580d..e6e706b8a849 100644 --- a/js/src/frontend/BytecodeEmitter.h +++ b/js/src/frontend/BytecodeEmitter.h @@ -173,6 +173,11 @@ struct MOZ_STACK_CLASS BytecodeEmitter { void updateDepth(ptrdiff_t target); + // ---- Try notes ---- + + CGTryNoteList& tryNoteList() { return tryNoteList_; }; + const CGTryNoteList& tryNoteList() const { return tryNoteList_; }; + private: // ---- Bytecode ---- @@ -199,6 +204,11 @@ struct MOZ_STACK_CLASS BytecodeEmitter { // Current stack depth in script frame. int32_t stackDepth_ = 0; + + // ---- Try notes ---- + + // List of emitted try notes. + CGTryNoteList tryNoteList_; }; BytecodeSection bytecodeSection_; @@ -269,7 +279,6 @@ struct MOZ_STACK_CLASS BytecodeEmitter { CGNumberList numberList; /* double and bigint values used by script */ CGObjectList objectList; /* list of emitted objects */ CGScopeList scopeList; /* list of emitted scopes */ - CGTryNoteList tryNoteList; /* list of emitted try notes */ CGScopeNoteList scopeNoteList; /* list of emitted block scope notes */ // Certain ops (yield, await, gosub) have an entry in the script's diff --git a/js/src/vm/JSScript.cpp b/js/src/vm/JSScript.cpp index 27a5549d7eca..8fb49fe74622 100644 --- a/js/src/vm/JSScript.cpp +++ b/js/src/vm/JSScript.cpp @@ -3241,7 +3241,7 @@ PrivateScriptData* PrivateScriptData::new_(JSContext* cx, uint32_t nscopes, uint32_t nscopes = bce->scopeList.length(); uint32_t nconsts = bce->numberList.length(); uint32_t nobjects = bce->objectList.length; - uint32_t ntrynotes = bce->tryNoteList.length(); + uint32_t ntrynotes = bce->bytecodeSection().tryNoteList().length(); uint32_t nscopenotes = bce->scopeNoteList.length(); uint32_t nresumeoffsets = bce->resumeOffsetList.length(); @@ -3263,7 +3263,7 @@ PrivateScriptData* PrivateScriptData::new_(JSContext* cx, uint32_t nscopes, bce->objectList.finish(data->objects()); } if (ntrynotes) { - bce->tryNoteList.finish(data->tryNotes()); + bce->bytecodeSection().tryNoteList().finish(data->tryNotes()); } if (nscopenotes) { bce->scopeNoteList.finish(data->scopeNotes()); From 795bae5f14956b6dc78225e6652fdd00db853b1c Mon Sep 17 00:00:00 2001 From: Tooru Fujisawa Date: Tue, 2 Apr 2019 18:20:42 +0900 Subject: [PATCH 08/22] Bug 1535994 - Part 6: Move scope to BytecodeSection class and PerScriptData class. r=jorendorff Differential Revision: https://phabricator.services.mozilla.com/D25736 --- js/src/frontend/BytecodeEmitter.cpp | 26 ++++++++------- js/src/frontend/BytecodeEmitter.h | 49 ++++++++++++++++++++++++----- js/src/frontend/EmitterScope.cpp | 20 ++++++------ js/src/vm/JSScript.cpp | 8 ++--- 4 files changed, 70 insertions(+), 33 deletions(-) diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index 4e75f5136b95..55fab7c05989 100644 --- a/js/src/frontend/BytecodeEmitter.cpp +++ b/js/src/frontend/BytecodeEmitter.cpp @@ -92,7 +92,9 @@ static bool ParseNodeRequiresSpecialLineNumberNotes(ParseNode* pn) { } BytecodeEmitter::BytecodeSection::BytecodeSection(JSContext* cx) - : code_(cx), notes_(cx), tryNoteList_(cx) {} + : code_(cx), notes_(cx), tryNoteList_(cx), scopeNoteList_(cx) {} + +BytecodeEmitter::PerScriptData::PerScriptData(JSContext* cx) : scopeList_(cx) {} BytecodeEmitter::BytecodeEmitter( BytecodeEmitter* parent, SharedContext* sc, HandleScript script, @@ -104,13 +106,12 @@ BytecodeEmitter::BytecodeEmitter( script(cx, script), lazyScript(cx, lazyScript), bytecodeSection_(cx), + perScriptData_(cx), currentLine_(lineNum), fieldInitializers_(fieldInitializers), atomIndices(cx->frontendCollectionPool()), firstLine(lineNum), numberList(cx), - scopeList(cx), - scopeNoteList(cx), resumeOffsetList(cx), emitterMode(emitterMode) { MOZ_ASSERT_IF(emitterMode == LazyFunction, lazyScript); @@ -667,15 +668,16 @@ class NonLocalExitControl { public: NonLocalExitControl(BytecodeEmitter* bce, Kind kind) : bce_(bce), - savedScopeNoteIndex_(bce->scopeNoteList.length()), + savedScopeNoteIndex_(bce->bytecodeSection().scopeNoteList().length()), savedDepth_(bce->bytecodeSection().stackDepth()), openScopeNoteIndex_(bce->innermostEmitterScope()->noteIndex()), kind_(kind) {} ~NonLocalExitControl() { - for (uint32_t n = savedScopeNoteIndex_; n < bce_->scopeNoteList.length(); - n++) { - bce_->scopeNoteList.recordEnd(n, bce_->bytecodeSection().offset()); + for (uint32_t n = savedScopeNoteIndex_; + n < bce_->bytecodeSection().scopeNoteList().length(); n++) { + bce_->bytecodeSection().scopeNoteList().recordEnd( + n, bce_->bytecodeSection().offset()); } bce_->bytecodeSection().setStackDepth(savedDepth_); } @@ -699,12 +701,12 @@ bool NonLocalExitControl::leaveScope(EmitterScope* es) { if (es->enclosingInFrame()) { enclosingScopeIndex = es->enclosingInFrame()->index(); } - if (!bce_->scopeNoteList.append(enclosingScopeIndex, - bce_->bytecodeSection().offset(), - openScopeNoteIndex_)) { + if (!bce_->bytecodeSection().scopeNoteList().append( + enclosingScopeIndex, bce_->bytecodeSection().offset(), + openScopeNoteIndex_)) { return false; } - openScopeNoteIndex_ = bce_->scopeNoteList.length() - 1; + openScopeNoteIndex_ = bce_->bytecodeSection().scopeNoteList().length() - 1; return true; } @@ -945,7 +947,7 @@ bool BytecodeEmitter::emitAtomOp(uint32_t atomIndex, JSOp op) { bool BytecodeEmitter::emitInternedScopeOp(uint32_t index, JSOp op) { MOZ_ASSERT(JOF_OPTYPE(op) == JOF_SCOPE); - MOZ_ASSERT(index < scopeList.length()); + MOZ_ASSERT(index < perScriptData().scopeList().length()); return emitIndex32(op, index); } diff --git a/js/src/frontend/BytecodeEmitter.h b/js/src/frontend/BytecodeEmitter.h index e6e706b8a849..4c8b24ba9d09 100644 --- a/js/src/frontend/BytecodeEmitter.h +++ b/js/src/frontend/BytecodeEmitter.h @@ -178,6 +178,11 @@ struct MOZ_STACK_CLASS BytecodeEmitter { CGTryNoteList& tryNoteList() { return tryNoteList_; }; const CGTryNoteList& tryNoteList() const { return tryNoteList_; }; + // ---- Scope ---- + + CGScopeNoteList& scopeNoteList() { return scopeNoteList_; }; + const CGScopeNoteList& scopeNoteList() const { return scopeNoteList_; }; + private: // ---- Bytecode ---- @@ -209,6 +214,11 @@ struct MOZ_STACK_CLASS BytecodeEmitter { // List of emitted try notes. CGTryNoteList tryNoteList_; + + // ---- Scope ---- + + // List of emitted block scope notes. + CGScopeNoteList scopeNoteList_; }; BytecodeSection bytecodeSection_; @@ -217,6 +227,31 @@ struct MOZ_STACK_CLASS BytecodeEmitter { BytecodeSection& bytecodeSection() { return bytecodeSection_; } const BytecodeSection& bytecodeSection() const { return bytecodeSection_; } + private: + // Data that is not directly associated with specific opcode/index inside + // bytecode, but referred from bytecode is stored in this class. + class PerScriptData { + public: + explicit PerScriptData(JSContext* cx); + + // ---- Scope ---- + + CGScopeList& scopeList() { return scopeList_; } + const CGScopeList& scopeList() const { return scopeList_; } + + private: + // ---- Scope ---- + + // List of emitted scopes. + CGScopeList scopeList_; + }; + + PerScriptData perScriptData_; + + public: + PerScriptData& perScriptData() { return perScriptData_; } + const PerScriptData& perScriptData() const { return perScriptData_; } + private: // Line number for srcnotes. // @@ -276,10 +311,8 @@ struct MOZ_STACK_CLASS BytecodeEmitter { return innermostEmitterScope_; } - CGNumberList numberList; /* double and bigint values used by script */ - CGObjectList objectList; /* list of emitted objects */ - CGScopeList scopeList; /* list of emitted scopes */ - CGScopeNoteList scopeNoteList; /* list of emitted block scope notes */ + CGNumberList numberList; /* double and bigint values used by script */ + CGObjectList objectList; /* list of emitted objects */ // Certain ops (yield, await, gosub) have an entry in the script's // resumeOffsets list. This can be used to map from the op's resumeIndex to @@ -440,11 +473,13 @@ struct MOZ_STACK_CLASS BytecodeEmitter { varEmitterScope = emitterScope; } - Scope* outermostScope() const { return scopeList.vector[0]; } + Scope* outermostScope() const { + return perScriptData().scopeList().vector[0]; + } Scope* innermostScope() const; Scope* bodyScope() const { - MOZ_ASSERT(bodyScopeIndex < scopeList.length()); - return scopeList.vector[bodyScopeIndex]; + MOZ_ASSERT(bodyScopeIndex < perScriptData().scopeList().length()); + return perScriptData().scopeList().vector[bodyScopeIndex]; } MOZ_ALWAYS_INLINE diff --git a/js/src/frontend/EmitterScope.cpp b/js/src/frontend/EmitterScope.cpp index 322720c9411b..6f0bf4bb0ea4 100644 --- a/js/src/frontend/EmitterScope.cpp +++ b/js/src/frontend/EmitterScope.cpp @@ -342,8 +342,8 @@ bool EmitterScope::internScope(BytecodeEmitter* bce, ScopeCreator createScope) { return false; } hasEnvironment_ = scope->hasEnvironment(); - scopeIndex_ = bce->scopeList.length(); - return bce->scopeList.append(scope); + scopeIndex_ = bce->perScriptData().scopeList().length(); + return bce->perScriptData().scopeList().append(scope); } template @@ -351,18 +351,18 @@ bool EmitterScope::internBodyScope(BytecodeEmitter* bce, ScopeCreator createScope) { MOZ_ASSERT(bce->bodyScopeIndex == UINT32_MAX, "There can be only one body scope"); - bce->bodyScopeIndex = bce->scopeList.length(); + bce->bodyScopeIndex = bce->perScriptData().scopeList().length(); return internScope(bce, createScope); } bool EmitterScope::appendScopeNote(BytecodeEmitter* bce) { MOZ_ASSERT(ScopeKindIsInBody(scope(bce)->kind()) && enclosingInFrame(), "Scope notes are not needed for body-level scopes."); - noteIndex_ = bce->scopeNoteList.length(); - return bce->scopeNoteList.append(index(), bce->bytecodeSection().offset(), - enclosingInFrame() - ? enclosingInFrame()->noteIndex() - : ScopeNote::NoScopeNoteIndex); + noteIndex_ = bce->bytecodeSection().scopeNoteList().length(); + return bce->bytecodeSection().scopeNoteList().append( + index(), bce->bytecodeSection().offset(), + enclosingInFrame() ? enclosingInFrame()->noteIndex() + : ScopeNote::NoScopeNoteIndex); } bool EmitterScope::deadZoneFrameSlotRange(BytecodeEmitter* bce, @@ -1060,7 +1060,7 @@ bool EmitterScope::leave(BytecodeEmitter* bce, bool nonLocal) { uint32_t offset = kind == ScopeKind::FunctionBodyVar ? UINT32_MAX : bce->bytecodeSection().offset(); - bce->scopeNoteList.recordEnd(noteIndex_, offset); + bce->bytecodeSection().scopeNoteList().recordEnd(noteIndex_, offset); } } @@ -1068,7 +1068,7 @@ bool EmitterScope::leave(BytecodeEmitter* bce, bool nonLocal) { } Scope* EmitterScope::scope(const BytecodeEmitter* bce) const { - return bce->scopeList.vector[index()]; + return bce->perScriptData().scopeList().vector[index()]; } NameLocation EmitterScope::lookup(BytecodeEmitter* bce, JSAtom* name) { diff --git a/js/src/vm/JSScript.cpp b/js/src/vm/JSScript.cpp index 8fb49fe74622..927438c34d1a 100644 --- a/js/src/vm/JSScript.cpp +++ b/js/src/vm/JSScript.cpp @@ -3238,11 +3238,11 @@ PrivateScriptData* PrivateScriptData::new_(JSContext* cx, uint32_t nscopes, /* static */ bool PrivateScriptData::InitFromEmitter( JSContext* cx, js::HandleScript script, frontend::BytecodeEmitter* bce) { - uint32_t nscopes = bce->scopeList.length(); + uint32_t nscopes = bce->perScriptData().scopeList().length(); uint32_t nconsts = bce->numberList.length(); uint32_t nobjects = bce->objectList.length; uint32_t ntrynotes = bce->bytecodeSection().tryNoteList().length(); - uint32_t nscopenotes = bce->scopeNoteList.length(); + uint32_t nscopenotes = bce->bytecodeSection().scopeNoteList().length(); uint32_t nresumeoffsets = bce->resumeOffsetList.length(); // Create and initialize PrivateScriptData @@ -3254,7 +3254,7 @@ PrivateScriptData* PrivateScriptData::new_(JSContext* cx, uint32_t nscopes, js::PrivateScriptData* data = script->data_; if (nscopes) { - bce->scopeList.finish(data->scopes()); + bce->perScriptData().scopeList().finish(data->scopes()); } if (nconsts) { bce->numberList.finish(data->consts()); @@ -3266,7 +3266,7 @@ PrivateScriptData* PrivateScriptData::new_(JSContext* cx, uint32_t nscopes, bce->bytecodeSection().tryNoteList().finish(data->tryNotes()); } if (nscopenotes) { - bce->scopeNoteList.finish(data->scopeNotes()); + bce->bytecodeSection().scopeNoteList().finish(data->scopeNotes()); } if (nresumeoffsets) { bce->resumeOffsetList.finish(data->resumeOffsets()); From efab8f2a50b8534fff95d2e9898b7600fef49d6c Mon Sep 17 00:00:00 2001 From: Tooru Fujisawa Date: Tue, 2 Apr 2019 18:21:04 +0900 Subject: [PATCH 09/22] Bug 1535994 - Part 7: Move object and number to PerScriptData class. r=jorendorff Differential Revision: https://phabricator.services.mozilla.com/D25737 --- js/src/frontend/BytecodeEmitter.cpp | 23 ++++++++++++----------- js/src/frontend/BytecodeEmitter.h | 19 ++++++++++++++++--- js/src/frontend/FunctionEmitter.cpp | 2 +- js/src/vm/JSScript.cpp | 12 ++++++------ 4 files changed, 35 insertions(+), 21 deletions(-) diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index 55fab7c05989..25540fca607c 100644 --- a/js/src/frontend/BytecodeEmitter.cpp +++ b/js/src/frontend/BytecodeEmitter.cpp @@ -94,7 +94,8 @@ static bool ParseNodeRequiresSpecialLineNumberNotes(ParseNode* pn) { BytecodeEmitter::BytecodeSection::BytecodeSection(JSContext* cx) : code_(cx), notes_(cx), tryNoteList_(cx), scopeNoteList_(cx) {} -BytecodeEmitter::PerScriptData::PerScriptData(JSContext* cx) : scopeList_(cx) {} +BytecodeEmitter::PerScriptData::PerScriptData(JSContext* cx) + : scopeList_(cx), numberList_(cx) {} BytecodeEmitter::BytecodeEmitter( BytecodeEmitter* parent, SharedContext* sc, HandleScript script, @@ -111,7 +112,6 @@ BytecodeEmitter::BytecodeEmitter( fieldInitializers_(fieldInitializers), atomIndices(cx->frontendCollectionPool()), firstLine(lineNum), - numberList(cx), resumeOffsetList(cx), emitterMode(emitterMode) { MOZ_ASSERT_IF(emitterMode == LazyFunction, lazyScript); @@ -953,18 +953,18 @@ bool BytecodeEmitter::emitInternedScopeOp(uint32_t index, JSOp op) { bool BytecodeEmitter::emitInternedObjectOp(uint32_t index, JSOp op) { MOZ_ASSERT(JOF_OPTYPE(op) == JOF_OBJECT); - MOZ_ASSERT(index < objectList.length); + MOZ_ASSERT(index < perScriptData().objectList().length); return emitIndex32(op, index); } bool BytecodeEmitter::emitObjectOp(ObjectBox* objbox, JSOp op) { - return emitInternedObjectOp(objectList.add(objbox), op); + return emitInternedObjectOp(perScriptData().objectList().add(objbox), op); } bool BytecodeEmitter::emitObjectPairOp(ObjectBox* objbox1, ObjectBox* objbox2, JSOp op) { - uint32_t index = objectList.add(objbox1); - objectList.add(objbox2); + uint32_t index = perScriptData().objectList().add(objbox1); + perScriptData().objectList().add(objbox2); return emitInternedObjectOp(index, op); } @@ -1723,7 +1723,7 @@ bool BytecodeEmitter::iteratorResultShape(unsigned* shape) { return false; } - *shape = objectList.add(objbox); + *shape = perScriptData().objectList().add(objbox); return true; } @@ -4966,10 +4966,10 @@ bool BytecodeEmitter::emitCopyDataProperties(CopyOption option) { } bool BytecodeEmitter::emitBigIntOp(BigInt* bigint) { - if (!numberList.append(BigIntValue(bigint))) { + if (!perScriptData().numberList().append(BigIntValue(bigint))) { return false; } - return emitIndex32(JSOP_BIGINT, numberList.length() - 1); + return emitIndex32(JSOP_BIGINT, perScriptData().numberList().length() - 1); } bool BytecodeEmitter::emitIterator() { @@ -8190,7 +8190,7 @@ bool BytecodeEmitter::replaceNewInitWithNewObject(JSObject* obj, JSOP_NEWINIT_LENGTH == JSOP_NEWOBJECT_LENGTH, "newinit and newobject must have equal length to edit in-place"); - uint32_t index = objectList.add(objbox); + uint32_t index = perScriptData().objectList().add(objbox); jsbytecode* code = bytecodeSection().code(offset); MOZ_ASSERT(code[0] == JSOP_NEWINIT); @@ -9241,7 +9241,8 @@ bool BytecodeEmitter::emitTree( break; case ParseNodeKind::RegExpExpr: - if (!emitRegExp(objectList.add(pn->as().objbox()))) { + if (!emitRegExp(perScriptData().objectList().add( + pn->as().objbox()))) { return false; } break; diff --git a/js/src/frontend/BytecodeEmitter.h b/js/src/frontend/BytecodeEmitter.h index 4c8b24ba9d09..aafd7c7789cf 100644 --- a/js/src/frontend/BytecodeEmitter.h +++ b/js/src/frontend/BytecodeEmitter.h @@ -239,11 +239,27 @@ struct MOZ_STACK_CLASS BytecodeEmitter { CGScopeList& scopeList() { return scopeList_; } const CGScopeList& scopeList() const { return scopeList_; } + // ---- Literals ---- + + CGNumberList& numberList() { return numberList_; } + const CGNumberList& numberList() const { return numberList_; } + + CGObjectList& objectList() { return objectList_; } + const CGObjectList& objectList() const { return objectList_; } + private: // ---- Scope ---- // List of emitted scopes. CGScopeList scopeList_; + + // ---- Literals ---- + + // List of double and bigint values used by script. + CGNumberList numberList_; + + // List of emitted objects. + CGObjectList objectList_; }; PerScriptData perScriptData_; @@ -311,9 +327,6 @@ struct MOZ_STACK_CLASS BytecodeEmitter { return innermostEmitterScope_; } - CGNumberList numberList; /* double and bigint values used by script */ - CGObjectList objectList; /* list of emitted objects */ - // Certain ops (yield, await, gosub) have an entry in the script's // resumeOffsets list. This can be used to map from the op's resumeIndex to // the bytecode offset of the next pc. This indirection makes it easy to diff --git a/js/src/frontend/FunctionEmitter.cpp b/js/src/frontend/FunctionEmitter.cpp index 0aefbdb94854..ec1f69486cf4 100644 --- a/js/src/frontend/FunctionEmitter.cpp +++ b/js/src/frontend/FunctionEmitter.cpp @@ -220,7 +220,7 @@ bool FunctionEmitter::emitAsmJSModule() { bool FunctionEmitter::emitFunction() { // Make the function object a literal in the outer script's pool. - unsigned index = bce_->objectList.add(funbox_); + unsigned index = bce_->perScriptData().objectList().add(funbox_); // [stack] diff --git a/js/src/vm/JSScript.cpp b/js/src/vm/JSScript.cpp index 927438c34d1a..affb3a2aa146 100644 --- a/js/src/vm/JSScript.cpp +++ b/js/src/vm/JSScript.cpp @@ -3239,8 +3239,8 @@ PrivateScriptData* PrivateScriptData::new_(JSContext* cx, uint32_t nscopes, /* static */ bool PrivateScriptData::InitFromEmitter( JSContext* cx, js::HandleScript script, frontend::BytecodeEmitter* bce) { uint32_t nscopes = bce->perScriptData().scopeList().length(); - uint32_t nconsts = bce->numberList.length(); - uint32_t nobjects = bce->objectList.length; + uint32_t nconsts = bce->perScriptData().numberList().length(); + uint32_t nobjects = bce->perScriptData().objectList().length; uint32_t ntrynotes = bce->bytecodeSection().tryNoteList().length(); uint32_t nscopenotes = bce->bytecodeSection().scopeNoteList().length(); uint32_t nresumeoffsets = bce->resumeOffsetList.length(); @@ -3257,10 +3257,10 @@ PrivateScriptData* PrivateScriptData::new_(JSContext* cx, uint32_t nscopes, bce->perScriptData().scopeList().finish(data->scopes()); } if (nconsts) { - bce->numberList.finish(data->consts()); + bce->perScriptData().numberList().finish(data->consts()); } if (nobjects) { - bce->objectList.finish(data->objects()); + bce->perScriptData().objectList().finish(data->objects()); } if (ntrynotes) { bce->bytecodeSection().tryNoteList().finish(data->tryNotes()); @@ -3566,7 +3566,7 @@ bool JSScript::fullyInitFromEmitter(JSContext* cx, HandleScript script, /* The counts of indexed things must be checked during code generation. */ MOZ_ASSERT(bce->atomIndices->count() <= INDEX_LIMIT); - MOZ_ASSERT(bce->objectList.length <= INDEX_LIMIT); + MOZ_ASSERT(bce->perScriptData().objectList().length <= INDEX_LIMIT); uint64_t nslots = bce->maxFixedSlots + @@ -3629,7 +3629,7 @@ bool JSScript::fullyInitFromEmitter(JSContext* cx, HandleScript script, // Part of the parse result – the scope containing each inner function – must // be stored in the inner function itself. Do this now that compilation is // complete and can no longer fail. - bce->objectList.finishInnerFunctions(); + bce->perScriptData().objectList().finishInnerFunctions(); #ifdef JS_STRUCTURED_SPEW // We want this to happen after line number initialization to allow filtering From 6304e9084e353ea98dafc6d4a4ada967029f4542 Mon Sep 17 00:00:00 2001 From: Tooru Fujisawa Date: Tue, 2 Apr 2019 18:21:06 +0900 Subject: [PATCH 10/22] Bug 1535994 - Part 8: Move resume/yield info to BytecodeSection class. r=jorendorff Differential Revision: https://phabricator.services.mozilla.com/D25738 --- js/src/frontend/BytecodeEmitter.cpp | 13 ++++++----- js/src/frontend/BytecodeEmitter.h | 32 +++++++++++++++++++--------- js/src/frontend/ForOfLoopControl.cpp | 4 ++-- js/src/vm/JSScript.cpp | 4 ++-- 4 files changed, 34 insertions(+), 19 deletions(-) diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index 25540fca607c..0c090e8930a8 100644 --- a/js/src/frontend/BytecodeEmitter.cpp +++ b/js/src/frontend/BytecodeEmitter.cpp @@ -92,7 +92,11 @@ static bool ParseNodeRequiresSpecialLineNumberNotes(ParseNode* pn) { } BytecodeEmitter::BytecodeSection::BytecodeSection(JSContext* cx) - : code_(cx), notes_(cx), tryNoteList_(cx), scopeNoteList_(cx) {} + : code_(cx), + notes_(cx), + tryNoteList_(cx), + scopeNoteList_(cx), + resumeOffsetList_(cx) {} BytecodeEmitter::PerScriptData::PerScriptData(JSContext* cx) : scopeList_(cx), numberList_(cx) {} @@ -112,7 +116,6 @@ BytecodeEmitter::BytecodeEmitter( fieldInitializers_(fieldInitializers), atomIndices(cx->frontendCollectionPool()), firstLine(lineNum), - resumeOffsetList(cx), emitterMode(emitterMode) { MOZ_ASSERT_IF(emitterMode == LazyFunction, lazyScript); @@ -2261,13 +2264,13 @@ bool BytecodeEmitter::allocateResumeIndex(ptrdiff_t offset, "resumeIndex * sizeof(uintptr_t) must fit in an int32. JIT code relies " "on this when loading resume entries from BaselineScript"); - *resumeIndex = resumeOffsetList.length(); + *resumeIndex = bytecodeSection().resumeOffsetList().length(); if (*resumeIndex > MaxResumeIndex) { reportError(nullptr, JSMSG_TOO_MANY_RESUME_INDEXES); return false; } - return resumeOffsetList.append(offset); + return bytecodeSection().resumeOffsetList().append(offset); } bool BytecodeEmitter::allocateResumeIndexRange(mozilla::Span offsets, @@ -2300,7 +2303,7 @@ bool BytecodeEmitter::emitYieldOp(JSOp op) { } if (op == JSOP_INITIALYIELD || op == JSOP_YIELD) { - numYields++; + bytecodeSection().addNumYields(); } uint32_t resumeIndex; diff --git a/js/src/frontend/BytecodeEmitter.h b/js/src/frontend/BytecodeEmitter.h index aafd7c7789cf..2739c9f8291f 100644 --- a/js/src/frontend/BytecodeEmitter.h +++ b/js/src/frontend/BytecodeEmitter.h @@ -183,6 +183,16 @@ struct MOZ_STACK_CLASS BytecodeEmitter { CGScopeNoteList& scopeNoteList() { return scopeNoteList_; }; const CGScopeNoteList& scopeNoteList() const { return scopeNoteList_; }; + // ---- Generator ---- + + CGResumeOffsetList& resumeOffsetList() { return resumeOffsetList_; } + const CGResumeOffsetList& resumeOffsetList() const { + return resumeOffsetList_; + } + + uint32_t numYields() const { return numYields_; } + void addNumYields() { numYields_++; } + private: // ---- Bytecode ---- @@ -219,6 +229,18 @@ struct MOZ_STACK_CLASS BytecodeEmitter { // List of emitted block scope notes. CGScopeNoteList scopeNoteList_; + + // ---- Generator ---- + + // Certain ops (yield, await, gosub) have an entry in the script's + // resumeOffsets list. This can be used to map from the op's resumeIndex to + // the bytecode offset of the next pc. This indirection makes it easy to + // resume in the JIT (because BaselineScript stores a resumeIndex => native + // code array). + CGResumeOffsetList resumeOffsetList_; + + // Number of yield instructions emitted. Does not include JSOP_AWAIT. + uint32_t numYields_ = 0; }; BytecodeSection bytecodeSection_; @@ -327,19 +349,9 @@ struct MOZ_STACK_CLASS BytecodeEmitter { return innermostEmitterScope_; } - // Certain ops (yield, await, gosub) have an entry in the script's - // resumeOffsets list. This can be used to map from the op's resumeIndex to - // the bytecode offset of the next pc. This indirection makes it easy to - // resume in the JIT (because BaselineScript stores a resumeIndex => native - // code array). - CGResumeOffsetList resumeOffsetList; - // Number of JOF_IC opcodes emitted. size_t numICEntries = 0; - // Number of yield instructions emitted. Does not include JSOP_AWAIT. - uint32_t numYields = 0; - // Number of JOF_TYPESET opcodes generated. uint16_t typesetCount = 0; diff --git a/js/src/frontend/ForOfLoopControl.cpp b/js/src/frontend/ForOfLoopControl.cpp index ec8bf910eca9..473242e8d875 100644 --- a/js/src/frontend/ForOfLoopControl.cpp +++ b/js/src/frontend/ForOfLoopControl.cpp @@ -30,7 +30,7 @@ bool ForOfLoopControl::emitBeginCodeNeedingIteratorClose(BytecodeEmitter* bce) { } MOZ_ASSERT(numYieldsAtBeginCodeNeedingIterClose_ == UINT32_MAX); - numYieldsAtBeginCodeNeedingIterClose_ = bce->numYields; + numYieldsAtBeginCodeNeedingIterClose_ = bce->bytecodeSection().numYields(); return true; } @@ -93,7 +93,7 @@ bool ForOfLoopControl::emitEndCodeNeedingIteratorClose(BytecodeEmitter* bce) { // If any yields were emitted, then this for-of loop is inside a star // generator and must handle the case of Generator.return. Like in // yield*, it is handled with a finally block. - uint32_t numYieldsEmitted = bce->numYields; + uint32_t numYieldsEmitted = bce->bytecodeSection().numYields(); if (numYieldsEmitted > numYieldsAtBeginCodeNeedingIterClose_) { if (!tryCatch_->emitFinally()) { return false; diff --git a/js/src/vm/JSScript.cpp b/js/src/vm/JSScript.cpp index affb3a2aa146..7fac708b2d58 100644 --- a/js/src/vm/JSScript.cpp +++ b/js/src/vm/JSScript.cpp @@ -3243,7 +3243,7 @@ PrivateScriptData* PrivateScriptData::new_(JSContext* cx, uint32_t nscopes, uint32_t nobjects = bce->perScriptData().objectList().length; uint32_t ntrynotes = bce->bytecodeSection().tryNoteList().length(); uint32_t nscopenotes = bce->bytecodeSection().scopeNoteList().length(); - uint32_t nresumeoffsets = bce->resumeOffsetList.length(); + uint32_t nresumeoffsets = bce->bytecodeSection().resumeOffsetList().length(); // Create and initialize PrivateScriptData if (!JSScript::createPrivateScriptData(cx, script, nscopes, nconsts, nobjects, @@ -3269,7 +3269,7 @@ PrivateScriptData* PrivateScriptData::new_(JSContext* cx, uint32_t nscopes, bce->bytecodeSection().scopeNoteList().finish(data->scopeNotes()); } if (nresumeoffsets) { - bce->resumeOffsetList.finish(data->resumeOffsets()); + bce->bytecodeSection().resumeOffsetList().finish(data->resumeOffsets()); } return true; From 3770586f7f058242ee09d31765e68be439b0a673 Mon Sep 17 00:00:00 2001 From: Tooru Fujisawa Date: Tue, 2 Apr 2019 18:21:07 +0900 Subject: [PATCH 11/22] Bug 1535994 - Part 9: Move atomIndices to PerScriptData class. r=jorendorff Differential Revision: https://phabricator.services.mozilla.com/D25739 --- js/src/frontend/BytecodeEmitter.cpp | 11 ++++++++--- js/src/frontend/BytecodeEmitter.h | 19 ++++++++++++++----- js/src/vm/JSScript.cpp | 6 +++--- 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index 0c090e8930a8..c4d0ca2fa7a7 100644 --- a/js/src/frontend/BytecodeEmitter.cpp +++ b/js/src/frontend/BytecodeEmitter.cpp @@ -99,7 +99,13 @@ BytecodeEmitter::BytecodeSection::BytecodeSection(JSContext* cx) resumeOffsetList_(cx) {} BytecodeEmitter::PerScriptData::PerScriptData(JSContext* cx) - : scopeList_(cx), numberList_(cx) {} + : scopeList_(cx), + numberList_(cx), + atomIndices_(cx->frontendCollectionPool()) {} + +bool BytecodeEmitter::PerScriptData::init(JSContext* cx) { + return atomIndices_.acquire(cx); +} BytecodeEmitter::BytecodeEmitter( BytecodeEmitter* parent, SharedContext* sc, HandleScript script, @@ -114,7 +120,6 @@ BytecodeEmitter::BytecodeEmitter( perScriptData_(cx), currentLine_(lineNum), fieldInitializers_(fieldInitializers), - atomIndices(cx->frontendCollectionPool()), firstLine(lineNum), emitterMode(emitterMode) { MOZ_ASSERT_IF(emitterMode == LazyFunction, lazyScript); @@ -153,7 +158,7 @@ void BytecodeEmitter::initFromBodyPosition(TokenPos bodyPosition) { setFunctionBodyEndPos(bodyPosition.end); } -bool BytecodeEmitter::init() { return atomIndices.acquire(cx); } +bool BytecodeEmitter::init() { return perScriptData_.init(cx); } template T* BytecodeEmitter::findInnermostNestableControl() const { diff --git a/js/src/frontend/BytecodeEmitter.h b/js/src/frontend/BytecodeEmitter.h index 2739c9f8291f..02d07563a0cc 100644 --- a/js/src/frontend/BytecodeEmitter.h +++ b/js/src/frontend/BytecodeEmitter.h @@ -256,6 +256,8 @@ struct MOZ_STACK_CLASS BytecodeEmitter { public: explicit PerScriptData(JSContext* cx); + MOZ_MUST_USE bool init(JSContext* cx); + // ---- Scope ---- CGScopeList& scopeList() { return scopeList_; } @@ -269,6 +271,11 @@ struct MOZ_STACK_CLASS BytecodeEmitter { CGObjectList& objectList() { return objectList_; } const CGObjectList& objectList() const { return objectList_; } + PooledMapPtr& atomIndices() { return atomIndices_; } + const PooledMapPtr& atomIndices() const { + return atomIndices_; + } + private: // ---- Scope ---- @@ -282,6 +289,9 @@ struct MOZ_STACK_CLASS BytecodeEmitter { // List of emitted objects. CGObjectList objectList_; + + // Map from atom to index. + PooledMapPtr atomIndices_; }; PerScriptData perScriptData_; @@ -320,7 +330,6 @@ struct MOZ_STACK_CLASS BytecodeEmitter { mozilla::Maybe ep_ = {}; BCEParserHandle* parser = nullptr; - PooledMapPtr atomIndices; /* literals indexed for mapping */ unsigned firstLine = 0; /* first line, for JSScript::initFromEmitter */ uint32_t maxFixedSlots = 0; /* maximum number of fixed frame slots so far */ @@ -509,15 +518,15 @@ struct MOZ_STACK_CLASS BytecodeEmitter { MOZ_ALWAYS_INLINE MOZ_MUST_USE bool makeAtomIndex(JSAtom* atom, uint32_t* indexp) { - MOZ_ASSERT(atomIndices); - AtomIndexMap::AddPtr p = atomIndices->lookupForAdd(atom); + MOZ_ASSERT(perScriptData().atomIndices()); + AtomIndexMap::AddPtr p = perScriptData().atomIndices()->lookupForAdd(atom); if (p) { *indexp = p->value(); return true; } - uint32_t index = atomIndices->count(); - if (!atomIndices->add(p, atom, index)) { + uint32_t index = perScriptData().atomIndices()->count(); + if (!perScriptData().atomIndices()->add(p, atom, index)) { ReportOutOfMemory(cx); return false; } diff --git a/js/src/vm/JSScript.cpp b/js/src/vm/JSScript.cpp index 7fac708b2d58..102fb81b5da9 100644 --- a/js/src/vm/JSScript.cpp +++ b/js/src/vm/JSScript.cpp @@ -3565,7 +3565,7 @@ bool JSScript::fullyInitFromEmitter(JSContext* cx, HandleScript script, mozilla::MakeScopeExit([&] { script->freeScriptData(); }); /* The counts of indexed things must be checked during code generation. */ - MOZ_ASSERT(bce->atomIndices->count() <= INDEX_LIMIT); + MOZ_ASSERT(bce->perScriptData().atomIndices()->count() <= INDEX_LIMIT); MOZ_ASSERT(bce->perScriptData().objectList().length <= INDEX_LIMIT); uint64_t nslots = @@ -4523,7 +4523,7 @@ bool JSScript::hasBreakpointsAt(jsbytecode* pc) { /* static */ bool SharedScriptData::InitFromEmitter( JSContext* cx, js::HandleScript script, frontend::BytecodeEmitter* bce) { - uint32_t natoms = bce->atomIndices->count(); + uint32_t natoms = bce->perScriptData().atomIndices()->count(); uint32_t codeLength = bce->bytecodeSection().code().length(); // The + 1 is to account for the final SN_MAKE_TERMINATOR that is appended @@ -4540,7 +4540,7 @@ bool JSScript::hasBreakpointsAt(jsbytecode* pc) { // Initialize trailing arrays std::copy_n(bce->bytecodeSection().code().begin(), codeLength, data->code()); bce->copySrcNotes(data->notes(), noteLength); - InitAtomMap(*bce->atomIndices, data->atoms()); + InitAtomMap(*bce->perScriptData().atomIndices(), data->atoms()); return true; } From 5e196c58f35c564bfc7b0e66f05100168ede9966 Mon Sep 17 00:00:00 2001 From: Tooru Fujisawa Date: Tue, 2 Apr 2019 18:21:07 +0900 Subject: [PATCH 12/22] Bug 1535994 - Part 10: Move JIT information to BytecodeSection class. r=jorendorff Differential Revision: https://phabricator.services.mozilla.com/D25740 --- js/src/frontend/BytecodeEmitter.cpp | 10 +++++----- js/src/frontend/BytecodeEmitter.h | 23 +++++++++++++++++------ js/src/vm/JSScript.cpp | 2 +- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index c4d0ca2fa7a7..51393a167276 100644 --- a/js/src/frontend/BytecodeEmitter.cpp +++ b/js/src/frontend/BytecodeEmitter.cpp @@ -126,7 +126,7 @@ BytecodeEmitter::BytecodeEmitter( if (sc->isFunctionBox()) { // Functions have IC entries for type monitoring |this| and arguments. - numICEntries = sc->asFunctionBox()->function()->nargs() + 1; + bytecodeSection().setNumICEntries(sc->asFunctionBox()->function()->nargs() + 1); } } @@ -243,13 +243,13 @@ bool BytecodeEmitter::emitCheck(JSOp op, ptrdiff_t delta, ptrdiff_t* offset) { // If op is JOF_TYPESET (see the type barriers comment in TypeInference.h), // reserve a type set to store its result. if (CodeSpec[op].format & JOF_TYPESET) { - if (typesetCount < JSScript::MaxBytecodeTypeSets) { - typesetCount++; + if (bytecodeSection().typesetCount() < JSScript::MaxBytecodeTypeSets) { + bytecodeSection().addTypesetCount(); } } if (BytecodeOpHasIC(op)) { - numICEntries++; + bytecodeSection().addNumICEntries(); } return true; @@ -361,7 +361,7 @@ bool BytecodeEmitter::emitN(JSOp op, size_t extra, ptrdiff_t* offset) { bool BytecodeEmitter::emitJumpTargetOp(JSOp op, ptrdiff_t* off) { MOZ_ASSERT(BytecodeIsJumpTarget(op)); - size_t numEntries = numICEntries; + size_t numEntries = bytecodeSection().numICEntries(); if (MOZ_UNLIKELY(numEntries > UINT32_MAX)) { reportError(nullptr, JSMSG_NEED_DIET, js_script_str); return false; diff --git a/js/src/frontend/BytecodeEmitter.h b/js/src/frontend/BytecodeEmitter.h index 02d07563a0cc..6d71370a8568 100644 --- a/js/src/frontend/BytecodeEmitter.h +++ b/js/src/frontend/BytecodeEmitter.h @@ -193,6 +193,15 @@ struct MOZ_STACK_CLASS BytecodeEmitter { uint32_t numYields() const { return numYields_; } void addNumYields() { numYields_++; } + // ---- JIT ---- + + size_t numICEntries() const { return numICEntries_; } + void addNumICEntries() { numICEntries_++; } + void setNumICEntries(size_t entries) { numICEntries_ = entries; } + + uint16_t typesetCount() const { return typesetCount_; } + void addTypesetCount() { typesetCount_++; } + private: // ---- Bytecode ---- @@ -241,6 +250,14 @@ struct MOZ_STACK_CLASS BytecodeEmitter { // Number of yield instructions emitted. Does not include JSOP_AWAIT. uint32_t numYields_ = 0; + + // ---- JIT ---- + + // Number of JOF_IC opcodes emitted. + size_t numICEntries_ = 0; + + // Number of JOF_TYPESET opcodes generated. + uint16_t typesetCount_ = 0; }; BytecodeSection bytecodeSection_; @@ -358,12 +375,6 @@ struct MOZ_STACK_CLASS BytecodeEmitter { return innermostEmitterScope_; } - // Number of JOF_IC opcodes emitted. - size_t numICEntries = 0; - - // Number of JOF_TYPESET opcodes generated. - uint16_t typesetCount = 0; - // Script contains singleton initializer JSOP_OBJECT. bool hasSingletons = false; diff --git a/js/src/vm/JSScript.cpp b/js/src/vm/JSScript.cpp index 102fb81b5da9..18a74af6a27c 100644 --- a/js/src/vm/JSScript.cpp +++ b/js/src/vm/JSScript.cpp @@ -3582,7 +3582,7 @@ bool JSScript::fullyInitFromEmitter(JSContext* cx, HandleScript script, script->nfixed_ = bce->maxFixedSlots; script->nslots_ = nslots; script->bodyScopeIndex_ = bce->bodyScopeIndex; - script->numBytecodeTypeSets_ = bce->typesetCount; + script->numBytecodeTypeSets_ = bce->bytecodeSection().typesetCount(); // Initialize script flags from BytecodeEmitter script->setFlag(ImmutableFlags::Strict, bce->sc->strict()); From a398acec16de5e6d815cc2cc6156b66c6a24ef56 Mon Sep 17 00:00:00 2001 From: Tooru Fujisawa Date: Tue, 2 Apr 2019 18:21:07 +0900 Subject: [PATCH 13/22] Bug 1535994 - Part 11: Move line/colum information to BytecodeSection class. r=jorendorff Differential Revision: https://phabricator.services.mozilla.com/D25741 --- js/src/frontend/BytecodeEmitter.cpp | 45 +++++++---------- js/src/frontend/BytecodeEmitter.h | 76 +++++++++++++++++++---------- js/src/frontend/CForEmitter.cpp | 4 +- 3 files changed, 69 insertions(+), 56 deletions(-) diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index 51393a167276..af1e9316b6b9 100644 --- a/js/src/frontend/BytecodeEmitter.cpp +++ b/js/src/frontend/BytecodeEmitter.cpp @@ -91,12 +91,14 @@ static bool ParseNodeRequiresSpecialLineNumberNotes(ParseNode* pn) { kind == ParseNodeKind::Function; } -BytecodeEmitter::BytecodeSection::BytecodeSection(JSContext* cx) +BytecodeEmitter::BytecodeSection::BytecodeSection(JSContext* cx, + uint32_t lineNum) : code_(cx), notes_(cx), tryNoteList_(cx), scopeNoteList_(cx), - resumeOffsetList_(cx) {} + resumeOffsetList_(cx), + currentLine_(lineNum) {} BytecodeEmitter::PerScriptData::PerScriptData(JSContext* cx) : scopeList_(cx), @@ -116,9 +118,8 @@ BytecodeEmitter::BytecodeEmitter( parent(parent), script(cx, script), lazyScript(cx, lazyScript), - bytecodeSection_(cx), + bytecodeSection_(cx, lineNum), perScriptData_(cx), - currentLine_(lineNum), fieldInitializers_(fieldInitializers), firstLine(lineNum), emitterMode(emitterMode) { @@ -126,7 +127,8 @@ BytecodeEmitter::BytecodeEmitter( if (sc->isFunctionBox()) { // Functions have IC entries for type monitoring |this| and arguments. - bytecodeSection().setNumICEntries(sc->asFunctionBox()->function()->nargs() + 1); + bytecodeSection().setNumICEntries(sc->asFunctionBox()->function()->nargs() + + 1); } } @@ -204,9 +206,7 @@ bool BytecodeEmitter::markStepBreakpoint() { // We track the location of the most recent separator for use in // markSimpleBreakpoint. Note that this means that the position must already // be set before markStepBreakpoint is called. - lastSeparatorOffet_ = bytecodeSection().code().length(); - lastSeparatorLine_ = currentLine_; - lastSeparatorColumn_ = lastColumn_; + bytecodeSection().updateSeparatorPosition(); return true; } @@ -220,10 +220,7 @@ bool BytecodeEmitter::markSimpleBreakpoint() { // expression start, we need to skip marking it breakable in order to avoid // having two breakpoints with the same line/column position. // Note: This assumes that the position for the call has already been set. - bool isDuplicateLocation = - lastSeparatorLine_ == currentLine_ && lastSeparatorColumn_ == lastColumn_; - - if (!isDuplicateLocation) { + if (!bytecodeSection().isDuplicateLocation()) { if (!newSrcNote(SRC_BREAKPOINT)) { return false; } @@ -528,14 +525,14 @@ bool BytecodeEmitter::updateLineNumberNotes(uint32_t offset) { ErrorReporter* er = &parser->errorReporter(); bool onThisLine; - if (!er->isOnThisLine(offset, currentLine(), &onThisLine)) { + if (!er->isOnThisLine(offset, bytecodeSection().currentLine(), &onThisLine)) { er->errorNoOffset(JSMSG_OUT_OF_MEMORY); return false; } if (!onThisLine) { unsigned line = er->lineAt(offset); - unsigned delta = line - currentLine(); + unsigned delta = line - bytecodeSection().currentLine(); /* * Encode any change in the current source line number by using @@ -548,7 +545,7 @@ bool BytecodeEmitter::updateLineNumberNotes(uint32_t offset) { * unsigned delta_ wrap to a very large number, which triggers a * SRC_SETLINE. */ - setCurrentLine(line); + bytecodeSection().setCurrentLine(line); if (delta >= LengthOfSetLine(line)) { if (!newSrcNote2(SRC_SETLINE, ptrdiff_t(line))) { return false; @@ -561,7 +558,7 @@ bool BytecodeEmitter::updateLineNumberNotes(uint32_t offset) { } while (--delta != 0); } - updateSeparatorPosition(); + bytecodeSection().updateSeparatorPositionIfPresent(); } return true; } @@ -578,7 +575,8 @@ bool BytecodeEmitter::updateSourceCoordNotes(uint32_t offset) { } uint32_t columnIndex = parser->errorReporter().columnAt(offset); - ptrdiff_t colspan = ptrdiff_t(columnIndex) - ptrdiff_t(lastColumn_); + ptrdiff_t colspan = + ptrdiff_t(columnIndex) - ptrdiff_t(bytecodeSection().lastColumn()); if (colspan != 0) { // If the column span is so large that we can't store it, then just // discard this information. This can happen with minimized or otherwise @@ -591,21 +589,12 @@ bool BytecodeEmitter::updateSourceCoordNotes(uint32_t offset) { if (!newSrcNote2(SRC_COLSPAN, SN_COLSPAN_TO_OFFSET(colspan))) { return false; } - lastColumn_ = columnIndex; - updateSeparatorPosition(); + bytecodeSection().setLastColumn(columnIndex); + bytecodeSection().updateSeparatorPositionIfPresent(); } return true; } -/* Updates the last separator position, if present */ -void BytecodeEmitter::updateSeparatorPosition() { - if (!inPrologue() && - lastSeparatorOffet_ == bytecodeSection().code().length()) { - lastSeparatorLine_ = currentLine_; - lastSeparatorColumn_ = lastColumn_; - } -} - Maybe BytecodeEmitter::getOffsetForLoop(ParseNode* nextpn) { if (!nextpn) { return Nothing(); diff --git a/js/src/frontend/BytecodeEmitter.h b/js/src/frontend/BytecodeEmitter.h index 6d71370a8568..83cd4a7fb6dd 100644 --- a/js/src/frontend/BytecodeEmitter.h +++ b/js/src/frontend/BytecodeEmitter.h @@ -128,7 +128,7 @@ struct MOZ_STACK_CLASS BytecodeEmitter { // bytecode is stored in this class. class BytecodeSection { public: - explicit BytecodeSection(JSContext* cx); + BytecodeSection(JSContext* cx, uint32_t lineNum); // ---- Bytecode ---- @@ -193,6 +193,34 @@ struct MOZ_STACK_CLASS BytecodeEmitter { uint32_t numYields() const { return numYields_; } void addNumYields() { numYields_++; } + // ---- Line and column ---- + + uint32_t currentLine() const { return currentLine_; } + uint32_t lastColumn() const { return lastColumn_; } + void setCurrentLine(uint32_t line) { + currentLine_ = line; + lastColumn_ = 0; + } + void setLastColumn(uint32_t column) { lastColumn_ = column; } + + void updateSeparatorPosition() { + lastSeparatorOffet_ = code().length(); + lastSeparatorLine_ = currentLine_; + lastSeparatorColumn_ = lastColumn_; + } + + void updateSeparatorPositionIfPresent() { + if (lastSeparatorOffet_ == code().length()) { + lastSeparatorLine_ = currentLine_; + lastSeparatorColumn_ = lastColumn_; + } + } + + bool isDuplicateLocation() const { + return lastSeparatorLine_ == currentLine_ && + lastSeparatorColumn_ == lastColumn_; + } + // ---- JIT ---- size_t numICEntries() const { return numICEntries_; } @@ -251,6 +279,27 @@ struct MOZ_STACK_CLASS BytecodeEmitter { // Number of yield instructions emitted. Does not include JSOP_AWAIT. uint32_t numYields_ = 0; + // ---- Line and column ---- + + // Line number for srcnotes. + // + // WARNING: If this becomes out of sync with already-emitted srcnotes, + // we can get undefined behavior. + uint32_t currentLine_; + + // Zero-based column index on currentLine_ of last SRC_COLSPAN-annotated + // opcode. + // + // WARNING: If this becomes out of sync with already-emitted srcnotes, + // we can get undefined behavior. + uint32_t lastColumn_ = 0; + + // The offset, line and column numbers of the last opcode for the + // breakpoint for step execution. + uint32_t lastSeparatorOffet_ = 0; + uint32_t lastSeparatorLine_ = 0; + uint32_t lastSeparatorColumn_ = 0; + // ---- JIT ---- // Number of JOF_IC opcodes emitted. @@ -318,23 +367,6 @@ struct MOZ_STACK_CLASS BytecodeEmitter { const PerScriptData& perScriptData() const { return perScriptData_; } private: - // Line number for srcnotes. - // - // WARNING: If this becomes out of sync with already-emitted srcnotes, - // we can get undefined behavior. - uint32_t currentLine_ = 0; - - // Zero-based column index on currentLine of last SRC_COLSPAN-annotated - // opcode. - // - // WARNING: If this becomes out of sync with already-emitted srcnotes, - // we can get undefined behavior. - uint32_t lastColumn_ = 0; - - uint32_t lastSeparatorOffet_ = 0; - uint32_t lastSeparatorLine_ = 0; - uint32_t lastSeparatorColumn_ = 0; - // switchToMain sets this to the bytecode offset of the main section. mozilla::Maybe mainOffset_ = {}; @@ -569,13 +601,6 @@ struct MOZ_STACK_CLASS BytecodeEmitter { mainOffset_.emplace(bytecodeSection().code().length()); } - unsigned currentLine() const { return currentLine_; } - - void setCurrentLine(uint32_t line) { - currentLine_ = line; - lastColumn_ = 0; - } - void setFunctionBodyEndPos(uint32_t pos) { functionBodyEndPos = mozilla::Some(pos); } @@ -648,7 +673,6 @@ struct MOZ_STACK_CLASS BytecodeEmitter { MOZ_MUST_USE bool markSimpleBreakpoint(); MOZ_MUST_USE bool updateLineNumberNotes(uint32_t offset); MOZ_MUST_USE bool updateSourceCoordNotes(uint32_t offset); - void updateSeparatorPosition(); JSOp strictifySetNameOp(JSOp op); diff --git a/js/src/frontend/CForEmitter.cpp b/js/src/frontend/CForEmitter.cpp index 7a397034c45c..fc6dce61297b 100644 --- a/js/src/frontend/CForEmitter.cpp +++ b/js/src/frontend/CForEmitter.cpp @@ -163,11 +163,11 @@ bool CForEmitter::emitCond(const Maybe& forPos, // Restore the absolute line number for source note readers. if (endPos) { uint32_t lineNum = bce_->parser->errorReporter().lineAt(*endPos); - if (bce_->currentLine() != lineNum) { + if (bce_->bytecodeSection().currentLine() != lineNum) { if (!bce_->newSrcNote2(SRC_SETLINE, ptrdiff_t(lineNum))) { return false; } - bce_->setCurrentLine(lineNum); + bce_->bytecodeSection().setCurrentLine(lineNum); } } } From fe94ecef81b89ff5113e04517deff65485ec28b5 Mon Sep 17 00:00:00 2001 From: Jon Coppeard Date: Tue, 9 Apr 2019 09:11:11 +0100 Subject: [PATCH 14/22] Bug 1543017 - Don't update test progress bar so often r=sfink Don't update the progress bar more than necessary by recording the last update time and checking in poke(). Network traffic is down to 12KB/s with this patch. --- js/src/tests/lib/progressbar.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/js/src/tests/lib/progressbar.py b/js/src/tests/lib/progressbar.py index 2ba90c4930fe..5faa2e19fefa 100644 --- a/js/src/tests/lib/progressbar.py +++ b/js/src/tests/lib/progressbar.py @@ -39,6 +39,8 @@ class ProgressBar(object): self.limit_digits = int(math.ceil(math.log10(self.limit))) # datetime: The start time. self.t0 = datetime.now() + # datetime: Optional, the last time update() ran. + self.last_update_time = None # Compute the width of the counters and build the format string. self.counters_width = 1 # [ @@ -79,7 +81,8 @@ class ProgressBar(object): sys.stdout.write(bar + '|') # Update the bar. - dt = datetime.now() - self.t0 + now = datetime.now() + dt = now - self.t0 dt = dt.seconds + dt.microseconds * 1e-6 sys.stdout.write('{:6.1f}s'.format(dt)) Terminal.clear_right() @@ -87,9 +90,13 @@ class ProgressBar(object): # Force redisplay, since we didn't write a \n. sys.stdout.flush() + self.last_update_time = now + def poke(self): if not self.prior: return + if datetime.now() - self.last_update_time < self.update_granularity(): + return self.update(*self.prior) def finish(self, complete=True): From 4855c6bd1825f6a5803331441a59a81f1734f54b Mon Sep 17 00:00:00 2001 From: Jon Coppeard Date: Tue, 9 Apr 2019 11:56:21 +0100 Subject: [PATCH 15/22] Bug 1543014 - Skip checking weakmap value marking for values owned by other runtimes r=sfink Differential Revision: https://phabricator.services.mozilla.com/D26691 --- js/src/gc/Verifier.cpp | 10 ++++++++-- js/src/jit-test/tests/gc/bug-1543014.js | 11 +++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 js/src/jit-test/tests/gc/bug-1543014.js diff --git a/js/src/gc/Verifier.cpp b/js/src/gc/Verifier.cpp index fd8f6eed2302..0d54c6e8f8f2 100644 --- a/js/src/gc/Verifier.cpp +++ b/js/src/gc/Verifier.cpp @@ -789,8 +789,14 @@ bool js::gc::CheckWeakMapEntryMarking(const WeakMapBase* map, Cell* key, } CellColor keyColor = GetCellColor(key); - CellColor valueColor = - valueZone->isGCMarking() ? GetCellColor(value) : CellColor::Black; + + // Values belonging to other runtimes or in uncollected zones are treated as + // black. + CellColor valueColor = CellColor::Black; + if (value->runtimeFromAnyThread() == zone->runtimeFromAnyThread() && + valueZone->isGCMarking()) { + valueColor = GetCellColor(value); + } if (valueColor < Min(mapColor, keyColor)) { fprintf(stderr, "WeakMap value is less marked than map and key\n"); diff --git a/js/src/jit-test/tests/gc/bug-1543014.js b/js/src/jit-test/tests/gc/bug-1543014.js new file mode 100644 index 000000000000..9f91ad35260c --- /dev/null +++ b/js/src/jit-test/tests/gc/bug-1543014.js @@ -0,0 +1,11 @@ +// |jit-test| skip-if: helperThreadCount() === 0 +gczeal(0); +evalInWorker(` + var sym4 = Symbol.match; + function basicSweeping() {}; + var wm1 = new WeakMap(); + wm1.set(basicSweeping, sym4); + startgc(100000, 'shrinking'); +`); +gczeal(2); +var d1 = newGlobal({}); From ac0bf72aa462a55cc69bfd6101539a40cbaac398 Mon Sep 17 00:00:00 2001 From: Jon Coppeard Date: Mon, 1 Apr 2019 18:38:26 +0100 Subject: [PATCH 16/22] Bug 1505622 - Skip last ditch GC and fail the allocation if we already did last ditch GC within the last minute r=sfink --- js/public/GCAPI.h | 8 +++++ js/src/builtin/TestingFunctions.cpp | 3 +- js/src/gc/Allocator.cpp | 43 +++++++++++++++++------ js/src/gc/GC.cpp | 14 +++++++- js/src/gc/GCRuntime.h | 6 ++-- js/src/gc/Scheduling.h | 12 +++++++ js/src/jit-test/tests/gc/bug-1505622.js | 45 +++++++++++++++++++++++++ 7 files changed, 117 insertions(+), 14 deletions(-) create mode 100644 js/src/jit-test/tests/gc/bug-1505622.js diff --git a/js/public/GCAPI.h b/js/public/GCAPI.h index 2220909cbf98..41823ded73d4 100644 --- a/js/public/GCAPI.h +++ b/js/public/GCAPI.h @@ -307,6 +307,14 @@ typedef enum JSGCParamKey { */ JSGC_MIN_NURSERY_BYTES = 31, + /* + * The minimum time to allow between triggering last ditch GCs in seconds. + * + * Default: 60 seconds + * Pref: None + */ + JSGC_MIN_LAST_DITCH_GC_PERIOD = 32, + } JSGCParamKey; /* diff --git a/js/src/builtin/TestingFunctions.cpp b/js/src/builtin/TestingFunctions.cpp index 51ae1a015244..a777eaf28ae6 100644 --- a/js/src/builtin/TestingFunctions.cpp +++ b/js/src/builtin/TestingFunctions.cpp @@ -488,7 +488,8 @@ static bool MinorGC(JSContext* cx, unsigned argc, Value* vp) { _("allocationThreshold", JSGC_ALLOCATION_THRESHOLD, true) \ _("minEmptyChunkCount", JSGC_MIN_EMPTY_CHUNK_COUNT, true) \ _("maxEmptyChunkCount", JSGC_MAX_EMPTY_CHUNK_COUNT, true) \ - _("compactingEnabled", JSGC_COMPACTING_ENABLED, true) + _("compactingEnabled", JSGC_COMPACTING_ENABLED, true) \ + _("minLastDitchGCPeriod", JSGC_MIN_LAST_DITCH_GC_PERIOD, true) static const struct ParamInfo { const char* name; diff --git a/js/src/gc/Allocator.cpp b/js/src/gc/Allocator.cpp index 684a42b48b2d..b51270037235 100644 --- a/js/src/gc/Allocator.cpp +++ b/js/src/gc/Allocator.cpp @@ -7,6 +7,7 @@ #include "gc/Allocator.h" #include "mozilla/DebugOnly.h" +#include "mozilla/TimeStamp.h" #include "gc/GCInternals.h" #include "gc/GCTrace.h" @@ -22,6 +23,9 @@ #include "gc/PrivateIterators-inl.h" #include "vm/JSObject-inl.h" +using mozilla::TimeDuration; +using mozilla::TimeStamp; + using namespace js; using namespace gc; @@ -268,19 +272,16 @@ T* GCRuntime::tryNewTenuredThing(JSContext* cx, AllocKind kind, // chunks available it may also allocate new memory directly. t = reinterpret_cast(refillFreeListFromAnyThread(cx, kind)); - if (MOZ_UNLIKELY(!t && allowGC)) { - if (!cx->helperThread()) { - // We have no memory available for a new chunk; perform an - // all-compartments, non-incremental, shrinking GC and wait for - // sweeping to finish. - JS::PrepareForFullGC(cx); - cx->runtime()->gc.gc(GC_SHRINK, JS::GCReason::LAST_DITCH); - cx->runtime()->gc.waitBackgroundSweepOrAllocEnd(); - + if (MOZ_UNLIKELY(!t)) { + if (allowGC) { + cx->runtime()->gc.attemptLastDitchGC(cx); t = tryNewTenuredThing(cx, kind, thingSize); } if (!t) { - ReportOutOfMemory(cx); + if (allowGC) { + ReportOutOfMemory(cx); + } + return nullptr; } } } @@ -294,6 +295,28 @@ T* GCRuntime::tryNewTenuredThing(JSContext* cx, AllocKind kind, return t; } +void GCRuntime::attemptLastDitchGC(JSContext* cx) { + // Either there was no memory available for a new chunk or the heap hit its + // size limit. Try to perform an all-compartments, non-incremental, shrinking + // GC and wait for it to finish. + + if (cx->helperThread()) { + return; + } + + if (!lastLastDitchTime.IsNull() && + TimeStamp::Now() - lastLastDitchTime <= tunables.minLastDitchGCPeriod()) { + return; + } + + JS::PrepareForFullGC(cx); + gc(GC_SHRINK, JS::GCReason::LAST_DITCH); + waitBackgroundAllocEnd(); + waitBackgroundFreeEnd(); + + lastLastDitchTime = mozilla::TimeStamp::Now(); +} + template bool GCRuntime::checkAllocatorState(JSContext* cx, AllocKind kind) { if (allowGC) { diff --git a/js/src/gc/GC.cpp b/js/src/gc/GC.cpp index 6a0229377fce..ae17d38ce577 100644 --- a/js/src/gc/GC.cpp +++ b/js/src/gc/GC.cpp @@ -364,6 +364,9 @@ static const float PretenureThreshold = 0.6f; /* JSGC_PRETENURE_GROUP_THRESHOLD */ static const float PretenureGroupThreshold = 3000; +/* JSGC_MIN_LAST_DITCH_GC_PERIOD */ +static const TimeDuration MinLastDitchGCPeriod = TimeDuration::FromSeconds(60); + } // namespace TuningDefaults } // namespace gc } // namespace js @@ -1521,6 +1524,9 @@ bool GCSchedulingTunables::setParameter(JSGCParamKey key, uint32_t value, } pretenureGroupThreshold_ = value; break; + case JSGC_MIN_LAST_DITCH_GC_PERIOD: + minLastDitchGCPeriod_ = TimeDuration::FromSeconds(value); + break; default: MOZ_CRASH("Unknown GC parameter."); } @@ -1613,7 +1619,8 @@ GCSchedulingTunables::GCSchedulingTunables() nurseryFreeThresholdForIdleCollectionFraction_( TuningDefaults::NurseryFreeThresholdForIdleCollectionFraction), pretenureThreshold_(TuningDefaults::PretenureThreshold), - pretenureGroupThreshold_(TuningDefaults::PretenureGroupThreshold) {} + pretenureGroupThreshold_(TuningDefaults::PretenureGroupThreshold), + minLastDitchGCPeriod_(TuningDefaults::MinLastDitchGCPeriod) {} void GCRuntime::resetParameter(JSGCParamKey key, AutoLockGC& lock) { switch (key) { @@ -1708,6 +1715,9 @@ void GCSchedulingTunables::resetParameter(JSGCParamKey key, case JSGC_PRETENURE_GROUP_THRESHOLD: pretenureGroupThreshold_ = TuningDefaults::PretenureGroupThreshold; break; + case JSGC_MIN_LAST_DITCH_GC_PERIOD: + minLastDitchGCPeriod_ = TuningDefaults::MinLastDitchGCPeriod; + break; default: MOZ_CRASH("Unknown GC parameter."); } @@ -1783,6 +1793,8 @@ uint32_t GCRuntime::getParameter(JSGCParamKey key, const AutoLockGC& lock) { return uint32_t(tunables.pretenureThreshold() * 100); case JSGC_PRETENURE_GROUP_THRESHOLD: return tunables.pretenureGroupThreshold(); + case JSGC_MIN_LAST_DITCH_GC_PERIOD: + return tunables.minLastDitchGCPeriod().ToSeconds(); default: MOZ_CRASH("Unknown parameter key"); } diff --git a/js/src/gc/GCRuntime.h b/js/src/gc/GCRuntime.h index 924a6002f06c..0811336803b2 100644 --- a/js/src/gc/GCRuntime.h +++ b/js/src/gc/GCRuntime.h @@ -310,8 +310,7 @@ class GCRuntime { bool isForegroundSweeping() const { return state() == State::Sweep; } bool isBackgroundSweeping() { return sweepTask.isRunning(); } void waitBackgroundSweepEnd(); - void waitBackgroundSweepOrAllocEnd() { - waitBackgroundSweepEnd(); + void waitBackgroundAllocEnd() { allocTask.cancelAndWait(); } void waitBackgroundFreeEnd(); @@ -535,6 +534,7 @@ class GCRuntime { AllocKind thingKind); static TenuredCell* refillFreeListFromHelperThread(JSContext* cx, AllocKind thingKind); + void attemptLastDitchGC(JSContext* cx); /* * Return the list of chunks that can be released outside the GC lock. @@ -1045,6 +1045,8 @@ class GCRuntime { minorGC(reason, gcstats::PhaseKind::EVICT_NURSERY); } + mozilla::TimeStamp lastLastDitchTime; + friend class MarkingValidator; friend class AutoEnterIteration; }; diff --git a/js/src/gc/Scheduling.h b/js/src/gc/Scheduling.h index 35a005353be3..a2e7eb8299c9 100644 --- a/js/src/gc/Scheduling.h +++ b/js/src/gc/Scheduling.h @@ -456,6 +456,14 @@ class GCSchedulingTunables { */ UnprotectedData pretenureGroupThreshold_; + /* + * JSGC_MIN_LAST_DITCH_GC_PERIOD + * + * Last ditch GC is skipped if allocation failure occurs less than this many + * seconds from the previous one. + */ + MainThreadData minLastDitchGCPeriod_; + public: GCSchedulingTunables(); @@ -502,6 +510,10 @@ class GCSchedulingTunables { float pretenureThreshold() const { return pretenureThreshold_; } uint32_t pretenureGroupThreshold() const { return pretenureGroupThreshold_; } + mozilla::TimeDuration minLastDitchGCPeriod() const { + return minLastDitchGCPeriod_; + } + MOZ_MUST_USE bool setParameter(JSGCParamKey key, uint32_t value, const AutoLockGC& lock); void resetParameter(JSGCParamKey key, const AutoLockGC& lock); diff --git a/js/src/jit-test/tests/gc/bug-1505622.js b/js/src/jit-test/tests/gc/bug-1505622.js new file mode 100644 index 000000000000..c6eb07a59a19 --- /dev/null +++ b/js/src/jit-test/tests/gc/bug-1505622.js @@ -0,0 +1,45 @@ +// Test that we don't repeatedly trigger last-ditch GCs. + +function allocUntilFail() { + gc(); + let initGCNumber = gcparam("gcNumber"); + let error; + try { + let a = []; + while (true) { + a.push(Symbol()); // Symbols are tenured. + } + } catch(err) { + error = err; + } + let finalGCNumber = gcparam("gcNumber"); + gc(); + assertEq(error, "out of memory"); + return finalGCNumber - initGCNumber; +} + +// Turn of any zeal which will disrupt GC number checks. +gczeal(0); + +// Set a small heap limit. +gcparam("maxBytes", 1024 * 1024); + +// Set the time limit for skipping last ditch GCs to 5 seconds. +gcparam("minLastDitchGCPeriod", 5); +assertEq(gcparam("minLastDitchGCPeriod"), 5); + +// Allocate up to the heap limit. This triggers a last ditch GC. +let gcCount = allocUntilFail(); +assertEq(gcCount, 1) + +// Allocate up to the limit again. The second time we fail without +// triggering a GC. +gcCount = allocUntilFail(); +assertEq(gcCount, 0) + +// Wait for time limit to expire. +sleep(6); + +// Check we trigger a GC again. +gcCount = allocUntilFail(); +assertEq(gcCount, 1) From c0cfe6280cfd6e9dca1a3db293cbb91afbd9fccf Mon Sep 17 00:00:00 2001 From: Jeff Walden Date: Tue, 2 Apr 2019 14:10:10 -0700 Subject: [PATCH 17/22] Bug 1506902 - Change JS::EvaluateUtf8 to directly compile the provided UTF-8 data, without inflating through UTF-16. r=jandem --HG-- extra : rebase_source : 6a4d99ce983776faa1a486b9e2805412f876ce0e --- js/src/vm/CompilationAndEvaluation.cpp | 31 ++++++++++++++++---------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/js/src/vm/CompilationAndEvaluation.cpp b/js/src/vm/CompilationAndEvaluation.cpp index 80bb89d76df1..d4ca5c02a03b 100644 --- a/js/src/vm/CompilationAndEvaluation.cpp +++ b/js/src/vm/CompilationAndEvaluation.cpp @@ -508,9 +508,10 @@ JS_PUBLIC_API bool JS::CloneAndExecuteScript(JSContext* cx, return ExecuteScript(cx, envChain, script, rval.address()); } +template static bool Evaluate(JSContext* cx, ScopeKind scopeKind, HandleObject env, const ReadOnlyCompileOptions& optionsArg, - SourceText& srcBuf, MutableHandleValue rval) { + SourceText& srcBuf, MutableHandleValue rval) { CompileOptions options(cx, optionsArg); MOZ_ASSERT(!cx->zone()->isAtomsZone()); AssertHeapIsIdle(); @@ -548,14 +549,8 @@ static bool Evaluate(JSContext* cx, HandleObjectVector envChain, extern JS_PUBLIC_API bool JS::EvaluateUtf8( JSContext* cx, const ReadOnlyCompileOptions& options, const char* bytes, size_t length, MutableHandle rval) { - auto chars = UniqueTwoByteChars( - UTF8CharsToNewTwoByteCharsZ(cx, UTF8Chars(bytes, length), &length).get()); - if (!chars) { - return false; - } - - SourceText srcBuf; - if (!srcBuf.init(cx, std::move(chars), length)) { + SourceText srcBuf; + if (!srcBuf.init(cx, bytes, length, SourceOwnership::Borrowed)) { return false; } @@ -594,7 +589,19 @@ JS_PUBLIC_API bool JS::EvaluateUtf8Path( CompileOptions options(cx, optionsArg); options.setFileAndLine(filename, 1); - return EvaluateUtf8(cx, options, - reinterpret_cast(buffer.begin()), - buffer.length(), rval); + auto contents = reinterpret_cast(buffer.begin()); + size_t length = buffer.length(); + auto chars = UniqueTwoByteChars( + UTF8CharsToNewTwoByteCharsZ(cx, UTF8Chars(contents, length), &length) + .get()); + if (!chars) { + return false; + } + + SourceText srcBuf; + if (!srcBuf.init(cx, std::move(chars), length)) { + return false; + } + + return Evaluate(cx, options, srcBuf, rval); } From 1cdceb7b22314a6b32b28a2d46e0c0b1584a7d66 Mon Sep 17 00:00:00 2001 From: Andrew Osmond Date: Tue, 9 Apr 2019 15:27:16 -0400 Subject: [PATCH 18/22] Bug 1543217 - Allow qualified Linux machines to get WebRender. r=jrmuizel Linux machines using Intel graphics with Mesa drivers being at least 18.2.8.0 and not 4k displays should be able to run WebRender well, given this is a common configuration used for testing already by Mozilla. This patch allows users meeting said requirements to join the WebRender experiments on nightly. WebRender will remain disabled by default for other configurations/devices. Differential Revision: https://phabricator.services.mozilla.com/D26796 --- gfx/thebes/gfxPlatform.cpp | 30 ++++++++++++++++++++++++++---- gfx/thebes/gfxPlatform.h | 2 +- gfx/thebes/gfxPlatformGtk.cpp | 2 +- widget/GfxInfoX11.cpp | 28 ++++++++++++++++++++++++++++ widget/windows/GfxInfo.cpp | 6 +++--- 5 files changed, 59 insertions(+), 9 deletions(-) diff --git a/gfx/thebes/gfxPlatform.cpp b/gfx/thebes/gfxPlatform.cpp index 0a930430e820..251138e76f58 100644 --- a/gfx/thebes/gfxPlatform.cpp +++ b/gfx/thebes/gfxPlatform.cpp @@ -923,6 +923,7 @@ void gfxPlatform::Init() { #else # error "No gfxPlatform implementation available" #endif + gPlatform->PopulateScreenInfo(); gPlatform->InitAcceleration(); gPlatform->InitWebRenderConfig(); // When using WebRender, we defer initialization of the D3D11 devices until @@ -960,7 +961,6 @@ void gfxPlatform::Init() { InitLayersIPC(); - gPlatform->PopulateScreenInfo(); gPlatform->ComputeTileSize(); #ifdef MOZ_ENABLE_FREETYPE @@ -2511,7 +2511,7 @@ static bool CalculateWrQualifiedPrefValue() { } static FeatureState& WebRenderHardwareQualificationStatus( - bool aHasBattery, nsCString& aOutFailureId) { + const IntSize& aScreenSize, bool aHasBattery, nsCString& aOutFailureId) { FeatureState& featureWebRenderQualified = gfxConfig::GetFeature(Feature::WEBRENDER_QUALIFIED); featureWebRenderQualified.EnableByDefault(); @@ -2576,7 +2576,8 @@ static FeatureState& WebRenderHardwareQualificationStatus( FeatureStatus::Blocked, "Device too old", NS_LITERAL_CSTRING("FEATURE_FAILURE_DEVICE_TOO_OLD")); } - } else if (adapterVendorID == u"0x8086") { // Intel + } else if (adapterVendorID == u"0x8086" || + adapterVendorID == u"mesa/i965") { // Intel const uint16_t supportedDevices[] = { 0x191d, // HD Graphics P530 0x192d, // Iris Pro Graphics P555 @@ -2605,6 +2606,18 @@ static FeatureState& WebRenderHardwareQualificationStatus( featureWebRenderQualified.Disable( FeatureStatus::Blocked, "Device too old", NS_LITERAL_CSTRING("FEATURE_FAILURE_DEVICE_TOO_OLD")); + } else if (adapterVendorID == u"mesa/i965") { + const int32_t maxPixels = 3440 * 1440; // UWQHD + int32_t pixels = aScreenSize.width * aScreenSize.height; + if (pixels > maxPixels) { + featureWebRenderQualified.Disable( + FeatureStatus::Blocked, "Screen size too large", + NS_LITERAL_CSTRING("FEATURE_FAILURE_SCREEN_SIZE_TOO_LARGE")); + } else if (pixels <= 0) { + featureWebRenderQualified.Disable( + FeatureStatus::Blocked, "Screen size unknown", + NS_LITERAL_CSTRING("FEATURE_FAILURE_SCREEN_SIZE_UNKNOWN")); + } } #endif } else { @@ -2653,7 +2666,8 @@ void gfxPlatform::InitWebRenderConfig() { nsCString failureId; FeatureState& featureWebRenderQualified = - WebRenderHardwareQualificationStatus(HasBattery(), failureId); + WebRenderHardwareQualificationStatus(GetScreenSize(), HasBattery(), + failureId); FeatureState& featureWebRender = gfxConfig::GetFeature(Feature::WEBRENDER); featureWebRender.DisableByDefault( @@ -2748,6 +2762,14 @@ void gfxPlatform::InitWebRenderConfig() { WebRenderDebugPrefChangeCallback, WR_DEBUG_PREF); } } +#ifdef XP_LINUX + else { + // Hardware compositing should be disabled by default if we aren't using + // WebRender, but may still have been enabled by prefs. + gfxConfig::Disable(Feature::HW_COMPOSITING, FeatureStatus::Blocked, + "Acceleration blocked by platform"); + } +#endif #ifdef XP_WIN if (Preferences::GetBool("gfx.webrender.dcomp-win.enabled", false)) { diff --git a/gfx/thebes/gfxPlatform.h b/gfx/thebes/gfxPlatform.h index 34cf85857b72..896dff0452ff 100644 --- a/gfx/thebes/gfxPlatform.h +++ b/gfx/thebes/gfxPlatform.h @@ -736,7 +736,7 @@ class gfxPlatform : public mozilla::layers::MemoryPressureListener { gfxPlatform(); virtual ~gfxPlatform(); - virtual bool HasBattery() { return true; } + virtual bool HasBattery() { return false; } virtual void InitAcceleration(); virtual void InitWebRenderConfig(); diff --git a/gfx/thebes/gfxPlatformGtk.cpp b/gfx/thebes/gfxPlatformGtk.cpp index 144af010f76a..0a6ecc42342d 100644 --- a/gfx/thebes/gfxPlatformGtk.cpp +++ b/gfx/thebes/gfxPlatformGtk.cpp @@ -330,7 +330,7 @@ uint32_t gfxPlatformGtk::MaxGenericSubstitions() { } bool gfxPlatformGtk::AccelerateLayersByDefault() { - return gfxPrefs::WebRenderAll(); + return true; } void gfxPlatformGtk::GetPlatformCMSOutputProfile(void*& mem, size_t& size) { diff --git a/widget/GfxInfoX11.cpp b/widget/GfxInfoX11.cpp index beb720660d40..13777676add5 100644 --- a/widget/GfxInfoX11.cpp +++ b/widget/GfxInfoX11.cpp @@ -307,6 +307,34 @@ const nsTArray &GfxInfo::GetGfxDriverInfo() { GfxDriverInfo::allDevices, GfxDriverInfo::allFeatures, nsIGfxInfo::FEATURE_BLOCKED_DRIVER_VERSION, DRIVER_LESS_THAN, V(13, 15, 100, 1), "FEATURE_FAILURE_OLD_FGLRX", "fglrx 13.15.100.1"); + + //////////////////////////////////// + // FEATURE_WEBRENDER + + // Mesa baseline (chosen arbitrarily as that which ships with + // Ubuntu 18.04 LTS). + APPEND_TO_DRIVER_BLOCKLIST( + OperatingSystem::Linux, + (nsAString &)GfxDriverInfo::GetDeviceVendor(VendorMesaAll), + GfxDriverInfo::allDevices, nsIGfxInfo::FEATURE_WEBRENDER, + nsIGfxInfo::FEATURE_BLOCKED_DRIVER_VERSION, DRIVER_LESS_THAN, + V(18, 2, 8, 0), "FEATURE_FAILURE_WEBRENDER_OLD_MESA", "Mesa 18.2.8.0"); + + // Disable on all NVIDIA devices for now. + APPEND_TO_DRIVER_BLOCKLIST( + OperatingSystem::Linux, + (nsAString &)GfxDriverInfo::GetDeviceVendor(VendorNVIDIA), + GfxDriverInfo::allDevices, nsIGfxInfo::FEATURE_WEBRENDER, + nsIGfxInfo::FEATURE_BLOCKED_DEVICE, DRIVER_COMPARISON_IGNORED, + V(0, 0, 0, 0), "FEATURE_FAILURE_WEBRENDER_NO_LINUX_NVIDIA", ""); + + // Disable on all ATI devices for now. + APPEND_TO_DRIVER_BLOCKLIST( + OperatingSystem::Linux, + (nsAString &)GfxDriverInfo::GetDeviceVendor(VendorATI), + GfxDriverInfo::allDevices, nsIGfxInfo::FEATURE_WEBRENDER, + nsIGfxInfo::FEATURE_BLOCKED_DEVICE, DRIVER_COMPARISON_IGNORED, + V(0, 0, 0, 0), "FEATURE_FAILURE_WEBRENDER_NO_LINUX_ATI", ""); } return *sDriverInfo; } diff --git a/widget/windows/GfxInfo.cpp b/widget/windows/GfxInfo.cpp index fe91244fbacb..e5213b20a7f1 100644 --- a/widget/windows/GfxInfo.cpp +++ b/widget/windows/GfxInfo.cpp @@ -1602,9 +1602,9 @@ const nsTArray& GfxInfo::GetGfxDriverInfo() { //////////////////////////////////// // FEATURE_WEBRENDER - // We are blocking all non-Nvidia cards in gfxPlatform.cpp where we check - // for the WEBRENDER_QUALIFIED feature. However we also want to block some - // specific Nvidia cards for being too low-powered, so we do that here. + // We are blocking most hardware explicitly in gfxPlatform.cpp where we + // check for the WEBRENDER_QUALIFIED feature. However we also want to block + // some specific Nvidia cards for being too low-powered, so we do that here. APPEND_TO_DRIVER_BLOCKLIST2( OperatingSystem::Windows10, (nsAString&)GfxDriverInfo::GetDeviceVendor(VendorNVIDIA), From b794b3160794146eb516443a362cc389cfc7b8da Mon Sep 17 00:00:00 2001 From: Csoregi Natalia Date: Wed, 10 Apr 2019 18:46:01 +0300 Subject: [PATCH 19/22] Backed out changeset d76b42f0d6ca (bug 1523636) for causing bug 1543015. a=backout --- docshell/base/BrowsingContext.cpp | 4 - docshell/base/BrowsingContext.h | 3 - dom/base/nsFrameLoader.cpp | 402 +++++++++++------------ dom/base/nsFrameLoader.h | 21 +- dom/base/nsGlobalWindowOuter.cpp | 1 - dom/html/nsGenericHTMLFrameElement.cpp | 4 +- dom/ipc/ContentParent.cpp | 20 +- dom/ipc/ContentParent.h | 1 - dom/ipc/tests/test_force_oop_iframe.html | 3 - dom/xul/XULFrameElement.cpp | 3 +- 10 files changed, 221 insertions(+), 241 deletions(-) diff --git a/docshell/base/BrowsingContext.cpp b/docshell/base/BrowsingContext.cpp index 9c9442713976..ec3f962f5e21 100644 --- a/docshell/base/BrowsingContext.cpp +++ b/docshell/base/BrowsingContext.cpp @@ -307,10 +307,6 @@ void BrowsingContext::CacheChildren(bool aFromIPC) { bool BrowsingContext::IsCached() { return sCachedBrowsingContexts->has(Id()); } -bool BrowsingContext::HasOpener() const { - return sBrowsingContexts->has(mOpenerId); -} - void BrowsingContext::GetChildren( nsTArray>& aChildren) { MOZ_ALWAYS_TRUE(aChildren.AppendElements(mChildren)); diff --git a/docshell/base/BrowsingContext.h b/docshell/base/BrowsingContext.h index 91c0b556575c..99338438f977 100644 --- a/docshell/base/BrowsingContext.h +++ b/docshell/base/BrowsingContext.h @@ -121,7 +121,6 @@ class BrowsingContext : public nsWrapperCache, // null if it's not. nsIDocShell* GetDocShell() { return mDocShell; } void SetDocShell(nsIDocShell* aDocShell); - void ClearDocShell() { mDocShell = nullptr; } // Get the outer window object for this BrowsingContext if it is in-process // and still has a docshell, or null otherwise. @@ -162,8 +161,6 @@ class BrowsingContext : public nsWrapperCache, SetOpenerId(aOpener ? aOpener->Id() : 0); } - bool HasOpener() const; - void GetChildren(nsTArray>& aChildren); BrowsingContextGroup* Group() { return mGroup; } diff --git a/dom/base/nsFrameLoader.cpp b/dom/base/nsFrameLoader.cpp index 30a1c5c1a3ad..71b45834cffc 100644 --- a/dom/base/nsFrameLoader.cpp +++ b/dom/base/nsFrameLoader.cpp @@ -152,8 +152,8 @@ typedef ScrollableLayerGuid::ViewID ViewID; // we'd need to re-institute a fixed version of bug 98158. #define MAX_DEPTH_CONTENT_FRAMES 10 -NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE(nsFrameLoader, mBrowsingContext, - mMessageManager, mChildMessageManager, +NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE(nsFrameLoader, mDocShell, mMessageManager, + mChildMessageManager, mOpener, mParentSHistory, mRemoteBrowser) NS_IMPL_CYCLE_COLLECTING_ADDREF(nsFrameLoader) NS_IMPL_CYCLE_COLLECTING_RELEASE(nsFrameLoader) @@ -165,11 +165,11 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(nsFrameLoader) NS_INTERFACE_MAP_ENTRY(nsISupports) NS_INTERFACE_MAP_END -nsFrameLoader::nsFrameLoader(Element* aOwner, BrowsingContext* aBrowsingContext, +nsFrameLoader::nsFrameLoader(Element* aOwner, nsPIDOMWindowOuter* aOpener, bool aNetworkCreated) - : mBrowsingContext(aBrowsingContext), - mOwnerContent(aOwner), + : mOwnerContent(aOwner), mDetachedSubdocFrame(nullptr), + mOpener(aOpener), mRemoteBrowser(nullptr), mChildID(0), mDepthTooGreat(false), @@ -185,15 +185,15 @@ nsFrameLoader::nsFrameLoader(Element* aOwner, BrowsingContext* aBrowsingContext, mIsRemoteFrame(false), mObservingOwnerContent(false) { mIsRemoteFrame = ShouldUseRemoteProcess(); - MOZ_ASSERT(!mIsRemoteFrame || !mBrowsingContext->HasOpener(), + MOZ_ASSERT(!mIsRemoteFrame || !aOpener, "Cannot pass aOpener for a remote frame!"); } -nsFrameLoader::nsFrameLoader(Element* aOwner, BrowsingContext* aBrowsingContext, +nsFrameLoader::nsFrameLoader(Element* aOwner, const mozilla::dom::RemotenessOptions& aOptions) - : mBrowsingContext(aBrowsingContext), - mOwnerContent(aOwner), + : mOwnerContent(aOwner), mDetachedSubdocFrame(nullptr), + mOpener(nullptr), mRemoteBrowser(nullptr), mChildID(0), mDepthTooGreat(false), @@ -212,6 +212,14 @@ nsFrameLoader::nsFrameLoader(Element* aOwner, BrowsingContext* aBrowsingContext, (!aOptions.mRemoteType.Value().IsVoid())) { mIsRemoteFrame = true; } + bool hasOpener = + aOptions.mOpener.WasPassed() && !aOptions.mOpener.Value().IsNull(); + MOZ_ASSERT(!mIsRemoteFrame || !hasOpener, + "Cannot pass aOpener for a remote frame!"); + if (hasOpener) { + // This seems slightly unwieldy. + mOpener = aOptions.mOpener.Value().Value().get()->GetDOMWindow(); + } } nsFrameLoader::~nsFrameLoader() { @@ -221,70 +229,9 @@ nsFrameLoader::~nsFrameLoader() { MOZ_RELEASE_ASSERT(mDestroyCalled); } -static void GetFrameName(Element* aOwnerContent, nsAString& aFrameName) { - int32_t namespaceID = aOwnerContent->GetNameSpaceID(); - if (namespaceID == kNameSpaceID_XHTML && !aOwnerContent->IsInHTMLDocument()) { - aOwnerContent->GetAttr(kNameSpaceID_None, nsGkAtoms::id, aFrameName); - } else { - aOwnerContent->GetAttr(kNameSpaceID_None, nsGkAtoms::name, aFrameName); - // XXX if no NAME then use ID, after a transition period this will be - // changed so that XUL only uses ID too (bug 254284). - if (aFrameName.IsEmpty() && namespaceID == kNameSpaceID_XUL) { - aOwnerContent->GetAttr(kNameSpaceID_None, nsGkAtoms::id, aFrameName); - } - } -} - -static already_AddRefed CreateBrowsingContext( - Element* aOwner, BrowsingContext* aOpener) { - Document* doc = aOwner->OwnerDoc(); - // Get our parent docshell off the document of mOwnerContent - // XXXbz this is such a total hack.... We really need to have a - // better setup for doing this. - - // Determine our parent nsDocShell - RefPtr parentDocShell = nsDocShell::Cast(doc->GetDocShell()); - - if (NS_WARN_IF(!parentDocShell)) { - return nullptr; - } - - RefPtr parentContext = parentDocShell->GetBrowsingContext(); - - MOZ_DIAGNOSTIC_ASSERT(parentContext, "docShell must have BrowsingContext"); - - // Determine the frame name for the new browsing context. - nsAutoString frameName; - GetFrameName(aOwner, frameName); - - // Check if our new context is chrome or content - bool isContent = - parentContext->IsContent() || - aOwner->AttrValueIs( - kNameSpaceID_None, - aOwner->IsXULElement() ? nsGkAtoms::type : nsGkAtoms::mozframetype, - nsGkAtoms::content, eIgnoreCase); - - // Force mozbrowser frames to always be content, even if the mozbrowser - // interfaces are disabled. - nsCOMPtr mozbrowser = aOwner->GetAsMozBrowserFrame(); - if (!isContent && mozbrowser) { - mozbrowser->GetMozbrowser(&isContent); - } - - // If we're content but our parent isn't, we're going to want to - // start a new browsing context tree. - if (isContent && !parentContext->IsContent()) { - parentContext = nullptr; - } - - BrowsingContext::Type type = isContent ? BrowsingContext::Type::Content - : BrowsingContext::Type::Chrome; - - return BrowsingContext::Create(parentContext, aOpener, frameName, type); -} - -nsFrameLoader* nsFrameLoader::Create(Element* aOwner, BrowsingContext* aOpener, +/* static */ +nsFrameLoader* nsFrameLoader::Create(Element* aOwner, + nsPIDOMWindowOuter* aOpener, bool aNetworkCreated) { NS_ENSURE_TRUE(aOwner, nullptr); Document* doc = aOwner->OwnerDoc(); @@ -314,8 +261,7 @@ nsFrameLoader* nsFrameLoader::Create(Element* aOwner, BrowsingContext* aOpener, doc->IsStaticDocument()), nullptr); - RefPtr context = CreateBrowsingContext(aOwner, aOpener); - return new nsFrameLoader(aOwner, context, aNetworkCreated); + return new nsFrameLoader(aOwner, aOpener, aNetworkCreated); } /* static */ @@ -325,21 +271,7 @@ nsFrameLoader* nsFrameLoader::Create( NS_ENSURE_TRUE(aOwner, nullptr); // This version of Create is only called for Remoteness updates, so we can // assume we need a FrameLoader here and skip the check in the other Create. - - bool hasOpener = - aOptions.mOpener.WasPassed() && !aOptions.mOpener.Value().IsNull(); - MOZ_ASSERT(!aOptions.mRemoteType.WasPassed() || - aOptions.mRemoteType.Value().IsVoid() || !hasOpener, - "Cannot pass aOpener for a remote frame!"); - - // This seems slightly unwieldy. - RefPtr opener; - if (hasOpener) { - opener = aOptions.mOpener.Value().Value().get(); - } - RefPtr context = CreateBrowsingContext(aOwner, opener); - - return new nsFrameLoader(aOwner, context, aOptions); + return new nsFrameLoader(aOwner, aOptions); } void nsFrameLoader::LoadFrame(bool aOriginalSrc) { @@ -497,8 +429,8 @@ nsresult nsFrameLoader::ReallyStartLoadingInternal() { if (NS_FAILED(rv)) { return rv; } - MOZ_ASSERT(GetDocShell(), - "MaybeCreateDocShell succeeded with a null docShell"); + NS_ASSERTION(mDocShell, + "MaybeCreateDocShell succeeded with a null mDocShell"); // Just to be safe, recheck uri. rv = CheckURILoad(mURIToLoad, mTriggeringPrincipal); @@ -600,7 +532,7 @@ nsresult nsFrameLoader::ReallyStartLoadingInternal() { mNeedsAsyncDestroy = true; loadState->SetLoadFlags(flags); loadState->SetFirstParty(false); - rv = GetDocShell()->LoadURI(loadState); + rv = mDocShell->LoadURI(loadState); mNeedsAsyncDestroy = tmpState; mURIToLoad = nullptr; NS_ENSURE_SUCCESS(rv, rv); @@ -659,11 +591,11 @@ nsDocShell* nsFrameLoader::GetDocShell(ErrorResult& aRv) { aRv.Throw(rv); return nullptr; } - MOZ_ASSERT(GetDocShell(), - "MaybeCreateDocShell succeeded, but null docShell"); + NS_ASSERTION(mDocShell, + "MaybeCreateDocShell succeeded, but null mDocShell"); } - return GetDocShell(); + return mDocShell; } static void SetTreeOwnerAndChromeEventHandlerOnDocshellTree( @@ -809,20 +741,20 @@ bool nsFrameLoader::Show(int32_t marginWidth, int32_t marginHeight, if (NS_FAILED(rv)) { return false; } - MOZ_ASSERT(GetDocShell(), "MaybeCreateDocShell succeeded, but null docShell"); - if (!GetDocShell()) { + NS_ASSERTION(mDocShell, "MaybeCreateDocShell succeeded, but null mDocShell"); + if (!mDocShell) { return false; } - GetDocShell()->SetMarginWidth(marginWidth); - GetDocShell()->SetMarginHeight(marginHeight); + mDocShell->SetMarginWidth(marginWidth); + mDocShell->SetMarginHeight(marginHeight); - GetDocShell()->SetDefaultScrollbarPreferences( - nsIScrollable::ScrollOrientation_X, scrollbarPrefX); - GetDocShell()->SetDefaultScrollbarPreferences( - nsIScrollable::ScrollOrientation_Y, scrollbarPrefY); + mDocShell->SetDefaultScrollbarPreferences(nsIScrollable::ScrollOrientation_X, + scrollbarPrefX); + mDocShell->SetDefaultScrollbarPreferences(nsIScrollable::ScrollOrientation_Y, + scrollbarPrefY); - nsCOMPtr presShell = GetDocShell()->GetPresShell(); + nsCOMPtr presShell = mDocShell->GetPresShell(); if (presShell) { // Ensure root scroll frame is reflowed in case scroll preferences or // margins have changed @@ -837,7 +769,7 @@ bool nsFrameLoader::Show(int32_t marginWidth, int32_t marginHeight, nsView* view = frame->EnsureInnerView(); if (!view) return false; - RefPtr baseWindow = GetDocShell(); + RefPtr baseWindow = mDocShell; baseWindow->InitWindow(nullptr, view->GetWidget(), 0, 0, size.width, size.height); // This is kinda whacky, this "Create()" call doesn't really @@ -845,13 +777,13 @@ bool nsFrameLoader::Show(int32_t marginWidth, int32_t marginHeight, // "Create"... baseWindow->Create(); baseWindow->SetVisibility(true); - NS_ENSURE_TRUE(GetDocShell(), false); + NS_ENSURE_TRUE(mDocShell, false); // Trigger editor re-initialization if midas is turned on in the // sub-document. This shouldn't be necessary, but given the way our // editor works, it is. See // https://bugzilla.mozilla.org/show_bug.cgi?id=284245 - presShell = GetDocShell()->GetPresShell(); + presShell = mDocShell->GetPresShell(); if (presShell) { Document* doc = presShell->GetDocument(); nsHTMLDocument* htmlDoc = @@ -864,7 +796,7 @@ bool nsFrameLoader::Show(int32_t marginWidth, int32_t marginHeight, if (designMode.EqualsLiteral("on")) { // Hold on to the editor object to let the document reattach to the // same editor object, instead of creating a new one. - RefPtr htmlEditor = GetDocShell()->GetHTMLEditor(); + RefPtr htmlEditor = mDocShell->GetHTMLEditor(); Unused << htmlEditor; htmlDoc->SetDesignMode(NS_LITERAL_STRING("off"), Nothing(), IgnoreErrors()); @@ -874,9 +806,9 @@ bool nsFrameLoader::Show(int32_t marginWidth, int32_t marginHeight, } else { // Re-initialize the presentation for contenteditable documents bool editable = false, hasEditingSession = false; - GetDocShell()->GetEditable(&editable); - GetDocShell()->GetHasEditingSession(&hasEditingSession); - RefPtr htmlEditor = GetDocShell()->GetHTMLEditor(); + mDocShell->GetEditable(&editable); + mDocShell->GetHasEditingSession(&hasEditingSession); + RefPtr htmlEditor = mDocShell->GetHTMLEditor(); if (editable && hasEditingSession && htmlEditor) { htmlEditor->PostCreate(); } @@ -896,24 +828,20 @@ bool nsFrameLoader::Show(int32_t marginWidth, int32_t marginHeight, void nsFrameLoader::MarginsChanged(uint32_t aMarginWidth, uint32_t aMarginHeight) { // We assume that the margins are always zero for remote frames. - if (IsRemoteFrame()) { - return; - } + if (IsRemoteFrame()) return; // If there's no docshell, we're probably not up and running yet. // nsFrameLoader::Show() will take care of setting the right // margins. - if (!GetDocShell()) { - return; - } + if (!mDocShell) return; // Set the margins - GetDocShell()->SetMarginWidth(aMarginWidth); - GetDocShell()->SetMarginHeight(aMarginHeight); + mDocShell->SetMarginWidth(aMarginWidth); + mDocShell->SetMarginHeight(aMarginHeight); // There's a cached property declaration block // that needs to be updated - if (Document* doc = GetDocShell()->GetDocument()) { + if (Document* doc = mDocShell->GetDocument()) { for (nsINode* cur = doc; cur; cur = cur->GetNextNode()) { if (cur->IsHTMLElement(nsGkAtoms::body)) { static_cast(cur)->ClearMappedServoStyle(); @@ -923,7 +851,7 @@ void nsFrameLoader::MarginsChanged(uint32_t aMarginWidth, // Trigger a restyle if there's a prescontext // FIXME: This could do something much less expensive. - if (nsPresContext* presContext = GetDocShell()->GetPresContext()) { + if (nsPresContext* presContext = mDocShell->GetPresContext()) { // rebuild, because now the same nsMappedAttributes* will produce // a different style presContext->RebuildAllStyleData(nsChangeHint(0), @@ -1012,15 +940,13 @@ void nsFrameLoader::Hide() { return; } - if (!GetDocShell()) { - return; - } + if (!mDocShell) return; nsCOMPtr contentViewer; - GetDocShell()->GetContentViewer(getter_AddRefs(contentViewer)); + mDocShell->GetContentViewer(getter_AddRefs(contentViewer)); if (contentViewer) contentViewer->SetSticky(false); - RefPtr baseWin = GetDocShell(); + RefPtr baseWin = mDocShell; baseWin->SetVisibility(false); baseWin->SetParentWidget(nullptr); } @@ -1731,26 +1657,26 @@ void nsFrameLoader::StartDestroy() { // Seems like this is a dynamic frame removal. if (dynamicSubframeRemoval) { - if (GetDocShell()) { - GetDocShell()->RemoveFromSessionHistory(); + if (mDocShell) { + mDocShell->RemoveFromSessionHistory(); } } // Let the tree owner know we're gone. if (mIsTopLevelContent) { - if (GetDocShell()) { + if (mDocShell) { nsCOMPtr parentItem; - GetDocShell()->GetParent(getter_AddRefs(parentItem)); + mDocShell->GetParent(getter_AddRefs(parentItem)); nsCOMPtr owner = do_GetInterface(parentItem); if (owner) { - owner->ContentShellRemoved(GetDocShell()); + owner->ContentShellRemoved(mDocShell); } } } // Let our window know that we are gone - if (GetDocShell()) { - nsCOMPtr win_private(GetDocShell()->GetWindow()); + if (mDocShell) { + nsCOMPtr win_private(mDocShell->GetWindow()); if (win_private) { win_private->SetFrameElementInternal(nullptr); } @@ -1830,11 +1756,10 @@ void nsFrameLoader::DestroyDocShell() { } // Destroy the docshell. - if (GetDocShell()) { - GetDocShell()->Destroy(); + if (mDocShell) { + mDocShell->Destroy(); } - - mBrowsingContext = nullptr; + mDocShell = nullptr; if (mChildMessageManager) { // Stop handling events in the in-process frame script. @@ -1966,8 +1891,37 @@ bool nsFrameLoader::ShouldUseRemoteProcess() { nsGkAtoms::_true, eCaseMatters); } +static already_AddRefed CreateBrowsingContext( + BrowsingContext* aParentContext, BrowsingContext* aOpenerContext, + const nsAString& aName, bool aIsContent) { + // If we're content but our parent isn't, we're going to want to start a new + // browsing context tree. + if (aIsContent && aParentContext && !aParentContext->IsContent()) { + aParentContext = nullptr; + } + + BrowsingContext::Type type = aIsContent ? BrowsingContext::Type::Content + : BrowsingContext::Type::Chrome; + + return BrowsingContext::Create(aParentContext, aOpenerContext, aName, type); +} + +static void GetFrameName(Element* aOwnerContent, nsAString& aFrameName) { + int32_t namespaceID = aOwnerContent->GetNameSpaceID(); + if (namespaceID == kNameSpaceID_XHTML && !aOwnerContent->IsInHTMLDocument()) { + aOwnerContent->GetAttr(kNameSpaceID_None, nsGkAtoms::id, aFrameName); + } else { + aOwnerContent->GetAttr(kNameSpaceID_None, nsGkAtoms::name, aFrameName); + // XXX if no NAME then use ID, after a transition period this will be + // changed so that XUL only uses ID too (bug 254284). + if (aFrameName.IsEmpty() && namespaceID == kNameSpaceID_XUL) { + aOwnerContent->GetAttr(kNameSpaceID_None, nsGkAtoms::id, aFrameName); + } + } +} + nsresult nsFrameLoader::MaybeCreateDocShell() { - if (GetDocShell()) { + if (mDocShell) { return NS_OK; } if (IsRemoteFrame()) { @@ -1996,20 +1950,43 @@ nsresult nsFrameLoader::MaybeCreateDocShell() { return NS_ERROR_NOT_AVAILABLE; } + // Determine our parent nsDocShell RefPtr parentDocShell = nsDocShell::Cast(doc->GetDocShell()); if (NS_WARN_IF(!parentDocShell)) { return NS_ERROR_UNEXPECTED; } - // nsDocShell::Create will attach itself to the passed browsing - // context inside of nsDocShell::Create - RefPtr docShell = nsDocShell::Create(mBrowsingContext); - NS_ENSURE_TRUE(docShell, NS_ERROR_FAILURE); + RefPtr parentBC = parentDocShell->GetBrowsingContext(); + MOZ_ASSERT(parentBC, "docShell must have BrowsingContext"); - mIsTopLevelContent = - mBrowsingContext->IsContent() && !mBrowsingContext->GetParent(); + // Determine the frame name for the new browsing context. + nsAutoString frameName; + GetFrameName(mOwnerContent, frameName); + + // Check if our new context is chrome or content + bool isContent = parentBC->IsContent() || + mOwnerContent->AttrValueIs(kNameSpaceID_None, TypeAttrName(), + nsGkAtoms::content, eIgnoreCase); + + // Force mozbrowser frames to always be content, even if the mozbrowser + // interfaces are disabled. + nsCOMPtr mozbrowser = + mOwnerContent->GetAsMozBrowserFrame(); + if (!isContent && mozbrowser) { + mozbrowser->GetMozbrowser(&isContent); + } + + RefPtr openerBC = + mOpener ? mOpener->GetBrowsingContext() : nullptr; + RefPtr browsingContext = + CreateBrowsingContext(parentBC, openerBC, frameName, isContent); + + mDocShell = nsDocShell::Create(browsingContext); + NS_ENSURE_TRUE(mDocShell, NS_ERROR_FAILURE); + + mIsTopLevelContent = isContent && !parentBC->IsContent(); if (!mNetworkCreated && !mIsTopLevelContent) { - docShell->SetCreatedDynamically(true); + mDocShell->SetCreatedDynamically(true); } if (mIsTopLevelContent) { @@ -2018,20 +1995,19 @@ nsresult nsFrameLoader::MaybeCreateDocShell() { // // XXX(nika): Consider removing the DocShellTree in the future, for // consistency between local and remote frames.. - parentDocShell->AddChild(docShell); + parentDocShell->AddChild(mDocShell); } // Now that we are part of the DocShellTree, attach our DocShell to our // parent's TreeOwner. nsCOMPtr parentTreeOwner; parentDocShell->GetTreeOwner(getter_AddRefs(parentTreeOwner)); - AddTreeItemToTreeOwner(docShell, parentTreeOwner); + AddTreeItemToTreeOwner(mDocShell, parentTreeOwner); // Make sure all nsDocShells have links back to the content element in the // nearest enclosing chrome shell. RefPtr chromeEventHandler; - bool parentIsContent = parentDocShell->GetBrowsingContext()->IsContent(); - if (parentIsContent) { + if (parentBC->IsContent()) { // Our parent shell is a content shell. Get the chrome event handler from it // and use that for our shell as well. parentDocShell->GetChromeEventHandler(getter_AddRefs(chromeEventHandler)); @@ -2041,15 +2017,15 @@ nsresult nsFrameLoader::MaybeCreateDocShell() { chromeEventHandler = mOwnerContent; } - docShell->SetChromeEventHandler(chromeEventHandler); + mDocShell->SetChromeEventHandler(chromeEventHandler); - // This is nasty, this code (the docShell->GetWindow() below) + // This is nasty, this code (the mDocShell->GetWindow() below) // *must* come *after* the above call to - // docShell->SetChromeEventHandler() for the global window to get + // mDocShell->SetChromeEventHandler() for the global window to get // the right chrome event handler. // Tell the window about the frame that hosts it. - nsCOMPtr newWindow = docShell->GetWindow(); + nsCOMPtr newWindow = mDocShell->GetWindow(); if (NS_WARN_IF(!newWindow)) { // Do not call Destroy() here. See bug 472312. NS_WARNING("Something wrong when creating the docshell for a frameloader!"); @@ -2058,10 +2034,13 @@ nsresult nsFrameLoader::MaybeCreateDocShell() { newWindow->SetFrameElementInternal(mOwnerContent); + // Set the opener window if we have one provided here XXX(nika): We + // should tell our BrowsingContext this as we create it. // TODO(farre): Remove this when nsGlobalWindowOuter::GetOpenerWindowOuter // starts using BrowsingContext::GetOpener. - if (RefPtr opener = mBrowsingContext->GetOpener()) { - newWindow->SetOpenerWindow(opener->GetDOMWindow(), true); + if (mOpener) { + newWindow->SetOpenerWindow(mOpener, true); + mOpener = nullptr; } // Allow scripts to close the docshell if specified. @@ -2075,6 +2054,7 @@ nsresult nsFrameLoader::MaybeCreateDocShell() { // This is kinda whacky, this call doesn't really create anything, // but it must be called to make sure things are properly // initialized. + RefPtr docShell = mDocShell; if (NS_FAILED(docShell->Create())) { // Do not call Destroy() here. See bug 472312. NS_WARNING("Something wrong when creating the docshell for a frameloader!"); @@ -2087,13 +2067,13 @@ nsresult nsFrameLoader::MaybeCreateDocShell() { if (mIsTopLevelContent && mOwnerContent->IsXULElement(nsGkAtoms::browser) && !mOwnerContent->HasAttr(kNameSpaceID_None, nsGkAtoms::disablehistory)) { // XXX(nika): Set this up more explicitly? - nsresult rv = docShell->InitSessionHistory(); + nsresult rv = mDocShell->InitSessionHistory(); NS_ENSURE_SUCCESS(rv, rv); mParentSHistory = new ParentSHistory(this); } OriginAttributes attrs; - if (parentDocShell->ItemType() == docShell->ItemType()) { + if (parentDocShell->ItemType() == mDocShell->ItemType()) { attrs = parentDocShell->GetOriginAttributes(); } @@ -2104,7 +2084,7 @@ nsresult nsFrameLoader::MaybeCreateDocShell() { // // For example, firstPartyDomain is computed from top-level document, it // doesn't exist in the top-level docshell. - if (parentIsContent && + if (parentBC->IsContent() && !nsContentUtils::IsSystemPrincipal(doc->NodePrincipal()) && !OwnerIsMozBrowserFrame()) { OriginAttributes oa = doc->NodePrincipal()->OriginAttributesRef(); @@ -2132,18 +2112,18 @@ nsresult nsFrameLoader::MaybeCreateDocShell() { if (OwnerIsMozBrowserFrame()) { attrs.mAppId = nsIScriptSecurityManager::NO_APP_ID; attrs.mInIsolatedMozBrowser = OwnerIsIsolatedMozBrowserFrame(); - docShell->SetFrameType(nsIDocShell::FRAME_TYPE_BROWSER); + mDocShell->SetFrameType(nsIDocShell::FRAME_TYPE_BROWSER); } else { nsCOMPtr parentCheck; - docShell->GetSameTypeParent(getter_AddRefs(parentCheck)); + mDocShell->GetSameTypeParent(getter_AddRefs(parentCheck)); if (!!parentCheck) { - docShell->SetIsFrame(); + mDocShell->SetIsFrame(); } } // Apply sandbox flags even if our owner is not an iframe, as this copies // flags from our owning content's owning document. - // Note: ApplySandboxFlags should be called after docShell->SetFrameType + // Note: ApplySandboxFlags should be called after mDocShell->SetFrameType // because we need to get the correct presentation URL in ApplySandboxFlags. uint32_t sandboxFlags = 0; HTMLIFrameElement* iframe = HTMLIFrameElement::FromNode(mOwnerContent); @@ -2169,16 +2149,16 @@ nsresult nsFrameLoader::MaybeCreateDocShell() { // For inproc frames, set the docshell properties. nsAutoString name; if (mOwnerContent->GetAttr(kNameSpaceID_None, nsGkAtoms::name, name)) { - docShell->SetName(name); + mDocShell->SetName(name); } - docShell->SetFullscreenAllowed( + mDocShell->SetFullscreenAllowed( mOwnerContent->HasAttr(kNameSpaceID_None, nsGkAtoms::allowfullscreen) || mOwnerContent->HasAttr(kNameSpaceID_None, nsGkAtoms::mozallowfullscreen)); bool isPrivate = mOwnerContent->HasAttr(kNameSpaceID_None, nsGkAtoms::mozprivatebrowsing); if (isPrivate) { - if (docShell->GetHasLoadedNonBlankURI()) { + if (mDocShell->GetHasLoadedNonBlankURI()) { nsContentUtils::ReportToConsoleNonLocalized( NS_LITERAL_STRING("We should not switch to Private Browsing after " "loading a document."), @@ -2192,28 +2172,30 @@ nsresult nsFrameLoader::MaybeCreateDocShell() { } } - docShell->SetOriginAttributes(attrs); + nsDocShell::Cast(mDocShell)->SetOriginAttributes(attrs); // Typically there will be a window, however for some cases such as printing // the document is cloned with a docshell that has no window. We check // that the window exists to ensure we don't try to gather ancestors for // those cases. nsCOMPtr win = doc->GetWindow(); - if (!docShell->GetIsMozBrowser() && - parentDocShell->ItemType() == docShell->ItemType() && + if (!mDocShell->GetIsMozBrowser() && + parentDocShell->ItemType() == mDocShell->ItemType() && !doc->IsStaticDocument() && win) { // Propagate through the ancestor principals. nsTArray> ancestorPrincipals; // Make a copy, so we can modify it. ancestorPrincipals = doc->AncestorPrincipals(); ancestorPrincipals.InsertElementAt(0, doc->NodePrincipal()); - docShell->SetAncestorPrincipals(std::move(ancestorPrincipals)); + nsDocShell::Cast(mDocShell)->SetAncestorPrincipals( + std::move(ancestorPrincipals)); // Repeat for outer window IDs. nsTArray ancestorOuterWindowIDs; ancestorOuterWindowIDs = doc->AncestorOuterWindowIDs(); ancestorOuterWindowIDs.InsertElementAt(0, win->WindowID()); - docShell->SetAncestorOuterWindowIDs(std::move(ancestorOuterWindowIDs)); + nsDocShell::Cast(mDocShell)->SetAncestorOuterWindowIDs( + std::move(ancestorOuterWindowIDs)); } ReallyLoadFrameScripts(); @@ -2270,19 +2252,19 @@ nsresult nsFrameLoader::CheckForRecursiveLoad(nsIURI* aURI) { if (NS_FAILED(rv)) { return rv; } - MOZ_ASSERT(GetDocShell(), "MaybeCreateDocShell succeeded, but null docShell"); - if (!GetDocShell()) { + NS_ASSERTION(mDocShell, "MaybeCreateDocShell succeeded, but null mDocShell"); + if (!mDocShell) { return NS_ERROR_FAILURE; } // Check that we're still in the docshell tree. nsCOMPtr treeOwner; - GetDocShell()->GetTreeOwner(getter_AddRefs(treeOwner)); + mDocShell->GetTreeOwner(getter_AddRefs(treeOwner)); NS_WARNING_ASSERTION(treeOwner, "Trying to load a new url to a docshell without owner!"); NS_ENSURE_STATE(treeOwner); - if (GetDocShell()->ItemType() != nsIDocShellTreeItem::typeContent) { + if (mDocShell->ItemType() != nsIDocShellTreeItem::typeContent) { // No need to do recursion-protection here XXXbz why not?? Do we really // trust people not to screw up with non-content docshells? return NS_OK; @@ -2291,7 +2273,7 @@ nsresult nsFrameLoader::CheckForRecursiveLoad(nsIURI* aURI) { // Bug 8065: Don't exceed some maximum depth in content frames // (MAX_DEPTH_CONTENT_FRAMES) nsCOMPtr parentAsItem; - GetDocShell()->GetSameTypeParent(getter_AddRefs(parentAsItem)); + mDocShell->GetSameTypeParent(getter_AddRefs(parentAsItem)); int32_t depth = 0; while (parentAsItem) { ++depth; @@ -2323,7 +2305,7 @@ nsresult nsFrameLoader::CheckForRecursiveLoad(nsIURI* aURI) { } } int32_t matchCount = 0; - GetDocShell()->GetSameTypeParent(getter_AddRefs(parentAsItem)); + mDocShell->GetSameTypeParent(getter_AddRefs(parentAsItem)); while (parentAsItem) { // Check the parent URI with the URI we're loading nsCOMPtr parentAsNav(do_QueryInterface(parentAsItem)); @@ -2601,15 +2583,27 @@ bool nsFrameLoader::TryRemoteBrowser() { // If we're in a content process, create a BrowserBridgeChild actor. if (XRE_IsContentProcess()) { + // Determine the frame name for the new browsing context. + nsAutoString frameName; + GetFrameName(mOwnerContent, frameName); + + RefPtr parentBC; + parentDocShell->GetBrowsingContext(getter_AddRefs(parentBC)); + MOZ_ASSERT(parentBC, "docShell must have BrowsingContext"); + + // XXX(nika): due to limitations with Browsing Context Groups and multiple + // processes, we can't link up aParent yet! (Bug 1532661) + RefPtr browsingContext = + CreateBrowsingContext(parentBC, nullptr, frameName, true); + mBrowserBridgeChild = BrowserBridgeChild::Create( - this, context, NS_LITERAL_STRING(DEFAULT_REMOTE_TYPE), - mBrowsingContext); + this, context, NS_LITERAL_STRING(DEFAULT_REMOTE_TYPE), browsingContext); return !!mBrowserBridgeChild; } - mRemoteBrowser = ContentParent::CreateBrowser( - context, ownerElement, mBrowsingContext, openerContentParent, - sameTabGroupAs, nextTabParentId); + mRemoteBrowser = + ContentParent::CreateBrowser(context, ownerElement, openerContentParent, + sameTabGroupAs, nextTabParentId); if (!mRemoteBrowser) { return false; } @@ -2668,7 +2662,7 @@ bool nsFrameLoader::TryRemoteBrowser() { bool nsFrameLoader::IsRemoteFrame() { if (mIsRemoteFrame) { - MOZ_ASSERT(!GetDocShell(), "Found a remote frame with a DocShell"); + MOZ_ASSERT(!mDocShell, "Found a remote frame with a DocShell"); return true; } return false; @@ -2741,13 +2735,13 @@ void nsFrameLoader::ActivateFrameEvent(const nsAString& aType, bool aCapture, nsresult nsFrameLoader::CreateStaticClone(nsFrameLoader* aDest) { aDest->MaybeCreateDocShell(); - NS_ENSURE_STATE(aDest->GetDocShell()); + NS_ENSURE_STATE(aDest->mDocShell); - nsCOMPtr kungFuDeathGrip = aDest->GetDocShell()->GetDocument(); + nsCOMPtr kungFuDeathGrip = aDest->mDocShell->GetDocument(); Unused << kungFuDeathGrip; nsCOMPtr viewer; - aDest->GetDocShell()->GetContentViewer(getter_AddRefs(viewer)); + aDest->mDocShell->GetContentViewer(getter_AddRefs(viewer)); NS_ENSURE_STATE(viewer); nsIDocShell* origDocShell = GetDocShell(IgnoreErrors()); @@ -2756,7 +2750,7 @@ nsresult nsFrameLoader::CreateStaticClone(nsFrameLoader* aDest) { nsCOMPtr doc = origDocShell->GetDocument(); NS_ENSURE_STATE(doc); - nsCOMPtr clonedDoc = doc->CreateStaticClone(aDest->GetDocShell()); + nsCOMPtr clonedDoc = doc->CreateStaticClone(aDest->mDocShell); viewer->SetDocument(clonedDoc); return NS_OK; @@ -2894,13 +2888,13 @@ nsresult nsFrameLoader::EnsureMessageManager() { if (NS_FAILED(rv)) { return rv; } - MOZ_ASSERT(GetDocShell(), - "MaybeCreateDocShell succeeded, but null docShell"); - if (!GetDocShell()) { + NS_ASSERTION(mDocShell, + "MaybeCreateDocShell succeeded, but null mDocShell"); + if (!mDocShell) { return NS_ERROR_FAILURE; } mChildMessageManager = InProcessTabChildMessageManager::Create( - GetDocShell(), mOwnerContent, mMessageManager); + mDocShell, mOwnerContent, mMessageManager); NS_ENSURE_TRUE(mChildMessageManager, NS_ERROR_UNEXPECTED); } return NS_OK; @@ -2946,7 +2940,7 @@ nsIFrame* nsFrameLoader::GetDetachedSubdocFrame( } void nsFrameLoader::ApplySandboxFlags(uint32_t sandboxFlags) { - if (GetDocShell()) { + if (mDocShell) { uint32_t parentSandboxFlags = mOwnerContent->OwnerDoc()->GetSandboxFlags(); // The child can only add restrictions, never remove them. @@ -2956,11 +2950,11 @@ void nsFrameLoader::ApplySandboxFlags(uint32_t sandboxFlags) { // sandboxed auxiliary navigation flag to sandboxFlags. See // https://w3c.github.io/presentation-api/#creating-a-receiving-browsing-context nsAutoString presentationURL; - nsContentUtils::GetPresentationURL(GetDocShell(), presentationURL); + nsContentUtils::GetPresentationURL(mDocShell, presentationURL); if (!presentationURL.IsEmpty()) { sandboxFlags |= SANDBOXED_AUXILIARY_NAVIGATION; } - GetDocShell()->SetSandboxFlags(sandboxFlags); + mDocShell->SetSandboxFlags(sandboxFlags); } } @@ -2986,13 +2980,13 @@ void nsFrameLoader::AttributeChanged(mozilla::dom::Element* aElement, // Notify our enclosing chrome that our type has changed. We only do this // if our parent is chrome, since in all other cases we're random content // subframes and the treeowner shouldn't worry about us. - if (!GetDocShell()) { + if (!mDocShell) { MaybeUpdatePrimaryTabParent(eTabParentChanged); return; } nsCOMPtr parentItem; - GetDocShell()->GetParent(getter_AddRefs(parentItem)); + mDocShell->GetParent(getter_AddRefs(parentItem)); if (!parentItem) { return; } @@ -3014,16 +3008,14 @@ void nsFrameLoader::AttributeChanged(mozilla::dom::Element* aElement, // when a content panel is no longer primary, hide any open popups it may have if (!is_primary) { nsXULPopupManager* pm = nsXULPopupManager::GetInstance(); - if (pm) { - pm->HidePopupsInDocShell(GetDocShell()); - } + if (pm) pm->HidePopupsInDocShell(mDocShell); } #endif - parentTreeOwner->ContentShellRemoved(GetDocShell()); + parentTreeOwner->ContentShellRemoved(mDocShell); if (aElement->AttrValueIs(kNameSpaceID_None, TypeAttrName(), nsGkAtoms::content, eIgnoreCase)) { - parentTreeOwner->ContentShellAdded(GetDocShell(), is_primary); + parentTreeOwner->ContentShellAdded(mDocShell, is_primary); } } @@ -3128,8 +3120,7 @@ already_AddRefed nsFrameLoader::DrawSnapshot( gfx::CrossProcessPaint::StartRemote(mRemoteBrowser->GetTabId(), rect, aScale, color, promise); } else { - gfx::CrossProcessPaint::StartLocal(GetDocShell(), rect, aScale, color, - promise); + gfx::CrossProcessPaint::StartLocal(mDocShell, rect, aScale, color, promise); } return promise.forget(); @@ -3164,7 +3155,7 @@ already_AddRefed nsFrameLoader::GetBrowsingContext() { browsingContext = mBrowserBridgeChild->GetBrowsingContext(); } } else if (GetDocShell(IgnoreErrors())) { - browsingContext = GetDocShell()->GetBrowsingContext(); + browsingContext = nsDocShell::Cast(mDocShell)->GetBrowsingContext(); } return browsingContext.forget(); } @@ -3211,8 +3202,7 @@ void nsFrameLoader::StartPersistence( return; } - nsCOMPtr rootDoc = - GetDocShell() ? GetDocShell()->GetDocument() : nullptr; + nsCOMPtr rootDoc = mDocShell ? mDocShell->GetDocument() : nullptr; nsCOMPtr foundDoc; if (aOuterWindowID) { foundDoc = nsContentUtils::GetSubdocumentWithOuterWindowId(rootDoc, diff --git a/dom/base/nsFrameLoader.h b/dom/base/nsFrameLoader.h index a0ee6490d22e..1f0afc6c8967 100644 --- a/dom/base/nsFrameLoader.h +++ b/dom/base/nsFrameLoader.h @@ -20,7 +20,6 @@ #include "nsIURI.h" #include "nsFrameMessageManager.h" #include "mozilla/dom/BindingUtils.h" -#include "mozilla/dom/BrowsingContext.h" #include "mozilla/dom/Element.h" #include "mozilla/dom/ParentSHistory.h" #include "mozilla/Attributes.h" @@ -50,6 +49,7 @@ namespace mozilla { class OriginAttributes; namespace dom { +class BrowsingContext; class ChromeMessageSender; class ContentParent; class InProcessTabChildMessageManager; @@ -99,7 +99,7 @@ class nsFrameLoader final : public nsStubMutationObserver, public: // Called by Frame Elements to create a new FrameLoader. static nsFrameLoader* Create(mozilla::dom::Element* aOwner, - mozilla::dom::BrowsingContext* aOpener, + nsPIDOMWindowOuter* aOpener, bool aNetworkCreated); // Called by nsFrameLoaderOwner::ChangeRemoteness when switching out @@ -118,9 +118,7 @@ class nsFrameLoader final : public nsStubMutationObserver, void StartDestroy(); void DestroyDocShell(); void DestroyComplete(); - nsIDocShell* GetExistingDocShell() const { - return mBrowsingContext ? mBrowsingContext->GetDocShell() : nullptr; - } + nsIDocShell* GetExistingDocShell() { return mDocShell; } mozilla::dom::InProcessTabChildMessageManager* GetTabChildMessageManager() const { return mChildMessageManager; @@ -370,11 +368,9 @@ class nsFrameLoader final : public nsStubMutationObserver, JS::Handle aGivenProto) override; private: - nsFrameLoader(mozilla::dom::Element* aOwner, - mozilla::dom::BrowsingContext* aBrowsingContext, + nsFrameLoader(mozilla::dom::Element* aOwner, nsPIDOMWindowOuter* aOpener, bool aNetworkCreated); nsFrameLoader(mozilla::dom::Element* aOwner, - mozilla::dom::BrowsingContext* aBrowsingContext, const mozilla::dom::RemotenessOptions& aOptions); ~nsFrameLoader(); @@ -404,10 +400,6 @@ class nsFrameLoader final : public nsStubMutationObserver, nsresult MaybeCreateDocShell(); nsresult EnsureMessageManager(); nsresult ReallyLoadFrameScripts(); - nsDocShell* GetDocShell() const { - return mBrowsingContext ? nsDocShell::Cast(mBrowsingContext->GetDocShell()) - : nullptr; - } // Updates the subdocument position and size. This gets called only // when we have our own in-process DocShell. @@ -451,7 +443,7 @@ class nsFrameLoader final : public nsStubMutationObserver, nsresult PopulateUserContextIdFromAttribute(mozilla::OriginAttributes& aAttr); - RefPtr mBrowsingContext; + RefPtr mDocShell; nsCOMPtr mURIToLoad; nsCOMPtr mTriggeringPrincipal; nsCOMPtr mCsp; @@ -472,6 +464,9 @@ class nsFrameLoader final : public nsStubMutationObserver, // a reframe, so that we know not to restore the presentation. RefPtr mContainerDocWhileDetached; + // An opener window which should be used when the docshell is created. + nsCOMPtr mOpener; + RefPtr mRemoteBrowser; uint64_t mChildID; diff --git a/dom/base/nsGlobalWindowOuter.cpp b/dom/base/nsGlobalWindowOuter.cpp index fb641e24e4ba..c3917c8a4880 100644 --- a/dom/base/nsGlobalWindowOuter.cpp +++ b/dom/base/nsGlobalWindowOuter.cpp @@ -2468,7 +2468,6 @@ void nsGlobalWindowOuter::DetachFromDocShell() { } mDocShell = nullptr; - mBrowsingContext->ClearDocShell(); if (mFrames) { mFrames->SetDocShell(nullptr); diff --git a/dom/html/nsGenericHTMLFrameElement.cpp b/dom/html/nsGenericHTMLFrameElement.cpp index 6a58074688b8..5e38df4b05f0 100644 --- a/dom/html/nsGenericHTMLFrameElement.cpp +++ b/dom/html/nsGenericHTMLFrameElement.cpp @@ -133,7 +133,9 @@ void nsGenericHTMLFrameElement::EnsureFrameLoader() { // Strangely enough, this method doesn't actually ensure that the // frameloader exists. It's more of a best-effort kind of thing. - mFrameLoader = nsFrameLoader::Create(this, mOpenerWindow, mNetworkCreated); + mFrameLoader = nsFrameLoader::Create( + this, mOpenerWindow ? mOpenerWindow->GetDOMWindow() : nullptr, + mNetworkCreated); } nsresult nsGenericHTMLFrameElement::CreateRemoteFrameLoader( diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index cdf579754728..c8b581193d4d 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -1087,7 +1087,6 @@ mozilla::ipc::IPCResult ContentParent::RecvLaunchRDDProcess( /*static*/ TabParent* ContentParent::CreateBrowser(const TabContext& aContext, Element* aFrameElement, - BrowsingContext* aBrowsingContext, ContentParent* aOpenerContentParent, TabParent* aSameTabGroupAs, uint64_t aNextTabParentId) { @@ -1148,9 +1147,16 @@ TabParent* ContentParent::CreateBrowser(const TabContext& aContext, } } + // FIXME: This BrowsingContext should be provided by the nsFrameLoader. + // (bug 1523636) + RefPtr browsingContext = + BrowsingContext::Create(nullptr, nullptr, EmptyString(), + BrowsingContext::Type::Content) + .downcast(); + // Ensure that our content process is subscribed to our newly created // BrowsingContextGroup. - aBrowsingContext->Group()->EnsureSubscribed(constructorSender); + browsingContext->Group()->EnsureSubscribed(constructorSender); ContentProcessManager* cpm = ContentProcessManager::GetSingleton(); cpm->RegisterRemoteFrame(tabId, ContentParentId(0), openerTabId, @@ -1182,19 +1188,17 @@ TabParent* ContentParent::CreateBrowser(const TabContext& aContext, if (tabId == 0) { return nullptr; } - RefPtr tp = - new TabParent(constructorSender, tabId, aContext, - aBrowsingContext->Canonical(), chromeFlags); + RefPtr tp = new TabParent(constructorSender, tabId, aContext, + browsingContext, chromeFlags); - aBrowsingContext->Canonical()->SetOwnerProcessId( - constructorSender->ChildID()); + browsingContext->SetOwnerProcessId(constructorSender->ChildID()); PBrowserParent* browser = constructorSender->SendPBrowserConstructor( // DeallocPBrowserParent() releases this ref. tp.forget().take(), tabId, aSameTabGroupAs ? aSameTabGroupAs->GetTabId() : TabId(0), aContext.AsIPCTabContext(), chromeFlags, constructorSender->ChildID(), - aBrowsingContext, constructorSender->IsForBrowser()); + browsingContext, constructorSender->IsForBrowser()); if (remoteType.EqualsLiteral(LARGE_ALLOCATION_REMOTE_TYPE)) { // Tell the TabChild object that it was created due to a Large-Allocation diff --git a/dom/ipc/ContentParent.h b/dom/ipc/ContentParent.h index 9fee6c425f0c..9513444e500e 100644 --- a/dom/ipc/ContentParent.h +++ b/dom/ipc/ContentParent.h @@ -208,7 +208,6 @@ class ContentParent final : public PContentParent, */ static TabParent* CreateBrowser(const TabContext& aContext, Element* aFrameElement, - BrowsingContext* aBrowsingContext, ContentParent* aOpenerContentParent, TabParent* aSameTabGroupAs, uint64_t aNextTabParentId); diff --git a/dom/ipc/tests/test_force_oop_iframe.html b/dom/ipc/tests/test_force_oop_iframe.html index 31d00242a2cd..b3343b59b62e 100644 --- a/dom/ipc/tests/test_force_oop_iframe.html +++ b/dom/ipc/tests/test_force_oop_iframe.html @@ -47,9 +47,6 @@ add_task(async function() { let frameLoader = SpecialPowers.wrap(iframe).frameLoader; is(frameLoader.docShell, null); is(frameLoader.tabParent, null); - let browsingContext = frameLoader.browsingContext; - isnot(browsingContext, null); - is(browsingContext.docShell, null); await contentCreated; diff --git a/dom/xul/XULFrameElement.cpp b/dom/xul/XULFrameElement.cpp index 63616133e6dd..0a01f2df70da 100644 --- a/dom/xul/XULFrameElement.cpp +++ b/dom/xul/XULFrameElement.cpp @@ -92,7 +92,8 @@ void XULFrameElement::LoadSrc() { // session history handling works like dynamic html:iframes. // Usually xul elements are used in chrome, which doesn't have // session history at all. - mFrameLoader = nsFrameLoader::Create(this, opener, false); + mFrameLoader = nsFrameLoader::Create( + this, opener ? opener->GetDOMWindow() : nullptr, false); if (NS_WARN_IF(!mFrameLoader)) { return; } From 02079beb91a37b8b1f3a16636f3621f567ef2fff Mon Sep 17 00:00:00 2001 From: Coroiu Cristina Date: Wed, 10 Apr 2019 20:31:00 +0300 Subject: [PATCH 20/22] Backed out changeset 34e912d9305a (bug 1543217) chrome failures at build/src/gfx/config/gfxFeature.cpp --- gfx/thebes/gfxPlatform.cpp | 30 ++++-------------------------- gfx/thebes/gfxPlatform.h | 2 +- gfx/thebes/gfxPlatformGtk.cpp | 2 +- widget/GfxInfoX11.cpp | 28 ---------------------------- widget/windows/GfxInfo.cpp | 6 +++--- 5 files changed, 9 insertions(+), 59 deletions(-) diff --git a/gfx/thebes/gfxPlatform.cpp b/gfx/thebes/gfxPlatform.cpp index 251138e76f58..0a930430e820 100644 --- a/gfx/thebes/gfxPlatform.cpp +++ b/gfx/thebes/gfxPlatform.cpp @@ -923,7 +923,6 @@ void gfxPlatform::Init() { #else # error "No gfxPlatform implementation available" #endif - gPlatform->PopulateScreenInfo(); gPlatform->InitAcceleration(); gPlatform->InitWebRenderConfig(); // When using WebRender, we defer initialization of the D3D11 devices until @@ -961,6 +960,7 @@ void gfxPlatform::Init() { InitLayersIPC(); + gPlatform->PopulateScreenInfo(); gPlatform->ComputeTileSize(); #ifdef MOZ_ENABLE_FREETYPE @@ -2511,7 +2511,7 @@ static bool CalculateWrQualifiedPrefValue() { } static FeatureState& WebRenderHardwareQualificationStatus( - const IntSize& aScreenSize, bool aHasBattery, nsCString& aOutFailureId) { + bool aHasBattery, nsCString& aOutFailureId) { FeatureState& featureWebRenderQualified = gfxConfig::GetFeature(Feature::WEBRENDER_QUALIFIED); featureWebRenderQualified.EnableByDefault(); @@ -2576,8 +2576,7 @@ static FeatureState& WebRenderHardwareQualificationStatus( FeatureStatus::Blocked, "Device too old", NS_LITERAL_CSTRING("FEATURE_FAILURE_DEVICE_TOO_OLD")); } - } else if (adapterVendorID == u"0x8086" || - adapterVendorID == u"mesa/i965") { // Intel + } else if (adapterVendorID == u"0x8086") { // Intel const uint16_t supportedDevices[] = { 0x191d, // HD Graphics P530 0x192d, // Iris Pro Graphics P555 @@ -2606,18 +2605,6 @@ static FeatureState& WebRenderHardwareQualificationStatus( featureWebRenderQualified.Disable( FeatureStatus::Blocked, "Device too old", NS_LITERAL_CSTRING("FEATURE_FAILURE_DEVICE_TOO_OLD")); - } else if (adapterVendorID == u"mesa/i965") { - const int32_t maxPixels = 3440 * 1440; // UWQHD - int32_t pixels = aScreenSize.width * aScreenSize.height; - if (pixels > maxPixels) { - featureWebRenderQualified.Disable( - FeatureStatus::Blocked, "Screen size too large", - NS_LITERAL_CSTRING("FEATURE_FAILURE_SCREEN_SIZE_TOO_LARGE")); - } else if (pixels <= 0) { - featureWebRenderQualified.Disable( - FeatureStatus::Blocked, "Screen size unknown", - NS_LITERAL_CSTRING("FEATURE_FAILURE_SCREEN_SIZE_UNKNOWN")); - } } #endif } else { @@ -2666,8 +2653,7 @@ void gfxPlatform::InitWebRenderConfig() { nsCString failureId; FeatureState& featureWebRenderQualified = - WebRenderHardwareQualificationStatus(GetScreenSize(), HasBattery(), - failureId); + WebRenderHardwareQualificationStatus(HasBattery(), failureId); FeatureState& featureWebRender = gfxConfig::GetFeature(Feature::WEBRENDER); featureWebRender.DisableByDefault( @@ -2762,14 +2748,6 @@ void gfxPlatform::InitWebRenderConfig() { WebRenderDebugPrefChangeCallback, WR_DEBUG_PREF); } } -#ifdef XP_LINUX - else { - // Hardware compositing should be disabled by default if we aren't using - // WebRender, but may still have been enabled by prefs. - gfxConfig::Disable(Feature::HW_COMPOSITING, FeatureStatus::Blocked, - "Acceleration blocked by platform"); - } -#endif #ifdef XP_WIN if (Preferences::GetBool("gfx.webrender.dcomp-win.enabled", false)) { diff --git a/gfx/thebes/gfxPlatform.h b/gfx/thebes/gfxPlatform.h index 896dff0452ff..34cf85857b72 100644 --- a/gfx/thebes/gfxPlatform.h +++ b/gfx/thebes/gfxPlatform.h @@ -736,7 +736,7 @@ class gfxPlatform : public mozilla::layers::MemoryPressureListener { gfxPlatform(); virtual ~gfxPlatform(); - virtual bool HasBattery() { return false; } + virtual bool HasBattery() { return true; } virtual void InitAcceleration(); virtual void InitWebRenderConfig(); diff --git a/gfx/thebes/gfxPlatformGtk.cpp b/gfx/thebes/gfxPlatformGtk.cpp index 0a6ecc42342d..144af010f76a 100644 --- a/gfx/thebes/gfxPlatformGtk.cpp +++ b/gfx/thebes/gfxPlatformGtk.cpp @@ -330,7 +330,7 @@ uint32_t gfxPlatformGtk::MaxGenericSubstitions() { } bool gfxPlatformGtk::AccelerateLayersByDefault() { - return true; + return gfxPrefs::WebRenderAll(); } void gfxPlatformGtk::GetPlatformCMSOutputProfile(void*& mem, size_t& size) { diff --git a/widget/GfxInfoX11.cpp b/widget/GfxInfoX11.cpp index 13777676add5..beb720660d40 100644 --- a/widget/GfxInfoX11.cpp +++ b/widget/GfxInfoX11.cpp @@ -307,34 +307,6 @@ const nsTArray &GfxInfo::GetGfxDriverInfo() { GfxDriverInfo::allDevices, GfxDriverInfo::allFeatures, nsIGfxInfo::FEATURE_BLOCKED_DRIVER_VERSION, DRIVER_LESS_THAN, V(13, 15, 100, 1), "FEATURE_FAILURE_OLD_FGLRX", "fglrx 13.15.100.1"); - - //////////////////////////////////// - // FEATURE_WEBRENDER - - // Mesa baseline (chosen arbitrarily as that which ships with - // Ubuntu 18.04 LTS). - APPEND_TO_DRIVER_BLOCKLIST( - OperatingSystem::Linux, - (nsAString &)GfxDriverInfo::GetDeviceVendor(VendorMesaAll), - GfxDriverInfo::allDevices, nsIGfxInfo::FEATURE_WEBRENDER, - nsIGfxInfo::FEATURE_BLOCKED_DRIVER_VERSION, DRIVER_LESS_THAN, - V(18, 2, 8, 0), "FEATURE_FAILURE_WEBRENDER_OLD_MESA", "Mesa 18.2.8.0"); - - // Disable on all NVIDIA devices for now. - APPEND_TO_DRIVER_BLOCKLIST( - OperatingSystem::Linux, - (nsAString &)GfxDriverInfo::GetDeviceVendor(VendorNVIDIA), - GfxDriverInfo::allDevices, nsIGfxInfo::FEATURE_WEBRENDER, - nsIGfxInfo::FEATURE_BLOCKED_DEVICE, DRIVER_COMPARISON_IGNORED, - V(0, 0, 0, 0), "FEATURE_FAILURE_WEBRENDER_NO_LINUX_NVIDIA", ""); - - // Disable on all ATI devices for now. - APPEND_TO_DRIVER_BLOCKLIST( - OperatingSystem::Linux, - (nsAString &)GfxDriverInfo::GetDeviceVendor(VendorATI), - GfxDriverInfo::allDevices, nsIGfxInfo::FEATURE_WEBRENDER, - nsIGfxInfo::FEATURE_BLOCKED_DEVICE, DRIVER_COMPARISON_IGNORED, - V(0, 0, 0, 0), "FEATURE_FAILURE_WEBRENDER_NO_LINUX_ATI", ""); } return *sDriverInfo; } diff --git a/widget/windows/GfxInfo.cpp b/widget/windows/GfxInfo.cpp index e5213b20a7f1..fe91244fbacb 100644 --- a/widget/windows/GfxInfo.cpp +++ b/widget/windows/GfxInfo.cpp @@ -1602,9 +1602,9 @@ const nsTArray& GfxInfo::GetGfxDriverInfo() { //////////////////////////////////// // FEATURE_WEBRENDER - // We are blocking most hardware explicitly in gfxPlatform.cpp where we - // check for the WEBRENDER_QUALIFIED feature. However we also want to block - // some specific Nvidia cards for being too low-powered, so we do that here. + // We are blocking all non-Nvidia cards in gfxPlatform.cpp where we check + // for the WEBRENDER_QUALIFIED feature. However we also want to block some + // specific Nvidia cards for being too low-powered, so we do that here. APPEND_TO_DRIVER_BLOCKLIST2( OperatingSystem::Windows10, (nsAString&)GfxDriverInfo::GetDeviceVendor(VendorNVIDIA), From 6d6f6dadce80a73644bc2e5fb488b81e79f2eeae Mon Sep 17 00:00:00 2001 From: Daniel Varga Date: Wed, 10 Apr 2019 21:09:46 +0300 Subject: [PATCH 21/22] Backed out 4 changesets (bug 1458385) for nightly updates fail. a=backout Backed out changeset 09338587b68e (bug 1458385) Backed out changeset f7791b680d46 (bug 1458385) Backed out changeset cc06a7beb3d1 (bug 1458385) Backed out changeset 3b10487587c3 (bug 1458385) --- build/sparse-profiles/taskgraph | 7 --- mfbt/moz.build | 2 +- other-licenses/bsdiff/moz.build | 1 - taskcluster/ci/repackage-l10n/kind.yml | 7 --- taskcluster/ci/repackage-msi/kind.yml | 3 - taskcluster/ci/repackage/kind.yml | 7 --- taskcluster/ci/toolchain/mingw.yml | 8 +++ taskcluster/ci/toolchain/misc.yml | 26 -------- taskcluster/docker/debian-build/Dockerfile | 1 - taskcluster/docker/mingw32-build/Dockerfile | 2 +- taskcluster/docker/toolchain-build/Dockerfile | 1 - taskcluster/scripts/misc/build-mar-tools.sh | 31 ---------- taskcluster/scripts/misc/build-upx.sh | 3 +- taskcluster/taskgraph/transforms/repackage.py | 61 ++++++++++++++----- .../taskgraph/transforms/repackage_partner.py | 15 ++++- .../configs/repackage/win32_partner.py | 8 +-- .../configs/repackage/win32_signed.py | 9 +-- .../configs/repackage/win64_partner.py | 8 +-- .../configs/repackage/win64_signed.py | 9 +-- testing/mozharness/scripts/repackage.py | 1 - tools/update-packaging/app.mozbuild | 9 --- tools/update-packaging/moz.configure | 0 22 files changed, 88 insertions(+), 131 deletions(-) delete mode 100755 taskcluster/scripts/misc/build-mar-tools.sh delete mode 100644 tools/update-packaging/app.mozbuild delete mode 100644 tools/update-packaging/moz.configure diff --git a/build/sparse-profiles/taskgraph b/build/sparse-profiles/taskgraph index 3df3d0b0b514..9a9d70ac9b88 100644 --- a/build/sparse-profiles/taskgraph +++ b/build/sparse-profiles/taskgraph @@ -56,10 +56,3 @@ path:.cron.yml # for the wrench-deps toolchain task path:gfx/wr/Cargo.lock - -# for the mar-tools toolchain task -path:mfbt/ -path:modules/libmar/ -path:other-licenses/bsdiff/ -path:other-licenses/nsis/Contrib/CityHash/cityhash/ -path:toolkit/mozapps/update/updater diff --git a/mfbt/moz.build b/mfbt/moz.build index 4d99966de045..5aa697a5e603 100644 --- a/mfbt/moz.build +++ b/mfbt/moz.build @@ -155,7 +155,7 @@ UNIFIED_SOURCES += [ 'Utf8.cpp', ] -if CONFIG['MOZ_BUILD_APP'] not in ('memory', 'tools/crashreporter', 'tools/update-packaging'): +if CONFIG['MOZ_BUILD_APP'] not in ('memory', 'tools/crashreporter'): # RecordReplay.cpp uses files from js/ UNIFIED_SOURCES += [ 'RecordReplay.cpp', diff --git a/other-licenses/bsdiff/moz.build b/other-licenses/bsdiff/moz.build index c7080a6e249f..424e096393b4 100644 --- a/other-licenses/bsdiff/moz.build +++ b/other-licenses/bsdiff/moz.build @@ -18,5 +18,4 @@ if CONFIG['HOST_OS_ARCH'] == 'WINNT': LOCAL_INCLUDES += [ '/toolkit/mozapps/update/updater', - '/toolkit/mozapps/update/updater/bspatch', ] diff --git a/taskcluster/ci/repackage-l10n/kind.yml b/taskcluster/ci/repackage-l10n/kind.yml index 5c9a68813fdf..57c24ac11894 100644 --- a/taskcluster/ci/repackage-l10n/kind.yml +++ b/taskcluster/ci/repackage-l10n/kind.yml @@ -37,9 +37,6 @@ only-for-build-platforms: - win64-aarch64-devedition-nightly/opt job-template: - worker-type: 'aws-provisioner-v1/gecko-{level}-b-linux' - worker: - docker-image: {"in-tree": "debian7-amd64-build"} mozharness: config: by-build-platform: @@ -80,7 +77,3 @@ job-template: macosx64\b.*: [mar, dmg] win32\b.*: [mar, installer] win64\b.*: [mar, installer] - fetches: - toolchain: - - linux64-mar-tools - - linux64-upx diff --git a/taskcluster/ci/repackage-msi/kind.yml b/taskcluster/ci/repackage-msi/kind.yml index b727b19eb3d4..13c7c6f08aa2 100644 --- a/taskcluster/ci/repackage-msi/kind.yml +++ b/taskcluster/ci/repackage-msi/kind.yml @@ -25,10 +25,7 @@ only-for-build-platforms: - win64-devedition-nightly/opt job-template: - worker-type: 'aws-provisioner-v1/gecko-{level}-b-win2012' mozharness: - use-magic-mh-args: false - use-caches: false config: by-build-platform: win32\b.*: diff --git a/taskcluster/ci/repackage/kind.yml b/taskcluster/ci/repackage/kind.yml index f839e175ba65..b0d0686f16a7 100644 --- a/taskcluster/ci/repackage/kind.yml +++ b/taskcluster/ci/repackage/kind.yml @@ -47,9 +47,6 @@ only-for-build-platforms: - win64-asan-reporter-nightly/opt job-template: - worker-type: 'aws-provisioner-v1/gecko-{level}-b-linux' - worker: - docker-image: {"in-tree": "debian7-amd64-build"} mozharness: config: by-build-platform: @@ -90,7 +87,3 @@ job-template: macosx64\b.*: [mar, dmg] win32\b.*: [mar, installer] win64\b.*: [mar, installer] - fetches: - toolchain: - - linux64-mar-tools - - linux64-upx diff --git a/taskcluster/ci/toolchain/mingw.yml b/taskcluster/ci/toolchain/mingw.yml index f0a2783860cd..c1c0145084a7 100644 --- a/taskcluster/ci/toolchain/mingw.yml +++ b/taskcluster/ci/toolchain/mingw.yml @@ -10,6 +10,14 @@ job-defaults: docker-image: {in-tree: mingw32-build} max-run-time: 3600 +linux64-upx: + description: "UPX build for MinGW32 Cross Compile" + treeherder: + symbol: TMW(upx) + run: + script: build-upx.sh + toolchain-artifact: public/build/upx.tar.xz + linux64-wine: description: "Wine build for MinGW32 Cross Compile" treeherder: diff --git a/taskcluster/ci/toolchain/misc.yml b/taskcluster/ci/toolchain/misc.yml index a663d6c93f8a..3767e875b3d9 100644 --- a/taskcluster/ci/toolchain/misc.yml +++ b/taskcluster/ci/toolchain/misc.yml @@ -70,23 +70,6 @@ linux64-libdmg: script: build-libdmg-hfsplus.sh toolchain-artifact: public/build/dmg.tar.xz -linux64-mar-tools: - description: "mar-tools toolchain build" - treeherder: - symbol: TL(mar-tools) - toolchains: - - linux64-clang-7 - - linux64-binutils - run: - script: build-mar-tools.sh - sparse-profile: null - toolchain-artifact: public/build/mar-tools.tar.xz - resources: - - modules/libmar/ - - other-licenses/nsis/Contrib/CityHash/cityhash/ - - other-licenses/bsdiff/ - - toolkit/mozapps/update/updater/bspatch/ - linux64-tup: description: "tup toolchain build" treeherder: @@ -104,15 +87,6 @@ linux64-tup: toolchains: - linux64-gcc-6 -linux64-upx: - description: "UPX build" - treeherder: - symbol: TL(upx) - tier: 1 - run: - script: build-upx.sh - toolchain-artifact: public/build/upx.tar.xz - linux64-custom-v8: description: "Custom v8 build" treeherder: diff --git a/taskcluster/docker/debian-build/Dockerfile b/taskcluster/docker/debian-build/Dockerfile index 1940920cdba9..5bf9fb2f1e6e 100644 --- a/taskcluster/docker/debian-build/Dockerfile +++ b/taskcluster/docker/debian-build/Dockerfile @@ -44,7 +44,6 @@ RUN apt-get update && \ gawk \ gcc-multilib \ gnupg \ - libucl1 \ p7zip-full \ procps \ python-pip \ diff --git a/taskcluster/docker/mingw32-build/Dockerfile b/taskcluster/docker/mingw32-build/Dockerfile index 44e427fb3bfb..7d0bd67289f4 100644 --- a/taskcluster/docker/mingw32-build/Dockerfile +++ b/taskcluster/docker/mingw32-build/Dockerfile @@ -24,7 +24,7 @@ RUN apt-get update && \ gawk \ g++-multilib \ gnupg \ - libucl1 \ + libucl-dev \ patch \ p7zip-full \ scons \ diff --git a/taskcluster/docker/toolchain-build/Dockerfile b/taskcluster/docker/toolchain-build/Dockerfile index d42c8d4a6dac..8e532d9dc100 100644 --- a/taskcluster/docker/toolchain-build/Dockerfile +++ b/taskcluster/docker/toolchain-build/Dockerfile @@ -29,7 +29,6 @@ RUN apt-get update && \ libcurl4-openssl-dev \ libssl-dev \ libtool \ - libucl-dev \ ninja-build \ p7zip-full \ procps \ diff --git a/taskcluster/scripts/misc/build-mar-tools.sh b/taskcluster/scripts/misc/build-mar-tools.sh deleted file mode 100755 index 2313f3125424..000000000000 --- a/taskcluster/scripts/misc/build-mar-tools.sh +++ /dev/null @@ -1,31 +0,0 @@ -#!/bin/bash -set -x -e -v - -# This script is for building mar and mbsdiff - -WORKSPACE=$HOME/workspace -UPLOAD_DIR=$HOME/artifacts -COMPRESS_EXT=xz - -cd $WORKSPACE/build/src - -. taskcluster/scripts/misc/tooltool-download.sh - -export MOZ_OBJDIR=obj-mar - -echo ac_add_options --enable-project=tools/update-packaging > .mozconfig - -TOOLCHAINS="binutils clang" - -for t in $TOOLCHAINS; do - PATH="$WORKSPACE/build/src/$t/bin:$PATH" -done - -./mach build -v - -mkdir mar-tools -cp $MOZ_OBJDIR/dist/host/bin/{mar,mbsdiff} mar-tools/ - -tar -acf mar-tools.tar.$COMPRESS_EXT mar-tools/ -mkdir -p $UPLOAD_DIR -cp mar-tools.tar.$COMPRESS_EXT $UPLOAD_DIR diff --git a/taskcluster/scripts/misc/build-upx.sh b/taskcluster/scripts/misc/build-upx.sh index e6c06f0b8c6e..9a3b7b15f929 100755 --- a/taskcluster/scripts/misc/build-upx.sh +++ b/taskcluster/scripts/misc/build-upx.sh @@ -12,8 +12,7 @@ cd $WORKSPACE git clone -n https://github.com/upx/upx.git upx-clone cd upx-clone -# https://github.com/upx/upx/releases/tag/v3.95 -git checkout 7a3637ff5a800b8bcbad20ae7f668d8c8449b014 # Asserts integrity of the clone (right?) +git checkout d31947e1f016e87f24f88b944439bbee892f0429 # Asserts integrity of the clone (right?) git submodule update --init --recursive cd src make -j$(nproc) diff --git a/taskcluster/taskgraph/transforms/repackage.py b/taskcluster/taskgraph/transforms/repackage.py index df58ab20df89..39f2efce8f84 100644 --- a/taskcluster/taskgraph/transforms/repackage.py +++ b/taskcluster/taskgraph/transforms/repackage.py @@ -17,10 +17,10 @@ from taskgraph.util.schema import ( resolve_keyed_by, ) from taskgraph.util.taskcluster import get_artifact_prefix -from taskgraph.util.platforms import archive_format, architecture +from taskgraph.util.platforms import archive_format, executable_extension, architecture from taskgraph.util.workertypes import worker_type_implementation from taskgraph.transforms.job import job_description_schema -from voluptuous import Required, Optional, Extra +from voluptuous import Required, Optional packaging_description_schema = schema.extend({ @@ -30,9 +30,6 @@ packaging_description_schema = schema.extend({ # unique label to describe this repackaging task Optional('label'): basestring, - Optional('worker-type'): basestring, - Optional('worker'): object, - # treeherder is allowed here to override any defaults we use for repackaging. See # taskcluster/taskgraph/transforms/task.py for the schema details, and the # below transforms for defaults of various values. @@ -59,7 +56,6 @@ packaging_description_schema = schema.extend({ # All l10n jobs use mozharness Required('mozharness'): { - Extra: object, # Config files passed to the mozharness script Required('config'): optionally_keyed_by('build-platform', [basestring]), @@ -80,7 +76,7 @@ packaging_description_schema = schema.extend({ # from mozharness. # - `inputs` are passed as long-options, with the filename prefixed by # `MOZ_FETCH_DIR`. The filename is interpolated by taskgraph -# (`{archive_format}`). +# (`{archive_format}`, `{executable_extension}`). # - `output` is passed to `--output`, with the filename prefixed by the output # directory. PACKAGE_FORMATS = { @@ -91,7 +87,7 @@ PACKAGE_FORMATS = { ], 'inputs': { 'input': 'target{archive_format}', - 'mar': 'mar-tools/mar', + 'mar': 'mar{executable_extension}', }, 'output': "target.complete.mar", }, @@ -102,7 +98,7 @@ PACKAGE_FORMATS = { ], 'inputs': { 'input': 'target{archive_format}', - 'mar': 'mar', + 'mar': 'mar{executable_extension}', }, 'output': "target.bz2.complete.mar", }, @@ -209,6 +205,10 @@ def make_job_description(config, jobs): # repackage-signing can end up with multiple deps... raise NotImplementedError( "Can't repackage a signing task with multiple dependencies") + signing_dependencies = dep_job.dependencies + # This is so we get the build task in our dependencies to + # have better beetmover support. + dependencies.update(signing_dependencies) attributes = copy_attributes_from_dependent_job(dep_job) attributes['repackage_type'] = 'repackage' @@ -230,6 +230,7 @@ def make_job_description(config, jobs): if config.kind == 'repackage-msi': treeherder['symbol'] = 'MSI({})'.format(locale or 'N') + build_task = None signing_task = None repackage_signing_task = None for dependency in dependencies.keys(): @@ -237,12 +238,21 @@ def make_job_description(config, jobs): repackage_signing_task = dependency elif 'signing' in dependency: signing_task = dependency + else: + build_task = dependency _fetch_subst_locale = 'en-US' if locale: + # XXXCallek: todo: rewrite dependency finding + # Use string splice to strip out 'nightly-l10n-' .. '-/opt' + # We need this additional dependency to support finding the mar binary + # Which is needed in order to generate a new complete.mar + dependencies['build'] = "build-{}/opt".format( + dependencies[build_task][13:dependencies[build_task].rfind('-')]) + build_task = 'build' _fetch_subst_locale = locale - worker_type = job['worker-type'] + level = config.params['level'] build_platform = attributes['build_platform'] use_stub = attributes.get('stub-installer') @@ -256,6 +266,7 @@ def make_job_description(config, jobs): command = copy.deepcopy(PACKAGE_FORMATS[format]) substs = { 'archive_format': archive_format(build_platform), + 'executable_extension': executable_extension(build_platform), '_locale': _fetch_subst_locale, 'architecture': architecture(build_platform), 'version_display': config.params['version'], @@ -287,18 +298,32 @@ def make_job_description(config, jobs): }, }) - worker = job.get('worker', {}) - worker.update({ + worker = { 'chain-of-trust': True, 'max-run-time': 7200 if build_platform.startswith('win') else 3600, # Don't add generic artifact directory. 'skip-artifacts': True, - }) + } if locale: # Make sure we specify the locale-specific upload dir worker.setdefault('env', {}).update(LOCALE=locale) + if build_platform.startswith('win'): + worker_type = 'aws-provisioner-v1/gecko-%s-b-win2012' % level + run['use-magic-mh-args'] = False + run['use-caches'] = False + else: + if build_platform.startswith(('linux', 'macosx')): + worker_type = 'aws-provisioner-v1/gecko-%s-b-linux' % level + else: + raise NotImplementedError( + 'Unsupported build_platform: "{}"'.format(build_platform) + ) + + run['tooltool-downloads'] = 'internal' + worker['docker-image'] = {"in-tree": "debian7-amd64-build"} + worker['artifacts'] = _generate_task_output_files( dep_job, worker_type_implementation(config.graph_config, worker_type), repackage_config=repackage_config, @@ -326,7 +351,7 @@ def make_job_description(config, jobs): 'extra': job.get('extra', {}), 'worker': worker, 'run': run, - 'fetches': _generate_download_config(dep_job, build_platform, + 'fetches': _generate_download_config(dep_job, build_platform, build_task, signing_task, repackage_signing_task, locale=locale, project=config.params["project"], @@ -343,7 +368,7 @@ def make_job_description(config, jobs): yield task -def _generate_download_config(task, build_platform, signing_task, +def _generate_download_config(task, build_platform, build_task, signing_task, repackage_signing_task, locale=None, project=None, existing_fetch=None): locale_path = '{}/'.format(locale) if locale else '' @@ -365,6 +390,9 @@ def _generate_download_config(task, build_platform, signing_task, 'extract': False, }, ], + build_task: [ + 'host/bin/mar', + ], }) elif build_platform.startswith('win'): fetch.update({ @@ -375,6 +403,9 @@ def _generate_download_config(task, build_platform, signing_task, }, '{}setup.exe'.format(locale_path), ], + build_task: [ + 'host/bin/mar.exe', + ], }) use_stub = task.attributes.get('stub-installer') diff --git a/taskcluster/taskgraph/transforms/repackage_partner.py b/taskcluster/taskgraph/transforms/repackage_partner.py index b8b77b1aed8f..8a78d6359701 100644 --- a/taskcluster/taskgraph/transforms/repackage_partner.py +++ b/taskcluster/taskgraph/transforms/repackage_partner.py @@ -167,8 +167,19 @@ def make_job_description(config, jobs): 'skip-artifacts': True, } - worker_type = 'aws-provisioner-v1/gecko-%s-b-linux' % level - worker['docker-image'] = {"in-tree": "debian7-amd64-build"} + if build_platform.startswith('win'): + worker_type = 'aws-provisioner-v1/gecko-%s-b-win2012' % level + run['use-magic-mh-args'] = False + else: + if build_platform.startswith('macosx'): + worker_type = 'aws-provisioner-v1/gecko-%s-b-linux' % level + else: + raise NotImplementedError( + 'Unsupported build_platform: "{}"'.format(build_platform) + ) + + run['tooltool-downloads'] = 'internal' + worker['docker-image'] = {"in-tree": "debian7-amd64-build"} worker['artifacts'] = _generate_task_output_files( dep_job, worker_type_implementation(config.graph_config, worker_type), diff --git a/testing/mozharness/configs/repackage/win32_partner.py b/testing/mozharness/configs/repackage/win32_partner.py index f73ce73c0416..b0be32fdf634 100644 --- a/testing/mozharness/configs/repackage/win32_partner.py +++ b/testing/mozharness/configs/repackage/win32_partner.py @@ -5,10 +5,10 @@ platform = "win32" config = { "repack_id": os.environ.get("REPACK_ID"), + # ToolTool + "tooltool_manifest_src": 'browser\\config\\tooltool-manifests\\{}\\releng.manifest'.format(platform), 'tooltool_url': 'https://tooltool.mozilla-releng.net/', - 'run_configure': False, + 'tooltool_cache': os.environ.get('TOOLTOOL_CACHE'), - 'env': { - 'PATH': "%(abs_input_dir)s/upx/bin:%(PATH)s", - } + 'run_configure': False, } diff --git a/testing/mozharness/configs/repackage/win32_signed.py b/testing/mozharness/configs/repackage/win32_signed.py index f07076f832a9..6f59b1501d04 100644 --- a/testing/mozharness/configs/repackage/win32_signed.py +++ b/testing/mozharness/configs/repackage/win32_signed.py @@ -5,9 +5,10 @@ platform = "win32" config = { "locale": os.environ.get("LOCALE"), - 'run_configure': False, + # ToolTool + "tooltool_manifest_src": 'browser\\config\\tooltool-manifests\\{}\\releng.manifest'.format(platform), + 'tooltool_url': 'https://tooltool.mozilla-releng.net/', + 'tooltool_cache': os.environ.get('TOOLTOOL_CACHE'), - 'env': { - 'PATH': "%(abs_input_dir)s/upx/bin:%(PATH)s", - } + 'run_configure': False, } diff --git a/testing/mozharness/configs/repackage/win64_partner.py b/testing/mozharness/configs/repackage/win64_partner.py index 54ec61f3a629..e524900ffd2e 100644 --- a/testing/mozharness/configs/repackage/win64_partner.py +++ b/testing/mozharness/configs/repackage/win64_partner.py @@ -5,10 +5,10 @@ platform = "win64" config = { "repack_id": os.environ.get("REPACK_ID"), + # ToolTool + "tooltool_manifest_src": 'browser\\config\\tooltool-manifests\\{}\\releng.manifest'.format(platform), 'tooltool_url': 'https://tooltool.mozilla-releng.net/', - 'run_configure': False, + 'tooltool_cache': os.environ.get('TOOLTOOL_CACHE'), - 'env': { - 'PATH': "%(abs_input_dir)s/upx/bin:%(PATH)s", - } + 'run_configure': False, } diff --git a/testing/mozharness/configs/repackage/win64_signed.py b/testing/mozharness/configs/repackage/win64_signed.py index ff359334be3c..13691abda962 100644 --- a/testing/mozharness/configs/repackage/win64_signed.py +++ b/testing/mozharness/configs/repackage/win64_signed.py @@ -5,9 +5,10 @@ platform = "win64" config = { "locale": os.environ.get("LOCALE"), - 'run_configure': False, + # ToolTool + "tooltool_manifest_src": 'browser\\config\\tooltool-manifests\\{}\\releng.manifest'.format(platform), + 'tooltool_url': 'https://tooltool.mozilla-releng.net/', + 'tooltool_cache': os.environ.get('TOOLTOOL_CACHE'), - 'env': { - 'PATH': "%(abs_input_dir)s/upx/bin:%(PATH)s", - } + 'run_configure': False, } diff --git a/testing/mozharness/scripts/repackage.py b/testing/mozharness/scripts/repackage.py index ab6ccfccd2b6..8ecf90bd5baf 100644 --- a/testing/mozharness/scripts/repackage.py +++ b/testing/mozharness/scripts/repackage.py @@ -93,7 +93,6 @@ class Repackage(BaseScript): command=command, cwd=dirs['abs_mozilla_dir'], halt_on_failure=True, - env=self.query_env(), ) def _run_tooltool(self): diff --git a/tools/update-packaging/app.mozbuild b/tools/update-packaging/app.mozbuild deleted file mode 100644 index 9053ec17f1dd..000000000000 --- a/tools/update-packaging/app.mozbuild +++ /dev/null @@ -1,9 +0,0 @@ -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, You can obtain one at http://mozilla.org/MPL/2.0/. - -DIRS += [ - '/modules/libmar/src', - '/modules/libmar/tool', - '/other-licenses/bsdiff', -] diff --git a/tools/update-packaging/moz.configure b/tools/update-packaging/moz.configure deleted file mode 100644 index e69de29bb2d1..000000000000 From dc64e9d9b75eff779725185366c47a8a9aa92b65 Mon Sep 17 00:00:00 2001 From: Andrew Osmond Date: Tue, 9 Apr 2019 15:27:16 -0400 Subject: [PATCH 22/22] Bug 1543217 - Allow qualified Linux machines to get WebRender. r=jrmuizel Linux machines using Intel graphics with Mesa drivers being at least 18.2.8.0 and not 4k displays should be able to run WebRender well, given this is a common configuration used for testing already by Mozilla. This patch allows users meeting said requirements to join the WebRender experiments on nightly. WebRender will remain disabled by default for other configurations/devices. Differential Revision: https://phabricator.services.mozilla.com/D26796 --- gfx/thebes/gfxPlatform.cpp | 32 ++++++++++++++++++++++++++++---- gfx/thebes/gfxPlatform.h | 2 +- gfx/thebes/gfxPlatformGtk.cpp | 2 +- widget/GfxInfoX11.cpp | 28 ++++++++++++++++++++++++++++ widget/windows/GfxInfo.cpp | 6 +++--- 5 files changed, 61 insertions(+), 9 deletions(-) diff --git a/gfx/thebes/gfxPlatform.cpp b/gfx/thebes/gfxPlatform.cpp index 0a930430e820..0e4f91b1a2d8 100644 --- a/gfx/thebes/gfxPlatform.cpp +++ b/gfx/thebes/gfxPlatform.cpp @@ -923,6 +923,7 @@ void gfxPlatform::Init() { #else # error "No gfxPlatform implementation available" #endif + gPlatform->PopulateScreenInfo(); gPlatform->InitAcceleration(); gPlatform->InitWebRenderConfig(); // When using WebRender, we defer initialization of the D3D11 devices until @@ -960,7 +961,6 @@ void gfxPlatform::Init() { InitLayersIPC(); - gPlatform->PopulateScreenInfo(); gPlatform->ComputeTileSize(); #ifdef MOZ_ENABLE_FREETYPE @@ -2511,7 +2511,7 @@ static bool CalculateWrQualifiedPrefValue() { } static FeatureState& WebRenderHardwareQualificationStatus( - bool aHasBattery, nsCString& aOutFailureId) { + const IntSize& aScreenSize, bool aHasBattery, nsCString& aOutFailureId) { FeatureState& featureWebRenderQualified = gfxConfig::GetFeature(Feature::WEBRENDER_QUALIFIED); featureWebRenderQualified.EnableByDefault(); @@ -2576,7 +2576,8 @@ static FeatureState& WebRenderHardwareQualificationStatus( FeatureStatus::Blocked, "Device too old", NS_LITERAL_CSTRING("FEATURE_FAILURE_DEVICE_TOO_OLD")); } - } else if (adapterVendorID == u"0x8086") { // Intel + } else if (adapterVendorID == u"0x8086" || + adapterVendorID == u"mesa/i965") { // Intel const uint16_t supportedDevices[] = { 0x191d, // HD Graphics P530 0x192d, // Iris Pro Graphics P555 @@ -2605,6 +2606,18 @@ static FeatureState& WebRenderHardwareQualificationStatus( featureWebRenderQualified.Disable( FeatureStatus::Blocked, "Device too old", NS_LITERAL_CSTRING("FEATURE_FAILURE_DEVICE_TOO_OLD")); + } else if (adapterVendorID == u"mesa/i965") { + const int32_t maxPixels = 3440 * 1440; // UWQHD + int32_t pixels = aScreenSize.width * aScreenSize.height; + if (pixels > maxPixels) { + featureWebRenderQualified.Disable( + FeatureStatus::Blocked, "Screen size too large", + NS_LITERAL_CSTRING("FEATURE_FAILURE_SCREEN_SIZE_TOO_LARGE")); + } else if (pixels <= 0) { + featureWebRenderQualified.Disable( + FeatureStatus::Blocked, "Screen size unknown", + NS_LITERAL_CSTRING("FEATURE_FAILURE_SCREEN_SIZE_UNKNOWN")); + } } #endif } else { @@ -2653,7 +2666,8 @@ void gfxPlatform::InitWebRenderConfig() { nsCString failureId; FeatureState& featureWebRenderQualified = - WebRenderHardwareQualificationStatus(HasBattery(), failureId); + WebRenderHardwareQualificationStatus(GetScreenSize(), HasBattery(), + failureId); FeatureState& featureWebRender = gfxConfig::GetFeature(Feature::WEBRENDER); featureWebRender.DisableByDefault( @@ -2748,6 +2762,16 @@ void gfxPlatform::InitWebRenderConfig() { WebRenderDebugPrefChangeCallback, WR_DEBUG_PREF); } } +#if defined(XP_LINUX) && !defined(MOZ_WIDGET_ANDROID) + else if (gfxConfig::IsEnabled(Feature::HW_COMPOSITING)) { + // Hardware compositing should be disabled by default if we aren't using + // WebRender. We had to check if it is enabled at all, because it may + // already have been forced disabled (e.g. safe mode, headless). It may + // still be forced on by the user, and if so, this should have no effect. + gfxConfig::Disable(Feature::HW_COMPOSITING, FeatureStatus::Blocked, + "Acceleration blocked by platform"); + } +#endif #ifdef XP_WIN if (Preferences::GetBool("gfx.webrender.dcomp-win.enabled", false)) { diff --git a/gfx/thebes/gfxPlatform.h b/gfx/thebes/gfxPlatform.h index 34cf85857b72..896dff0452ff 100644 --- a/gfx/thebes/gfxPlatform.h +++ b/gfx/thebes/gfxPlatform.h @@ -736,7 +736,7 @@ class gfxPlatform : public mozilla::layers::MemoryPressureListener { gfxPlatform(); virtual ~gfxPlatform(); - virtual bool HasBattery() { return true; } + virtual bool HasBattery() { return false; } virtual void InitAcceleration(); virtual void InitWebRenderConfig(); diff --git a/gfx/thebes/gfxPlatformGtk.cpp b/gfx/thebes/gfxPlatformGtk.cpp index 144af010f76a..0a6ecc42342d 100644 --- a/gfx/thebes/gfxPlatformGtk.cpp +++ b/gfx/thebes/gfxPlatformGtk.cpp @@ -330,7 +330,7 @@ uint32_t gfxPlatformGtk::MaxGenericSubstitions() { } bool gfxPlatformGtk::AccelerateLayersByDefault() { - return gfxPrefs::WebRenderAll(); + return true; } void gfxPlatformGtk::GetPlatformCMSOutputProfile(void*& mem, size_t& size) { diff --git a/widget/GfxInfoX11.cpp b/widget/GfxInfoX11.cpp index beb720660d40..13777676add5 100644 --- a/widget/GfxInfoX11.cpp +++ b/widget/GfxInfoX11.cpp @@ -307,6 +307,34 @@ const nsTArray &GfxInfo::GetGfxDriverInfo() { GfxDriverInfo::allDevices, GfxDriverInfo::allFeatures, nsIGfxInfo::FEATURE_BLOCKED_DRIVER_VERSION, DRIVER_LESS_THAN, V(13, 15, 100, 1), "FEATURE_FAILURE_OLD_FGLRX", "fglrx 13.15.100.1"); + + //////////////////////////////////// + // FEATURE_WEBRENDER + + // Mesa baseline (chosen arbitrarily as that which ships with + // Ubuntu 18.04 LTS). + APPEND_TO_DRIVER_BLOCKLIST( + OperatingSystem::Linux, + (nsAString &)GfxDriverInfo::GetDeviceVendor(VendorMesaAll), + GfxDriverInfo::allDevices, nsIGfxInfo::FEATURE_WEBRENDER, + nsIGfxInfo::FEATURE_BLOCKED_DRIVER_VERSION, DRIVER_LESS_THAN, + V(18, 2, 8, 0), "FEATURE_FAILURE_WEBRENDER_OLD_MESA", "Mesa 18.2.8.0"); + + // Disable on all NVIDIA devices for now. + APPEND_TO_DRIVER_BLOCKLIST( + OperatingSystem::Linux, + (nsAString &)GfxDriverInfo::GetDeviceVendor(VendorNVIDIA), + GfxDriverInfo::allDevices, nsIGfxInfo::FEATURE_WEBRENDER, + nsIGfxInfo::FEATURE_BLOCKED_DEVICE, DRIVER_COMPARISON_IGNORED, + V(0, 0, 0, 0), "FEATURE_FAILURE_WEBRENDER_NO_LINUX_NVIDIA", ""); + + // Disable on all ATI devices for now. + APPEND_TO_DRIVER_BLOCKLIST( + OperatingSystem::Linux, + (nsAString &)GfxDriverInfo::GetDeviceVendor(VendorATI), + GfxDriverInfo::allDevices, nsIGfxInfo::FEATURE_WEBRENDER, + nsIGfxInfo::FEATURE_BLOCKED_DEVICE, DRIVER_COMPARISON_IGNORED, + V(0, 0, 0, 0), "FEATURE_FAILURE_WEBRENDER_NO_LINUX_ATI", ""); } return *sDriverInfo; } diff --git a/widget/windows/GfxInfo.cpp b/widget/windows/GfxInfo.cpp index fe91244fbacb..e5213b20a7f1 100644 --- a/widget/windows/GfxInfo.cpp +++ b/widget/windows/GfxInfo.cpp @@ -1602,9 +1602,9 @@ const nsTArray& GfxInfo::GetGfxDriverInfo() { //////////////////////////////////// // FEATURE_WEBRENDER - // We are blocking all non-Nvidia cards in gfxPlatform.cpp where we check - // for the WEBRENDER_QUALIFIED feature. However we also want to block some - // specific Nvidia cards for being too low-powered, so we do that here. + // We are blocking most hardware explicitly in gfxPlatform.cpp where we + // check for the WEBRENDER_QUALIFIED feature. However we also want to block + // some specific Nvidia cards for being too low-powered, so we do that here. APPEND_TO_DRIVER_BLOCKLIST2( OperatingSystem::Windows10, (nsAString&)GfxDriverInfo::GetDeviceVendor(VendorNVIDIA),