Notes looking at sweettooth-plugin

[Owen Taylor <otaylor redhat com>]

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