D7194: Detach before setting the d pointer

[Friedrich W. H. Kossebau]

kossebau added subscribers: whiting, kossebau.
kossebau added a comment.


  @apol  @whiting @leinir This change might have broken quite some things: EntryInternal has been an explicitly shared data object for some years (cmp. `QExplicitlySharedDataPointer<Private> d;`), and quite some logic seems to depend on that. E.g `ItemsModel`, which keeps a separate copy of the list of EntryInternal objects, does not update those objects on changes, but relies on this to happen automagically due to the explicitely shared data, https://phabricator.kde.org/R304:c32c8d002e1216373560c94738841a7a5e1b976b tries to work-around that, but actually makes things more convoluted.
  Same with `DownloadWidgetPrivate` and `Cache`, which both keep a set of changed entries as a `QSet<EntryInternal>`, which they maintain by `QSet::insert()`, just that this will due to same uniqueId not update the contained item to the latest version. Still those (potentially outdated) objects are then passed via signals to e.g. library consumers, which get outdated or incomplete data (like installedFiles values).
  Similar issue with `AtticaProvider::entryFromAtticaContent()` which also assumes in the logic explicitly shared data so the cache object gets all the updated data as well.
  
  Given this is a public class, this is quite some behavioural change. One could assume this breaks 3rd-party code in a similar way. There is no specification of the behaviour in the API dox though, so unsure myself what is the best solution: https://api.kde.org/frameworks/knewstuff/html/classKNSCore_1_1EntryInternal.html
  
  One fallout is at least this bug: https://bugs.kde.org/show_bug.cgi?id=386156
  
  I had started a patch to fix this, but it has grown a lot to make up for the now no longer explicitely shared nature of this class, and there are still issues in some places. So I wonder if it is better to simply revert this commit instead, document the old behaviour properly in the API and fix the places which rely on non-explicitely shared data objects. That might be only some newer code, given the old behaviour has stayed around for some years.
  
  What do you think?

REPOSITORY
  R304 KNewStuff

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

To: apol, leinir
Cc: kossebau, whiting, mutlaqja, broulik, #frameworks
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20171025/0aa67e19/attachment.html>