586603 - HTMLContentSink::BeginContext should null check mCurrentContext before creating sc

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[timeless]

1760 HTMLContentSink::BeginContext(PRInt32 aPosition)

too early:
1764   // Create new context
1765   SinkContext* sc = new SinkContext(this);
1766   if (!sc) {
1767     return NS_ERROR_OUT_OF_MEMORY;
1768   }

1770   if (!mCurrentContext) {
1771     NS_ERROR("Nonexistent context");
leaked:
1773     return NS_ERROR_FAILURE;
1774   }
1775 
1776   // Flush everything in the current context so that we don't have
1777   // to worry about insertions resulting in inconsistent frame creation.
1778   mCurrentContext->FlushTags();
1779 
1780   // Sanity check.
1781   if (mCurrentContext->mStackPos <= aPosition) {
1782     NS_ERROR("Out of bounds position");
leaked:
1783     return NS_ERROR_FAILURE;
1784   }
1785 
1786   PRInt32 insertionPoint = -1;
1787   nsHTMLTag nodeType      = mCurrentContext->mStack[aPosition].mType;
1788   nsGenericHTMLElement* content = mCurrentContext->mStack[aPosition].mContent;
1789 
1790   // If the content under which the new context is created
1791   // has a child on the stack, the insertion point is
1792   // before the last child.
1793   if (aPosition < (mCurrentContext->mStackPos - 1)) {
1794     insertionPoint = content->GetChildCount() - 1;
1795   }

just right:
1797   sc->Begin(nodeType,
1798             content,
1799             mCurrentContext->mStack[aPosition].mNumFlushed,
1800             insertionPoint);
1801   NS_ADDREF(sc->mSink);
1802 
1803   mContextStack.AppendElement(mCurrentContext);
1804   mCurrentContext = sc;
1805   return NS_OK;
1806 }

Comment 1

[timeless]

Attachment: patch

Comment 2

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Might as well remove the
1766   if (!sc) {
1767     return NS_ERROR_OUT_OF_MEMORY;
1768   }
as new is infallible...

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 465175 [details] [diff] [review]
patch

Actually, there is no need for null check.
new is infallible.

Comment 4

[timeless]

Attachment: less code

Comment 5

[Johnny Stenback (:jst)]

Comment on attachment 466570 [details] [diff] [review]
less code

Mass minusing patch approval that don't have high return. Please renominate if this is more important for 2.0 than it appears.

Comment 6

[(no longer active)]

http://hg.mozilla.org/projects/cedar/rev/d7cbfae806d9

Comment 7

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/d7cbfae806d9