From db34f0e7be38df19f7d04b99fdd369983c4926e4 Mon Sep 17 00:00:00 2001 From: David Parks Date: Fri, 6 Dec 2019 12:28:18 +0100 Subject: [PATCH 1/6] Properly detect failure in receiving plugin NPObjects. Properly handles NPError reporting and makes sure that, in the case of failure, it does not return junk for the NPObject. --- dom/plugins/ipc/PluginInstanceChild.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/dom/plugins/ipc/PluginInstanceChild.cpp b/dom/plugins/ipc/PluginInstanceChild.cpp index 1aaaa92f39..97b4072112 100644 --- a/dom/plugins/ipc/PluginInstanceChild.cpp +++ b/dom/plugins/ipc/PluginInstanceChild.cpp @@ -310,9 +310,10 @@ PluginInstanceChild::InternalGetNPObjectForValue(NPNVariable aValue, switch (aValue) { case NPNVWindowNPObject: if (!(actor = mCachedWindowActor)) { + result = NPERR_GENERIC_ERROR; PPluginScriptableObjectChild* actorProtocol; - CallNPN_GetValue_NPNVWindowNPObject(&actorProtocol, &result); - if (result == NPERR_NO_ERROR) { + if (CallNPN_GetValue_NPNVWindowNPObject(&actorProtocol, &result) && + result == NPERR_NO_ERROR) { actor = mCachedWindowActor = static_cast(actorProtocol); NS_ASSERTION(actor, "Null actor!"); @@ -324,10 +325,10 @@ PluginInstanceChild::InternalGetNPObjectForValue(NPNVariable aValue, case NPNVPluginElementNPObject: if (!(actor = mCachedElementActor)) { + result = NPERR_GENERIC_ERROR; PPluginScriptableObjectChild* actorProtocol; - CallNPN_GetValue_NPNVPluginElementNPObject(&actorProtocol, - &result); - if (result == NPERR_NO_ERROR) { + if (CallNPN_GetValue_NPNVPluginElementNPObject(&actorProtocol, &result) && + result == NPERR_NO_ERROR) { actor = mCachedElementActor = static_cast(actorProtocol); NS_ASSERTION(actor, "Null actor!"); @@ -338,6 +339,7 @@ PluginInstanceChild::InternalGetNPObjectForValue(NPNVariable aValue, break; default: + result = NPERR_GENERIC_ERROR; NS_NOTREACHED("Don't know what to do with this value type!"); } @@ -434,6 +436,7 @@ PluginInstanceChild::NPN_GetValue(NPNVariable aVar, case NPNVWindowNPObject: // Intentional fall-through case NPNVPluginElementNPObject: { NPObject* object; + *((NPObject**)aValue) = nullptr; NPError result = InternalGetNPObjectForValue(aVar, &object); if (result == NPERR_NO_ERROR) { *((NPObject**)aValue) = object; From 38439fd244ce26e57e1224e01d3541b15bdd71ac Mon Sep 17 00:00:00 2001 From: Valentin Gosu Date: Fri, 6 Dec 2019 13:01:55 +0100 Subject: [PATCH 2/6] Use mutex in PACResolver when accessing mRequest from multiple threads. --- netwerk/base/ProxyAutoConfig.cpp | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/netwerk/base/ProxyAutoConfig.cpp b/netwerk/base/ProxyAutoConfig.cpp index 4d7a6c1fd5..6a95776693 100644 --- a/netwerk/base/ProxyAutoConfig.cpp +++ b/netwerk/base/ProxyAutoConfig.cpp @@ -271,6 +271,7 @@ public: PACResolver() : mStatus(NS_ERROR_FAILURE) + , mMutex("PACResolver::Mutex") { } @@ -279,9 +280,15 @@ public: nsIDNSRecord *record, nsresult status) override { - if (mTimer) { - mTimer->Cancel(); - mTimer = nullptr; + nsCOMPtr timer; + { + MutexAutoLock lock(mMutex); + timer.swap(mTimer); + mRequest = nullptr; + } + + if (timer) { + timer->Cancel(); } mRequest = nullptr; @@ -293,9 +300,15 @@ public: // nsITimerCallback NS_IMETHOD Notify(nsITimer *timer) override { - if (mRequest) - mRequest->Cancel(NS_ERROR_NET_TIMEOUT); - mTimer = nullptr; + nsCOMPtr request; + { + MutexAutoLock lock(mMutex); + request.swap(mRequest); + mTimer = nullptr; + } + if (request) { + request->Cancel(NS_ERROR_NET_TIMEOUT); + } return NS_OK; } @@ -303,6 +316,7 @@ public: nsCOMPtr mRequest; nsCOMPtr mResponse; nsCOMPtr mTimer; + Mutex mMutex; private: ~PACResolver() {} From c0a28dbd30a65ff511dd5ba98b17fb2d7d0c65ac Mon Sep 17 00:00:00 2001 From: Jonathan Kew Date: Fri, 6 Dec 2019 13:42:37 +0100 Subject: [PATCH 3/6] Check if we're already at the end of the frame's content. --- layout/generic/nsTextFrame.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/layout/generic/nsTextFrame.cpp b/layout/generic/nsTextFrame.cpp index 0641c7439a..59ef020ce2 100644 --- a/layout/generic/nsTextFrame.cpp +++ b/layout/generic/nsTextFrame.cpp @@ -7525,7 +7525,8 @@ nsTextFrame::GetCharacterRectsInRange(int32_t aInOffset, gfxSkipCharsIterator nextIter(iter); nextIter.AdvanceOriginal(1); if (!nextIter.IsOriginalCharSkipped() && - !mTextRun->IsClusterStart(nextIter.GetSkippedOffset())) { + !mTextRun->IsClusterStart(nextIter.GetSkippedOffset()) && + nextIter.GetOriginalOffset() < kContentEnd) { FindClusterEnd(mTextRun, kContentEnd, &nextIter); } From 39cc10dde887d4995ce5bceb7c22544e330cbbf6 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Fri, 6 Dec 2019 14:07:01 +0100 Subject: [PATCH 4/6] Update identifier map entries and notify if they get removed. This can happen through DestroyElementMaps() Based on work by Markus Stange and Edgar Chen. --- dom/base/nsDocument.cpp | 35 +++++++++++--- dom/base/nsDocument.h | 5 ++ .../filters/liveness-document-open.html | 46 +++++++++++++++++++ dom/canvas/test/reftest/filters/reftest.list | 1 + 4 files changed, 81 insertions(+), 6 deletions(-) create mode 100644 dom/canvas/test/reftest/filters/liveness-document-open.html diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp index 6b8e11db05..e2be6b6645 100644 --- a/dom/base/nsDocument.cpp +++ b/dom/base/nsDocument.cpp @@ -395,6 +395,21 @@ nsIdentifierMapEntry::FireChangeCallbacks(Element* aOldElement, } } +void +nsIdentifierMapEntry::ClearAndNotify() +{ + Element* currentElement = mIdContentList.SafeElementAt(0); + mIdContentList.Clear(); + if (currentElement) { + FireChangeCallbacks(currentElement, nullptr); + } + mNameContentList = nullptr; + if (mImageElement) { + SetImageElement(nullptr); + } + mChangeCallbacks = nullptr; +} + namespace { struct PositionComparator @@ -1422,12 +1437,12 @@ nsDocument::~nsDocument() delete mSubDocuments; mSubDocuments = nullptr; + nsAutoScriptBlocker scriptBlocker; + // Destroy link map now so we don't waste time removing // links one by one DestroyElementMaps(); - nsAutoScriptBlocker scriptBlocker; - for (uint32_t indx = mChildren.ChildCount(); indx-- != 0; ) { mChildren.ChildAt(indx)->UnbindFromTree(); mChildren.RemoveChildAt(indx); @@ -1972,15 +1987,16 @@ nsDocument::ResetToURI(nsIURI *aURI, nsILoadGroup *aLoadGroup, delete mSubDocuments; mSubDocuments = nullptr; - // Destroy link map now so we don't waste time removing - // links one by one - DestroyElementMaps(); - bool oldVal = mInUnlinkOrDeletion; mInUnlinkOrDeletion = true; uint32_t count = mChildren.ChildCount(); { // Scope for update MOZ_AUTO_DOC_UPDATE(this, UPDATE_CONTENT_MODEL, true); + + // Destroy link map now so we don't waste time removing + // links one by one + DestroyElementMaps(); + for (int32_t i = int32_t(count) - 1; i >= 0; i--) { nsCOMPtr content = mChildren.ChildAt(i); @@ -8955,7 +8971,14 @@ nsDocument::DestroyElementMaps() mStyledLinksCleared = true; #endif mStyledLinks.Clear(); + + // Notify ID change listeners before clearing the identifier map. + for (auto iter = mIdentifierMap.Iter(); !iter.Done(); iter.Next()) { + iter.Get()->ClearAndNotify(); + } + mIdentifierMap.Clear(); + ++mExpandoAndGeneration.generation; } diff --git a/dom/base/nsDocument.h b/dom/base/nsDocument.h index 2b29b98fa5..ac600eb433 100644 --- a/dom/base/nsDocument.h +++ b/dom/base/nsDocument.h @@ -216,6 +216,11 @@ public: void RemoveContentChangeCallback(nsIDocument::IDTargetObserver aCallback, void* aData, bool aForImage); + /** + * Remove all elements and notify change listeners. + */ + void ClearAndNotify(); + void Traverse(nsCycleCollectionTraversalCallback* aCallback); struct ChangeCallback { diff --git a/dom/canvas/test/reftest/filters/liveness-document-open.html b/dom/canvas/test/reftest/filters/liveness-document-open.html new file mode 100644 index 0000000000..b3d76e550a --- /dev/null +++ b/dom/canvas/test/reftest/filters/liveness-document-open.html @@ -0,0 +1,46 @@ + + + +canvas filters: remove referenced filter element through document.open() + + + + + + + + + + + + diff --git a/dom/canvas/test/reftest/filters/reftest.list b/dom/canvas/test/reftest/filters/reftest.list index 9830307159..f5d671e4dc 100644 --- a/dom/canvas/test/reftest/filters/reftest.list +++ b/dom/canvas/test/reftest/filters/reftest.list @@ -6,6 +6,7 @@ default-preferences pref(canvas.filters.enabled,true) fuzzy-if(azureSkia,1,1500) == global-alpha.html global-alpha-ref.html == global-composite-operation.html global-composite-operation-ref.html == liveness.html ref.html +== liveness-document-open.html data:text/html,PASS == multiple-drop-shadows.html shadow-ref.html == shadow.html shadow-ref.html == subregion-fill-paint.html subregion-ref.html From 5a102074d10e1ed05feea2b41d0a2051c59ff24d Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Fri, 6 Dec 2019 14:54:57 +0100 Subject: [PATCH 5/6] Fix incorrect values for saving ordered lists as text. --- dom/base/nsPlainTextSerializer.cpp | 58 ++++++++++++++++-------------- dom/base/nsPlainTextSerializer.h | 4 +-- 2 files changed, 33 insertions(+), 29 deletions(-) diff --git a/dom/base/nsPlainTextSerializer.cpp b/dom/base/nsPlainTextSerializer.cpp index 8097c4ec8c..98c9cfe322 100644 --- a/dom/base/nsPlainTextSerializer.cpp +++ b/dom/base/nsPlainTextSerializer.cpp @@ -53,7 +53,6 @@ static int32_t GetUnicharStringWidth(const char16_t* pwcs, int32_t n); // Someday may want to make this non-const: static const uint32_t TagStackSize = 500; -static const uint32_t OLStackSize = 100; nsresult NS_NewPlainTextSerializer(nsIContentSerializer** aSerializer) @@ -100,10 +99,6 @@ nsPlainTextSerializer::nsPlainTextSerializer() mTagStackIndex = 0; mIgnoreAboveIndex = (uint32_t)kNotFound; - // initialize the OL stack, where numbers for ordered lists are kept - mOLStack = new int32_t[OLStackSize]; - mOLStackIndex = 0; - mULCount = 0; mIgnoredChildNodeLevel = 0; @@ -112,7 +107,6 @@ nsPlainTextSerializer::nsPlainTextSerializer() nsPlainTextSerializer::~nsPlainTextSerializer() { delete[] mTagStack; - delete[] mOLStack; NS_WARNING_ASSERTION(mHeadLevel == 0, "Wrong head level!"); } @@ -189,6 +183,8 @@ nsPlainTextSerializer::Init(uint32_t aFlags, uint32_t aWrapColumn, // XXX We should let the caller decide whether to do this or not mFlags &= ~nsIDocumentEncoder::OutputNoFramesContent; + MOZ_ASSERT(mOLStack.IsEmpty()); + return NS_OK; } @@ -438,6 +434,8 @@ nsPlainTextSerializer::AppendDocumentStart(nsIDocument *aDocument, return NS_OK; } +int32_t kOlStackDummyValue = 0; + nsresult nsPlainTextSerializer::DoOpenContainer(nsIAtom* aTag) { @@ -616,44 +614,45 @@ nsPlainTextSerializer::DoOpenContainer(nsIAtom* aTag) } else if (aTag == nsGkAtoms::ul) { // Indent here to support nested lists, which aren't included in li :-( - EnsureVerticalSpace(mULCount + mOLStackIndex == 0 ? 1 : 0); - // Must end the current line before we change indention + EnsureVerticalSpace(IsInOLOrUL() ? 0 : 1); + // Must end the current line before we change indention mIndent += kIndentSizeList; mULCount++; } else if (aTag == nsGkAtoms::ol) { - EnsureVerticalSpace(mULCount + mOLStackIndex == 0 ? 1 : 0); + EnsureVerticalSpace(IsInOLOrUL() ? 0 : 1); if (mFlags & nsIDocumentEncoder::OutputFormatted) { // Must end the current line before we change indention - if (mOLStackIndex < OLStackSize) { - nsAutoString startAttr; - int32_t startVal = 1; - if (NS_SUCCEEDED(GetAttributeValue(nsGkAtoms::start, startAttr))) { - nsresult rv = NS_OK; - startVal = startAttr.ToInteger(&rv); - if (NS_FAILED(rv)) - startVal = 1; + nsAutoString startAttr; + int32_t startVal = 1; + if (NS_SUCCEEDED(GetAttributeValue(nsGkAtoms::start, startAttr))) { + nsresult rv = NS_OK; + startVal = startAttr.ToInteger(&rv); + if (NS_FAILED(rv)) { + startVal = 1; } - mOLStack[mOLStackIndex++] = startVal; } + mOLStack.AppendElement(startVal); } else { - mOLStackIndex++; + mOLStack.AppendElement(kOlStackDummyValue); } mIndent += kIndentSizeList; // see ul } else if (aTag == nsGkAtoms::li && (mFlags & nsIDocumentEncoder::OutputFormatted)) { if (mTagStackIndex > 1 && IsInOL()) { - if (mOLStackIndex > 0) { + if (!mOLStack.IsEmpty()) { nsAutoString valueAttr; if (NS_SUCCEEDED(GetAttributeValue(nsGkAtoms::value, valueAttr))) { nsresult rv = NS_OK; int32_t valueAttrVal = valueAttr.ToInteger(&rv); - if (NS_SUCCEEDED(rv)) - mOLStack[mOLStackIndex-1] = valueAttrVal; + if (NS_SUCCEEDED(rv)) { + mOLStack.LastElement() = valueAttrVal; + } } // This is what nsBulletFrame does for OLs: - mInIndentString.AppendInt(mOLStack[mOLStackIndex-1]++, 10); + mInIndentString.AppendInt(mOLStack.LastElement(), 10); + mOLStack.LastElement()++; } else { mInIndentString.Append(char16_t('#')); @@ -878,7 +877,8 @@ nsPlainTextSerializer::DoCloseContainer(nsIAtom* aTag) else if (aTag == nsGkAtoms::ul) { FlushLine(); mIndent -= kIndentSizeList; - if (--mULCount + mOLStackIndex == 0) { + --mULCount; + if (!IsInOLOrUL()) { mFloatingLines = 1; mLineBreakDue = true; } @@ -886,9 +886,9 @@ nsPlainTextSerializer::DoCloseContainer(nsIAtom* aTag) else if (aTag == nsGkAtoms::ol) { FlushLine(); // Doing this after decreasing OLStackIndex would be wrong. mIndent -= kIndentSizeList; - NS_ASSERTION(mOLStackIndex, "Wrong OLStack level!"); - mOLStackIndex--; - if (mULCount + mOLStackIndex == 0) { + NS_ASSERTION(!mOLStack.IsEmpty(), "Wrong OLStack level!"); + mOLStack.RemoveElementAt(mOLStack.Length() - 1); + if (!IsInOLOrUL()) { mFloatingLines = 1; mLineBreakDue = true; } @@ -1861,6 +1861,10 @@ nsPlainTextSerializer::IsInOL() return false; } +bool nsPlainTextSerializer::IsInOLOrUL() const { + return (mULCount > 0) || !mOLStack.IsEmpty(); +} + /* @return 0 = no header, 1 = h1, ..., 6 = h6 */ diff --git a/dom/base/nsPlainTextSerializer.h b/dom/base/nsPlainTextSerializer.h index 5055c75a08..650a8e3e73 100644 --- a/dom/base/nsPlainTextSerializer.h +++ b/dom/base/nsPlainTextSerializer.h @@ -81,6 +81,7 @@ private: void Write(const nsAString& aString); bool IsInPre(); bool IsInOL(); + bool IsInOLOrUL() const; bool IsCurrentNodeConverted(); bool MustSuppressLeaf(); @@ -218,8 +219,7 @@ private: uint32_t mIgnoreAboveIndex; // The stack for ordered lists - int32_t *mOLStack; - uint32_t mOLStackIndex; + AutoTArray mOLStack; uint32_t mULCount; From 72e5396dfacefad2361abe644f3fed6f1e4bff67 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Fri, 6 Dec 2019 15:08:10 +0100 Subject: [PATCH 6/6] Initialize all fields of GdkEvents in nsClipboard.cpp --- widget/gtk/nsClipboard.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/widget/gtk/nsClipboard.cpp b/widget/gtk/nsClipboard.cpp index eecae3e888..950be1dc4a 100644 --- a/widget/gtk/nsClipboard.cpp +++ b/widget/gtk/nsClipboard.cpp @@ -806,7 +806,7 @@ void GetHTMLCharset(guchar * data, int32_t dataLength, nsCString& str) static void DispatchSelectionNotifyEvent(GtkWidget *widget, XEvent *xevent) { - GdkEvent event; + GdkEvent event = {}; event.selection.type = GDK_SELECTION_NOTIFY; event.selection.window = gtk_widget_get_window(widget); event.selection.selection = gdk_x11_xatom_to_atom(xevent->xselection.selection); @@ -822,7 +822,7 @@ DispatchPropertyNotifyEvent(GtkWidget *widget, XEvent *xevent) { GdkWindow *window = gtk_widget_get_window(widget); if ((gdk_window_get_events(window)) & GDK_PROPERTY_CHANGE_MASK) { - GdkEvent event; + GdkEvent event = {}; event.property.type = GDK_PROPERTY_NOTIFY; event.property.window = window; event.property.atom = gdk_x11_xatom_to_atom(xevent->xproperty.atom);