Merge lp:~alexlauni/unity/optional-debugging into lp:unity

[Alex Launi]

dbarth doesn't want introspection on by default in the final unity release, this makes it optional with a compiz option that can be enabled/disabled at runtime and a cmake option to set whether or not the option is on or off by default.

[Neil J. Patel (njpatel)]

Although the code looks fine, I don't believe there is a reason for this to be merged. The introspection has very little impact at startup (especially as gdbus bus connections are async) and no impact during runtime unless someone actually uses it.

review: Disapprove

Unmerged revisions

683. By Alex Launi on 2010-12-09

Allow introspection interface to be enabled/disabled with a compiz option

682. By Alex Launi on 2010-12-09

merge trunk

681. By Alex Launi on 2010-12-09

Make dbus introspection optional at runtime

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2010-12-09 13:30:35 +0000
+++ CMakeLists.txt	2010-12-09 17:11:52 +0000
@@ -84,6 +84,16 @@
                CFLAGSADD "-DINSTALLPREFIX='\"${CMAKE_INSTALL_PREFIX}\"' -DPKGDATADIR='\"${CMAKE_INSTALL_PREFIX}/share/unity/3\"' -I${CMAKE_BINARY_DIR}"
                LIBRARIES "unity"
                )
+#
+# Release build to disable DBus introspection
+#
+option (ENABLE_RELEASE_BUILD "Enable release build" OFF)
+if (ENABLE_RELEASE_BUILD)
+    set (USE_DEBUGGING "false")
+else (ENABLE_RELEASE_BUILD)
+    set (USE_DEBUGGING "true")
+endif (ENABLE_RELEASE_BUILD)
+configure_file (${CMAKE_CURRENT_SOURCE_DIR}/unityshell.xml.in.in ${CMAKE_CURRENT_SOURCE_DIR}/unityshell.xml.in)
 
 #
 # GSettings Schema

=== modified file 'src/IntrospectionDBusInterface.cpp'
--- src/IntrospectionDBusInterface.cpp	2010-12-01 22:28:50 +0000
+++ src/IntrospectionDBusInterface.cpp	2010-12-09 17:11:52 +0000
@@ -72,10 +72,13 @@
   NULL,
 };
 
-static Introspectable      *_introspectable;
+static bool             _initialized;
+static GDBusConnection *_connection;
+static Introspectable  *_introspectable;
 
 IntrospectionDBusInterface::IntrospectionDBusInterface (Introspectable *introspectable)
 {
+  _initialized = false;
   _introspectable = introspectable;
   _owner_id = g_bus_own_name (G_BUS_TYPE_SESSION,
                               UNITY_STATE_DEBUG_BUS_NAME,
@@ -95,40 +98,63 @@
 void 
 IntrospectionDBusInterface::OnBusAcquired (GDBusConnection *connection, const gchar *name, gpointer data)
 {
+  _connection = connection;
+  _initialized = true;
+  
+  IntrospectionDBusInterface *self = (IntrospectionDBusInterface*) data;
+  
+  if (self != NULL && !self->_delayed_register.empty())
+    self->_delayed_register ();
+}
+
+void
+IntrospectionDBusInterface::OnNameAcquired (GDBusConnection *connection, const gchar *name, gpointer data)
+{
+}
+
+void
+IntrospectionDBusInterface::OnNameLost (GDBusConnection *connection, const gchar *name, gpointer data)
+{
+}
+
+void
+IntrospectionDBusInterface::Register ()
+{
+  if (!_initialized) {
+    _delayed_register = sigc::mem_fun (this, &IntrospectionDBusInterface::Register);
+    return;
+  }
+  
   GError *error = NULL;
-  g_dbus_connection_register_object (connection,
-                                     "/com/canonical/Unity/Debug/Introspection",
-                                     (GDBusInterfaceInfo *) &si_iface_info,
-                                     &si_vtable,
-                                     NULL,
-                                     NULL,
-                                     &error);
+  _object_id = g_dbus_connection_register_object (_connection,
+                                                  "/com/canonical/Unity/Debug/Introspection",
+                                                  (GDBusInterfaceInfo *) &si_iface_info,
+                                                  &si_vtable,
+                                                  NULL,
+                                                  NULL,
+                                                  &error);
   if (error != NULL)
   {
     g_warning ("Could not register Introspection object onto d-bus");
-	g_error_free (error);
+	  g_error_free (error);
   }
 }
 
 void
-IntrospectionDBusInterface::OnNameAcquired (GDBusConnection *connection, const gchar *name, gpointer data)
-{
-}
-
-void
-IntrospectionDBusInterface::OnNameLost (GDBusConnection *connection, const gchar *name, gpointer data)
-{
+IntrospectionDBusInterface::Unregister ()
+{
+  g_dbus_connection_unregister_object (_connection, _object_id);
 }
 
 void
 DBusMethodCall (GDBusConnection *connection,
-                                                 const gchar *sender,
-                                                 const gchar *objectPath,
-                                                 const gchar *ifaceName,
-                                                 const gchar *methodName,
-                                                 GVariant *parameters,
-                                                 GDBusMethodInvocation *invocation,
-                                                 gpointer data)
+                const gchar *sender,
+                const gchar *objectPath,
+                const gchar *ifaceName,
+                const gchar *methodName,
+                GVariant *parameters,
+                GDBusMethodInvocation *invocation,
+                gpointer data)
 {
   if (g_strcmp0 (methodName, "GetState") == 0)
   {

=== modified file 'src/IntrospectionDBusInterface.h'
--- src/IntrospectionDBusInterface.h	2010-11-24 15:25:50 +0000
+++ src/IntrospectionDBusInterface.h	2010-12-09 17:11:52 +0000
@@ -21,19 +21,22 @@
 
 #include <glib.h>
 #include <gio/gio.h>
+#include <sigc++/sigc++.h>
 #include "Introspectable.h"
 
-class IntrospectionDBusInterface
+typedef sigc::slot<void> RegisterCallback;
+
+class IntrospectionDBusInterface : public sigc::trackable
 {
 public:
   IntrospectionDBusInterface  (Introspectable *introspectable);
   ~IntrospectionDBusInterface ();
-
-  /*static void DBusMethodCall (GDBusConnection *connection, const gchar *sender,
-                              const gchar *objectPath, const gchar *ifaceName,
-                              const gchar *methodName, GVariant *parameters,
-                              GDBusMethodInvocation *invocation, gpointer data);	*/
-
+  
+  void Register ();
+  void Unregister ();
+  
+  RegisterCallback _delayed_register;
+  
 private:
   /* methods */
 
@@ -43,12 +46,11 @@
 
   static void OnNameLost (GDBusConnection *connection, const gchar *name, gpointer data);
 
-  //static GVariant *GetState (const char *piece);
-
   static GVariant *BuildFakeReturn ();
 
   /* members */
-  guint						_owner_id;
+  guint						 _owner_id;
+  guint            _object_id;
 };
 
 #endif

=== modified file 'src/unity.cpp'
--- src/unity.cpp	2010-12-09 13:30:35 +0000
+++ src/unity.cpp	2010-12-09 17:11:52 +0000
@@ -384,6 +384,12 @@
     case UnityshellOptions::LauncherFloat:
       launcher->SetFloating (optionGetLauncherFloat ());
       break;
+    case UnityshellOptions::DebuggingIntrospection:
+      if (optionGetDebuggingIntrospection ())
+        debugger->Register ();
+      else
+        debugger->Unregister ();
+      break;        
     default:
       break;
   }
@@ -430,13 +436,18 @@
 
   wt->Run (NULL);
   uScreen = this;
-
+    
   debugger = new IntrospectionDBusInterface (this);
-
+  if (optionGetDebuggingIntrospection ())
+    debugger->Register ();
+  else 
+    debugger->Unregister ();
+  
   PluginAdapter::Initialize (screen);
 
   optionSetLauncherAutohideNotify (boost::bind (&UnityScreen::optionChanged, this, _1, _2));
   optionSetLauncherFloatNotify (boost::bind (&UnityScreen::optionChanged, this, _1, _2));
+  optionSetDebuggingIntrospectionNotify (boost::bind (&UnityScreen::optionChanged, this, _1, _2));
 
   g_timeout_add (0, &UnityScreen::initPluginActions, this);
   END_FUNCTION ();

=== renamed file 'unityshell.xml.in' => 'unityshell.xml.in.in'
--- unityshell.xml.in	2010-11-30 19:31:08 +0000
+++ unityshell.xml.in.in	2010-12-09 17:11:52 +0000
@@ -46,6 +46,10 @@
 	    <_long>Make the launcher appear above other windows</_long>
 	    <default>false</default>
 	    </option>
+	    <option name="debugging_introspection" type="bool">
+	    <_short>Enable DBus introspection</_short>
+	    <default>@USE_DEBUGGING@</default>
+	    </option>
 	</options>
     </plugin>
 </compiz>