Initialize LibraryFeature member of TreeItem consistently

14 files changed, 102 insertions, 81 deletions

diff --git a/src/library/autodj/autodjfeature.cpp b/src/library/autodj/autodjfeature.cpp
index 836633bc72..8cd5a0dbb7 100644
--- a/src/library/autodj/autodjfeature.cpp
+++ b/src/library/autodj/autodjfeature.cpp
@@ -69,7 +69,7 @@ AutoDJFeature::AutoDJFeature(Library* pLibrary,
     m_playlistDao.setAutoDJProcessor(m_pAutoDJProcessor);
 
     // Create the "Crates" tree-item under the root item.
-    auto pRootItem = std::make_unique<TreeItem>(this);
+    std::unique_ptr<TreeItem> pRootItem = TreeItem::newRoot(this);
     m_pCratesTreeItem = pRootItem->appendChild(tr("Crates"));
     m_pCratesTreeItem->setIcon(QIcon(":/images/library/ic_library_crates.svg"));
 
@@ -215,7 +215,7 @@ void AutoDJFeature::slotCrateChanged(CrateId crateId) {
         // No child item for crate found
         // -> Create and append a new child item for this crate
         QList<TreeItem*> rows;
-        rows.append(new TreeItem(this, crate.getName(), crate.getId().toVariant()));
+        rows.append(new TreeItem(crate.getName(), crate.getId().toVariant()));
         QModelIndex parentIndex = m_childModel.index(0, 0);
         m_childModel.insertTreeItemRows(rows, m_crateList.length(), parentIndex);
         DEBUG_ASSERT(rows.isEmpty()); // ownership passed to m_childModel
diff --git a/src/library/banshee/bansheefeature.cpp b/src/library/banshee/bansheefeature.cpp
index 0f4b3ffa1a..9cb2ffac97 100644
--- a/src/library/banshee/bansheefeature.cpp
+++ b/src/library/banshee/bansheefeature.cpp
@@ -91,7 +91,7 @@ void BansheeFeature::activate() {
 
         m_isActivated =  true;
 
-        auto pRootItem = std::make_unique<TreeItem>(this);
+        std::unique_ptr<TreeItem> pRootItem = TreeItem::newRoot(this);
         QList<BansheeDbConnection::Playlist> playlists = m_connection.getPlaylists();
         for (const BansheeDbConnection::Playlist& playlist: playlists) {
             qDebug() << playlist.name;
diff --git a/src/library/baseplaylistfeature.cpp b/src/library/baseplaylistfeature.cpp
index 5a41369f7d..149d5f635d 100644
--- a/src/library/baseplaylistfeature.cpp
+++ b/src/library/baseplaylistfeature.cpp
@@ -677,7 +677,7 @@ QModelIndex BasePlaylistFeature::constructChildModel(int selected_id) {
         }
 
         // Create the TreeItem whose parent is the invisible root item
-        TreeItem* item = new TreeItem(this, playlistLabel, playlistId);
+        TreeItem* item = new TreeItem(playlistLabel, playlistId);
         item->setBold(m_playlistsSelectedTrackIsIn.contains(playlistId));
 
         decorateChild(item, playlistId);
diff --git a/src/library/browse/browsefeature.cpp b/src/library/browse/browsefeature.cpp
index 7ac1450a9e..545f55b6b9 100644
--- a/src/library/browse/browsefeature.cpp
+++ b/src/library/browse/browsefeature.cpp
@@ -71,7 +71,7 @@ BrowseFeature::BrowseFeature(
     m_proxyModel.setDynamicSortFilter(true);
 
     // The invisible root item of the child model
-    auto pRootItem = std::make_unique<TreeItem>(this);
+    std::unique_ptr<TreeItem> pRootItem = TreeItem::newRoot(this);
 
     m_pQuickLinkItem = pRootItem->appendChild(tr("Quick Links"), QUICK_LINK_NODE);
 
@@ -153,7 +153,7 @@ void BrowseFeature::slotAddQuickLink() {
     QString name = extractNameFromPath(spath);
 
     QModelIndex parent = m_childModel.index(m_pQuickLinkItem->parentRow(), 0);
-    auto pNewChild = std::make_unique<TreeItem>(this, name, vpath);
+    auto pNewChild = std::make_unique<TreeItem>(name, vpath);
     QList<TreeItem*> rows;
     rows.append(pNewChild.get());
     pNewChild.release();
@@ -312,7 +312,7 @@ void BrowseFeature::onRightClickChild(const QPoint& globalPos, QModelIndex index
 
 namespace {
 // Get the list of devices (under "Removable Devices" section).
-QList<TreeItem*> getRemovableDevices(LibraryFeature* pFeature) {
+QList<TreeItem*> getRemovableDevices() {
     QList<TreeItem*> ret;
 #if defined(__WINDOWS__)
     // Repopulate drive list
@@ -332,7 +332,6 @@ QList<TreeItem*> getRemovableDevices(LibraryFeature* pFeature) {
             display_path.chop(1);
         }
         TreeItem* driveLetter = new TreeItem(
-            pFeature,
             display_path, // Displays C:
             drive.filePath()); // Displays C:/
         ret << driveLetter;
@@ -354,13 +353,10 @@ QList<TreeItem*> getRemovableDevices(LibraryFeature* pFeature) {
     // Convert devices into a QList<TreeItem*> for display.
     foreach(QFileInfo device, devices) {
         TreeItem* folder = new TreeItem(
-            pFeature,
             device.fileName(),
             QVariant(device.filePath() + QStringLiteral("/")));
         ret << folder;
     }
-#else
-    Q_UNUSED(pFeature);
 #endif
     return ret;
 }
@@ -404,7 +400,7 @@ void BrowseFeature::onLazyChildExpandation(const QModelIndex& index) {
 
     // If we are on the special device node
     if (path == DEVICE_NODE) {
-        folders += getRemovableDevices(this);
+        folders += getRemovableDevices();
     } else {
         // we assume that the path refers to a folder in the file system
         // populate childs
@@ -424,7 +420,6 @@ void BrowseFeature::onLazyChildExpandation(const QModelIndex& index) {
             // Once the items are added to the TreeItemModel,
             // the models takes ownership of them and ensures their deletion
             TreeItem* folder = new TreeItem(
-                this,
                 one.fileName(),
                 QVariant(one.absoluteFilePath() + QStringLiteral("/")));
             folders << folder;
diff --git a/src/library/crate/cratefeature.cpp b/src/library/crate/cratefeature.cpp
index bd8c18cf28..859e0070f9 100644
--- a/src/library/crate/cratefeature.cpp
+++ b/src/library/crate/cratefeature.cpp
@@ -52,7 +52,7 @@ CrateFeature::CrateFeature(Library* pLibrary,
     initActions();
 
     // construct child model
-    m_childModel.setRootItem(std::make_unique<TreeItem>(this));
+    m_childModel.setRootItem(TreeItem::newRoot(this));
     rebuildChildModel();
 
     connectLibrary(pLibrary);
@@ -192,7 +192,7 @@ QString CrateFeature::formatRootViewHtml() const {
 
 std::unique_ptr<TreeItem> CrateFeature::newTreeItemForCrateSummary(
         const CrateSummary& crateSummary) {
-    auto pTreeItem = std::make_unique<TreeItem>(this);
+    auto pTreeItem = TreeItem::newRoot(this);
     updateTreeItemForCrateSummary(pTreeItem.get(), crateSummary);
     return pTreeItem;
 }
diff --git a/src/library/itunes/itunesfeature.cpp b/src/library/itunes/itunesfeature.cpp
index 82c60b2046..dc3203e2c2 100644
--- a/src/library/itunes/itunesfeature.cpp
+++ b/src/library/itunes/itunesfeature.cpp
@@ -643,7 +643,7 @@ void ITunesFeature::parseTrack(QXmlStreamReader& xml, QSqlQuery& query) {
 
 TreeItem* ITunesFeature::parsePlaylists(QXmlStreamReader& xml) {
     qDebug() << "Parse iTunes playlists";
-    TreeItem* rootItem = new TreeItem(this);
+    std::unique_ptr<TreeItem> pRootItem = TreeItem::newRoot(this);
     QSqlQuery query_insert_to_playlists(m_database);
     query_insert_to_playlists.prepare("INSERT INTO itunes_playlists (id, name) "
                                       "VALUES (:id, :name)");
@@ -660,7 +660,7 @@ TreeItem* ITunesFeature::parsePlaylists(QXmlStreamReader& xml) {
             parsePlaylist(xml,
                           query_insert_to_playlists,
                           query_insert_to_playlist_tracks,
-                          rootItem);
+                          pRootItem.get());
             continue;
         }
         if (xml.isEndElement()) {
@@ -668,7 +668,7 @@ TreeItem* ITunesFeature::parsePlaylists(QXmlStreamReader& xml) {
                 break;
         }
     }
-    return rootItem;
+    return pRootItem.release();
 }
 
 bool ITunesFeature::readNextStartElement(QXmlStreamReader& xml) {
diff --git a/src/library/mixxxlibraryfeature.cpp b/src/library/mixxxlibraryfeature.cpp
index a30b5cde87..829e0be7f2 100644
--- a/src/library/mixxxlibraryfeature.cpp
+++ b/src/library/mixxxlibraryfeature.cpp
@@ -95,7 +95,7 @@ MixxxLibraryFeature::MixxxLibraryFeature(Library* pLibrary,
     // These rely on the 'default' track source being present.
     m_pLibraryTableModel = new LibraryTableModel(this, pLibrary->trackCollections(), "mixxx.db.model.library");
 
-    auto pRootItem = std::make_unique<TreeItem>(this);
+    std::unique_ptr<TreeItem> pRootItem = TreeItem::newRoot(this);
     pRootItem->appendChild(kMissingTitle);
     pRootItem->appendChild(kHiddenTitle);
 
diff --git a/src/library/playlistfeature.cpp b/src/library/playlistfeature.cpp
index 2b3f0c018a..92d06625b5 100644
--- a/src/library/playlistfeature.cpp
+++ b/src/library/playlistfeature.cpp
@@ -49,7 +49,7 @@ PlaylistFeature::PlaylistFeature(
             "mixxx.db.model.playlist"));
 
     //construct child model
-    auto pRootItem = std::make_unique<TreeItem>(this);
+    std::unique_ptr<TreeItem> pRootItem = TreeItem::newRoot(this);
     m_childModel.setRootItem(std::move(pRootItem));
     constructChildModel(-1);
 }
diff --git a/src/library/rekordbox/rekordboxfeature.cpp b/src/library/rekordbox/rekordboxfeature.cpp
index 7cbdecf755..872d8ae937 100644
--- a/src/library/rekordbox/rekordboxfeature.cpp
+++ b/src/library/rekordbox/rekordboxfeature.cpp
@@ -63,7 +63,7 @@ void clearTable(QSqlDatabase& database, QString tableName) {
 }
 
 // This function is executed in a separate thread other than the main thread
-QList<TreeItem*> findRekordboxDevices(RekordboxFeature* rekordboxFeature) {
+QList<TreeItem*> findRekordboxDevices() {
     QThread* thisThread = QThread::currentThread();
     thisThread->setPriority(QThread::LowPriority);
 
@@ -86,20 +86,16 @@ QList<TreeItem*> findRekordboxDevices(RekordboxFeature* rekordboxFeature) {
         QFileInfo rbDBFileInfo(drive.filePath() + kPdbPath);
 
         if (rbDBFileInfo.exists() && rbDBFileInfo.isFile()) {
-            TreeItem* foundDevice = new TreeItem(rekordboxFeature);
-            QList<QString> data;
-
             QString displayPath = drive.filePath();
             if (displayPath.endsWith("/")) {
                 displayPath.chop(1);
             }
-
+            QList<QString> data;
             data << drive.filePath();
             data << IS_RECORDBOX_DEVICE;
-
-            foundDevice->setLabel(displayPath);
-            foundDevice->setData(QVariant(data));
-
+            TreeItem* foundDevice = new TreeItem(
+                    std::move(displayPath),
+                    QVariant(data));
             foundDevices << foundDevice;
         }
     }
@@ -126,15 +122,12 @@ QList<TreeItem*> findRekordboxDevices(RekordboxFeature* rekordboxFeature) {
         QFileInfo rbDBFileInfo(device.filePath() + QStringLiteral("/") + kPdbPath);
 
         if (rbDBFileInfo.exists() && rbDBFileInfo.isFile()) {
-            TreeItem* foundDevice = new TreeItem(rekordboxFeature);
             QList<QString> data;
-
             data << device.filePath();
             data << IS_RECORDBOX_DEVICE;
-
-            foundDevice->setLabel(device.fileName());
-            foundDevice->setData(QVariant(data));
-
+            TreeItem* foundDevice = new TreeItem(
+                    device.fileName(),
+                    QVariant(data));
             foundDevices << foundDevice;
         }
     }
@@ -145,15 +138,12 @@ QList<TreeItem*> findRekordboxDevices(RekordboxFeature* rekordboxFeature) {
         QFileInfo rbDBFileInfo(device.filePath() + QStringLiteral("/") + kPdbPath);
 
         if (rbDBFileInfo.exists() && rbDBFileInfo.isFile()) {
-            TreeItem* foundDevice = new TreeItem(rekordboxFeature);
             QList<QString> data;
-
             data << device.filePath();
             data << IS_RECORDBOX_DEVICE;
-
-            foundDevice->setLabel(device.fileName());
-            foundDevice->setData(QVariant(data));
-
+            auto* foundDevice = new TreeItem(
+                    device.fileName(),
+                    QVariant(data));
             foundDevices << foundDevice;
         }
     }
@@ -1063,7 +1053,7 @@ RekordboxFeature::RekordboxFeature(
     connect(&m_devicesFutureWatcher, SIGNAL(finished()), this, SLOT(onRekordboxDevicesFound()));
     connect(&m_tracksFutureWatcher, SIGNAL(finished()), this, SLOT(onTracksFound()));
     // initialize the model
-    m_childModel.setRootItem(std::make_unique<TreeItem>(this));
+    m_childModel.setRootItem(TreeItem::newRoot(this));
 }
 
 RekordboxFeature::~RekordboxFeature() {
@@ -1150,7 +1140,7 @@ void RekordboxFeature::activate() {
     qDebug() << "RekordboxFeature::activate()";
 
     // Let a worker thread do the XML parsing
-    m_devicesFuture = QtConcurrent::run(findRekordboxDevices, this);
+    m_devicesFuture = QtConcurrent::run(findRekordboxDevices);
     m_devicesFutureWatcher.setFuture(m_devicesFuture);
     m_title = tr("(loading) Rekordbox");
     //calls a slot in the sidebar model such that 'Rekordbox (isLoading)' is displayed.
diff --git a/src/library/rhythmbox/rhythmboxfeature.cpp b/src/library/rhythmbox/rhythmboxfeature.cpp
index 5056dcd61f..2277843b5c 100644
--- a/src/library/rhythmbox/rhythmboxfeature.cpp
+++ b/src/library/rhythmbox/rhythmboxfeature.cpp
@@ -216,7 +216,7 @@ TreeItem* RhythmboxFeature::importPlaylists() {
             "INSERT INTO rhythmbox_playlist_tracks (playlist_id, track_id, position) "
             "VALUES (:playlist_id, :track_id, :position)");
     //The tree structure holding the playlists
-    TreeItem* rootItem = new TreeItem(this);
+    std::unique_ptr<TreeItem> rootItem = TreeItem::newRoot(this);
 
     QXmlStreamReader xml(&db);
     while (!xml.atEnd() && !m_cancelImport) {
@@ -253,13 +253,11 @@ TreeItem* RhythmboxFeature::importPlaylists() {
         // do error handling
         qDebug() << "Cannot process Rhythmbox music collection";
         qDebug() << "XML ERROR: " << xml.errorString();
-        delete rootItem;
-        return NULL;
+        return nullptr;
     }
     db.close();
 
-    return rootItem;
-
+    return rootItem.release();
 }
 
 void RhythmboxFeature::importTrack(QXmlStreamReader &xml, QSqlQuery &query) {
diff --git a/src/library/setlogfeature.cpp b/src/library/setlogfeature.cpp
index d9ebbe810a..c5e472c842 100644
--- a/src/library/setlogfeature.cpp
+++ b/src/library/setlogfeature.cpp
@@ -34,8 +34,7 @@ SetlogFeature::SetlogFeature(
             /*show-all-tracks*/ true));
 
     //construct child model
-    auto pRootItem = std::make_unique<TreeItem>(this);
-    m_childModel.setRootItem(std::move(pRootItem));
+    m_childModel.setRootItem(TreeItem::newRoot(this));
     constructChildModel(-1);
 
     m_pJoinWithPreviousAction = new QAction(tr("Join with previous"), this);
diff --git a/src/library/traktor/traktorfeature.cpp b/src/library/traktor/traktorfeature.cpp
index 22fafa7c80..4af74f8107 100644
--- a/src/library/traktor/traktorfeature.cpp
+++ b/src/library/traktor/traktorfeature.cpp
@@ -390,8 +390,8 @@ TreeItem* TraktorFeature::parsePlaylists(QXmlStreamReader &xml) {
 
     QString delimiter = "-->";
 
-    TreeItem *rootItem = new TreeItem(this);
-    TreeItem * parent = rootItem;
+    std::unique_ptr<TreeItem> rootItem = TreeItem::newRoot(this);
+    TreeItem* parent = rootItem.get();
 
     QSqlQuery query_insert_to_playlists(m_database);
     query_insert_to_playlists.prepare("INSERT INTO traktor_playlists (name) "
@@ -452,7 +452,7 @@ TreeItem* TraktorFeature::parsePlaylists(QXmlStreamReader &xml) {
             }
         }
     }
-    return rootItem;
+    return rootItem.release();
 }
 
 void TraktorFeature::parsePlaylistEntries(
diff --git a/src/library/treeitem.cpp b/src/library/treeitem.cpp
index af4e38bf12..af39995597 100644
--- a/src/library/treeitem.cpp
+++ b/src/library/treeitem.cpp
@@ -25,22 +25,15 @@
  * - *feature.cpp
  */
 
-TreeItem::TreeItem()
-    : m_pFeature(nullptr),
-      m_pParent(nullptr),
-      m_bold(false) {
-}
-
 TreeItem::TreeItem(
         LibraryFeature* pFeature,
-        const QString& label,
-        const QVariant& data)
+        QString label,
+        QVariant data)
     : m_pFeature(pFeature),
       m_pParent(nullptr),
-      m_label(label),
-      m_data(data),
+      m_label(std::move(label)),
+      m_data(std::move(data)),
       m_bold(false) {
-    DEBUG_ASSERT(m_pFeature != nullptr);
 }
 
 TreeItem::~TreeItem() {
@@ -64,12 +57,27 @@ TreeItem* TreeItem::child(int row) const {
 }
 
 void TreeItem::appendChild(TreeItem* pChild) {
-    DEBUG_ASSERT(feature() != nullptr);
-    DEBUG_ASSERT(pChild != nullptr);
-    DEBUG_ASSERT(pChild->feature() == feature());
-    DEBUG_ASSERT(!pChild->hasParent());
+    DEBUG_ASSERT(pChild);
+    DEBUG_ASSERT(!pChild->m_pParent);
+    DEBUG_ASSERT(!pChild->m_pFeature ||
+            pChild->m_pFeature == m_pFeature);
     m_children.append(pChild);
     pChild->m_pParent = this;
+    pChild->initFeatureRecursively(m_pFeature);
+}
+
+void TreeItem::initFeatureRecursively(LibraryFeature* pFeature) {
+    DEBUG_ASSERT(!m_pFeature ||
+            m_pFeature == pFeature);
+    DEBUG_ASSERT(!m_pParent ||
+            m_pParent->m_pFeature == pFeature);
+    if (m_pFeature == pFeature) {
+        return;
+    }
+    m_pFeature = pFeature;
+    for (auto* pChild : qAsConst(m_children)) {
+        pChild->initFeatureRecursively(pFeature);
+    }
 }
 
 TreeItem* TreeItem::appendChild(std::unique_ptr<TreeItem> pChild) {
@@ -78,9 +86,9 @@ TreeItem* TreeItem::appendChild(std::unique_ptr<TreeItem> pChild) {
 }
 
 TreeItem* TreeItem::appendChild(
-        const QString& label,
-        const QVariant& data) {
-    auto pNewChild = std::make_unique<TreeItem>(feature(), label, data);
+        QString label,
+        QVariant data) {
+    auto pNewChild = std::make_unique<TreeItem>(std::move(label), std::move(data));
     TreeItem* pChild = pNewChild.get();
     appendChild(pChild); // transfer ownership
     pNewChild.release(); // release ownership (afterwards)
diff --git a/src/library/treeitem.h b/src/library/treeitem.h
index 42f08f80f1..5afd4b7085 100644
--- a/src/library/treeitem.h
+++ b/src/library/treeitem.h
@@ -1,5 +1,4 @@
-#ifndef MIXXX_TREEITEM_H
-#define MIXXX_TREEITEM_H
+#pragma once
 
 #include <QList>
 #include <QString>
@@ -11,14 +10,31 @@
 
 
 class TreeItem final {
+    struct PrivateRootTag {};
+
   public:
-    static const int kInvalidRow = -1;
+    static constexpr int kInvalidRow = -1;
+
+    static std::unique_ptr<TreeItem> newRoot(
+            LibraryFeature* pFeature) {
+        DEBUG_ASSERT(pFeature);
+        return std::make_unique<TreeItem>(pFeature, PrivateRootTag{});
+    }
 
-    TreeItem();
     explicit TreeItem(
+            QString label = QString(),
+            QVariant data = QVariant())
+            : TreeItem(nullptr, std::move(label), std::move(data)) {
+    }
+    // This constructor should actually be private. But that wouldn't
+    // work for std::make_unique(). The private, nested tag essentially
+    // makes this constructor unavailable for everyone else.
+    TreeItem(
             LibraryFeature* pFeature,
-            const QString& label = QString(),
-            const QVariant& data = QVariant());
+            PrivateRootTag)
+            : TreeItem(pFeature) {
+    }
+
     ~TreeItem();
 
 
@@ -27,6 +43,9 @@ class TreeItem final {
     /////////////////////////////////////////////////////////////////////////
 
     LibraryFeature* feature() const {
+        DEBUG_ASSERT(
+                !m_pParent ||
+                m_pParent->m_pFeature == m_pFeature);
         return m_pFeature;
     }
 
@@ -68,11 +87,15 @@ class TreeItem final {
     TreeItem* appendChild(
             std::unique_ptr<TreeItem> pChild);
     TreeItem* appendChild(
-            const QString& label,
-            const QVariant& data = QVariant());
+            QString label,
+            QVariant data = QVariant());
     void removeChild(int row);
 
     // multiple child items
+    void insertChildren(QList<TreeItem*> children, int row) {
+        insertChildren(children, row, children.size());
+        DEBUG_ASSERT(children.isEmpty()); // took ownership
+    }
     void insertChildren(QList<TreeItem*>& children, int row, int count); // take ownership
     void removeChildren(int row, int count);
 
@@ -110,11 +133,21 @@ class TreeItem final {
     }
 
   private:
+    explicit TreeItem(
+            LibraryFeature* pFeature,
+            QString label = QString(),
+            QVariant data = QVariant());
+
     void appendChild(TreeItem* pChild);
+    void initFeatureRecursively(LibraryFeature* pFeature);
 
+    // The library feature is inherited from the parent.
+    // For all child items this is just a shortcut to the
+    // library feature of the root item!
     LibraryFeature* m_pFeature;
 
     TreeItem* m_pParent;
+
     QList<TreeItem*> m_children; // owned child items
 
     QString m_label;
@@ -122,5 +155,3 @@ class TreeItem final {
     QIcon m_icon;
     bool m_bold;
 };
-
-#endif // MIXXX_TREEITEM_H