Bug 1793485: Fixed memory issue in NodeWillBeDestroyed() for MultiMutationObserver classes. r=smaug

Differential Revision: https://phabricator.services.mozilla.com/D158671
This commit is contained in:
Jan-Niklas Jaeschke 2022-10-13 14:26:07 +00:00
parent ba9267985d
commit 311f68ad19
17 changed files with 26 additions and 24 deletions

View File

@ -28,7 +28,7 @@ JSObject* StyleSheetList::WrapObject(JSContext* aCx,
return StyleSheetList_Binding::Wrap(aCx, this, aGivenProto); return StyleSheetList_Binding::Wrap(aCx, this, aGivenProto);
} }
void StyleSheetList::NodeWillBeDestroyed(const nsINode* aNode) { void StyleSheetList::NodeWillBeDestroyed(nsINode* aNode) {
mDocumentOrShadowRoot = nullptr; mDocumentOrShadowRoot = nullptr;
} }

View File

@ -590,7 +590,7 @@ int32_t nsContentList::IndexOf(nsIContent* aContent) {
return IndexOf(aContent, true); return IndexOf(aContent, true);
} }
void nsContentList::NodeWillBeDestroyed(const nsINode* aNode) { void nsContentList::NodeWillBeDestroyed(nsINode* aNode) {
// We shouldn't do anything useful from now on // We shouldn't do anything useful from now on
RemoveFromCaches(); RemoveFromCaches();

View File

@ -374,7 +374,7 @@ void nsMutationReceiver::ContentRemoved(nsIContent* aChild,
Observer()->ScheduleForRun(); Observer()->ScheduleForRun();
} }
void nsMutationReceiver::NodeWillBeDestroyed(const nsINode* aNode) { void nsMutationReceiver::NodeWillBeDestroyed(nsINode* aNode) {
NS_ASSERTION(!mParent, "Shouldn't have mParent here!"); NS_ASSERTION(!mParent, "Shouldn't have mParent here!");
Disconnect(true); Disconnect(true);
} }

View File

@ -288,7 +288,7 @@ class nsIMutationObserver
* the observer. The observer is responsible for making sure it * the observer. The observer is responsible for making sure it
* stays alive for the duration of the call as needed. * stays alive for the duration of the call as needed.
*/ */
virtual void NodeWillBeDestroyed(const nsINode* aNode) = 0; virtual void NodeWillBeDestroyed(nsINode* aNode) = 0;
/** /**
* Notification that the node's parent chain has changed. This * Notification that the node's parent chain has changed. This
@ -345,7 +345,7 @@ NS_DEFINE_STATIC_IID_ACCESSOR(nsIMutationObserver, NS_IMUTATION_OBSERVER_IID)
nsIContent* aPreviousSibling) override; nsIContent* aPreviousSibling) override;
#define NS_DECL_NSIMUTATIONOBSERVER_NODEWILLBEDESTROYED \ #define NS_DECL_NSIMUTATIONOBSERVER_NODEWILLBEDESTROYED \
virtual void NodeWillBeDestroyed(const nsINode* aNode) override; virtual void NodeWillBeDestroyed(nsINode* aNode) override;
#define NS_DECL_NSIMUTATIONOBSERVER_PARENTCHAINCHANGED \ #define NS_DECL_NSIMUTATIONOBSERVER_PARENTCHAINCHANGED \
virtual void ParentChainChanged(nsIContent* aContent) override; virtual void ParentChainChanged(nsIContent* aContent) override;
@ -363,7 +363,7 @@ NS_DEFINE_STATIC_IID_ACCESSOR(nsIMutationObserver, NS_IMUTATION_OBSERVER_IID)
NS_DECL_NSIMUTATIONOBSERVER_PARENTCHAINCHANGED NS_DECL_NSIMUTATIONOBSERVER_PARENTCHAINCHANGED
#define NS_IMPL_NSIMUTATIONOBSERVER_CORE_STUB(_class) \ #define NS_IMPL_NSIMUTATIONOBSERVER_CORE_STUB(_class) \
void _class::NodeWillBeDestroyed(const nsINode* aNode) {} void _class::NodeWillBeDestroyed(nsINode* aNode) {}
#define NS_IMPL_NSIMUTATIONOBSERVER_CONTENT(_class) \ #define NS_IMPL_NSIMUTATIONOBSERVER_CONTENT(_class) \
void _class::CharacterDataWillChange( \ void _class::CharacterDataWillChange( \

View File

@ -100,11 +100,12 @@ class MutationObserverWrapper final : public nsIMutationObserver {
mOwner->ContentRemoved(aChild, aPreviousSibling); mOwner->ContentRemoved(aChild, aPreviousSibling);
} }
void NodeWillBeDestroyed(const nsINode* aNode) override { void NodeWillBeDestroyed(nsINode* aNode) override {
MOZ_ASSERT(mOwner); MOZ_ASSERT(mOwner);
AddRefWrapper();
RefPtr<nsMultiMutationObserver> owner = mOwner; RefPtr<nsMultiMutationObserver> owner = mOwner;
owner->NodeWillBeDestroyed(aNode); owner->NodeWillBeDestroyed(aNode);
owner->mWrapperForNode.Remove(const_cast<nsINode*>(aNode)); owner->RemoveMutationObserverFromNode(aNode);
mOwner = nullptr; mOwner = nullptr;
ReleaseWrapper(); ReleaseWrapper();
} }
@ -115,8 +116,9 @@ class MutationObserverWrapper final : public nsIMutationObserver {
} }
MozExternalRefCountType AddRefWrapper() { MozExternalRefCountType AddRefWrapper() {
NS_LOG_ADDREF(this, mRefCnt, "MutationObserverWrapper", sizeof(*this)); nsrefcnt count = ++mRefCnt;
return ++mRefCnt; NS_LOG_ADDREF(this, count, "MutationObserverWrapper", sizeof(*this));
return count;
} }
MozExternalRefCountType ReleaseWrapper() { MozExternalRefCountType ReleaseWrapper() {
@ -124,9 +126,8 @@ class MutationObserverWrapper final : public nsIMutationObserver {
NS_LOG_RELEASE(this, mRefCnt, "MutationObserverWrapper"); NS_LOG_RELEASE(this, mRefCnt, "MutationObserverWrapper");
if (mRefCnt == 0) { if (mRefCnt == 0) {
mRefCnt = 1; mRefCnt = 1;
auto refCnt = MozExternalRefCountType(mRefCnt);
delete this; delete this;
return refCnt; return MozExternalRefCountType(0);
} }
return mRefCnt; return mRefCnt;
} }

View File

@ -240,7 +240,7 @@ void nsAttributeTextNode::AttributeChanged(Element* aElement,
} }
} }
void nsAttributeTextNode::NodeWillBeDestroyed(const nsINode* aNode) { void nsAttributeTextNode::NodeWillBeDestroyed(nsINode* aNode) {
NS_ASSERTION(aNode == static_cast<nsINode*>(mGrandparent), "Wrong node!"); NS_ASSERTION(aNode == static_cast<nsINode*>(mGrandparent), "Wrong node!");
mGrandparent = nullptr; mGrandparent = nullptr;
} }

View File

@ -489,7 +489,7 @@ void TableRowsCollection::ContentRemoved(nsIContent* aChild,
} }
} }
void TableRowsCollection::NodeWillBeDestroyed(const nsINode* aNode) { void TableRowsCollection::NodeWillBeDestroyed(nsINode* aNode) {
// Set mInitialized to false so CleanUp doesn't try to remove our mutation // Set mInitialized to false so CleanUp doesn't try to remove our mutation
// observer, as we're going away. CleanUp() will reset mInitialized to true as // observer, as we're going away. CleanUp() will reset mInitialized to true as
// it returns. // it returns.

View File

@ -236,7 +236,7 @@ void SVGUseElement::ContentRemoved(nsIContent* aChild,
} }
} }
void SVGUseElement::NodeWillBeDestroyed(const nsINode* aNode) { void SVGUseElement::NodeWillBeDestroyed(nsINode* aNode) {
nsCOMPtr<nsIMutationObserver> kungFuDeathGrip(this); nsCOMPtr<nsIMutationObserver> kungFuDeathGrip(this);
UnlinkSource(); UnlinkSource();
} }

View File

@ -177,7 +177,7 @@ void nsXMLPrettyPrinter::ContentRemoved(nsIContent* aChild,
MaybeUnhook(aChild->GetParent()); MaybeUnhook(aChild->GetParent());
} }
void nsXMLPrettyPrinter::NodeWillBeDestroyed(const nsINode* aNode) { void nsXMLPrettyPrinter::NodeWillBeDestroyed(nsINode* aNode) {
mDocument = nullptr; mDocument = nullptr;
NS_RELEASE_THIS(); NS_RELEASE_THIS();
} }

View File

@ -96,7 +96,7 @@ nsINode* XPathResult::IterateNext(ErrorResult& aRv) {
return mResultNodes.SafeElementAt(mCurrentPos++); return mResultNodes.SafeElementAt(mCurrentPos++);
} }
void XPathResult::NodeWillBeDestroyed(const nsINode* aNode) { void XPathResult::NodeWillBeDestroyed(nsINode* aNode) {
nsCOMPtr<nsIMutationObserver> kungFuDeathGrip(this); nsCOMPtr<nsIMutationObserver> kungFuDeathGrip(this);
// Set to null to avoid unregistring unnecessarily // Set to null to avoid unregistring unnecessarily
mDocument = nullptr; mDocument = nullptr;

View File

@ -1052,7 +1052,7 @@ nsresult txMozillaXSLTProcessor::ensureStylesheet() {
return TX_CompileStylesheet(style, this, getter_AddRefs(mStylesheet)); return TX_CompileStylesheet(style, this, getter_AddRefs(mStylesheet));
} }
void txMozillaXSLTProcessor::NodeWillBeDestroyed(const nsINode* aNode) { void txMozillaXSLTProcessor::NodeWillBeDestroyed(nsINode* aNode) {
nsCOMPtr<nsIMutationObserver> kungFuDeathGrip(this); nsCOMPtr<nsIMutationObserver> kungFuDeathGrip(this);
if (NS_FAILED(mCompileResult)) { if (NS_FAILED(mCompileResult)) {
return; return;

View File

@ -199,7 +199,7 @@ void ChromeObserver::AttributeChanged(dom::Element* aElement,
} }
} }
void ChromeObserver::NodeWillBeDestroyed(const nsINode* aNode) { void ChromeObserver::NodeWillBeDestroyed(nsINode* aNode) {
mDocument = nullptr; mDocument = nullptr;
} }

View File

@ -115,7 +115,7 @@ void ElementDeletionObserver::ParentChainChanged(nsIContent* aContent) {
NS_RELEASE_THIS(); NS_RELEASE_THIS();
} }
void ElementDeletionObserver::NodeWillBeDestroyed(const nsINode* aNode) { void ElementDeletionObserver::NodeWillBeDestroyed(nsINode* aNode) {
NS_ASSERTION(aNode == mNativeAnonNode || aNode == mObservedElement, NS_ASSERTION(aNode == mNativeAnonNode || aNode == mObservedElement,
"Wrong aNode!"); "Wrong aNode!");
if (aNode == mNativeAnonNode) { if (aNode == mNativeAnonNode) {

View File

@ -276,7 +276,7 @@ void BFCachePreventionObserver::ContentRemoved(nsIContent* aChild,
MutationHappened(); MutationHappened();
} }
void BFCachePreventionObserver::NodeWillBeDestroyed(const nsINode* aNode) { void BFCachePreventionObserver::NodeWillBeDestroyed(nsINode* aNode) {
mDocument = nullptr; mDocument = nullptr;
} }

View File

@ -955,7 +955,7 @@ void nsTreeContentView::ContentRemoved(nsIContent* aChild,
} }
} }
void nsTreeContentView::NodeWillBeDestroyed(const nsINode* aNode) { void nsTreeContentView::NodeWillBeDestroyed(nsINode* aNode) {
// XXXbz do we need this strong ref? Do we drop refs to self in ClearRows? // XXXbz do we need this strong ref? Do we drop refs to self in ClearRows?
nsCOMPtr<nsIMutationObserver> kungFuDeathGrip(this); nsCOMPtr<nsIMutationObserver> kungFuDeathGrip(this);
ClearRows(); ClearRows();

View File

@ -200,10 +200,11 @@ void nsFormFillController::NativeAnonymousChildListChange(nsIContent* aContent,
void nsFormFillController::ParentChainChanged(nsIContent* aContent) {} void nsFormFillController::ParentChainChanged(nsIContent* aContent) {}
MOZ_CAN_RUN_SCRIPT_BOUNDARY MOZ_CAN_RUN_SCRIPT_BOUNDARY
void nsFormFillController::NodeWillBeDestroyed(const nsINode* aNode) { void nsFormFillController::NodeWillBeDestroyed(nsINode* aNode) {
MOZ_LOG(sLogger, LogLevel::Verbose, ("NodeWillBeDestroyed: %p", aNode)); MOZ_LOG(sLogger, LogLevel::Verbose, ("NodeWillBeDestroyed: %p", aNode));
mPwmgrInputs.Remove(aNode); mPwmgrInputs.Remove(aNode);
mAutofillInputs.Remove(aNode); mAutofillInputs.Remove(aNode);
MaybeRemoveMutationObserver(aNode);
if (aNode == mListNode) { if (aNode == mListNode) {
mListNode = nullptr; mListNode = nullptr;
RevalidateDataList(); RevalidateDataList();

View File

@ -53,7 +53,7 @@ void nsMenuGroupOwnerX::ContentAppended(nsIContent* aFirstNewContent) {
} }
} }
void nsMenuGroupOwnerX::NodeWillBeDestroyed(const nsINode* aNode) {} void nsMenuGroupOwnerX::NodeWillBeDestroyed(nsINode* aNode) {}
void nsMenuGroupOwnerX::AttributeWillChange(dom::Element* aElement, int32_t aNameSpaceID, void nsMenuGroupOwnerX::AttributeWillChange(dom::Element* aElement, int32_t aNameSpaceID,
nsAtom* aAttribute, int32_t aModType) {} nsAtom* aAttribute, int32_t aModType) {}