Some review comments reading the code to sweettooth-plugin: * I don't see any reason that this shouldn't just live in the gnome-shell tree - that would at least reduce the problem of the plugin and gnome-shell having incompatible versions.
It would also avoid having packages with the sweettooth-plugin name; it's fine if the code deployed on extensions.gnome.org has a funky codename, but I don't want people listing software on a computer to have to know what sweettooth-plugin is. (Distributions can still have a gnome-shell-browser-plugin sub-package if they choose.) I think we would just stick npapi.h into the gnome-shell tree to avoid sticky build-time dependencies. * The 'funcs' variable should be static. * We should require the origin to be https://extensions.gnome.org * The origin checking code, as far as I can see, will consider http://extensions.gnome.org.mysite.com to be valid. It would be best if we can actually use an established URI parser to parse the URL. Could just link to libsoup and use soup-uri. * For extra safety, check that the null-terminated length of document.location.href matches the length in the NPString. * You should be checking the return value of funcs.getvalue, funcs.getproperty. I also think it's better not to g_assert() when checking for the right types, just fail cleanly. * It seems like there should be a callback if the shell restarts while the plugin is running to allow the webpage to react/restart. * The handling of G_DBUS_ERROR_NAME_HAS_NO_OWNER in NPP_New is leaky and not as intended. * Might be good to have a uuid-validation function that validates to characters that should be in a uuid and call that on all uuids. I don't see anything in the shell code that would obviously have problems with a uuid like '../../foo' but better to catch such things upfront. * I don't particularly like the use of g_return_if_fail() for checking for invalid plugin API use. While I think we can be confident that nobody is going to compile the plugin with -DG_DISABLE_ASSERT, g_return_if_fail() is stylistically supposed to be a debugging aid and not a reliable check. * plugin_object_get_property() doesn't properly propagate back errors from plugin_get_api_version/plugin_get_shell_version _______________________________________________ gnome-shell-list mailing list gnome-shell-list@gnome.org http://mail.gnome.org/mailman/listinfo/gnome-shell-list