[webkit-reviews] review denied: [Bug 16815] Crash with navigator.plugins and navigator.mimeTypes after plugins.refresh : [Attachment 18364] Proposed patch (concept)

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Simon Hausmann <hausmann at kde.org>'s
request for review:
Bug 16815: Crash with navigator.plugins and navigator.mimeTypes after
plugins.refresh
http://bugs.webkit.org/show_bug.cgi?id=16815

Attachment 18364: Proposed patch (concept)
http://bugs.webkit.org/attachment.cgi?id=18364&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
Here are a few comments:

If you're going to put frame pointers into all of these separately reference
counted objects, you either need to make the frame pointers get zeroed out when
the frame goes away or you need to make them RefPtr<Frame>.

The direction I'd like to see this move is to make these objects more like
normal DOM objects, with automatically generated bindings.

I don't see why the list of plug-ins should be per-frame or per-page. Having
each web page keep a separate list of plug-ins seems unnecessary. I'm concerned
that we're taking too many lower-level things and involving the page in them.

On the other hand, I could be wrong. If we are going to make the plug-in data
structures be per-page we should do it cross platform with as much
cross-platform code as possible.

If the plug-in information is really per-page, then the functions should be
members of Page or a class with a page pointer in them. I don't think it's a
good pattern to have a class PlugInInfoStore and pass a frame pointer to all
the functions of that class.

This looks like a Qt-only patch.

+	 PluginBase(ExecState *exec, Frame* frame);

Normally we would not name these arguments when their purpose is completely
clear from their type.