Em Wednesday 20 June 2012, Daniel Nicoletti escreveu:
> Hi,
> 
> recently I reported a bug as the PowerManager library
> was reporting the wrong PowerSaveStatus, Lukas took
> a look, saw nothing wrong but proposed a patch, that well
> sort of worked. After doing several tests with Apped KDED
> module I realized the problem was when the DBus
> interface was not available since Apper is loaded before
> PM kded module.
> 
> So I decided to look at the code to find the issue, and
> well there are actually some:
> * QDBusPendingReply doesn't provide a default value
> in case the call fails, which is why it returned true
> when PM interface wasn't available, which in case the
> best default would of course be false.
> * If KDED dies for some reason (we can't think it must
> work, bugs happen), it doesn't watch for service registration
> so it won't update the client if the power save mode
> has changed.
> 
> To the first I have two ways of fixing it, create a function
> for cold plug which checks if the reply is valid otherwise
> defaults to false. OR actually have the values at
> the private class declared as QDBusPendingReply, which
> when the user calls appShouldConserveResources() we
> check if the reply is valid and if not we try getting the
> value again, otherwise just return.
> 
> For the second it's a bit more complex, since the current
> service watcher need to watch another path, and will
> need to cold plug everything if the service is registered.
> 
> I started coding a cold plug function but well the code will
> be quite big, so depending on the preferred approach I'll
> do my codings
> 
> QDBusPendingReply<bool> psReply = managerIface.GetPowerSaveStatus();
>     reply.waitForFinished();

        Avoid using waitForFinished or QDBusReply, they can cause freezes in 
the 
desktop. Always try the asynchronous approach first. Unfortunately in this 
case we will need them to retrieve the initial status of some variables. 
However, restrict the usage of waitForFinished and QDBusReply to early 
initialization. Do not use them in slots that can be called several times 
during the lifetime of the class.

>     bool psValue = psReply.isValid() ? psReply.value() : false;
>     if (psValue != powerSaveStatus) {
>         slotPowerSaveStatusChanged(psValue);
>     }
> 
> this is for only one of the values, there are more 2,
> so well, the patch will be quite intrusive...

        Well, I created the attached patch to fix the problem Lukas described 
yesterday on #solid. Can you test it for me? If you wish you can add it to 
git.reviewboard.kde.org too.

-- 
Lamarque V. Souza
KDE's Network Management maintainer
http://planetkde.org/pt-br
diff -Nru --show-c-function kdelibs-4.8.4.orig/solid/solid/powermanagement.cpp kdelibs-4.8.4/solid/solid/powermanagement.cpp
--- kdelibs-4.8.4.orig/solid/solid/powermanagement.cpp	2012-06-06 17:49:52.829041616 -0300
+++ kdelibs-4.8.4/solid/solid/powermanagement.cpp	2012-06-20 14:44:19.818873945 -0300
@@ -28,34 +28,24 @@
 SOLID_GLOBAL_STATIC(Solid::PowerManagementPrivate, globalPowerManager)
 
 Solid::PowerManagementPrivate::PowerManagementPrivate()
-    : managerIface("org.freedesktop.PowerManagement",
+    : managerIface(new OrgFreedesktopPowerManagementInterface("org.freedesktop.PowerManagement",
                    "/org/freedesktop/PowerManagement",
-                   QDBusConnection::sessionBus()),
-      policyAgentIface("org.kde.Solid.PowerManagement.PolicyAgent",
+                   QDBusConnection::sessionBus())),
+      policyAgentIface(new OrgKdeSolidPowerManagementPolicyAgentInterface("org.kde.Solid.PowerManagement.PolicyAgent",
                        "/org/kde/Solid/PowerManagement/PolicyAgent",
-                       QDBusConnection::sessionBus()),
-      inhibitIface("org.freedesktop.PowerManagement.Inhibit",
+                       QDBusConnection::sessionBus())),
+      inhibitIface(new OrgFreedesktopPowerManagementInhibitInterface("org.freedesktop.PowerManagement.Inhibit",
                    "/org/freedesktop/PowerManagement/Inhibit",
-                   QDBusConnection::sessionBus()),
+                   QDBusConnection::sessionBus())),
       serviceWatcher("org.kde.Solid.PowerManagement",
                      QDBusConnection::sessionBus(),
-                     QDBusServiceWatcher::WatchForRegistration)
+                     QDBusServiceWatcher::WatchForRegistration |
+		     QDBusServiceWatcher::WatchForUnregistration)
 {
-    powerSaveStatus = managerIface.GetPowerSaveStatus();
-
-    if (managerIface.CanSuspend())
-        supportedSleepStates+= Solid::PowerManagement::SuspendState;
-    if (managerIface.CanHibernate())
-        supportedSleepStates+= Solid::PowerManagement::HibernateState;
-
-    connect(&managerIface, SIGNAL(CanSuspendChanged(bool)),
-            this, SLOT(slotCanSuspendChanged(bool)));
-    connect(&managerIface, SIGNAL(CanHibernateChanged(bool)),
-            this, SLOT(slotCanHibernateChanged(bool)));
-    connect(&managerIface, SIGNAL(PowerSaveStatusChanged(bool)),
-            this, SLOT(slotPowerSaveStatusChanged(bool)));
     connect(&serviceWatcher, SIGNAL(serviceRegistered(QString)),
-            this, SLOT(slotServiceRegistered(QString)));
+            this, SLOT(slotServiceRegistered()));
+    connect(&serviceWatcher, SIGNAL(serviceUnregistered(QString)),
+            this, SLOT(slotServiceUnregistered()));
 
     // If the service is registered, trigger the connection immediately
     if (QDBusConnection::sessionBus().interface()->isServiceRegistered("org.kde.Solid.PowerManagement")) {
@@ -95,10 +85,10 @@ void Solid::PowerManagement::requestSlee
     case StandbyState:
         break;
     case SuspendState:
-        globalPowerManager->managerIface.Suspend();
+        globalPowerManager->managerIface->Suspend();
         break;
     case HibernateState:
-        globalPowerManager->managerIface.Hibernate();
+        globalPowerManager->managerIface->Hibernate();
         break;
     }
 }
@@ -106,13 +96,13 @@ void Solid::PowerManagement::requestSlee
 int Solid::PowerManagement::beginSuppressingSleep(const QString &reason)
 {
     QDBusReply<uint> reply;
-    if (globalPowerManager->policyAgentIface.isValid()) {
-        reply = globalPowerManager->policyAgentIface.AddInhibition(
+    if (globalPowerManager->policyAgentIface->isValid()) {
+        reply = globalPowerManager->policyAgentIface->AddInhibition(
             (uint)PowerManagementPrivate::InterruptSession,
             QCoreApplication::applicationName(), reason);
     } else {
         // Fallback to the fd.o Inhibit interface
-        reply = globalPowerManager->inhibitIface.Inhibit(QCoreApplication::applicationName(), reason);
+        reply = globalPowerManager->inhibitIface->Inhibit(QCoreApplication::applicationName(), reason);
     }
 
     if (reply.isValid())
@@ -123,18 +113,18 @@ int Solid::PowerManagement::beginSuppres
 
 bool Solid::PowerManagement::stopSuppressingSleep(int cookie)
 {
-    if (globalPowerManager->policyAgentIface.isValid()) {
-        return globalPowerManager->policyAgentIface.ReleaseInhibition(cookie).isValid();
+    if (globalPowerManager->policyAgentIface->isValid()) {
+        return globalPowerManager->policyAgentIface->ReleaseInhibition(cookie).isValid();
     } else {
         // Fallback to the fd.o Inhibit interface
-        return globalPowerManager->inhibitIface.UnInhibit(cookie).isValid();
+        return globalPowerManager->inhibitIface->UnInhibit(cookie).isValid();
     }
 }
 
 int Solid::PowerManagement::beginSuppressingScreenPowerManagement(const QString& reason)
 {
-    if (globalPowerManager->policyAgentIface.isValid()) {
-        QDBusReply<uint> reply = globalPowerManager->policyAgentIface.AddInhibition(
+    if (globalPowerManager->policyAgentIface->isValid()) {
+        QDBusReply<uint> reply = globalPowerManager->policyAgentIface->AddInhibition(
             (uint)PowerManagementPrivate::ChangeScreenSettings,
             QCoreApplication::applicationName(), reason);
 
@@ -162,8 +152,8 @@ int Solid::PowerManagement::beginSuppres
 
 bool Solid::PowerManagement::stopSuppressingScreenPowerManagement(int cookie)
 {
-    if (globalPowerManager->policyAgentIface.isValid()) {
-        bool result = globalPowerManager->policyAgentIface.ReleaseInhibition(cookie).isValid();
+    if (globalPowerManager->policyAgentIface->isValid()) {
+        bool result = globalPowerManager->policyAgentIface->ReleaseInhibition(cookie).isValid();
 
         if (globalPowerManager->screensaverCookiesForPowerDevilCookies.contains(cookie)) {
             QDBusMessage message = QDBusMessage::createMethodCall("org.freedesktop.ScreenSaver", "/ScreenSaver",
@@ -204,6 +194,9 @@ void Solid::PowerManagementPrivate::slot
 
 void Solid::PowerManagementPrivate::slotPowerSaveStatusChanged(bool newState)
 {
+    if (newState == powerSaveStatus) {
+        return;
+    }
     powerSaveStatus = newState;
     emit appShouldConserveResourcesChanged(powerSaveStatus);
 }
@@ -217,8 +210,7 @@ void Solid::PowerManagementPrivate::slot
                                                        "/org/kde/Solid/PowerManagement",
                                                        "org.kde.Solid.PowerManagement",
                                                        "backendCapabilities");
-    QDBusPendingReply< uint > reply = QDBusConnection::sessionBus().asyncCall(call);
-    reply.waitForFinished();
+    QDBusReply< uint > reply = QDBusConnection::sessionBus().asyncCall(call);
 
     if (reply.isValid() && reply.value() > 0) {
         // Connect the signal
@@ -229,6 +221,53 @@ void Solid::PowerManagementPrivate::slot
                                               this,
                                               SIGNAL(resumingFromSuspend()));
     }
+
+    if (managerIface) {
+        managerIface->deleteLater();
+    }
+
+    if (policyAgentIface) {
+        policyAgentIface->deleteLater();
+    }
+
+    if (inhibitIface) {
+        inhibitIface->deleteLater();
+    }
+
+    managerIface = new OrgFreedesktopPowerManagementInterface("org.freedesktop.PowerManagement",
+                   "/org/freedesktop/PowerManagement",
+                   QDBusConnection::sessionBus()),
+    policyAgentIface = new OrgKdeSolidPowerManagementPolicyAgentInterface("org.kde.Solid.PowerManagement.PolicyAgent",
+                       "/org/kde/Solid/PowerManagement/PolicyAgent",
+                       QDBusConnection::sessionBus()),
+    inhibitIface = new OrgFreedesktopPowerManagementInhibitInterface("org.freedesktop.PowerManagement.Inhibit",
+                   "/org/freedesktop/PowerManagement/Inhibit",
+                   QDBusConnection::sessionBus()),
+
+    connect(managerIface, SIGNAL(CanSuspendChanged(bool)),
+            this, SLOT(slotCanSuspendChanged(bool)));
+    connect(managerIface, SIGNAL(CanHibernateChanged(bool)),
+            this, SLOT(slotCanHibernateChanged(bool)));
+    connect(managerIface, SIGNAL(PowerSaveStatusChanged(bool)),
+            this, SLOT(slotPowerSaveStatusChanged(bool)));
+
+    QDBusReply<bool> psReply = managerIface->GetPowerSaveStatus();
+    slotPowerSaveStatusChanged(psReply.isValid() ? psReply.value() : false);
+
+    QDBusReply<bool> sReply = managerIface->CanSuspend();
+    slotCanSuspendChanged(sReply.isValid() ? sReply.value() : false);
+
+    QDBusReply<bool> hReply = managerIface->CanHibernate();
+    slotCanHibernateChanged(hReply.isValid() ? hReply.value() : false);
+}
+
+void Solid::PowerManagementPrivate::slotServiceUnregistered(const QString &serviceName)
+{
+    Q_UNUSED(serviceName);
+
+    slotPowerSaveStatusChanged(false);
+    slotCanSuspendChanged(false);
+    slotCanHibernateChanged(false);
 }
 
 #include "powermanagement_p.moc"
diff -Nru --show-c-function kdelibs-4.8.4.orig/solid/solid/powermanagement_p.h kdelibs-4.8.4/solid/solid/powermanagement_p.h
--- kdelibs-4.8.4.orig/solid/solid/powermanagement_p.h	2012-06-06 17:49:52.829041616 -0300
+++ kdelibs-4.8.4/solid/solid/powermanagement_p.h	2012-06-20 14:36:09.964861045 -0300
@@ -45,16 +45,19 @@ namespace Solid
         PowerManagementPrivate();
         ~PowerManagementPrivate();
 
+        void init();
+
     public Q_SLOTS:
         void slotCanSuspendChanged(bool newState);
         void slotCanHibernateChanged(bool newState);
         void slotPowerSaveStatusChanged(bool newState);
         void slotServiceRegistered(const QString &serviceName);
+        void slotServiceUnregistered(const QString &serviceName);
 
     public:
-        OrgFreedesktopPowerManagementInterface managerIface;
-        OrgKdeSolidPowerManagementPolicyAgentInterface policyAgentIface;
-        OrgFreedesktopPowerManagementInhibitInterface inhibitIface;
+        OrgFreedesktopPowerManagementInterface * managerIface;
+        OrgKdeSolidPowerManagementPolicyAgentInterface * policyAgentIface;
+        OrgFreedesktopPowerManagementInhibitInterface * inhibitIface;
         QDBusServiceWatcher serviceWatcher;
 
         bool powerSaveStatus;
_______________________________________________
Kde-hardware-devel mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/kde-hardware-devel

Reply via email to