From 205d40e2d7c06a4a260857777bd4054c1a9ce1e4 Mon Sep 17 00:00:00 2001 From: Andrea Marchesini Date: Mon, 15 Jan 2018 17:18:03 +0100 Subject: [PATCH 01/12] Bug 1425440 - Get rid of GetChildAt_Deprecated in nsDocumentEncoder, r=catalinb --- dom/base/nsDocumentEncoder.cpp | 140 ++++++--------------------------- 1 file changed, 23 insertions(+), 117 deletions(-) diff --git a/dom/base/nsDocumentEncoder.cpp b/dom/base/nsDocumentEncoder.cpp index 6207cfe537be..d591f61c6244 100644 --- a/dom/base/nsDocumentEncoder.cpp +++ b/dom/base/nsDocumentEncoder.cpp @@ -633,112 +633,6 @@ nsDocumentEncoder::FlushText(nsAString& aString, bool aForce) return rv; } -#if 0 // This code is really fast at serializing a range, but unfortunately - // there are problems with it so we don't use it now, maybe later... -static nsresult ChildAt(nsIDOMNode* aNode, int32_t aIndex, nsIDOMNode*& aChild) -{ - nsCOMPtr content(do_QueryInterface(aNode)); - - aChild = nullptr; - - NS_ENSURE_TRUE(content, NS_ERROR_FAILURE); - - nsIContent *child = content->GetChildAt_Deprecated(aIndex); - - if (child) - return CallQueryInterface(child, &aChild); - - return NS_OK; -} - -static int32_t IndexOf(nsIDOMNode* aParent, nsIDOMNode* aChild) -{ - nsCOMPtr parent(do_QueryInterface(aParent)); - nsCOMPtr child(do_QueryInterface(aChild)); - - if (!parent) - return -1; - - return parent->IndexOf(child); -} - -static inline int32_t GetIndex(nsTArray& aIndexArray) -{ - int32_t count = aIndexArray.Length(); - - if (count) { - return aIndexArray.ElementAt(count - 1); - } - - return 0; -} - -static nsresult GetNextNode(nsIDOMNode* aNode, nsTArray& aIndexArray, - nsIDOMNode*& aNextNode, - nsRangeIterationDirection& aDirection) -{ - bool hasChildren; - - aNextNode = nullptr; - - aNode->HasChildNodes(&hasChildren); - - if (hasChildren && aDirection == kDirectionIn) { - ChildAt(aNode, 0, aNextNode); - NS_ENSURE_TRUE(aNextNode, NS_ERROR_FAILURE); - - aIndexArray.AppendElement(0); - - aDirection = kDirectionIn; - } else if (aDirection == kDirectionIn) { - aNextNode = aNode; - - NS_ADDREF(aNextNode); - - aDirection = kDirectionOut; - } else { - nsCOMPtr parent; - - aNode->GetParentNode(getter_AddRefs(parent)); - NS_ENSURE_TRUE(parent, NS_ERROR_FAILURE); - - int32_t count = aIndexArray.Length(); - - if (count) { - int32_t indx = aIndexArray.ElementAt(count - 1); - - ChildAt(parent, indx + 1, aNextNode); - - if (aNextNode) - aIndexArray.ElementAt(count - 1) = indx + 1; - else - aIndexArray.RemoveElementAt(count - 1); - } else { - int32_t indx = IndexOf(parent, aNode); - - if (indx >= 0) { - ChildAt(parent, indx + 1, aNextNode); - - if (aNextNode) - aIndexArray.AppendElement(indx + 1); - } - } - - if (aNextNode) { - aDirection = kDirectionIn; - } else { - aDirection = kDirectionOut; - - aNextNode = parent; - - NS_ADDREF(aNextNode); - } - } - - return NS_OK; -} -#endif - static bool IsTextNode(nsINode *aNode) { return aNode && aNode->IsNodeOfType(nsINode::eTEXT); @@ -817,14 +711,12 @@ nsDocumentEncoder::SerializeRangeNodes(nsRange* aRange, // do some calculations that will tell us which children of this // node are in the range. - nsIContent* childAsNode = nullptr; int32_t startOffset = 0, endOffset = -1; if (startNode == content && mStartRootIndex >= aDepth) startOffset = mStartOffsets[mStartRootIndex - aDepth]; if (endNode == content && mEndRootIndex >= aDepth) endOffset = mEndOffsets[mEndRootIndex - aDepth]; // generated content will cause offset values of -1 to be returned. - int32_t j; uint32_t childCount = content->GetChildCount(); if (startOffset == -1) startOffset = 0; @@ -842,17 +734,31 @@ nsDocumentEncoder::SerializeRangeNodes(nsRange* aRange, endOffset++; } } - // serialize the children of this node that are in the range - for (j=startOffset; jGetChildAt_Deprecated(j); - if ((j==startOffset) || (j==endOffset-1)) - rv = SerializeRangeNodes(aRange, childAsNode, aString, aDepth+1); - else - rv = SerializeToStringRecursive(childAsNode, aString, false); + if (endOffset) { + // serialize the children of this node that are in the range + nsIContent* childAsNode = content->GetFirstChild(); + int32_t j = 0; - NS_ENSURE_SUCCESS(rv, rv); + for (; j < startOffset && childAsNode; ++j) { + childAsNode = childAsNode->GetNextSibling(); + } + + NS_ENSURE_TRUE(!!childAsNode, NS_ERROR_FAILURE); + MOZ_ASSERT(j == startOffset); + + for (; childAsNode && j < endOffset; ++j) + { + if ((j==startOffset) || (j==endOffset-1)) { + rv = SerializeRangeNodes(aRange, childAsNode, aString, aDepth+1); + } else { + rv = SerializeToStringRecursive(childAsNode, aString, false); + } + + NS_ENSURE_SUCCESS(rv, rv); + + childAsNode = childAsNode->GetNextSibling(); + } } // serialize the end of this node From 41d4da5cef330788ea6654301b4281ac53e167c0 Mon Sep 17 00:00:00 2001 From: Andrea Marchesini Date: Mon, 15 Jan 2018 17:18:38 +0100 Subject: [PATCH 02/12] Bug 1425440 - Introduce nsINode::RemoveChildNode, r=catalinb --- dom/base/Attr.cpp | 5 ++ dom/base/Attr.h | 1 + dom/base/FragmentOrElement.cpp | 6 ++ dom/base/FragmentOrElement.h | 1 + dom/base/nsDocument.cpp | 22 +++++++ dom/base/nsDocument.h | 1 + dom/base/nsGenericDOMDataNode.cpp | 5 ++ dom/base/nsGenericDOMDataNode.h | 1 + dom/base/nsINode.cpp | 2 +- dom/base/nsINode.h | 11 ++++ dom/html/HTMLFieldSetElement.cpp | 26 +++++++++ dom/html/HTMLFieldSetElement.h | 1 + dom/html/HTMLOptGroupElement.cpp | 8 +++ dom/html/HTMLOptGroupElement.h | 1 + dom/html/HTMLPictureElement.cpp | 24 ++++++++ dom/html/HTMLPictureElement.h | 1 + dom/html/HTMLSelectElement.cpp | 7 ++- dom/html/HTMLSelectElement.h | 1 + dom/svg/SVGSwitchElement.cpp | 7 +++ dom/svg/SVGSwitchElement.h | 1 + dom/xul/nsXULElement.cpp | 95 +++++++++++++++++++++++++++++++ dom/xul/nsXULElement.h | 1 + 22 files changed, 226 insertions(+), 2 deletions(-) diff --git a/dom/base/Attr.cpp b/dom/base/Attr.cpp index 6c23af968adb..4942216f5143 100644 --- a/dom/base/Attr.cpp +++ b/dom/base/Attr.cpp @@ -331,6 +331,11 @@ Attr::RemoveChildAt_Deprecated(uint32_t aIndex, bool aNotify) { } +void +Attr::RemoveChildNode(nsIContent* aKid, bool aNotify) +{ +} + nsresult Attr::GetEventTargetParent(EventChainPreVisitor& aVisitor) { diff --git a/dom/base/Attr.h b/dom/base/Attr.h index f037c41d1c5b..53a7154a4eda 100644 --- a/dom/base/Attr.h +++ b/dom/base/Attr.h @@ -70,6 +70,7 @@ public: virtual nsresult InsertChildAt(nsIContent* aKid, uint32_t aIndex, bool aNotify) override; virtual void RemoveChildAt_Deprecated(uint32_t aIndex, bool aNotify) override; + virtual void RemoveChildNode(nsIContent* aKid, bool aNotify) override; virtual nsresult Clone(mozilla::dom::NodeInfo *aNodeInfo, nsINode **aResult, bool aPreallocateChildren) const override; virtual already_AddRefed GetBaseURI(bool aTryUseXHRDocBaseURI = false) const override; diff --git a/dom/base/FragmentOrElement.cpp b/dom/base/FragmentOrElement.cpp index 0d2bdb48358b..8bd7acaec4f6 100644 --- a/dom/base/FragmentOrElement.cpp +++ b/dom/base/FragmentOrElement.cpp @@ -1177,6 +1177,12 @@ FragmentOrElement::RemoveChildAt_Deprecated(uint32_t aIndex, bool aNotify) } } +void +FragmentOrElement::RemoveChildNode(nsIContent* aKid, bool aNotify) +{ + doRemoveChildAt(IndexOf(aKid), aNotify, aKid, mAttrsAndChildren); +} + void FragmentOrElement::GetTextContentInternal(nsAString& aTextContent, OOMReporter& aError) diff --git a/dom/base/FragmentOrElement.h b/dom/base/FragmentOrElement.h index a7d054eeee87..7d99d6d172db 100644 --- a/dom/base/FragmentOrElement.h +++ b/dom/base/FragmentOrElement.h @@ -123,6 +123,7 @@ public: virtual nsresult InsertChildAt(nsIContent* aKid, uint32_t aIndex, bool aNotify) override; virtual void RemoveChildAt_Deprecated(uint32_t aIndex, bool aNotify) override; + virtual void RemoveChildNode(nsIContent* aKid, bool aNotify) override; virtual void GetTextContentInternal(nsAString& aTextContent, mozilla::OOMReporter& aError) override; virtual void SetTextContentInternal(const nsAString& aTextContent, diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp index 93e44ec4b866..f8b129b12c86 100644 --- a/dom/base/nsDocument.cpp +++ b/dom/base/nsDocument.cpp @@ -4401,6 +4401,28 @@ nsDocument::RemoveChildAt_Deprecated(uint32_t aIndex, bool aNotify) "(maybe somebody called GetRootElement() too early?)"); } +void +nsDocument::RemoveChildNode(nsIContent* aKid, bool aNotify) +{ + if (aKid->IsElement()) { + // Destroy the link map up front before we mess with the child list. + DestroyElementMaps(); + } + + // Preemptively clear mCachedRootElement, since we may be about to remove it + // from our child list, and we don't want to return this maybe-obsolete value + // from any GetRootElement() calls that happen inside of doRemoveChildAt(). + // (NOTE: for this to be useful, doRemoveChildAt() must NOT trigger any + // GetRootElement() calls until after it's removed the child from mChildren. + // Any call before that point would restore this soon-to-be-obsolete cached + // answer, and our clearing here would be fruitless.) + mCachedRootElement = nullptr; + doRemoveChildAt(IndexOf(aKid), aNotify, aKid, mChildren); + MOZ_ASSERT(mCachedRootElement != aKid, + "Stale pointer in mCachedRootElement, after we tried to clear it " + "(maybe somebody called GetRootElement() too early?)"); +} + void nsDocument::EnsureOnDemandBuiltInUASheet(StyleSheet* aSheet) { diff --git a/dom/base/nsDocument.h b/dom/base/nsDocument.h index 12568bab94fc..ee6bad8b997f 100644 --- a/dom/base/nsDocument.h +++ b/dom/base/nsDocument.h @@ -560,6 +560,7 @@ public: virtual nsresult InsertChildAt(nsIContent* aKid, uint32_t aIndex, bool aNotify) override; virtual void RemoveChildAt_Deprecated(uint32_t aIndex, bool aNotify) override; + virtual void RemoveChildNode(nsIContent* aKid, bool aNotify) override; virtual nsresult Clone(mozilla::dom::NodeInfo *aNodeInfo, nsINode **aResult, bool aPreallocateChildren) const override { diff --git a/dom/base/nsGenericDOMDataNode.cpp b/dom/base/nsGenericDOMDataNode.cpp index 3837bf3035c1..e9e7dbd36590 100644 --- a/dom/base/nsGenericDOMDataNode.cpp +++ b/dom/base/nsGenericDOMDataNode.cpp @@ -663,6 +663,11 @@ nsGenericDOMDataNode::RemoveChildAt_Deprecated(uint32_t aIndex, bool aNotify) { } +void +nsGenericDOMDataNode::RemoveChildNode(nsIContent* aKid, bool aNotify) +{ +} + nsXBLBinding * nsGenericDOMDataNode::DoGetXBLBinding() const { diff --git a/dom/base/nsGenericDOMDataNode.h b/dom/base/nsGenericDOMDataNode.h index 598945253324..60b0fe139982 100644 --- a/dom/base/nsGenericDOMDataNode.h +++ b/dom/base/nsGenericDOMDataNode.h @@ -110,6 +110,7 @@ public: virtual nsresult InsertChildAt(nsIContent* aKid, uint32_t aIndex, bool aNotify) override; virtual void RemoveChildAt_Deprecated(uint32_t aIndex, bool aNotify) override; + virtual void RemoveChildNode(nsIContent* aKid, bool aNotify) override; virtual void GetTextContentInternal(nsAString& aTextContent, mozilla::OOMReporter& aError) override { diff --git a/dom/base/nsINode.cpp b/dom/base/nsINode.cpp index fee9f09b7806..8f4300031b9e 100644 --- a/dom/base/nsINode.cpp +++ b/dom/base/nsINode.cpp @@ -1927,7 +1927,7 @@ nsINode::doRemoveChildAt(uint32_t aIndex, bool aNotify, // nsIDocument::GetRootElement() calls until *after* it has removed aKid from // aChildArray. Any calls before then could potentially restore a stale // value for our cached root element, per note in - // nsDocument::RemoveChildAt_Deprecated(). + // nsDocument::RemoveChildNode(). MOZ_ASSERT(aKid && aKid->GetParentNode() == this && aKid == GetChildAt_Deprecated(aIndex) && IndexOf(aKid) == (int32_t)aIndex, "Bogus aKid"); diff --git a/dom/base/nsINode.h b/dom/base/nsINode.h index fcfd303e6971..bd5d3fb4cf90 100644 --- a/dom/base/nsINode.h +++ b/dom/base/nsINode.h @@ -778,6 +778,17 @@ public: */ virtual void RemoveChildAt_Deprecated(uint32_t aIndex, bool aNotify) = 0; + /** + * Remove a child from this node. This method handles calling UnbindFromTree + * on the child appropriately. + * + * @param aKid the content to remove + * @param aNotify whether to notify the document (current document for + * nsIContent, and |this| for nsIDocument) that the remove has + * occurred + */ + virtual void RemoveChildNode(nsIContent* aKid, bool aNotify) = 0; + /** * Get a property associated with this node. * diff --git a/dom/html/HTMLFieldSetElement.cpp b/dom/html/HTMLFieldSetElement.cpp index 1821ae177546..51bf95c2daaa 100644 --- a/dom/html/HTMLFieldSetElement.cpp +++ b/dom/html/HTMLFieldSetElement.cpp @@ -195,6 +195,32 @@ HTMLFieldSetElement::RemoveChildAt_Deprecated(uint32_t aIndex, bool aNotify) } } +void +HTMLFieldSetElement::RemoveChildNode(nsIContent* aKid, bool aNotify) +{ + bool firstLegendHasChanged = false; + + if (mFirstLegend && aKid == mFirstLegend) { + // If we are removing the first legend we have to found another one. + nsIContent* child = mFirstLegend->GetNextSibling(); + mFirstLegend = nullptr; + firstLegendHasChanged = true; + + for (; child; child = child->GetNextSibling()) { + if (child->IsHTMLElement(nsGkAtoms::legend)) { + mFirstLegend = child; + break; + } + } + } + + nsGenericHTMLFormElement::RemoveChildNode(aKid, aNotify); + + if (firstLegendHasChanged) { + NotifyElementsForFirstLegendChange(aNotify); + } +} + void HTMLFieldSetElement::AddElement(nsGenericHTMLFormElement* aElement) { diff --git a/dom/html/HTMLFieldSetElement.h b/dom/html/HTMLFieldSetElement.h index 68b18c501435..8fe1773b3552 100644 --- a/dom/html/HTMLFieldSetElement.h +++ b/dom/html/HTMLFieldSetElement.h @@ -44,6 +44,7 @@ public: virtual nsresult InsertChildAt(nsIContent* aChild, uint32_t aIndex, bool aNotify) override; virtual void RemoveChildAt_Deprecated(uint32_t aIndex, bool aNotify) override; + virtual void RemoveChildNode(nsIContent* aKid, bool aNotify) override; // nsIFormControl NS_IMETHOD Reset() override; diff --git a/dom/html/HTMLOptGroupElement.cpp b/dom/html/HTMLOptGroupElement.cpp index d287e7b4f7a9..2347afc03058 100644 --- a/dom/html/HTMLOptGroupElement.cpp +++ b/dom/html/HTMLOptGroupElement.cpp @@ -95,6 +95,14 @@ HTMLOptGroupElement::RemoveChildAt_Deprecated(uint32_t aIndex, bool aNotify) nsGenericHTMLElement::RemoveChildAt_Deprecated(aIndex, aNotify); } +void +HTMLOptGroupElement::RemoveChildNode(nsIContent* aKid, bool aNotify) +{ + SafeOptionListMutation safeMutation(GetSelect(), this, nullptr, IndexOf(aKid), + aNotify); + nsGenericHTMLElement::RemoveChildNode(aKid, aNotify); +} + nsresult HTMLOptGroupElement::AfterSetAttr(int32_t aNameSpaceID, nsAtom* aName, const nsAttrValue* aValue, diff --git a/dom/html/HTMLOptGroupElement.h b/dom/html/HTMLOptGroupElement.h index 0c1c6f414f97..989604cc92c9 100644 --- a/dom/html/HTMLOptGroupElement.h +++ b/dom/html/HTMLOptGroupElement.h @@ -28,6 +28,7 @@ public: virtual nsresult InsertChildAt(nsIContent* aKid, uint32_t aIndex, bool aNotify) override; virtual void RemoveChildAt_Deprecated(uint32_t aIndex, bool aNotify) override; + virtual void RemoveChildNode(nsIContent* aKid, bool aNotify) override; // nsIContent virtual nsresult GetEventTargetParent( diff --git a/dom/html/HTMLPictureElement.cpp b/dom/html/HTMLPictureElement.cpp index 5d5d4cc60f26..1415a7c9ed34 100644 --- a/dom/html/HTMLPictureElement.cpp +++ b/dom/html/HTMLPictureElement.cpp @@ -58,6 +58,30 @@ HTMLPictureElement::RemoveChildAt_Deprecated(uint32_t aIndex, bool aNotify) nsGenericHTMLElement::RemoveChildAt_Deprecated(aIndex, aNotify); } +void +HTMLPictureElement::RemoveChildNode(nsIContent* aKid, bool aNotify) +{ + if (aKid && aKid->IsHTMLElement(nsGkAtoms::img)) { + HTMLImageElement* img = HTMLImageElement::FromContent(aKid); + if (img) { + img->PictureSourceRemoved(aKid->AsContent()); + } + } else if (aKid && aKid->IsHTMLElement(nsGkAtoms::source)) { + // Find all img siblings after this to notify them of its demise + nsCOMPtr nextSibling = aKid->GetNextSibling(); + if (nextSibling && nextSibling->GetParentNode() == this) { + do { + HTMLImageElement* img = HTMLImageElement::FromContent(nextSibling); + if (img) { + img->PictureSourceRemoved(aKid->AsContent()); + } + } while ( (nextSibling = nextSibling->GetNextSibling()) ); + } + } + + nsGenericHTMLElement::RemoveChildNode(aKid, aNotify); +} + nsresult HTMLPictureElement::InsertChildAt(nsIContent* aKid, uint32_t aIndex, bool aNotify) { diff --git a/dom/html/HTMLPictureElement.h b/dom/html/HTMLPictureElement.h index 361d2ceba8d2..72e5ce15ecfa 100644 --- a/dom/html/HTMLPictureElement.h +++ b/dom/html/HTMLPictureElement.h @@ -24,6 +24,7 @@ public: virtual nsresult Clone(mozilla::dom::NodeInfo* aNodeInfo, nsINode** aResult, bool aPreallocateChildren) const override; virtual void RemoveChildAt_Deprecated(uint32_t aIndex, bool aNotify) override; + virtual void RemoveChildNode(nsIContent* aKid, bool aNotify) override; virtual nsresult InsertChildAt(nsIContent* aKid, uint32_t aIndex, bool aNotify) override; protected: diff --git a/dom/html/HTMLSelectElement.cpp b/dom/html/HTMLSelectElement.cpp index b1549236e69d..8ea93d48abbb 100644 --- a/dom/html/HTMLSelectElement.cpp +++ b/dom/html/HTMLSelectElement.cpp @@ -224,7 +224,12 @@ HTMLSelectElement::RemoveChildAt_Deprecated(uint32_t aIndex, bool aNotify) nsGenericHTMLFormElementWithState::RemoveChildAt_Deprecated(aIndex, aNotify); } - +void +HTMLSelectElement::RemoveChildNode(nsIContent* aKid, bool aNotify) +{ + SafeOptionListMutation safeMutation(this, this, nullptr, IndexOf(aKid), aNotify); + nsGenericHTMLFormElementWithState::RemoveChildNode(aKid, aNotify); +} void HTMLSelectElement::InsertOptionsIntoList(nsIContent* aOptions, diff --git a/dom/html/HTMLSelectElement.h b/dom/html/HTMLSelectElement.h index 7e54a38fb586..8b4b398dce3b 100644 --- a/dom/html/HTMLSelectElement.h +++ b/dom/html/HTMLSelectElement.h @@ -294,6 +294,7 @@ public: virtual nsresult InsertChildAt(nsIContent* aKid, uint32_t aIndex, bool aNotify) override; virtual void RemoveChildAt_Deprecated(uint32_t aIndex, bool aNotify) override; + virtual void RemoveChildNode(nsIContent* aKid, bool aNotify) override; // Overriden nsIFormControl methods NS_IMETHOD Reset() override; diff --git a/dom/svg/SVGSwitchElement.cpp b/dom/svg/SVGSwitchElement.cpp index a5df9850db77..35f35734ccbc 100644 --- a/dom/svg/SVGSwitchElement.cpp +++ b/dom/svg/SVGSwitchElement.cpp @@ -100,6 +100,13 @@ SVGSwitchElement::RemoveChildAt_Deprecated(uint32_t aIndex, bool aNotify) MaybeInvalidate(); } +void +SVGSwitchElement::RemoveChildNode(nsIContent* aKid, bool aNotify) +{ + SVGSwitchElementBase::RemoveChildNode(aKid, aNotify); + MaybeInvalidate(); +} + //---------------------------------------------------------------------- // nsIContent methods diff --git a/dom/svg/SVGSwitchElement.h b/dom/svg/SVGSwitchElement.h index 137aea94deb9..ddc0d2256c15 100644 --- a/dom/svg/SVGSwitchElement.h +++ b/dom/svg/SVGSwitchElement.h @@ -43,6 +43,7 @@ public: virtual nsresult InsertChildAt(nsIContent* aKid, uint32_t aIndex, bool aNotify) override; virtual void RemoveChildAt_Deprecated(uint32_t aIndex, bool aNotify) override; + virtual void RemoveChildNode(nsIContent* aKid, bool aNotify) override; // nsIContent NS_IMETHOD_(bool) IsAttributeMapped(const nsAtom* aAttribute) const override; diff --git a/dom/xul/nsXULElement.cpp b/dom/xul/nsXULElement.cpp index 674b686c9964..edeceb2b388c 100644 --- a/dom/xul/nsXULElement.cpp +++ b/dom/xul/nsXULElement.cpp @@ -949,6 +949,101 @@ nsXULElement::RemoveChildAt_Deprecated(uint32_t aIndex, bool aNotify) } } +void +nsXULElement::RemoveChildNode(nsIContent* aKid, bool aNotify) +{ + // On the removal of a , , or element, + // the possibility exists that some of the items in the removed subtree + // are selected (and therefore need to be deselected). We need to account for this. + nsCOMPtr controlElement; + nsCOMPtr listBox; + bool fireSelectionHandler = false; + + // -1 = do nothing, -2 = null out current item + // anything else = index to re-set as current + int32_t newCurrentIndex = -1; + + if (aKid->NodeInfo()->Equals(nsGkAtoms::listitem, kNameSpaceID_XUL)) { + // This is the nasty case. We have (potentially) a slew of selected items + // and cells going away. + // First, retrieve the tree. + // Check first whether this element IS the tree + controlElement = do_QueryObject(this); + + // If it's not, look at our parent + if (!controlElement) + GetParentTree(getter_AddRefs(controlElement)); + nsCOMPtr controlContent(do_QueryInterface(controlElement)); + RefPtr xulElement = FromContentOrNull(controlContent); + + nsCOMPtr oldKidElem = do_QueryInterface(aKid); + if (xulElement && oldKidElem) { + // Iterate over all of the items and find out if they are contained inside + // the removed subtree. + int32_t length; + controlElement->GetSelectedCount(&length); + for (int32_t i = 0; i < length; i++) { + nsCOMPtr node; + controlElement->MultiGetSelectedItem(i, getter_AddRefs(node)); + // we need to QI here to do an XPCOM-correct pointercompare + nsCOMPtr selElem = do_QueryInterface(node); + if (selElem == oldKidElem && + NS_SUCCEEDED(controlElement->RemoveItemFromSelection(node))) { + length--; + i--; + fireSelectionHandler = true; + } + } + + nsCOMPtr curItem; + controlElement->GetCurrentItem(getter_AddRefs(curItem)); + nsCOMPtr curNode = do_QueryInterface(curItem); + if (curNode && nsContentUtils::ContentIsDescendantOf(curNode, aKid)) { + // Current item going away + IgnoredErrorResult ignored; + nsCOMPtr box = xulElement->GetBoxObject(ignored); + listBox = do_QueryInterface(box); + if (listBox && oldKidElem) { + listBox->GetIndexOfItem(oldKidElem, &newCurrentIndex); + } + + // If any of this fails, we'll just set the current item to null + if (newCurrentIndex == -1) + newCurrentIndex = -2; + } + } + } + + nsStyledElement::RemoveChildNode(aKid, aNotify); + + if (newCurrentIndex == -2) { + controlElement->SetCurrentItem(nullptr); + } else if (newCurrentIndex > -1) { + // Make sure the index is still valid + int32_t treeRows; + listBox->GetRowCount(&treeRows); + if (treeRows > 0) { + newCurrentIndex = std::min((treeRows - 1), newCurrentIndex); + nsCOMPtr newCurrentItem; + listBox->GetItemAtIndex(newCurrentIndex, getter_AddRefs(newCurrentItem)); + nsCOMPtr xulCurItem = do_QueryInterface(newCurrentItem); + if (xulCurItem) + controlElement->SetCurrentItem(xulCurItem); + } else { + controlElement->SetCurrentItem(nullptr); + } + } + + nsIDocument* doc; + if (fireSelectionHandler && (doc = GetComposedDoc())) { + nsContentUtils::DispatchTrustedEvent(doc, + static_cast(this), + NS_LITERAL_STRING("select"), + false, + true); + } +} + void nsXULElement::UnregisterAccessKey(const nsAString& aOldValue) { diff --git a/dom/xul/nsXULElement.h b/dom/xul/nsXULElement.h index ce68e94ddecb..bcda3485d8d9 100644 --- a/dom/xul/nsXULElement.h +++ b/dom/xul/nsXULElement.h @@ -367,6 +367,7 @@ public: bool aCompileEventHandlers) override; virtual void UnbindFromTree(bool aDeep, bool aNullParent) override; virtual void RemoveChildAt_Deprecated(uint32_t aIndex, bool aNotify) override; + virtual void RemoveChildNode(nsIContent* aKid, bool aNotify) override; virtual void DestroyContent() override; #ifdef DEBUG From e164d9c087e2e65878dbf6e3ec20f4c249c5ec96 Mon Sep 17 00:00:00 2001 From: Jonathan Kew Date: Mon, 15 Jan 2018 16:36:47 +0000 Subject: [PATCH 03/12] Bug 1430552 - Handle possible freetype failures in gfxFT2FontBase::GetFTGlyphAdvance to avoid risk of crashes. r=lsalzman --- gfx/thebes/gfxFT2FontBase.cpp | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/gfx/thebes/gfxFT2FontBase.cpp b/gfx/thebes/gfxFT2FontBase.cpp index 8cacd4260db7..ee4e9e2cbede 100644 --- a/gfx/thebes/gfxFT2FontBase.cpp +++ b/gfx/thebes/gfxFT2FontBase.cpp @@ -511,14 +511,22 @@ FT_Fixed gfxFT2FontBase::GetFTGlyphAdvance(uint16_t aGID) { gfxFT2LockedFace face(this); + MOZ_ASSERT(face.get()); + if (!face.get()) { + // Failed to get the FT_Face? Give up already. + return 0; + } int32_t flags = gfxPlatform::GetPlatform()->FontHintingEnabled() ? FT_LOAD_ADVANCE_ONLY : FT_LOAD_ADVANCE_ONLY | FT_LOAD_NO_AUTOHINT | FT_LOAD_NO_HINTING; - FT_Fixed advance = 0; - mozilla::DebugOnly ftError = - FT_Load_Glyph(face.get(), aGID, flags); + FT_Error ftError = FT_Load_Glyph(face.get(), aGID, flags); MOZ_ASSERT(!ftError); + if (ftError != FT_Err_Ok) { + // FT_Face was somehow broken/invalid? Don't try to access glyph slot. + return 0; + } + FT_Fixed advance = 0; if (face.get()->face_flags & FT_FACE_FLAG_SCALABLE) { advance = face.get()->glyph->linearHoriAdvance; } else { From 0538f8c6cec1f8be6111d5a9a3cdf5c89cdd55a2 Mon Sep 17 00:00:00 2001 From: Neil Deakin Date: Mon, 15 Jan 2018 15:16:56 -0500 Subject: [PATCH 04/12] Bug 1427449, don't close the menu in nsMenuBarFrame::FindMenuWithShortcut when just checking if such a menu shortcut key exists from the keydown event handler, also for extra safety this should only happen for menus not panels, r=felipe --- .../test/popupNotifications/browser.ini | 2 + .../browser_popupNotification_accesskey.js | 43 +++++++++++++++++++ layout/xul/nsMenuBarFrame.cpp | 32 +++++++------- layout/xul/nsMenuBarFrame.h | 2 +- layout/xul/nsMenuBarListener.cpp | 8 ++-- layout/xul/nsMenuBarListener.h | 5 ++- layout/xul/nsXULPopupManager.cpp | 2 +- 7 files changed, 71 insertions(+), 23 deletions(-) create mode 100644 browser/base/content/test/popupNotifications/browser_popupNotification_accesskey.js diff --git a/browser/base/content/test/popupNotifications/browser.ini b/browser/base/content/test/popupNotifications/browser.ini index e8a1cb0c6ec6..3921514f5f05 100644 --- a/browser/base/content/test/popupNotifications/browser.ini +++ b/browser/base/content/test/popupNotifications/browser.ini @@ -14,6 +14,8 @@ skip-if = (os == "linux" && (debug || asan)) skip-if = (os == "linux" && (debug || asan)) [browser_popupNotification_5.js] skip-if = true # bug 1332646 +[browser_popupNotification_accesskey.js] +skip-if = (os == "linux" && (debug || asan)) || os == "mac" [browser_popupNotification_checkbox.js] skip-if = (os == "linux" && (debug || asan)) [browser_popupNotification_selection_required.js] diff --git a/browser/base/content/test/popupNotifications/browser_popupNotification_accesskey.js b/browser/base/content/test/popupNotifications/browser_popupNotification_accesskey.js new file mode 100644 index 000000000000..e5964db5671c --- /dev/null +++ b/browser/base/content/test/popupNotifications/browser_popupNotification_accesskey.js @@ -0,0 +1,43 @@ +/* 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/. */ + +function test() { + waitForExplicitFinish(); + + ok(PopupNotifications, "PopupNotifications object exists"); + ok(PopupNotifications.panel, "PopupNotifications panel exists"); + + setup(); +} + +let buttonPressed = false; + +function commandTriggered() { + buttonPressed = true; +} + +var tests = [ + // This test ensures that the accesskey closes the popup. + { id: "Test#1", + run() { + this.notifyObj = new BasicNotification(this.id); + showNotification(this.notifyObj); + }, + onShown(popup) { + window.addEventListener("command", commandTriggered, true); + checkPopup(popup, this.notifyObj); + EventUtils.synthesizeKey("VK_ALT", { type: "keydown" }); + EventUtils.synthesizeKey("M", { altKey: true }); + EventUtils.synthesizeKey("VK_ALT", { type: "keyup" }); + + // If bug xxx was present, then the popup would be in the + // process of being hidden right now. + isnot(popup.state, "hiding", "popup is not hiding"); + }, + onHidden(popup) { + window.removeEventListener("command", commandTriggered, true); + ok(buttonPressed, "button pressed"); + } + } + ]; diff --git a/layout/xul/nsMenuBarFrame.cpp b/layout/xul/nsMenuBarFrame.cpp index f207df25c2f4..dfd4f3abda63 100644 --- a/layout/xul/nsMenuBarFrame.cpp +++ b/layout/xul/nsMenuBarFrame.cpp @@ -144,7 +144,7 @@ nsMenuBarFrame::ToggleMenuActiveState() } nsMenuFrame* -nsMenuBarFrame::FindMenuWithShortcut(nsIDOMKeyEvent* aKeyEvent) +nsMenuBarFrame::FindMenuWithShortcut(nsIDOMKeyEvent* aKeyEvent, bool aPeek) { uint32_t charCode; aKeyEvent->GetCharCode(&charCode); @@ -202,22 +202,24 @@ nsMenuBarFrame::FindMenuWithShortcut(nsIDOMKeyEvent* aKeyEvent) // didn't find a matching menu item #ifdef XP_WIN - // behavior on Windows - this item is on the menu bar, beep and deactivate the menu bar - if (mIsActive) { - nsCOMPtr soundInterface = do_CreateInstance("@mozilla.org/sound;1"); - if (soundInterface) - soundInterface->Beep(); - } + if (!aPeek) { + // behavior on Windows - this item is on the menu bar, beep and deactivate the menu bar + if (mIsActive) { + nsCOMPtr soundInterface = do_CreateInstance("@mozilla.org/sound;1"); + if (soundInterface) + soundInterface->Beep(); + } - nsXULPopupManager* pm = nsXULPopupManager::GetInstance(); - if (pm) { - nsIFrame* popup = pm->GetTopPopup(ePopupTypeAny); - if (popup) - pm->HidePopup(popup->GetContent(), true, true, true, false); - } + nsXULPopupManager* pm = nsXULPopupManager::GetInstance(); + if (pm) { + nsIFrame* popup = pm->GetTopPopup(ePopupTypeMenu); + if (popup) + pm->HidePopup(popup->GetContent(), true, true, true, false); + } - SetCurrentMenuItem(nullptr); - SetActive(false); + SetCurrentMenuItem(nullptr); + SetActive(false); + } #endif // #ifdef XP_WIN diff --git a/layout/xul/nsMenuBarFrame.h b/layout/xul/nsMenuBarFrame.h index be2e1de67217..a425b16389f3 100644 --- a/layout/xul/nsMenuBarFrame.h +++ b/layout/xul/nsMenuBarFrame.h @@ -85,7 +85,7 @@ public: nsMenuFrame* Enter(mozilla::WidgetGUIEvent* aEvent); // Used to handle ALT+key combos - nsMenuFrame* FindMenuWithShortcut(nsIDOMKeyEvent* aKeyEvent); + nsMenuFrame* FindMenuWithShortcut(nsIDOMKeyEvent* aKeyEvent, bool aPeek); virtual bool IsFrameOfType(uint32_t aFlags) const override { diff --git a/layout/xul/nsMenuBarListener.cpp b/layout/xul/nsMenuBarListener.cpp index a49e2eb53516..446b6ae6a494 100644 --- a/layout/xul/nsMenuBarListener.cpp +++ b/layout/xul/nsMenuBarListener.cpp @@ -324,7 +324,7 @@ nsMenuBarListener::KeyPress(nsIDOMEvent* aKeyEvent) } #endif // !XP_MACOSX - nsMenuFrame* menuFrameForKey = GetMenuForKeyEvent(keyEvent); + nsMenuFrame* menuFrameForKey = GetMenuForKeyEvent(keyEvent, false); if (!menuFrameForKey) { return NS_OK; } @@ -382,7 +382,7 @@ nsMenuBarListener::GetModifiersForAccessKey(nsIDOMKeyEvent* aKeyEvent) } nsMenuFrame* -nsMenuBarListener::GetMenuForKeyEvent(nsIDOMKeyEvent* aKeyEvent) +nsMenuBarListener::GetMenuForKeyEvent(nsIDOMKeyEvent* aKeyEvent, bool aPeek) { if (!IsAccessKeyPressed(aKeyEvent)) { return nullptr; @@ -404,7 +404,7 @@ nsMenuBarListener::GetMenuForKeyEvent(nsIDOMKeyEvent* aKeyEvent) // Do shortcut navigation. // A letter was pressed. We want to see if a shortcut gets matched. If // so, we'll know the menu got activated. - return mMenuBarFrame->FindMenuWithShortcut(aKeyEvent); + return mMenuBarFrame->FindMenuWithShortcut(aKeyEvent, aPeek); } return nullptr; @@ -493,7 +493,7 @@ nsMenuBarListener::KeyDown(nsIDOMEvent* aKeyEvent) } if (capturing && mAccessKey) { - nsMenuFrame* menuFrameForKey = GetMenuForKeyEvent(keyEvent); + nsMenuFrame* menuFrameForKey = GetMenuForKeyEvent(keyEvent, true); if (menuFrameForKey) { ReserveKeyIfNeeded(aKeyEvent); } diff --git a/layout/xul/nsMenuBarListener.h b/layout/xul/nsMenuBarListener.h index a154ec1be6e6..4baf28936355 100644 --- a/layout/xul/nsMenuBarListener.h +++ b/layout/xul/nsMenuBarListener.h @@ -77,9 +77,10 @@ protected: /** * Given a key event for an Alt+shortcut combination, - * return the menu, if any, that would be opened. + * return the menu, if any, that would be opened. If aPeek + * is false, then play a beep and deactivate the menubar on Windows. */ - nsMenuFrame* GetMenuForKeyEvent(nsIDOMKeyEvent* aKeyEvent); + nsMenuFrame* GetMenuForKeyEvent(nsIDOMKeyEvent* aKeyEvent, bool aPeek); /** * Call MarkAsReservedByChrome if the user's preferences indicate that diff --git a/layout/xul/nsXULPopupManager.cpp b/layout/xul/nsXULPopupManager.cpp index 20b370d9505d..c4a07ec1352c 100644 --- a/layout/xul/nsXULPopupManager.cpp +++ b/layout/xul/nsXULPopupManager.cpp @@ -2133,7 +2133,7 @@ nsXULPopupManager::HandleShortcutNavigation(nsIDOMKeyEvent* aKeyEvent, } if (mActiveMenuBar) { - nsMenuFrame* result = mActiveMenuBar->FindMenuWithShortcut(aKeyEvent); + nsMenuFrame* result = mActiveMenuBar->FindMenuWithShortcut(aKeyEvent, false); if (result) { mActiveMenuBar->SetActive(true); result->OpenMenu(true); From bc44f5df81b396693f014e7eccd872ace91e3030 Mon Sep 17 00:00:00 2001 From: archit Date: Tue, 16 Jan 2018 01:39:50 +0530 Subject: [PATCH 05/12] Bug 1428336 - Remove fallback border and background from the synced tabs sidebar's search field. r=dao MozReview-Commit-ID: FWqpaXyMvqM --HG-- extra : rebase_source : a353d22919ab0869a3bc38b4ee7893dca543c33d --- browser/themes/osx/syncedtabs/sidebar.css | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/browser/themes/osx/syncedtabs/sidebar.css b/browser/themes/osx/syncedtabs/sidebar.css index ebdb7b10339a..e56955563ce2 100644 --- a/browser/themes/osx/syncedtabs/sidebar.css +++ b/browser/themes/osx/syncedtabs/sidebar.css @@ -106,18 +106,7 @@ font-size: 12px; cursor: text; margin: 4px; - border-width: 3px; - border-style: solid; - border-color: currentcolor; - border-image: none; - -moz-border-top-colors: transparent #888 #000; - -moz-border-right-colors: transparent #FFF #000; - -moz-border-bottom-colors: transparent #FFF #000; - -moz-border-left-colors: transparent #888 #000; - border-top-right-radius: 2px; - border-bottom-left-radius: 2px; - background-color: #FFF; - color: #000; + color: -moz-fieldText; -moz-user-select: text; text-shadow: none; } @@ -130,13 +119,6 @@ height: 11px; } -.search-box[focused="true"] { - -moz-border-top-colors: -moz-mac-focusring -moz-mac-focusring #000000; - -moz-border-right-colors: -moz-mac-focusring -moz-mac-focusring #000000; - -moz-border-bottom-colors: -moz-mac-focusring -moz-mac-focusring #000000; - -moz-border-left-colors: -moz-mac-focusring -moz-mac-focusring #000000; -} - .search-box.compact { padding: 0px; /* font size is in px because the XUL it was copied from uses px */ From 345a5d6fda7876aa1192e871f349649e14d03f80 Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Tue, 9 Jan 2018 10:44:04 -0700 Subject: [PATCH 06/12] Bug 1430006: Allow configuring scriptworkers per-graph config; r=aki Differential Revision: https://phabricator.services.mozilla.com/D379 --HG-- extra : rebase_source : c52684c1a65017698d29979f35afed646ca5c90c --- taskcluster/ci/config.yml | 8 +++++ taskcluster/docs/taskgraph.rst | 4 ++- taskcluster/taskgraph/config.py | 4 +++ .../taskgraph/transforms/checksums_signing.py | 12 +++---- .../taskgraph/transforms/repackage_signing.py | 12 +++---- taskcluster/taskgraph/transforms/signing.py | 12 +++---- taskcluster/taskgraph/util/scriptworker.py | 35 ++++++++++++++++--- 7 files changed, 61 insertions(+), 26 deletions(-) diff --git a/taskcluster/ci/config.yml b/taskcluster/ci/config.yml index 0e10bc0d6821..e04541af1e16 100644 --- a/taskcluster/ci/config.yml +++ b/taskcluster/ci/config.yml @@ -96,3 +96,11 @@ try: 'win32': - 'sm-plain-win32' - 'sm-compacting-win32' + +scriptworker: + worker-types: + 'scriptworker-prov-v1/signing-linux-v1': + - 'project:releng:signing:cert:release-signing' + - 'project:releng:signing:cert:nightly-signing' + 'scriptworker-prov-v1/depsigning': + - 'project:releng:signing:cert:dep-signing' diff --git a/taskcluster/docs/taskgraph.rst b/taskcluster/docs/taskgraph.rst index f219ceba4c7e..2995bb841e7e 100644 --- a/taskcluster/docs/taskgraph.rst +++ b/taskcluster/docs/taskgraph.rst @@ -176,6 +176,8 @@ using simple parameterized values, as follows: Multiple labels may be substituted in a single string, and ``<<>`` can be used to escape a literal ``<``. +.. _taskgraph-graph-config: + Graph Configuration ------------------- @@ -192,7 +194,7 @@ Trust Domain When publishing and signing releases, that tasks verify their definition and all upstream tasks come from a decision task based on a trusted tree. (see -`chain-of-trust verification `). +`chain-of-trust verification `_). Firefox and Thunderbird share the taskgraph code and in particular, they have separate taskgraph configurations and in particular distinct decision tasks. Although they use identical docker images and toolchains, in order to track the diff --git a/taskcluster/taskgraph/config.py b/taskcluster/taskgraph/config.py index 4aeb05b75de9..adfc0d92e8af 100644 --- a/taskcluster/taskgraph/config.py +++ b/taskcluster/taskgraph/config.py @@ -26,6 +26,10 @@ graph_config_schema = Schema({ # all" Required('ridealong-builds', default={}): {basestring: [basestring]}, }, + Required('scriptworker'): { + # Mapping of scriptworker types to scopes they accept + Required('worker-types'): {basestring: [basestring]} + }, }) diff --git a/taskcluster/taskgraph/transforms/checksums_signing.py b/taskcluster/taskgraph/transforms/checksums_signing.py index 5e73e406bbef..3b411a6110d8 100644 --- a/taskcluster/taskgraph/transforms/checksums_signing.py +++ b/taskcluster/taskgraph/transforms/checksums_signing.py @@ -10,7 +10,10 @@ from __future__ import absolute_import, print_function, unicode_literals from taskgraph.transforms.base import TransformSequence from taskgraph.util.attributes import copy_attributes_from_dependent_job from taskgraph.util.schema import validate_schema, Schema -from taskgraph.util.scriptworker import get_signing_cert_scope +from taskgraph.util.scriptworker import ( + get_signing_cert_scope, + get_worker_type_for_scope, +) from taskgraph.transforms.task import task_description_schema from voluptuous import Any, Required, Optional @@ -89,7 +92,7 @@ def make_checksums_signing_description(config, jobs): task = { 'label': label, 'description': description, - 'worker-type': _generate_worker_type(signing_cert_scope), + 'worker-type': get_worker_type_for_scope(config, signing_cert_scope), 'worker': {'implementation': 'scriptworker-signing', 'upstream-artifacts': upstream_artifacts, 'max-run-time': 3600}, @@ -104,8 +107,3 @@ def make_checksums_signing_description(config, jobs): } yield task - - -def _generate_worker_type(signing_cert_scope): - worker_type = 'depsigning' if 'dep-signing' in signing_cert_scope else 'signing-linux-v1' - return 'scriptworker-prov-v1/{}'.format(worker_type) diff --git a/taskcluster/taskgraph/transforms/repackage_signing.py b/taskcluster/taskgraph/transforms/repackage_signing.py index a8869506b87d..b1cbfb22ac0d 100644 --- a/taskcluster/taskgraph/transforms/repackage_signing.py +++ b/taskcluster/taskgraph/transforms/repackage_signing.py @@ -10,7 +10,10 @@ from __future__ import absolute_import, print_function, unicode_literals from taskgraph.transforms.base import TransformSequence from taskgraph.util.attributes import copy_attributes_from_dependent_job from taskgraph.util.schema import validate_schema, Schema -from taskgraph.util.scriptworker import get_signing_cert_scope_per_platform +from taskgraph.util.scriptworker import ( + get_signing_cert_scope_per_platform, + get_worker_type_for_scope, +) from taskgraph.transforms.task import task_description_schema from voluptuous import Required, Optional @@ -121,7 +124,7 @@ def make_repackage_signing_description(config, jobs): task = { 'label': label, 'description': description, - 'worker-type': _generate_worker_type(signing_cert_scope), + 'worker-type': get_worker_type_for_scope(config, signing_cert_scope), 'worker': {'implementation': 'scriptworker-signing', 'upstream-artifacts': upstream_artifacts, 'max-run-time': 3600}, @@ -133,8 +136,3 @@ def make_repackage_signing_description(config, jobs): } yield task - - -def _generate_worker_type(signing_cert_scope): - worker_type = 'depsigning' if 'dep-signing' in signing_cert_scope else 'signing-linux-v1' - return 'scriptworker-prov-v1/{}'.format(worker_type) diff --git a/taskcluster/taskgraph/transforms/signing.py b/taskcluster/taskgraph/transforms/signing.py index 83fab344a1f3..91673b9e73a7 100644 --- a/taskcluster/taskgraph/transforms/signing.py +++ b/taskcluster/taskgraph/transforms/signing.py @@ -10,7 +10,10 @@ from __future__ import absolute_import, print_function, unicode_literals from taskgraph.transforms.base import TransformSequence from taskgraph.util.attributes import copy_attributes_from_dependent_job from taskgraph.util.schema import validate_schema, Schema -from taskgraph.util.scriptworker import get_signing_cert_scope_per_platform +from taskgraph.util.scriptworker import ( + get_signing_cert_scope_per_platform, + get_worker_type_for_scope, +) from taskgraph.transforms.task import task_description_schema from voluptuous import Any, Required, Optional @@ -135,7 +138,7 @@ def make_task_description(config, jobs): task = { 'label': label, 'description': description, - 'worker-type': _generate_worker_type(signing_cert_scope), + 'worker-type': get_worker_type_for_scope(config, signing_cert_scope), 'worker': {'implementation': 'scriptworker-signing', 'upstream-artifacts': job['upstream-artifacts'], 'max-run-time': 3600}, @@ -157,8 +160,3 @@ def _generate_treeherder_platform(dep_th_platform, build_platform, build_type): def _generate_treeherder_symbol(is_nightly): return 'tc(Ns)' if is_nightly else 'tc(Bs)' - - -def _generate_worker_type(signing_cert_scope): - worker_type = 'depsigning' if 'dep-signing' in signing_cert_scope else 'signing-linux-v1' - return 'scriptworker-prov-v1/{}'.format(worker_type) diff --git a/taskcluster/taskgraph/util/scriptworker.py b/taskcluster/taskgraph/util/scriptworker.py index ba839c778a8d..d79fe80f30a2 100644 --- a/taskcluster/taskgraph/util/scriptworker.py +++ b/taskcluster/taskgraph/util/scriptworker.py @@ -12,6 +12,8 @@ project branch, and merge day uplifts more user friendly. In the future, we may adjust scopes by other settings as well, e.g. different scopes for `push-to-candidates` rather than `push-to-releases`, even if both happen on mozilla-beta and mozilla-release. + +Additional configuration is found in the :ref:`graph config `. """ from __future__ import absolute_import, print_function, unicode_literals import functools @@ -296,7 +298,7 @@ def get_scope_from_project(alias_to_project_map, alias_to_scope_map, config): alias_to_project_map (list of lists): each list pair contains the alias and the set of projects that match. This is ordered. alias_to_scope_map (dict): the alias alias to scope - config (dict): the task config that defines the project. + config (TransformConfig): The configuration for the kind being transformed. Returns: string: the scope to use. @@ -314,7 +316,7 @@ def get_scope_from_target_method(alias_to_tasks_map, alias_to_scope_map, config) alias_to_tasks_map (list of lists): each list pair contains the alias and the set of target methods that match. This is ordered. alias_to_scope_map (dict): the alias alias to scope - config (dict): the task config that defines the target task method. + config (TransformConfig): The configuration for the kind being transformed. Returns: string: the scope to use. @@ -340,7 +342,7 @@ def get_scope_from_target_method_and_project(alias_to_tasks_map, alias_to_projec alias_to_project_map (list of lists): each list pair contains the alias and the set of projects that match. This is ordered. aliases_to_scope_map (dict of dicts): the task alias to project alias to scope - config (dict): the task config that defines the target task method and project. + config (TransformConfig): The configuration for the kind being transformed. Returns: string: the scope to use. @@ -436,7 +438,7 @@ def get_release_config(config): Currently only applies to beetmover tasks. Args: - config (dict): the task config that defines the target task method. + config (TransformConfig): The configuration for the kind being transformed. Returns: dict: containing both `build_number` and `version`. This can be used to @@ -484,3 +486,28 @@ def get_signing_cert_scope_per_platform(build_platform, is_nightly, config): return get_signing_cert_scope(config) else: return 'project:releng:signing:cert:dep-signing' + + +def get_worker_type_for_scope(config, scope): + """Get the scriptworker type that will accept the given scope. + + Args: + config (TransformConfig): The configuration for the kind being transformed. + scope (string): The scope being used. + + Returns: + string: The worker-type to use. + """ + for worker_type, scopes in config.graph_config['scriptworker']['worker-types'].items(): + if scope in scopes: + return worker_type + raise RuntimeError( + "Unsupported scriptworker scope {scope}. (supported scopes: {available_scopes})".format( + scope=scope, + available_scopes=sorted( + scope + for scopes in config.graph_config['scriptworker']['worker-types'].values() + for scope in scopes + ), + ) + ) From 4cedc60165163216b79dc89011ac53a0f6f7c613 Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Fri, 12 Jan 2018 13:20:23 -0700 Subject: [PATCH 07/12] Bug 1430006: Move push-apk settings into the kind; r=aki,jlorenzo Differential Revision: https://phabricator.services.mozilla.com/D383 --HG-- extra : rebase_source : 63bd62cd05399bbd685f57862e14e4e8b1035373 --- taskcluster/ci/push-apk-breakpoint/kind.yml | 6 +++ taskcluster/ci/push-apk/kind.yml | 22 +++++++- taskcluster/taskgraph/transforms/push_apk.py | 20 ++++--- .../transforms/push_apk_breakpoint.py | 13 ++--- taskcluster/taskgraph/transforms/task.py | 2 +- taskcluster/taskgraph/util/scriptworker.py | 52 ------------------- 6 files changed, 47 insertions(+), 68 deletions(-) diff --git a/taskcluster/ci/push-apk-breakpoint/kind.yml b/taskcluster/ci/push-apk-breakpoint/kind.yml index c7b54d5e99bd..59c115e9f989 100644 --- a/taskcluster/ci/push-apk-breakpoint/kind.yml +++ b/taskcluster/ci/push-apk-breakpoint/kind.yml @@ -20,6 +20,12 @@ jobs: shipping-phase: ship shipping-product: fennec worker-type: # see transforms + by-project: + mozilla-central: aws-provisioner-v1/taskcluster-generic + mozilla-beta: null-provisioner/human-breakpoint + mozilla-release: null-provisioner/human-breakpoint + maple: aws-provisioner-v1/taskcluster-generic + default: invalid/invalid worker: implementation: push-apk-breakpoint treeherder: diff --git a/taskcluster/ci/push-apk/kind.yml b/taskcluster/ci/push-apk/kind.yml index 63e0a9f84a87..6c9faa31cd03 100644 --- a/taskcluster/ci/push-apk/kind.yml +++ b/taskcluster/ci/push-apk/kind.yml @@ -29,9 +29,27 @@ jobs: default: scriptworker-prov-v1/dep-pushapk worker: upstream-artifacts: # see transforms - google-play-track: # see transforms + google-play-track: + # See https://github.com/mozilla-releng/pushapkscript#aurora-beta-release-vs-alpha-beta-production + by-project: + mozilla-central: 'beta' + mozilla-beta: 'rollout' + mozilla-release: 'rollout' + default: 'invalid' implementation: push-apk - commit: # see transforms + commit: + by-project: + mozilla-central: true + mozilla-beta: true + mozilla-release: true + default: false + rollout-percentage: + by-project: + # XXX Please make sure to change PUSH_APK_GOOGLE_PLAY_TRACT to + # 'rollout' if you add a new supported project + mozilla-release: 10 + mozilla-beta: 10 + default: null requires: all-resolved scopes: # see transforms treeherder: diff --git a/taskcluster/taskgraph/transforms/push_apk.py b/taskcluster/taskgraph/transforms/push_apk.py index c477ce2bf92b..783fa9ce6476 100644 --- a/taskcluster/taskgraph/transforms/push_apk.py +++ b/taskcluster/taskgraph/transforms/push_apk.py @@ -12,8 +12,7 @@ import functools from taskgraph.transforms.base import TransformSequence from taskgraph.transforms.task import task_description_schema from taskgraph.util.schema import optionally_keyed_by, resolve_keyed_by, Schema -from taskgraph.util.scriptworker import get_push_apk_scope, get_push_apk_track, \ - get_push_apk_commit_option, get_push_apk_rollout_percentage +from taskgraph.util.scriptworker import get_push_apk_scope from taskgraph.util.push_apk import fill_labels_tranform, validate_jobs_schema_transform_partial, \ validate_dependent_tasks_transform, delete_non_required_fields_transform, generate_dependencies @@ -62,12 +61,19 @@ def make_task_description(config, jobs): for job in jobs: job['dependencies'] = generate_dependencies(job['dependent-tasks']) job['worker']['upstream-artifacts'] = generate_upstream_artifacts(job['dependencies']) - job['worker']['google-play-track'] = get_push_apk_track(config) - job['worker']['commit'] = get_push_apk_commit_option(config) + resolve_keyed_by( + job, 'worker.google-play-track', item_name=job['name'], + project=config.params['project'] + ) + resolve_keyed_by( + job, 'worker.commit', item_name=job['name'], + project=config.params['project'] + ) - rollout_percentage = get_push_apk_rollout_percentage(config) - if rollout_percentage is not None: - job['worker']['rollout-percentage'] = rollout_percentage + resolve_keyed_by( + job, 'worker.rollout-percentage', item_name=job['name'], + project=config.params['project'] + ) job['scopes'] = [get_push_apk_scope(config)] diff --git a/taskcluster/taskgraph/transforms/push_apk_breakpoint.py b/taskcluster/taskgraph/transforms/push_apk_breakpoint.py index eb1eb128ba3c..c4c70caa2680 100644 --- a/taskcluster/taskgraph/transforms/push_apk_breakpoint.py +++ b/taskcluster/taskgraph/transforms/push_apk_breakpoint.py @@ -11,8 +11,7 @@ import functools from taskgraph.transforms.base import TransformSequence from taskgraph.transforms.task import task_description_schema -from taskgraph.util.schema import Schema -from taskgraph.util.scriptworker import get_push_apk_breakpoint_worker_type +from taskgraph.util.schema import optionally_keyed_by, resolve_keyed_by, Schema from taskgraph.util.push_apk import fill_labels_tranform, validate_jobs_schema_transform_partial, \ validate_dependent_tasks_transform, delete_non_required_fields_transform, generate_dependencies from voluptuous import Required @@ -32,7 +31,7 @@ push_apk_breakpoint_description_schema = Schema({ Required('description'): basestring, Required('job-from'): basestring, Required('attributes'): object, - Required('worker-type'): None, + Required('worker-type'): optionally_keyed_by('project', basestring), Required('worker'): object, Required('treeherder'): object, Required('run-on-projects'): list, @@ -57,10 +56,12 @@ def make_task_description(config, jobs): for job in jobs: job['dependencies'] = generate_dependencies(job['dependent-tasks']) - worker_type = get_push_apk_breakpoint_worker_type(config) - job['worker-type'] = worker_type + resolve_keyed_by( + job, 'worker-type', item_name=job['name'], + project=config.params['project'] + ) - job['worker']['payload'] = {} if 'human' in worker_type else { + job['worker']['payload'] = {} if 'human' in job['worker-type'] else { 'image': 'ubuntu:16.10', 'command': [ '/bin/bash', diff --git a/taskcluster/taskgraph/transforms/task.py b/taskcluster/taskgraph/transforms/task.py index 93a926f9bd6c..d34c1a7c7e2c 100644 --- a/taskcluster/taskgraph/transforms/task.py +++ b/taskcluster/taskgraph/transforms/task.py @@ -581,7 +581,7 @@ task_description_schema = Schema({ # "Invalid" is a noop for try and other non-supported branches Required('google-play-track'): Any('production', 'beta', 'alpha', 'rollout', 'invalid'), Required('commit'): bool, - Optional('rollout-percentage'): int, + Optional('rollout-percentage'): Any(int, None), }), }) diff --git a/taskcluster/taskgraph/util/scriptworker.py b/taskcluster/taskgraph/util/scriptworker.py index d79fe80f30a2..8d311707de50 100644 --- a/taskcluster/taskgraph/util/scriptworker.py +++ b/taskcluster/taskgraph/util/scriptworker.py @@ -257,37 +257,9 @@ PUSH_APK_SCOPES = { 'default': 'project:releng:googleplay:invalid', } -# See https://github.com/mozilla-releng/pushapkscript#aurora-beta-release-vs-alpha-beta-production -PUSH_APK_GOOGLE_PLAY_TRACT = { - 'central': 'beta', - 'beta': 'rollout', - 'release': 'rollout', - 'default': 'invalid', -} -PUSH_APK_BREAKPOINT_WORKER_TYPE = { - 'central': 'aws-provisioner-v1/taskcluster-generic', - 'beta': 'null-provisioner/human-breakpoint', - 'release': 'null-provisioner/human-breakpoint', - 'maple': 'aws-provisioner-v1/taskcluster-generic', - 'default': 'invalid/invalid', -} -PUSH_APK_COMMIT_OPTION = { - 'central': True, - 'beta': True, - 'maple': False, - 'release': True, - 'default': False, -} -PUSH_APK_ROLLOUT_PERCENTAGE = { - # XXX Please make sure to change PUSH_APK_GOOGLE_PLAY_TRACT to 'rollout' if you add a new - # supported project - 'release': 10, - 'beta': 10, - 'default': None, -} # scope functions {{{1 @@ -406,30 +378,6 @@ get_push_apk_scope = functools.partial( PUSH_APK_SCOPES ) -get_push_apk_track = functools.partial( - get_scope_from_project, - PUSH_APK_SCOPE_ALIAS_TO_PROJECT, - PUSH_APK_GOOGLE_PLAY_TRACT -) - -get_push_apk_breakpoint_worker_type = functools.partial( - get_scope_from_project, - PUSH_APK_SCOPE_ALIAS_TO_PROJECT, - PUSH_APK_BREAKPOINT_WORKER_TYPE -) - -get_push_apk_commit_option = functools.partial( - get_scope_from_project, - PUSH_APK_SCOPE_ALIAS_TO_PROJECT, - PUSH_APK_COMMIT_OPTION -) - -get_push_apk_rollout_percentage = functools.partial( - get_scope_from_project, - PUSH_APK_SCOPE_ALIAS_TO_PROJECT, - PUSH_APK_ROLLOUT_PERCENTAGE -) - # release_config {{{1 def get_release_config(config): From d2e34db9b36603e7d51dd947ed4c5f639ea7ee30 Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Fri, 12 Jan 2018 14:04:42 -0700 Subject: [PATCH 08/12] Bug 1430006: Add separate function for calculating phases; r=aki Differential Revision: https://phabricator.services.mozilla.com/D384 --HG-- extra : rebase_source : ab17d3a92265aa0965996a1c7535aaf366023e8c --- taskcluster/taskgraph/util/scriptworker.py | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/taskcluster/taskgraph/util/scriptworker.py b/taskcluster/taskgraph/util/scriptworker.py index 8d311707de50..aeb665d04d4e 100644 --- a/taskcluster/taskgraph/util/scriptworker.py +++ b/taskcluster/taskgraph/util/scriptworker.py @@ -329,6 +329,24 @@ def get_scope_from_target_method_and_project(alias_to_tasks_map, alias_to_projec return aliases_to_scope_map['default'] +def get_phase_from_target_method(alias_to_tasks_map, alias_to_phase_map, config): + """Determine the phase from `config.params['target_tasks_method']`. + + Args: + alias_to_tasks_map (list of lists): each list pair contains the + alias and the set of target methods that match. This is ordered. + alias_to_phase_map (dict): the alias to phase map + config (TransformConfig): The configuration for the kind being transformed. + + Returns: + string: the phase to use. + """ + for alias, tasks in alias_to_tasks_map: + if config.params['target_tasks_method'] in tasks and alias in alias_to_phase_map: + return alias_to_phase_map[alias] + return alias_to_phase_map['default'] + + get_signing_cert_scope = functools.partial( get_scope_from_project, SIGNING_SCOPE_ALIAS_TO_PROJECT, @@ -355,7 +373,7 @@ get_beetmover_action_scope = functools.partial( ) get_phase = functools.partial( - get_scope_from_target_method, + get_phase_from_target_method, BEETMOVER_SCOPE_ALIAS_TO_TARGET_TASK, PHASES ) From 3a19777b1f4f31fafefafeb3411b2b5be8584d5b Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Fri, 12 Jan 2018 01:08:54 -0700 Subject: [PATCH 09/12] Bug 1430006: Allow specifying the scope prefix to be used for script workers; r=aki Differential Revision: https://phabricator.services.mozilla.com/D382 --HG-- extra : rebase_source : d011bc53c290468c846c1c3ed77e083a43fda2fa --- taskcluster/ci/config.yml | 2 + taskcluster/taskgraph/config.py | 2 + taskcluster/taskgraph/util/scriptworker.py | 194 +++++++++++++-------- 3 files changed, 121 insertions(+), 77 deletions(-) diff --git a/taskcluster/ci/config.yml b/taskcluster/ci/config.yml index e04541af1e16..cf89fdd5098a 100644 --- a/taskcluster/ci/config.yml +++ b/taskcluster/ci/config.yml @@ -98,6 +98,8 @@ try: - 'sm-compacting-win32' scriptworker: + # See additional configuration in taskcluster/taskgraph/util/scriptworker.py + scope-prefix: 'project:releng' worker-types: 'scriptworker-prov-v1/signing-linux-v1': - 'project:releng:signing:cert:release-signing' diff --git a/taskcluster/taskgraph/config.py b/taskcluster/taskgraph/config.py index adfc0d92e8af..34641a0a051c 100644 --- a/taskcluster/taskgraph/config.py +++ b/taskcluster/taskgraph/config.py @@ -27,6 +27,8 @@ graph_config_schema = Schema({ Required('ridealong-builds', default={}): {basestring: [basestring]}, }, Required('scriptworker'): { + # Prefix to add to scopes controlling scriptworkers + Required('scope-prefix'): basestring, # Mapping of scriptworker types to scopes they accept Required('worker-types'): {basestring: [basestring]} }, diff --git a/taskcluster/taskgraph/util/scriptworker.py b/taskcluster/taskgraph/util/scriptworker.py index aeb665d04d4e..af1c18a3f62e 100644 --- a/taskcluster/taskgraph/util/scriptworker.py +++ b/taskcluster/taskgraph/util/scriptworker.py @@ -55,9 +55,9 @@ SIGNING_SCOPE_ALIAS_TO_PROJECT = [[ """Map the signing scope aliases to the actual scopes. """ SIGNING_CERT_SCOPES = { - 'all-release-branches': 'project:releng:signing:cert:release-signing', - 'all-nightly-branches': 'project:releng:signing:cert:nightly-signing', - 'default': 'project:releng:signing:cert:dep-signing', + 'all-release-branches': 'signing:cert:release-signing', + 'all-nightly-branches': 'signing:cert:nightly-signing', + 'default': 'signing:cert:dep-signing', } DEVEDITION_SIGNING_SCOPE_ALIAS_TO_PROJECT = [[ @@ -67,8 +67,8 @@ DEVEDITION_SIGNING_SCOPE_ALIAS_TO_PROJECT = [[ ]] DEVEDITION_SIGNING_CERT_SCOPES = { - 'beta': 'project:releng:signing:cert:nightly-signing', - 'default': 'project:releng:signing:cert:dep-signing', + 'beta': 'signing:cert:nightly-signing', + 'default': 'signing:cert:dep-signing', } """Map beetmover scope aliases to sets of projects. @@ -93,7 +93,7 @@ Used for both `BEETMOVER_SCOPE_ALIAS_TO_TARGET_TASK` and `get_release_build_numb BEETMOVER_CANDIDATES_TARGET_TASKS = set([ 'promote_fennec', 'promote_firefox', - 'promote_devedition' + 'promote_devedition', ]) BEETMOVER_PUSH_TARGET_TASKS = set([ 'push_fennec', @@ -130,24 +130,24 @@ BEETMOVER_SCOPE_ALIAS_TO_TARGET_TASK = [[ """ BEETMOVER_BUCKET_SCOPES = { 'all-candidates-tasks': { - 'all-release-branches': 'project:releng:beetmover:bucket:release', + 'all-release-branches': 'beetmover:bucket:release', }, 'all-push-tasks': { - 'all-release-branches': 'project:releng:beetmover:bucket:release', + 'all-release-branches': 'beetmover:bucket:release', }, 'all-nightly-tasks': { - 'all-nightly-branches': 'project:releng:beetmover:bucket:nightly', + 'all-nightly-branches': 'beetmover:bucket:nightly', }, - 'default': 'project:releng:beetmover:bucket:dep', + 'default': 'beetmover:bucket:dep', } """Map the beetmover tasks aliases to the actual action scopes. """ BEETMOVER_ACTION_SCOPES = { - 'all-candidates-tasks': 'project:releng:beetmover:action:push-to-candidates', - 'all-push-tasks': 'project:releng:beetmover:action:push-to-releases', - 'all-nightly-tasks': 'project:releng:beetmover:action:push-to-nightly', - 'default': 'project:releng:beetmover:action:push-to-staging', + 'all-candidates-tasks': 'beetmover:action:push-to-candidates', + 'all-push-tasks': 'beetmover:action:push-to-releases', + 'all-nightly-tasks': 'beetmover:action:push-to-nightly', + 'default': 'beetmover:action:push-to-staging', } @@ -184,54 +184,54 @@ BALROG_SCOPE_ALIAS_TO_PROJECT = [[ """Map the balrog scope aliases to the actual scopes. """ BALROG_SERVER_SCOPES = { - 'nightly': 'project:releng:balrog:server:nightly', - 'aurora': 'project:releng:balrog:server:aurora', - 'beta': 'project:releng:balrog:server:beta', - 'release': 'project:releng:balrog:server:release', - 'esr': 'project:releng:balrog:server:esr', - 'default': 'project:releng:balrog:server:dep', + 'nightly': 'balrog:server:nightly', + 'aurora': 'balrog:server:aurora', + 'beta': 'balrog:server:beta', + 'release': 'balrog:server:release', + 'esr': 'balrog:server:esr', + 'default': 'balrog:server:dep', } """Map the balrog scope aliases to the actual channel scopes. """ BALROG_CHANNEL_SCOPES = { 'nightly': [ - 'project:releng:balrog:channel:nightly', - 'project:releng:balrog:channel:nightly-old-id', - 'project:releng:balrog:channel:aurora' + 'balrog:channel:nightly', + 'balrog:channel:nightly-old-id', + 'balrog:channel:aurora', ], 'aurora': [ - 'project:releng:balrog:channel:aurora' + 'balrog:channel:aurora', ], 'beta': [ - 'project:releng:balrog:channel:beta', - 'project:releng:balrog:channel:beta-localtest', - 'project:releng:balrog:channel:beta-cdntest' + 'balrog:channel:beta', + 'balrog:channel:beta-localtest', + 'balrog:channel:beta-cdntest', ], 'release': [ - 'project:releng:balrog:channel:release', - 'project:releng:balrog:channel:release-localtest', - 'project:releng:balrog:channel:release-cdntest' + 'balrog:channel:release', + 'balrog:channel:release-localtest', + 'balrog:channel:release-cdntest', ], 'esr': [ - 'project:releng:balrog:channel:esr', - 'project:releng:balrog:channel:esr-localtest', - 'project:releng:balrog:channel:esr-cdntest' + 'balrog:channel:esr', + 'balrog:channel:esr-localtest', + 'balrog:channel:esr-cdntest', ], 'default': [ - 'project:releng:balrog:channel:nightly', - 'project:releng:balrog:channel:nightly-old-id', - 'project:releng:balrog:channel:aurora' - 'project:releng:balrog:channel:beta', - 'project:releng:balrog:channel:beta-localtest', - 'project:releng:balrog:channel:beta-cdntest', - 'project:releng:balrog:channel:release', - 'project:releng:balrog:channel:release-localtest', - 'project:releng:balrog:channel:release-cdntest', - 'project:releng:balrog:channel:esr', - 'project:releng:balrog:channel:esr-localtest', - 'project:releng:balrog:channel:esr-cdntest' - ] + 'balrog:channel:nightly', + 'balrog:channel:nightly-old-id', + 'balrog:channel:aurora', + 'balrog:channel:beta', + 'balrog:channel:beta-localtest', + 'balrog:channel:beta-cdntest', + 'balrog:channel:release', + 'balrog:channel:release-localtest', + 'balrog:channel:release-cdntest', + 'balrog:channel:esr', + 'balrog:channel:esr-localtest', + 'balrog:channel:esr-cdntest', + ], } @@ -251,26 +251,64 @@ PUSH_APK_SCOPE_ALIAS_TO_PROJECT = [[ PUSH_APK_SCOPES = { - 'central': 'project:releng:googleplay:aurora', - 'beta': 'project:releng:googleplay:beta', - 'release': 'project:releng:googleplay:release', - 'default': 'project:releng:googleplay:invalid', + 'central': 'googleplay:aurora', + 'beta': 'googleplay:beta', + 'release': 'googleplay:release', + 'default': 'googleplay:invalid', } +def add_scope_prefix(config, scope): + """ + Prepends the scriptworker scope prefix from the :ref:`graph config + `. + + Args: + config (TransformConfig): The configuration for the kind being transformed. + scope (string): The suffix of the scope + + Returns: + string: the scope to use. + """ + return "{prefix}:{scope}".format( + prefix=config.graph_config['scriptworker']['scope-prefix'], + scope=scope, + ) +def with_scope_prefix(f): + """ + Wraps a function, calling :py:func:`add_scope_prefix` on the result of + calling the wrapped function. + + Args: + f (callable): A function that takes a ``config`` and some keyword + arguments, and returns a scope suffix. + + Returns: + callable: the wrapped function + """ + @functools.wraps(f) + def wrapper(config, **kwargs): + scope_or_scopes = f(config, **kwargs) + if isinstance(scope_or_scopes, list): + return map(functools.partial(add_scope_prefix, config), scope_or_scopes) + else: + return add_scope_prefix(config, scope_or_scopes) + + return wrapper # scope functions {{{1 -def get_scope_from_project(alias_to_project_map, alias_to_scope_map, config): +@with_scope_prefix +def get_scope_from_project(config, alias_to_project_map, alias_to_scope_map): """Determine the restricted scope from `config.params['project']`. Args: + config (TransformConfig): The configuration for the kind being transformed. alias_to_project_map (list of lists): each list pair contains the alias and the set of projects that match. This is ordered. alias_to_scope_map (dict): the alias alias to scope - config (TransformConfig): The configuration for the kind being transformed. Returns: string: the scope to use. @@ -281,14 +319,15 @@ def get_scope_from_project(alias_to_project_map, alias_to_scope_map, config): return alias_to_scope_map['default'] -def get_scope_from_target_method(alias_to_tasks_map, alias_to_scope_map, config): +@with_scope_prefix +def get_scope_from_target_method(config, alias_to_tasks_map, alias_to_scope_map): """Determine the restricted scope from `config.params['target_tasks_method']`. Args: + config (TransformConfig): The configuration for the kind being transformed. alias_to_tasks_map (list of lists): each list pair contains the alias and the set of target methods that match. This is ordered. alias_to_scope_map (dict): the alias alias to scope - config (TransformConfig): The configuration for the kind being transformed. Returns: string: the scope to use. @@ -299,8 +338,9 @@ def get_scope_from_target_method(alias_to_tasks_map, alias_to_scope_map, config) return alias_to_scope_map['default'] -def get_scope_from_target_method_and_project(alias_to_tasks_map, alias_to_project_map, - aliases_to_scope_map, config): +@with_scope_prefix +def get_scope_from_target_method_and_project(config, alias_to_tasks_map, + alias_to_project_map, aliases_to_scope_map): """Determine the restricted scope from both `target_tasks_method` and `project`. On certain branches, we'll need differing restricted scopes based on @@ -309,12 +349,12 @@ def get_scope_from_target_method_and_project(alias_to_tasks_map, alias_to_projec checks both. Args: + config (TransformConfig): The configuration for the kind being transformed. alias_to_tasks_map (list of lists): each list pair contains the alias and the set of target methods that match. This is ordered. alias_to_project_map (list of lists): each list pair contains the alias and the set of projects that match. This is ordered. aliases_to_scope_map (dict of dicts): the task alias to project alias to scope - config (TransformConfig): The configuration for the kind being transformed. Returns: string: the scope to use. @@ -329,14 +369,14 @@ def get_scope_from_target_method_and_project(alias_to_tasks_map, alias_to_projec return aliases_to_scope_map['default'] -def get_phase_from_target_method(alias_to_tasks_map, alias_to_phase_map, config): +def get_phase_from_target_method(config, alias_to_tasks_map, alias_to_phase_map): """Determine the phase from `config.params['target_tasks_method']`. Args: + config (TransformConfig): The configuration for the kind being transformed. alias_to_tasks_map (list of lists): each list pair contains the alias and the set of target methods that match. This is ordered. alias_to_phase_map (dict): the alias to phase map - config (TransformConfig): The configuration for the kind being transformed. Returns: string: the phase to use. @@ -349,51 +389,51 @@ def get_phase_from_target_method(alias_to_tasks_map, alias_to_phase_map, config) get_signing_cert_scope = functools.partial( get_scope_from_project, - SIGNING_SCOPE_ALIAS_TO_PROJECT, - SIGNING_CERT_SCOPES + alias_to_project_map=SIGNING_SCOPE_ALIAS_TO_PROJECT, + alias_to_scope_map=SIGNING_CERT_SCOPES, ) get_devedition_signing_cert_scope = functools.partial( get_scope_from_project, - DEVEDITION_SIGNING_SCOPE_ALIAS_TO_PROJECT, - DEVEDITION_SIGNING_CERT_SCOPES + alias_to_project_map=DEVEDITION_SIGNING_SCOPE_ALIAS_TO_PROJECT, + alias_to_scope_map=DEVEDITION_SIGNING_CERT_SCOPES, ) get_beetmover_bucket_scope = functools.partial( get_scope_from_target_method_and_project, - BEETMOVER_SCOPE_ALIAS_TO_TARGET_TASK, - BEETMOVER_SCOPE_ALIAS_TO_PROJECT, - BEETMOVER_BUCKET_SCOPES + alias_to_tasks_map=BEETMOVER_SCOPE_ALIAS_TO_TARGET_TASK, + alias_to_project_map=BEETMOVER_SCOPE_ALIAS_TO_PROJECT, + aliases_to_scope_map=BEETMOVER_BUCKET_SCOPES, ) get_beetmover_action_scope = functools.partial( get_scope_from_target_method, - BEETMOVER_SCOPE_ALIAS_TO_TARGET_TASK, - BEETMOVER_ACTION_SCOPES + alias_to_tasks_map=BEETMOVER_SCOPE_ALIAS_TO_TARGET_TASK, + alias_to_scope_map=BEETMOVER_ACTION_SCOPES, ) get_phase = functools.partial( get_phase_from_target_method, - BEETMOVER_SCOPE_ALIAS_TO_TARGET_TASK, - PHASES + alias_to_tasks_map=BEETMOVER_SCOPE_ALIAS_TO_TARGET_TASK, + alias_to_phase_map=PHASES, ) get_balrog_server_scope = functools.partial( get_scope_from_project, - BALROG_SCOPE_ALIAS_TO_PROJECT, - BALROG_SERVER_SCOPES + alias_to_project_map=BALROG_SCOPE_ALIAS_TO_PROJECT, + alias_to_scope_map=BALROG_SERVER_SCOPES, ) get_balrog_channel_scopes = functools.partial( get_scope_from_project, - BALROG_SCOPE_ALIAS_TO_PROJECT, - BALROG_CHANNEL_SCOPES + alias_to_project_map=BALROG_SCOPE_ALIAS_TO_PROJECT, + alias_to_scope_map=BALROG_CHANNEL_SCOPES, ) get_push_apk_scope = functools.partial( get_scope_from_project, - PUSH_APK_SCOPE_ALIAS_TO_PROJECT, - PUSH_APK_SCOPES + alias_to_project_map=PUSH_APK_SCOPE_ALIAS_TO_PROJECT, + alias_to_scope_map=PUSH_APK_SCOPES, ) @@ -451,7 +491,7 @@ def get_signing_cert_scope_per_platform(build_platform, is_nightly, config): elif is_nightly: return get_signing_cert_scope(config) else: - return 'project:releng:signing:cert:dep-signing' + return add_scope_prefix(config, 'signing:cert:dep-signing') def get_worker_type_for_scope(config, scope): From bef381375c67e01a7899e8b239b81a61754b911b Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Sun, 14 Jan 2018 17:40:09 -0800 Subject: [PATCH 10/12] Bug 1430508: Return 0 for ProcessId() when channel IPC is closed. r=dragana There are some corner cases where we try to attach StreamFilter endpoints to a channel after its IPC has been closed from from the other side, but request listeners haven't been notified. This causes crashes in any of several places. This patch changes nsHttpChannel::ProcessId to return 0 when IPC is closed, so callers can detect that it's no longer possible to attach endpoints to it. MozReview-Commit-ID: BZTOqezih0P --HG-- extra : rebase_source : dfdb5bf7a11fccea51a1fbb161e688f10167da30 --- netwerk/protocol/http/HttpChannelParent.cpp | 9 +++++++++ netwerk/protocol/http/HttpChannelParent.h | 2 ++ .../extensions/webrequest/StreamFilterParent.cpp | 5 ++++- 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/netwerk/protocol/http/HttpChannelParent.cpp b/netwerk/protocol/http/HttpChannelParent.cpp index b0934084b9d3..064c5659e9ed 100644 --- a/netwerk/protocol/http/HttpChannelParent.cpp +++ b/netwerk/protocol/http/HttpChannelParent.cpp @@ -265,6 +265,15 @@ HttpChannelParent::CleanupBackgroundChannel() } } +base::ProcessId +HttpChannelParent::OtherPid() const +{ + if (mIPCClosed) { + return 0; + } + return Manager()->OtherPid(); +} + //----------------------------------------------------------------------------- // HttpChannelParent::nsISupports //----------------------------------------------------------------------------- diff --git a/netwerk/protocol/http/HttpChannelParent.h b/netwerk/protocol/http/HttpChannelParent.h index 3f5896c4ed68..572d7f177d7a 100644 --- a/netwerk/protocol/http/HttpChannelParent.h +++ b/netwerk/protocol/http/HttpChannelParent.h @@ -120,6 +120,8 @@ public: // Callback while background channel is destroyed. void OnBackgroundParentDestroyed(); + base::ProcessId OtherPid() const override; + protected: // used to connect redirected-to channel in parent with just created // ChildChannel. Used during redirects. diff --git a/toolkit/components/extensions/webrequest/StreamFilterParent.cpp b/toolkit/components/extensions/webrequest/StreamFilterParent.cpp index 68788e150580..cf51127358c3 100644 --- a/toolkit/components/extensions/webrequest/StreamFilterParent.cpp +++ b/toolkit/components/extensions/webrequest/StreamFilterParent.cpp @@ -136,9 +136,12 @@ StreamFilterParent::Create(dom::ContentParent* aContentParent, uint64_t aChannel RefPtr chan = do_QueryObject(channel); NS_ENSURE_TRUE(chan, false); + auto channelPid = chan->ProcessId(); + NS_ENSURE_TRUE(channelPid, false); + Endpoint parent; Endpoint child; - nsresult rv = PStreamFilter::CreateEndpoints(chan->ProcessId(), + nsresult rv = PStreamFilter::CreateEndpoints(channelPid, aContentParent ? aContentParent->OtherPid() : base::GetCurrentProcId(), &parent, &child); From d1e788a7b6b747b57b32ae089ba1f8f12a8d4c3b Mon Sep 17 00:00:00 2001 From: Geoff Brown Date: Mon, 15 Jan 2018 15:51:13 -0700 Subject: [PATCH 11/12] Bug 1321605 - Follow-up to dump Android bogomips in main test log; r=me, a=test-only We use bogomips as a convenient indicator or emulator performance/health and make that available in the android-performance.log artifact. When the task times out, that artifact is not uploaded, leaving me curious about what the bogomips were; let's dump it to the test log. --- testing/mozharness/scripts/android_emulator_unittest.py | 1 + 1 file changed, 1 insertion(+) diff --git a/testing/mozharness/scripts/android_emulator_unittest.py b/testing/mozharness/scripts/android_emulator_unittest.py index e8ad38fb02ab..be73074109bc 100644 --- a/testing/mozharness/scripts/android_emulator_unittest.py +++ b/testing/mozharness/scripts/android_emulator_unittest.py @@ -667,6 +667,7 @@ class AndroidEmulatorTest(BlobUploadMixin, TestingMixin, EmulatorMixin, VCSMixin if bogomips < 250: self.fatal('INFRA-ERROR: insufficient Android bogomips (%d < 250)' % bogomips, EXIT_STATUS_DICT[TBPL_RETRY]) + self.info("Found Android bogomips: %d" % bogomips) break def verify_emulator(self): From c27dfe6822b365a00bee12ea329a3242137705e6 Mon Sep 17 00:00:00 2001 From: Geoff Brown Date: Mon, 15 Jan 2018 15:51:13 -0700 Subject: [PATCH 12/12] Bug 1430668 - Increase chunks for linux32/debug jsreftest; r=me, a=test-only Attempt to avoid intermittent task timeouts in these tests. --- taskcluster/ci/test/reftest.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/taskcluster/ci/test/reftest.yml b/taskcluster/ci/test/reftest.yml index 6540ea5026a0..30de090e3ed3 100644 --- a/taskcluster/ci/test/reftest.yml +++ b/taskcluster/ci/test/reftest.yml @@ -64,6 +64,7 @@ jsreftest: windows10-64-ccov/debug: 5 linux64-ccov/.*: 5 linux64-qr/.*: 4 + linux32/debug: 5 macosx.*: 2 default: 3 e10s: