D16471: [RFC] Listen for added DBus interfaces instead of registered services

[Jan Grulich]

jgrulich accepted this revision.
jgrulich added a comment.
This revision is now accepted and ready to land.


  Seems to work, let's push this for now and we will see if there is no 
regression.

REPOSITORY
  R282 NetworkManagerQt

BRANCH
  ifadded

REVISION DETAIL
  https://phabricator.kde.org/D16471

To: fvogt, #frameworks, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16471: [RFC] Listen for added DBus interfaces instead of registered services

[Fabian Vogt]

fvogt added a comment.


  In D16471#354635 , @jgrulich wrote:
  
  > Is this supposed to fix also the issue where when you log into a Plasma 
session, you first get a notification that it failed to activate a connection 
and then it gets activated successfuly?
  
  
  I only saw that once and was unable to reproduce it, but someone else 
reported it downstream and posted an NM trace log: 
https://bugzilla.suse.com/attachment.cgi?id=777407
  
  A more or less educated guess:
  
  - User logs in
  - NM tries autologin, but no secrets available
  - NM tries WPS as fallback
  - Agent registers
  - WPS returns without success -> notification shown
  - As the agent registered, it queued another autoactivate event
  
  If that's true though it's a bug in NM and this won't fix it.

REPOSITORY
  R282 NetworkManagerQt

REVISION DETAIL
  https://phabricator.kde.org/D16471

To: fvogt, #frameworks, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16471: [RFC] Listen for added DBus interfaces instead of registered services

[Jan Grulich]

jgrulich added a comment.


  Is this supposed to fix also the issue where when you log into a Plasma 
session, you first get a notification that it failed to activate a connection 
and then it gets activated successfuly?

REPOSITORY
  R282 NetworkManagerQt

REVISION DETAIL
  https://phabricator.kde.org/D16471

To: fvogt, #frameworks, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16471: [RFC] Listen for added DBus interfaces instead of registered services

[Jan Grulich]

jgrulich added a comment.


  > This is RFC because:
  > 
  > I'm not sure whether it's guaranteed that InterfacesAdded is emitted on 
service registration as well or both connections are necessary
  
  I'm not sure about this either. I'll build it locally and try to use it for a 
while.
  
  > I don't know how it's possible to test this as org.kde.fakenetwork does not 
support the org.freedesktop.ObjectManager API
  
  The fakenetwork stuff supports or simulates only a small portion of NM dbus, 
we don't currently test secret agent at all.

REPOSITORY
  R282 NetworkManagerQt

REVISION DETAIL
  https://phabricator.kde.org/D16471

To: fvogt, #frameworks, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16471: [RFC] Listen for added DBus interfaces instead of registered services

[Fabian Vogt]

fvogt updated this revision to Diff 44334.
fvogt added a comment.


  Fix bad merge

REPOSITORY
  R282 NetworkManagerQt

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16471?vs=44332=44334

BRANCH
  ifadded

REVISION DETAIL
  https://phabricator.kde.org/D16471

AFFECTED FILES
  src/manager.cpp
  src/manager_p.h
  src/secretagent.cpp
  src/secretagent.h
  src/secretagent_p.h

To: fvogt, #frameworks, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16471: [RFC] Listen for added DBus interfaces instead of registered services

[Fabian Vogt]

fvogt created this revision.
fvogt added reviewers: Frameworks, jgrulich.
Herald added a project: Frameworks.
fvogt requested review of this revision.

REVISION SUMMARY
  When the service is registered, the interfaces might not be available yet.
  So wait for those instead.
  
  BUG: 400359
  
  This is RFC because:
  
  - I'm not sure whether it's guaranteed that InterfacesAdded is emitted on 
service registration as well or both connections are necessary
  - I don't know how it's possible to test this as org.kde.fakenetwork does not 
support the org.freedesktop.ObjectManager API

TEST PLAN
  Restarting NM while kded5 keeps running caused "nmcli up" to fail because of
  missing secrets.
  With this patch, it works successfully.

REPOSITORY
  R282 NetworkManagerQt

BRANCH
  ifadded

REVISION DETAIL
  https://phabricator.kde.org/D16471

AFFECTED FILES
  src/manager.cpp
  src/manager_p.h
  src/secretagent.cpp
  src/secretagent.h
  src/secretagent_p.h

To: fvogt, #frameworks, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns