Bug 7065 – refactor and simplify ChildFrame in preparation for removal

• Component: Frames
• Status: RESOLVED FIXED

[Maciej Stachowiak]

Refactor and clean up ChildFrame in preparation for removing it; most fields are removed now.

[Maciej Stachowiak]

Created attachment 6242 [details]
the cleanup

[Darin Adler]

Comment on attachment 6242 [details]
the cleanup

Looks great. r=me

When reviewing this I saw some minor issues in the old code.

When going from the render tree to the DOM tree, I recommend using node() rather than element(). Despite the misleading names of these two functions, the differences between the two are:

    1) element() returns 0 if the RenderObject is anonymous
    2) element() is overriden in derived classes to return an object of the right type for convenience to avoid the need to cast

So in MacFrame::createFrame I would be using renderer->node(), since we don't need the null checks.

I'm wondering about this (old) code:

+    // Some JS code in the load event may have destroyed the plugin, in that case, abort
+    if (!child->m_renderer)
+        return false;

What guarantees in this case that the child pointer is still good?

[Maciej Stachowiak]

I don't think the node() vs. element() thing matters much for frames, since frames cannot (currently anyway) correspond anonymous render objects. I don't think any code is doing unnecessary nil checks either.

After landing I will try to make a test case for the "destroyed the plugin" case, I think the reason the child pointer is still good right now is that nothing gets removed from m_objects ever except when you leave the page.