[krita] libs: Fix a memory leak: the popup action should "own" the image collection

[Friedrich W. H. Kossebau]

Am Montag, 12. September 2016, 13:37:26 CEST schrieb Boudewijn Rempt:
> Git commit deff966ee3041104f0e2bda587c94d58403670af by Boudewijn Rempt.
> Committed on 12/09/2016 at 13:37.
> Pushed by rempt into branch 'master'.
> 
> Fix a memory leak: the popup action should "own" the image collection
> 
> And the pattern background should check whether the popup still
> exists. This bug probably is also in Calligra.
> 
> CCMAIL:calligra-devel at kde.org
> 
> M  +15   -6    libs/flake/KoPatternBackground.cpp
> M  +4    -3    libs/widgets/KoResourcePopupAction.cpp
> 
> http://commits.kde.org/krita/deff966ee3041104f0e2bda587c94d58403670af
> 
> diff --git a/libs/flake/KoPatternBackground.cpp
> b/libs/flake/KoPatternBackground.cpp index a2289b7..9f0a170 100644
> --- a/libs/flake/KoPatternBackground.cpp
> +++ b/libs/flake/KoPatternBackground.cpp
> @@ -37,6 +37,7 @@
> 
>  #include <QBrush>
>  #include <QPainter>
> +#include <QPointer>
> 
>  class KoPatternBackgroundPrivate : public KoShapeBackgroundPrivate
>  {
> @@ -123,7 +124,7 @@ public:
>      QSizeF targetImageSizePercent;
>      QPointF refPointOffsetPercent;
>      QPointF tileRepeatOffsetPercent;
> -    KoImageCollection * imageCollection;
> +    QPointer<KoImageCollection> imageCollection;
>      KoImageData * imageData;
>  };
> 
> @@ -131,7 +132,7 @@ public:
>  // ----------------------------------------------------------------
> 
> 
> -KoPatternBackground::KoPatternBackground(KoImageCollection *
> imageCollection)
> +KoPatternBackground::KoPatternBackground(KoImageCollection
> *imageCollection)
>          : KoShapeBackground(*(new KoPatternBackgroundPrivate()))
> 
>  {
>      Q_D(KoPatternBackground);
> @@ -160,7 +161,9 @@ void KoPatternBackground::setPattern(const QImage
> &pattern) {
>      Q_D(KoPatternBackground);
>      delete d->imageData;
> -    d->imageData = d->imageCollection->createImageData(pattern);
> +    if (d->imageCollection) {
> +        d->imageData = d->imageCollection->createImageData(pattern);
> +    }
>  }
> 
>  void KoPatternBackground::setPattern(KoImageData *imageData)
> @@ -358,7 +361,9 @@ void KoPatternBackground::fillStyle(KoGenStyle &style,
> KoShapeSavingContext &con style.addProperty("draw:fill", "bitmap");
>      style.addProperty("draw:fill-image-name", patternStyleName);
> 
> -    context.addDataCenter(d->imageCollection);
> +    if (d->imageCollection) {
> +        context.addDataCenter(d->imageCollection);
> +    }
>  }
> 
>  bool KoPatternBackground::loadStyle(KoOdfLoadingContext &context, const
> QSizeF &) @@ -383,9 +388,13 @@ bool
> KoPatternBackground::loadStyle(KoOdfLoadingContext &context, const QSizeF &
> return false;
> 
>      delete d->imageData;
> -    d->imageData = d->imageCollection->createImageData(href,
> context.store()); -    if (! d->imageData)
> +    d->imageData = 0;
> +    if (d->imageCollection) {
> +        d->imageData = d->imageCollection->createImageData(href,
> context.store()); +    }
> +    if (! d->imageData) {
>          return false;
> +    }
> 
>      // read the pattern repeat style
>      QString style = styleStack.property(KoXmlNS::style, "repeat");
> diff --git a/libs/widgets/KoResourcePopupAction.cpp
> b/libs/widgets/KoResourcePopupAction.cpp index 95f0192..1c27b0c 100644
> --- a/libs/widgets/KoResourcePopupAction.cpp
> +++ b/libs/widgets/KoResourcePopupAction.cpp
> @@ -50,6 +50,7 @@ public:
>      QMenu *menu;
>      KoResourceItemView *resourceList;
>      QSharedPointer<KoShapeBackground> background;
> +    KoImageCollection *imageCollection;
>      KoCheckerBoardPainter checkerPainter;
>  };
> 
> @@ -83,8 +84,8 @@
> KoResourcePopupAction::KoResourcePopupAction(QSharedPointer<KoAbstractResou
> rceSe qg->setCoordinateMode(QGradient::ObjectBoundingMode);
>          d->background = QSharedPointer<KoShapeBackground>(new
> KoGradientBackground(qg)); } else if (pattern) {
> -        KoImageCollection *collection = new KoImageCollection();
> -        d->background = QSharedPointer<KoShapeBackground>(new
> KoPatternBackground(collection)); +        d->imageCollection = new
> KoImageCollection();
> +        d->background = QSharedPointer<KoShapeBackground>(new
> KoPatternBackground(d->imageCollection));
> static_cast<KoPatternBackground*>(d->background.data())->setPattern(pattern
> ->pattern()); }
> 
> @@ -116,7 +117,7 @@ KoResourcePopupAction::~KoResourcePopupAction()
>      }
> 
>      delete d->menu;
> -
> +    delete d->imageCollection;
>      delete d;
>  }


Thanks for the cc:, Boud.

Just, this one results in a crash in ~KoResourcePopupAction() on the new
	delete d->imageCollection;
when quitting the app (e.g. calligrastage) so this patch at least does not 
apply like is to Calligra codebase.

I have no big picture of the usage of imageCollection, so myself for now 
ignoring this memory leak reported here.

Cheers
Friedrich