[webkit-reviews] review denied: [Bug 28677] Allow excluding certain plugins from loading : [Attachment 38475] Proposed patch to implement the plugin loading delegation described in the feature request.

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Marius Renn <damarvy at gmail.com>'s
request for review:
Bug 28677: Allow excluding certain plugins from loading
https://bugs.webkit.org/show_bug.cgi?id=28677

Attachment 38475: Proposed patch to implement the plugin loading delegation
described in the feature request.
https://bugs.webkit.org/attachment.cgi?id=38475&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
I would probably have broken:
   if (!m_client || m_client->shouldLoadPluginAtPath(*it)) {
 121		 RefPtr<PluginPackage> package =
PluginPackage::createPackage(*it, lastModified);
 122		 if (package) {
 123		     if ((!m_client ||
m_client->shouldLoadPluginPackage(package)) && add(package))
 124			 pluginSetChanged = true;
 125		     package.release();
 126		 }
 127	     }
out into a little function to make early return possible (for when !package)
for instance.  You'd have to pass the Client* and the path, and likey return a
PassRefPtr<PluginPackage>  It would get rid of the need for explicit release
though.

Probably would look something like this:

RefPtr<PluginPackage> package = loadPluginAtPath(*it, m_client);
if (addPluginPackage(package.release(), this))
    pluginSetChanged = true;

Actually, I'm not sure that's cleaner since you have to pass "this" in oder to
get access to "add()"

Anyway, you can re-work that function if you'd like, or it's OK as is.	But I'm
going to r- this for the wrong copyright here:

+ * Copyright (C) 2007 Apple Inc. All rights reserved.	Should show your
copyright

Otherwise this looks fine to me.  I'll CC the plugin experts in case they have
comment.