move stars_up/_down COs to BaseTrackPlayer to avoid hitting rare DEBU…

…G_ASSERT on skin reload

If owned by WStarRating, it can happen that the COs are not deleted when the skin is destroyed.

This is not a fix, only a workaround, for the yet unknown root cause.
#12560

src/library/dlgtrackinfo.cpp

@@ -45,7 +45,7 @@ DlgTrackInfo::DlgTrackInfo(
           m_tapFilter(this, kFilterLength, kMaxInterval),
           m_pWCoverArtMenu(make_parented<WCoverArtMenu>(this)),
           m_pWCoverArtLabel(make_parented<WCoverArtLabel>(this, m_pWCoverArtMenu)),
-          m_pWStarRating(make_parented<WStarRating>(nullptr, this)),
+          m_pWStarRating(make_parented<WStarRating>(this)),
           m_pColorPicker(make_parented<WColorPickerAction>(
                   WColorPicker::Option::AllowNoColor |
                           // TODO(xxx) remove this once the preferences are themed via QSS

src/mixer/basetrackplayer.cpp

@@ -709,6 +709,35 @@ void BaseTrackPlayerImpl::loadTrackFromGroup(const QString& group) {
     slotLoadTrack(pTrack, false);
 }
 
+void BaseTrackPlayerImpl::maybeCreateStarControls() {
+    if (m_pStarsUp == nullptr) {
+        m_pStarsUp = std::make_unique<ControlPushButton>(ConfigKey(getGroup(), "stars_up"));
+        connect(m_pStarsUp.get(),
+                &ControlObject::valueChanged,
+                this,
+                &BaseTrackPlayerImpl::slotStarsUp);
+    }
+    if (m_pStarsDown == nullptr) {
+        m_pStarsDown = std::make_unique<ControlPushButton>(ConfigKey(getGroup(), "stars_down"));
+        connect(m_pStarsDown.get(),
+                &ControlObject::valueChanged,
+                this,
+                &BaseTrackPlayerImpl::slotStarsDown);
+    }
+}
+
+void BaseTrackPlayerImpl::slotStarsUp(double v) {
+    if (v > 0) {
+        emit trackRatingChangeRequest(1);
+    }
+}
+
+void BaseTrackPlayerImpl::slotStarsDown(double v) {
+    if (v > 0) {
+        emit trackRatingChangeRequest(-1);
+    }
+}
+
 void BaseTrackPlayerImpl::slotSetReplayGain(mixxx::ReplayGain replayGain) {
     // Do not change replay gain when track is playing because
     // this may lead to an unexpected volume change.

src/mixer/basetrackplayer.h

@@ -37,6 +37,7 @@ class BaseTrackPlayer : public BasePlayer {
 
     virtual TrackPointer getLoadedTrack() const = 0;
     virtual void setupEqControls() = 0;
+    virtual void maybeCreateStarControls() = 0;
 
   public slots:
     virtual void slotLoadTrack(TrackPointer pTrack, bool bPlay = false) = 0;
@@ -52,6 +53,7 @@ class BaseTrackPlayer : public BasePlayer {
     void playerEmpty();
     void noVinylControlInputConfigured();
     void trackRatingChanged(int rating);
+    void trackRatingChangeRequest(int change);
 };
 
 class BaseTrackPlayerImpl : public BaseTrackPlayer {
@@ -76,6 +78,11 @@ class BaseTrackPlayerImpl : public BaseTrackPlayer {
 
     void setupEqControls() final;
 
+    // Controls to change the star rating with controllers.
+    // Created request only, because we need them only if there is a star
+    // rating widget for this player.
+    void maybeCreateStarControls() final;
+
     // For testing, loads a fake track.
     TrackPointer loadFakeTrack(bool bPlay, double filebpm);
 
@@ -109,6 +116,8 @@ class BaseTrackPlayerImpl : public BaseTrackPlayer {
     void slotWaveformZoomSetDefault(double pressed);
     void slotShiftCuesMillis(double milliseconds);
     void slotShiftCuesMillisButton(double value, double milliseconds);
+    void slotStarsUp(double v);
+    void slotStarsDown(double v);
     void slotUpdateReplayGainFromPregain(double pressed);
 
   private:
@@ -164,6 +173,9 @@ class BaseTrackPlayerImpl : public BaseTrackPlayer {
     std::unique_ptr<ControlPushButton> m_pShiftCuesLaterSmall;
     std::unique_ptr<ControlObject> m_pShiftCues;
 
+    std::unique_ptr<ControlPushButton> m_pStarsUp;
+    std::unique_ptr<ControlPushButton> m_pStarsDown;
+
     std::unique_ptr<ControlObject> m_pUpdateReplayGainFromPregain;
 
     parented_ptr<ControlProxy> m_pReplayGain;

src/skin/legacy/legacyskinparser.cpp

@@ -1171,14 +1171,23 @@ QWidget* LegacySkinParser::parseStarRating(const QDomElement& node) {
         return nullptr;
     }
 
-    WStarRating* pStarRating = new WStarRating(group, m_pParent);
+    // Create stars_up/down if they haven't been created, yet.
+    // Previously created connections should have been removed
+    //  when the previous skin/widget has been destroyed.
+    pPlayer->maybeCreateStarControls();
+
+    WStarRating* pStarRating = new WStarRating(m_pParent);
     commonWidgetSetup(node, pStarRating, false);
     pStarRating->setup(node, *m_pContext);
 
     connect(pPlayer,
             &BaseTrackPlayer::trackRatingChanged,
             pStarRating,
             &WStarRating::slotSetRating);
+    connect(pPlayer,
+            &BaseTrackPlayer::trackRatingChangeRequest,
+            pStarRating,
+            &WStarRating::slotRatingUpDownRequest);
     connect(pStarRating,
             &WStarRating::ratingChanged,
             pPlayer,

src/test/autodjprocessor_test.cpp

@@ -88,6 +88,7 @@ class FakeDeck : public BaseTrackPlayer {
     }
 
     void setupEqControls() override{};
+    void maybeCreateStarControls() override{};
 
     // This method emulates requesting a track load to a player and emits no
     // signals. Normally, the reader thread attempts to load the file and emits

src/widget/wstarrating.cpp

@@ -9,22 +9,10 @@
 class QEvent;
 class QWidgets;
 
-WStarRating::WStarRating(const QString& group, QWidget* pParent)
+WStarRating::WStarRating(QWidget* pParent)
         : WWidget(pParent),
           m_starCount(0),
           m_visualStarRating(m_starCount, 5) {
-    // Controls to change the star rating with controllers.
-    // Note that 'group' maybe NULLPTR, e.g. when called from DlgTrackInfo,
-    // so only create rate change COs if there's a group passed when creating deck widgets.
-    if (!group.isEmpty()) {
-        m_pStarsUp = std::make_unique<ControlPushButton>(ConfigKey(group, "stars_up"));
-        m_pStarsDown = std::make_unique<ControlPushButton>(ConfigKey(group, "stars_down"));
-        connect(m_pStarsUp.get(), &ControlObject::valueChanged, this, &WStarRating::slotStarsUp);
-        connect(m_pStarsDown.get(),
-                &ControlObject::valueChanged,
-                this,
-                &WStarRating::slotStarsDown);
-    }
 }
 
 void WStarRating::setup(const QDomNode& node, const SkinContext& context) {
@@ -57,6 +45,10 @@ void WStarRating::slotSetRating(int starCount) {
     emit ratingChanged(m_starCount);
 }
 
+void WStarRating::slotRatingUpDownRequest(int change) {
+    slotSetRating(m_starCount + change);
+}
+
 void WStarRating::paintEvent(QPaintEvent * /*unused*/) {
     QStyleOption option;
     option.initFrom(this);
@@ -83,18 +75,6 @@ void WStarRating::mouseMoveEvent(QMouseEvent *event) {
     }
 }
 
-void WStarRating::slotStarsUp(double v) {
-    if (v > 0 && m_starCount < m_visualStarRating.maxStarCount()) {
-        slotSetRating(m_starCount + 1);
-    }
-}
-
-void WStarRating::slotStarsDown(double v) {
-    if (v > 0 && m_starCount > StarRating::kMinStarCount) {
-        slotSetRating(m_starCount - 1);
-    }
-}
-
 void WStarRating::leaveEvent(QEvent* /*unused*/) {
     resetVisualRating();
 }

src/widget/wstarrating.h

@@ -1,32 +1,26 @@
 #pragma once
 
-#include "control/controlpushbutton.h"
 #include "library/starrating.h"
 #include "widget/wwidget.h"
 
-class ControlObject;
-class ControlPushButton;
 class QDomNode;
 class SkinContext;
 
 class WStarRating : public WWidget {
     Q_OBJECT
   public:
-    WStarRating(const QString& group, QWidget* pParent);
+    WStarRating(QWidget* pParent);
 
     virtual void setup(const QDomNode& node, const SkinContext& context);
     QSize sizeHint() const override;
 
   public slots:
     void slotSetRating(int starCount);
+    void slotRatingUpDownRequest(int change);
 
   signals:
     void ratingChanged(int starCount);
 
-  private slots:
-    void slotStarsUp(double v);
-    void slotStarsDown(double v);
-
   protected:
     void paintEvent(QPaintEvent* e) override;
     void mouseMoveEvent(QMouseEvent *event) override;
@@ -44,7 +38,4 @@ class WStarRating : public WWidget {
     void resetVisualRating() {
         updateVisualRating(m_starCount);
     }
-
-    std::unique_ptr<ControlPushButton> m_pStarsUp;
-    std::unique_ptr<ControlPushButton> m_pStarsDown;
 };