Bug 6754 – Deploy RefPtr throughout more of WebCore

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Eric Seidel (no email)]

Deploy RefPtr throughout more of WebCore

I was in the mood tonight... I spread a bit more of the RefPtr love throughout WebCore.

[Eric Seidel (no email)]

Created attachment 5905 [details]
proposed patch

[Darin Adler]

Comment on attachment 5905 [details]
proposed patch

This looks wrong:

-    : AttributeImpl(name, value), m_styleDecl(decl)
+    : AttributeImpl(name, value)

and occurs twice. I wonder why this did not show up in testing.

+    void setDecl(CSSMappedAttributeDeclarationImpl* decl) { m_styleDecl = decl; }

This should take a PassRefPtr.

+    bool ret = dispatchGenericEvent(evt, exceptioncode);

Since "evt" is a PassRefPtr, once you pass it to dispatchGenericEvent it will change to 0, so you won't be able to call finishedWithEvent. So in this case, you need to pass evt.get() even though you have a PassRefPtr and the target function takes a PassRefPtr.

You should see obvious test failures from some of the problems above.

In HTMLElementImpl.cpp, the code assumes that it's OK to just keep nextNode in a raw pointer. But since functions like insertBefore and removeChild send DOM mutation events, it's entirely possible that nextNode will be deallocated. So nextNode needs to be a RefPtr too.

+                fragment->insertBefore(child, node.get(), ignoredExceptionCode);

Should just be node here, not node.get().

+            fragment->removeChild(node.get(), ignoredExceptionCode);

Same here.

+        RefPtr<NodeImpl> n = it.current();
+        removeChild(n.get(), exceptioncode);

This can just be removeChild(it.current(), exceptioncode). There used to be reason for the caller to ref/deref, but removeChild now takes a PassRefPtr.

+    RefPtr<NodeImpl> item = items[listIndex];
+    removeChild(item.get(), exceptioncode);

Same here.

+    if(index >= 0 && index < numRows) {

Needs a space after the if (two times).

+        RefPtr<NodeImpl> row = children->item(index);
+        HTMLElementImpl::removeChild(row.get(), exceptioncode);

Same thing, no need to use a RefPtr for this. (Twice.)

[Eric Seidel (no email)]

Created attachment 5975 [details]
addressed darin's concerns