Use smart pointers with AP classes and Instance

22 files changed, 83 insertions, 65 deletions

diff --git a/src/activitypub/apactivity.cpp b/src/activitypub/apactivity.cpp
index b44f87a..ebd9d18 100644
--- a/src/activitypub/apactivity.cpp
+++ b/src/activitypub/apactivity.cpp
@@ -2,8 +2,6 @@
 #include "apactor.h"
 #include "src/settings_interface.h"
 
-#include <QDebug>
-
 APActivity::APActivity() {}
 
 APActivity::APActivity(APActivityFields fields) {
@@ -22,10 +20,6 @@ APActivity::APActivity(APActivityFields fields) {
     object = fields.object;
 }
 
-APActivity::~APActivity() {
-    delete object;
-}
-
 QString APActivity::get_html_render(HtmlRenderDetails render_info) {
     QString html(get_html_template(QStringLiteral("apactivity")));
 
diff --git a/src/activitypub/apactivity.h b/src/activitypub/apactivity.h
index fcfb86c..dd6c09e 100644
--- a/src/activitypub/apactivity.h
+++ b/src/activitypub/apactivity.h
@@ -5,6 +5,8 @@
 #include "apobject.h"
 #include "../types.h"
 #include <QDateTime>
+#include <QMetaType>
+#include <memory>
 
 enum struct APActivityType {CREATE, ANNOUNCE, UNKNOWN};
 
@@ -14,7 +16,7 @@ struct APActivityFields {
     QStringList cc_actors;
     QString object_url; // maps to "id", represents the Activity, *not* the URL of the object it contains
     QString published;
-    APObject* object; // will generally be a APPost or APReblog
+    APObjectPtr object; // will generally be a APPost or APReblog
     StatusType visibility;
     APActivityType type = APActivityType::UNKNOWN;
 };
@@ -25,9 +27,9 @@ public:
 
     // An Activity that can easily be constructed from an ActivityStreams JSON Activity. Make sure to pass the APObject.
     APActivity(APActivityFields fields);
-    ~APActivity();
+    ~APActivity() {};
 
-    APObject* object = nullptr; // the object that's manipulated by this Activity, use APPost for something useful and "manually" create it
+    APObjectPtr object; // the object that's manipulated by this Activity, use APPost for something useful and "manually" create it
 
     QString get_html_render(HtmlRenderDetails render_info);
 
@@ -43,3 +45,6 @@ private:
 
     QDateTime published;
 };
+
+typedef std::shared_ptr<APActivity> APActivityPtr;
+Q_DECLARE_METATYPE(APActivityPtr);
diff --git a/src/activitypub/apactor.h b/src/activitypub/apactor.h
index ad8d689..70d80de 100644
--- a/src/activitypub/apactor.h
+++ b/src/activitypub/apactor.h
@@ -3,6 +3,8 @@
 #include "apbase.h"
 #include "apattachment.h"
 #include <QDateTime>
+#include <QMetaType>
+#include <memory>
 #include <vector>
 
 struct APPropertyValue {
@@ -57,6 +59,9 @@ private:
     // APActorList known_as;
 };
 
+typedef std::shared_ptr<APActor> APActorPtr;
+Q_DECLARE_METATYPE(APActorPtr);
+
 class APActorList : public std::vector<APActor>, APBase {
 public:
     QString get_html_render(HtmlRenderDetails render_info);
diff --git a/src/activitypub/apobject.h b/src/activitypub/apobject.h
index 62e60c8..5f17e80 100644
--- a/src/activitypub/apobject.h
+++ b/src/activitypub/apobject.h
@@ -5,6 +5,8 @@
 #include "apattachment.h"
 #include "../types.h"
 #include <QDateTime>
+#include <QMetaType>
+#include <memory>
 
 class APObject : protected APBase {
 public:
@@ -34,3 +36,6 @@ protected:
 
     APAttachmentList attachments;
 };
+
+typedef std::shared_ptr<APObject> APObjectPtr;
+Q_DECLARE_METATYPE(APObjectPtr);
diff --git a/src/activitypub/appost.h b/src/activitypub/appost.h
index ccbedd9..8fe6430 100644
--- a/src/activitypub/appost.h
+++ b/src/activitypub/appost.h
@@ -3,6 +3,8 @@
 #include "apobject.h"
 #include "apbase.h"
 #include "fields.h"
+#include <QMetaType>
+#include <memory>
 #include <vector>
 
 // APPost represents an ActivityPub Note Object
@@ -23,3 +25,6 @@ protected:
 
     QString get_html_status_languages();
 };
+
+typedef std::shared_ptr<APPost> APPostPtr;
+Q_DECLARE_METATYPE(APPostPtr);
diff --git a/src/activitypub/apreblog.h b/src/activitypub/apreblog.h
index cc340b8..66a6382 100644
--- a/src/activitypub/apreblog.h
+++ b/src/activitypub/apreblog.h
@@ -3,6 +3,8 @@
 #include "apobject.h"
 #include "apobject.h"
 #include "src/types.h"
+#include <QMetaType>
+#include <memory>
 #include <vector>
 
 struct APReblogFields {
@@ -25,3 +27,6 @@ protected:
     APObject object; // the object that was reblogged (impossible to be an APReblog)
     StatusType visibility;
 };
+
+typedef std::shared_ptr<APReblog> APReblogPtr;
+Q_DECLARE_METATYPE(APReblogPtr);
diff --git a/src/archive/base_archive.cpp b/src/archive/base_archive.cpp
index 472abb1..0e96c61 100644
--- a/src/archive/base_archive.cpp
+++ b/src/archive/base_archive.cpp
@@ -16,6 +16,6 @@ const QString Archive::get_instance_address() {
     return "";
 }
 
-APActor* Archive::get_main_actor() {
+APActorPtr Archive::get_main_actor() {
     return nullptr;
 }
diff --git a/src/archive/base_archive.h b/src/archive/base_archive.h
index 27259b3..c361fd5 100644
--- a/src/archive/base_archive.h
+++ b/src/archive/base_archive.h
@@ -34,10 +34,10 @@ public:
     static Archive* create_archive(ArchiveType archive_type, const QString& main_filename);
 
     virtual void update_status_list(ViewStatusTypes allowed_types, QListWidget *parent) = 0;
-    virtual APActivity* get_activity(ArchiveItemRef index, Hinting_t hinting) = 0;
+    virtual APActivityPtr get_activity(ArchiveItemRef index, Hinting_t hinting) = 0;
     // Get the Actor that represents the data export, the user who's account data has been exported.
     // Return nullptr if the Actor cannot be retrieved
-    virtual APActor* get_main_actor();
+    virtual APActorPtr get_main_actor();
     virtual const QString get_html_status_text(ArchiveItemRef status_index) = 0;
     virtual const QString get_instance_address();
 
diff --git a/src/archive/mastodon.cpp b/src/archive/mastodon.cpp
index 121c366..044056c 100644
--- a/src/archive/mastodon.cpp
+++ b/src/archive/mastodon.cpp
@@ -1,4 +1,5 @@
 #include "mastodon.h"
+#include "src/activitypub/apactor.h"
 #include "src/list_item.h"
 #include "src/types.h"
 #include "src/activitypub/apactivity.h"
@@ -237,7 +238,7 @@ std::vector<APAttachmentFields> MastodonArchive::get_status_attachments_list(QJs
     return list;
 }
 
-APActivity* MastodonArchive::get_activity(ArchiveItemRef index, Hinting_t hinting) {
+APActivityPtr MastodonArchive::get_activity(ArchiveItemRef index, Hinting_t hinting) {
     // the JSON AP Activity
     QJsonObject activity = outbox_items->at(index).toObject();
 
@@ -320,17 +321,17 @@ APActivity* MastodonArchive::get_activity(ArchiveItemRef index, Hinting_t hintin
         obj_url = activity["object"].toString();
 
     if (not obj_url.isNull())
-        act_fields.object = new APReblog({obj_url, act_fields.visibility});
+        act_fields.object = std::make_shared<APReblog>(APReblogFields {obj_url, act_fields.visibility});
     else switch(obj_type) {
         case APObjectType::UNKNOWN:
         case APObjectType::NOTE:
-            act_fields.object = new APPost(obj_fields); break;
+            act_fields.object = std::make_shared<APPost>(obj_fields); break;
         case APObjectType::QUESTION:
-            act_fields.object = new APQuestion(obj_fields); break;
+            act_fields.object = std::make_shared<APQuestion>(obj_fields); break;
     }
 
     // TODO: it is currently a waste to create this APActivity object that will be immediately destroyed but it allows us to extend archive parsing to something that will become abstract (and an abstract base class) and separate from display (the final goal) which will allow us to add other sources that feed us with posts, reblogs and Actor information. furthermore, these objects can be cached for reuse in a session.
-    return new APActivity(act_fields);
+    return std::make_shared<APActivity>(act_fields);
 }
 
 // TODO: make this use an APActivity object that will be present as an StatusListItem member.
@@ -355,7 +356,7 @@ const QString MastodonArchive::get_html_status_text(ArchiveItemRef index) {
     return text;
 }
 
-APActor* MastodonArchive::get_main_actor() {
+APActorPtr MastodonArchive::get_main_actor() {
     // Avoid recreating the Actor each time by caching it
     if (actor) return actor;
 
@@ -436,7 +437,7 @@ APActor* MastodonArchive::get_main_actor() {
             obj_fields.header = new APAttachment(att_fields);
         }
 
-        actor = new APActor(obj_fields);
+        actor = std::make_shared<APActor>(obj_fields);
 
         // TODO: query type. it seems to be always "Person", but maybe if the account is a bot, it changes?
         // The code also currently ignores possible featured tags and users
diff --git a/src/archive/mastodon.h b/src/archive/mastodon.h
index f148047..355210c 100644
--- a/src/archive/mastodon.h
+++ b/src/archive/mastodon.h
@@ -19,8 +19,8 @@ public:
     InitError init();
 
     void update_status_list(ViewStatusTypes allowed_types, QListWidget *parent);
-    APActivity* get_activity(ArchiveItemRef index, Hinting_t hinting = {});
-    APActor* get_main_actor();
+    APActivityPtr get_activity(ArchiveItemRef index, Hinting_t hinting = {});
+    APActorPtr get_main_actor();
     const QString get_html_status_text(ArchiveItemRef status_index);
     const QString get_instance_address();
 private:
@@ -30,7 +30,7 @@ private:
 
     QJsonObject *outbox_json = nullptr;
     QJsonArray *outbox_items = nullptr;
-    APActor* actor = nullptr;
+    APActorPtr actor;
 
     bool is_status_type_allowed(StatusType status_type, ViewStatusTypes allowed_types);
     StatusType get_status_type(QJsonObject obj);
diff --git a/src/list_item.cpp b/src/list_item.cpp
index d93b81f..16eb9e8 100644
--- a/src/list_item.cpp
+++ b/src/list_item.cpp
@@ -50,13 +50,12 @@ StatusType StatusListItem::get_status_type() {
     return status_type;
 }
 
-APActivity* StatusListItem::get_activity() {
+APActivityPtr StatusListItem::get_activity() {
     return data_archive->get_activity(status_index, {status_type});
 }
 
 const QString StatusListItem::get_info_html(int text_zone_width, QLocale* locale) {
-    APActivity* activity = data_archive->get_activity(status_index, {status_type});
+    APActivityPtr activity = data_archive->get_activity(status_index, {status_type});
     QString html = activity->get_html_render({text_zone_width, locale});
-    delete activity; activity = nullptr;
     return html;
 }
diff --git a/src/list_item.h b/src/list_item.h
index 22debe7..0edeb8a 100644
--- a/src/list_item.h
+++ b/src/list_item.h
@@ -14,7 +14,7 @@ public:
     StatusListItem(const QString &text, StatusType status_type, bool has_attachement, Archive* data_archive, QListWidget *parent = nullptr, int index = 0);
     int get_status_index();
     StatusType get_status_type();
-    APActivity* get_activity();
+    APActivityPtr get_activity();
     const QString get_info_html(int text_zone_width, QLocale* locale);
 
 private:
diff --git a/src/net/instance.cpp b/src/net/instance.cpp
index 6982f64..8b2ad2d 100644
--- a/src/net/instance.cpp
+++ b/src/net/instance.cpp
@@ -4,15 +4,15 @@
 #endif
 #include "src/settings_interface.h"
 
-Instance* Instance::create_instance() {
+InstancePtr Instance::create_instance() {
     return create_instance(SettingsInterface::quick_read_setting<AppSettingsTypes::InstanceType>("net/instance/type"));
 }
 
-Instance* Instance::create_instance(AppSettingsTypes::InstanceType type) {
+InstancePtr Instance::create_instance(AppSettingsTypes::InstanceType type) {
     switch (type) {
 #ifdef MASTODONPP_BUILD
         case AppSettingsTypes::InstanceType::MASTODON:
-            return new MastodonInstance;
+            return std::make_shared<MastodonInstance>();
 #endif
         default:
             return nullptr;
diff --git a/src/net/instance.h b/src/net/instance.h
index 2b7eec7..79c0b12 100644
--- a/src/net/instance.h
+++ b/src/net/instance.h
@@ -1,6 +1,11 @@
 #pragma once
 #include "src/activitypub/appost.h"
 #include "src/settings_interface.h"
+#include <QMetaType>
+#include <memory>
+
+class Instance;
+typedef std::shared_ptr<Instance> InstancePtr;
 
 class Instance {
 public:
@@ -10,7 +15,7 @@ public:
     };
 
     virtual ~Instance() {};
-    virtual APPost* get_post_from_url(const QString &url) = 0;
+    virtual APPostPtr get_post_from_url(const QString &url) = 0;
     // Returns the URI the user has to visit in their browser
     virtual QString oauth2_step1();
     // auth_code is the code given by the user through the URI
@@ -18,7 +23,7 @@ public:
     virtual OAuth2Step2 oauth2_step2(const QString &auth_code);
     bool supports_oauth2 = false;
     // Creates an instance by reading the type from settings
-    static Instance* create_instance();
+    static InstancePtr create_instance();
     // Creates a specific type of instance, for use with SettingsDialog
-    static Instance* create_instance(AppSettingsTypes::InstanceType type);
+    static InstancePtr create_instance(AppSettingsTypes::InstanceType type);
 };
diff --git a/src/net/mastodon_instance.cpp b/src/net/mastodon_instance.cpp
index 8b88221..17d23ca 100644
--- a/src/net/mastodon_instance.cpp
+++ b/src/net/mastodon_instance.cpp
@@ -1,6 +1,7 @@
 #include "mastodon_instance.h"
 #include "src/activitypub/appost.h"
 #include "src/activitypub/apquestion.h"
+#include "src/activitypub/fields.h"
 #include "src/settings_interface.h"
 #include "src/types.h"
 
@@ -10,7 +11,7 @@
 #include <QUrl>
 #include <QDebug>
 
-APPost* post_from_json(const QJsonObject &status);
+APPostPtr post_from_json(const QJsonObject &status);
 
 MastodonInstance::MastodonInstance() : instance(SettingsInterface::quick_read_setting<QString>("net/instance/address").toStdString(), SettingsInterface::quick_read_setting<QString>("net/instance/token").toStdString()), connection(instance) {
 
@@ -43,7 +44,7 @@ Instance::OAuth2Step2 MastodonInstance::oauth2_step2(const QString &auth_code) {
     return {(bool)answer, token};
 }
 
-APPost* MastodonInstance::get_post_from_url(const QString &url) {
+APPostPtr MastodonInstance::get_post_from_url(const QString &url) {
     auto answer{connection.get(mastodonpp::API::v2::search, {
         {"q", QUrl::toPercentEncoding(url).toStdString()},
         {"type", "statuses"},
@@ -57,18 +58,18 @@ APPost* MastodonInstance::get_post_from_url(const QString &url) {
         qDebug() << doc;
         if (obj.contains("statuses")) {
             QJsonObject status = obj["statuses"].toArray().first().toObject();
-            if (status.isEmpty()) return new APPost({.content="invalid"}); // Invalid
+            if (status.isEmpty()) return std::make_shared<APPost>(APObjectFields {.content="invalid"}); // Invalid post
             return post_from_json(status);
         }
     }
 
-    return new APPost({.content="connection error"});
+    return std::make_shared<APPost>(APObjectFields {.content="connection error"});
 }
 
 // Create a filled APPost object from a Status entity (https://docs.joinmastodon.org/entities/Status/)
 // We have to return a pointer as else I can't get derived classes of APPost to render properly: the additional methods are ignored.
 // Furthermore, using pointers removes unecessary copying, even if that can be reduced by return value optimization.
-APPost* post_from_json(const QJsonObject &status) {
+APPostPtr post_from_json(const QJsonObject &status) {
     APObjectFields fields;
     bool is_question = status.contains("poll");
 
@@ -128,7 +129,7 @@ APPost* post_from_json(const QJsonObject &status) {
     }
 
     if (is_question)
-        return new APQuestion(fields);
+        return std::make_shared<APQuestion>(fields);
     else
-        return new APPost(fields);
+        return std::make_shared<APPost>(fields);
 }
diff --git a/src/net/mastodon_instance.h b/src/net/mastodon_instance.h
index 983bef6..7f9b058 100644
--- a/src/net/mastodon_instance.h
+++ b/src/net/mastodon_instance.h
@@ -7,7 +7,7 @@ class MastodonInstance : public Instance {
 public:
     MastodonInstance();
     ~MastodonInstance();
-    APPost* get_post_from_url(const QString &url);
+    APPostPtr get_post_from_url(const QString &url);
     QString oauth2_step1();
     OAuth2Step2 oauth2_step2(const QString &auth_code);
     bool supports_oauth2 = true;
diff --git a/src/settingsdialog.cpp b/src/settingsdialog.cpp
index 051091f..abb481e 100644
--- a/src/settingsdialog.cpp
+++ b/src/settingsdialog.cpp
@@ -72,7 +72,7 @@ void SettingsDialog::on_instanceActionsLabel_linkActivated(const QString& link)
         ui->instanceAddressLineEdit->setText(instance_address);
         update_ui_in_progress = false;
     } else if (link == "action:request-token") {
-        Instance* instance = Instance::create_instance((AppSettingsTypes::InstanceType)ui->instanceTypeComboBox->currentIndex());
+        InstancePtr instance = Instance::create_instance((AppSettingsTypes::InstanceType)ui->instanceTypeComboBox->currentIndex());
 
         QString url = instance->oauth2_step1();
         if (not QDesktopServices::openUrl(url))
diff --git a/src/widgets/status_info.cpp b/src/widgets/status_info.cpp
index 8ff90df..e3fe968 100644
--- a/src/widgets/status_info.cpp
+++ b/src/widgets/status_info.cpp
@@ -24,7 +24,6 @@ StatusInfoWidget::~StatusInfoWidget() {
     worker_thread.quit();
     worker_thread.wait();
 
-    delete displayed_activity;
     delete ui;
 }
 
@@ -46,9 +45,9 @@ void StatusInfoWidget::resizeEvent(QResizeEvent* event) {
 }
 
 void StatusInfoWidget::show_list_item(StatusListItem* new_item) {
-    qDebug() << "got item" << new_item << "old is" << displayed_item << displayed_activity;
+    qDebug() << "got item" << new_item << "old is" << displayed_item << displayed_activity.get();
     if (new_item != displayed_item) {
-        delete displayed_activity; displayed_activity = nullptr;
+        displayed_activity.reset();
         displayed_item = new_item;
         emit do_process_item(new_item);
     }
@@ -56,14 +55,9 @@ void StatusInfoWidget::show_list_item(StatusListItem* new_item) {
 
 // Update the GUI with the displayed Activity.
 // Has to be done on the GUI thread, no other way in Qt
-void StatusInfoWidget::postProcess(const QString &html, APActivity* activity) {
-    // Make sure to not delete the displayed_activity if we are reprocessing it after a resize event, else you get a crash that requires nasty debugging (don't ask how i found out ;)
-    if (activity != displayed_activity) {
-        // We are doing "double" deletion of old displayed_activity but sometimes that's important when really scrubbing the search results bar very quickly (we sometimes do reach that code and it hopefully helps against memory leaks)
-        if (displayed_activity)
-            delete displayed_activity;
-        displayed_activity = activity;
-    }
+void StatusInfoWidget::postProcess(const QString &html, APActivityPtr activity) {
+    // This shoudln't memory leak as we are now using smart pointers
+    displayed_activity = activity;
     ui->statusInfoText->setHtml(html);
 }
 
@@ -74,11 +68,11 @@ void StatusInfoWidget::postProcess(const QString &html, APActivity* activity) {
  */
 
 void StatusInfoWidgetWorker::process_item(StatusListItem* item) {
-    APActivity* activity = item->get_activity();
+    APActivityPtr activity = item->get_activity();
     process_activity(activity);
 }
 
-void StatusInfoWidgetWorker::process_activity(APActivity* activity) {
+void StatusInfoWidgetWorker::process_activity(APActivityPtr activity) {
     QString html = activity->get_html_render(render_info);
     emit itemProcessed(html, activity);
 }
diff --git a/src/widgets/status_info.h b/src/widgets/status_info.h
index 988f930..5eea7d0 100644
--- a/src/widgets/status_info.h
+++ b/src/widgets/status_info.h
@@ -12,11 +12,11 @@ class StatusInfoWidgetWorker : public QObject {
 
 public slots:
     void process_item(StatusListItem* item);
-    void process_activity(APActivity* activity);
+    void process_activity(APActivityPtr activity);
     void set_text_zone_width(int width) {render_info.text_zone_width = width;}
 
 signals:
-    void itemProcessed(const QString &html, APActivity* activity = nullptr);
+    void itemProcessed(const QString &html, APActivityPtr activity = nullptr);
 
 private:
     QLocale locale_context;
@@ -38,12 +38,12 @@ public:
 
 public slots:
     void show_list_item(StatusListItem* status); // Call from GUI thread
-    void postProcess(const QString &html, APActivity* activity);
+    void postProcess(const QString &html, APActivityPtr activity);
 
 signals:
     // These signals are called privetely by StatusInfoWidget
     void do_process_item(StatusListItem* item);
-    void do_process_activity(APActivity* activity);
+    void do_process_activity(APActivityPtr activity);
 
 private:
     Ui::StatusInfo* ui;
@@ -52,5 +52,5 @@ private:
 
     // Used for determining if we are dealing with a new list item or not
     StatusListItem* displayed_item = nullptr;
-    APActivity* displayed_activity = nullptr;
+    APActivityPtr displayed_activity;
 };
diff --git a/src/widgets/tab_activity_list.cpp b/src/widgets/tab_activity_list.cpp
index bd396c5..1254315 100644
--- a/src/widgets/tab_activity_list.cpp
+++ b/src/widgets/tab_activity_list.cpp
@@ -1,5 +1,6 @@
 #include "tab_activity_list.h"
 #include "./ui_tab_activity_list.h"
+#include "src/activitypub/apactivity.h"
 #include "status_info.h"
 #include "src/net/instance.h"
 
@@ -91,13 +92,12 @@ void TabActivityList::actionOpen_URL_triggered(bool checked) {
     // TODO: Reuse the Instance object
     // Really hacky code but works as a PoC and allows testing
     if (ok and not url.isEmpty()) {
-        Instance* instance = Instance::create_instance();
+        InstancePtr instance = Instance::create_instance();
         if (instance) {
-            APPost* post = instance->get_post_from_url(url);
+            APPostPtr post = instance->get_post_from_url(url);
             // Activity will be freed by StatusInfoWidget
-            APActivity* activity = new APActivity({.object = post});
+            auto activity = std::make_shared<APActivity>(APActivityFields {.object = post});
             status_info_widget->do_process_activity(activity);
-            delete instance; instance = nullptr;
         } else
             // Best is to remove option from menu, should be douable if the preprocessor touches .ui files
             QMessageBox::critical(this, tr("Open status from URL"), tr("Could not create Instance object.\nPerhaps network support was disabled at compile time?"));
diff --git a/src/widgets/tab_actor_info.cpp b/src/widgets/tab_actor_info.cpp
index c769297..9a289e0 100644
--- a/src/widgets/tab_actor_info.cpp
+++ b/src/widgets/tab_actor_info.cpp
@@ -26,7 +26,6 @@ TabActorInfo::TabActorInfo(Archive* archive, QWidget* parent)
 }
 
 TabActorInfo::~TabActorInfo() {
-    if (actor) delete actor;
     delete ui;
 }
 
diff --git a/src/widgets/tab_actor_info.h b/src/widgets/tab_actor_info.h
index c6a4b88..1ec9ca4 100644
--- a/src/widgets/tab_actor_info.h
+++ b/src/widgets/tab_actor_info.h
@@ -25,5 +25,5 @@ private:
 
     void paintEvent(QPaintEvent* event);
 
-    APActor* actor = nullptr;
+    APActorPtr actor;
 };