Refactor|libappfw: Use reference-counted Action instances

skyjake · Feb 3, 2014 · b61e600 · b61e600
1 parent eb973f9
commit b61e600
Show file tree

Hide file tree

Showing 9 changed files with 62 additions and 62 deletions.
diff --git a/doomsday/libappfw/include/de/framework/actionitem.h b/doomsday/libappfw/include/de/framework/actionitem.h
@@ -33,36 +33,50 @@ namespace ui {
 class LIBAPPFW_PUBLIC ActionItem : public Item
 {
 public:
-    ActionItem(de::String const &label = "", de::Action *action = 0)
-        : Item(ShownAsButton | ActivationClosesPopup, label), _action(action) {}
+    ActionItem(String const &label   = "",
+               RefArg<Action> action = RefArg<Action>())
+        : Item(ShownAsButton | ActivationClosesPopup, label)
+        , _action(action.holdRef()) {}
 
-    ActionItem(Semantics semantics, de::String const &label = "", de::Action *action = 0)
-        : Item(semantics, label), _action(action) {}
+    ActionItem(Semantics semantics,
+               String const &label   = "",
+               RefArg<Action> action = RefArg<Action>())
+        : Item(semantics, label)
+        , _action(action.holdRef()) {}
 
-    ActionItem(Semantics semantics, de::Image const &img, de::String const &label = "", de::Action *action = 0)
-        : Item(semantics, label), _action(action), _image(img) {}
+    ActionItem(Semantics semantics,
+               Image const &img,
+               String const &label   = "",
+               RefArg<Action> action = RefArg<Action>())
+        : Item(semantics, label)
+        , _action(action.holdRef())
+        , _image(img) {}
 
-    ActionItem(de::Image const &img, de::String const &label = "", de::Action *action = 0)
-        : Item(ShownAsButton | ActivationClosesPopup, label), _action(action), _image(img) {}
+    ActionItem(Image const &img,
+               String const &label   = "",
+               RefArg<Action> action = RefArg<Action>())
+        : Item(ShownAsButton | ActivationClosesPopup, label)
+        , _action(action.holdRef())
+        , _image(img) {}
 
-    de::Action *action() const { return _action.data(); } // ownership kept
-    de::Image const &image() const { return _image; }
+    Action const *action() const { return _action; }
+    Image const &image() const { return _image; }
 
-    void setAction(de::Action *action)
+    void setAction(RefArg<Action> action)
     {
         _action.reset(action);
         notifyChange();
     }
 
-    void setImage(de::Image const &image)
+    void setImage(Image const &image)
     {
         _image = image;
         notifyChange();
     }
 
 private:
-    QScopedPointer<de::Action> _action;
-    de::Image _image;
+    AutoRef<Action> _action;
+    Image _image;
 };
 
 } // namespace ui

diff --git a/doomsday/libappfw/include/de/framework/signalaction.h b/doomsday/libappfw/include/de/framework/signalaction.h
@@ -33,7 +33,6 @@ class LIBAPPFW_PUBLIC SignalAction : public QObject, public Action
     SignalAction(QObject *target, char const *slot);
 
     void trigger();
-    SignalAction *duplicate() const;
 
 signals:
     void triggered();

diff --git a/doomsday/libappfw/include/de/widgets/buttonwidget.h b/doomsday/libappfw/include/de/widgets/buttonwidget.h
@@ -73,12 +73,11 @@ class LIBAPPFW_PUBLIC ButtonWidget : public LabelWidget
      * Sets the action of the button. It gets triggered when the button is
      * pressed.
      *
-     * @param action  Action instance. Widget takes ownership.
+     * @param action  Action instance. Widget holds a reference.
      */
-    void setAction(Action *action);
+    void setAction(RefArg<Action> action);
 
-    Action *action() const;
-    Action *takeAction();
+    Action const *action() const;
 
     State state() const;
 

diff --git a/doomsday/libappfw/include/de/widgets/dialogwidget.h b/doomsday/libappfw/include/de/widgets/dialogwidget.h
@@ -109,18 +109,18 @@ class LIBAPPFW_PUBLIC DialogWidget : public PopupWidget
          * @param flags  Role flags for the button.
          * @param label  Label for the button. If empty, the default label will be used.
          */
-        ButtonItem(RoleFlags flags, de::String const &label = "");
+        ButtonItem(RoleFlags flags, String const &label = "");
 
         /**
          * Button with custom action.
          * @param flags  Role flags for the button.
          * @param label  Label for the button. If empty, the default label will be used.
          */
-        ButtonItem(RoleFlags flags, de::String const &label, de::Action *action);
+        ButtonItem(RoleFlags flags, String const &label, RefArg<de::Action> action);
 
-        ButtonItem(RoleFlags flags, de::Image const &image, de::Action *action);
+        ButtonItem(RoleFlags flags, Image const &image, RefArg<de::Action> action);
 
-        ButtonItem(RoleFlags flags, de::Image const &image, de::String const &label, de::Action *action);
+        ButtonItem(RoleFlags flags, Image const &image, String const &label, RefArg<de::Action> action);
 
         RoleFlags role() const { return _role; }
 
@@ -129,7 +129,7 @@ class LIBAPPFW_PUBLIC DialogWidget : public PopupWidget
     };
 
 public:
-    DialogWidget(de::String const &name = "", Flags const &flags = DefaultFlags);
+    DialogWidget(String const &name = "", Flags const &flags = DefaultFlags);
 
     Modality modality() const;
 
@@ -155,7 +155,7 @@ class LIBAPPFW_PUBLIC DialogWidget : public PopupWidget
 
     ui::Data &buttons();
 
-    ButtonWidget &buttonWidget(de::String const &label) const;
+    ButtonWidget &buttonWidget(String const &label) const;
 
     ButtonWidget *buttonWidget(int roleId) const;
 
@@ -180,7 +180,7 @@ class LIBAPPFW_PUBLIC DialogWidget : public PopupWidget
 
     // Events.
     void update();
-    bool handleEvent(de::Event const &event);
+    bool handleEvent(Event const &event);
 
 public slots:
     void accept(int result = 1);

diff --git a/doomsday/libappfw/src/signalaction.cpp b/doomsday/libappfw/src/signalaction.cpp
@@ -32,9 +32,4 @@ void SignalAction::trigger()
     emit triggered();
 }
 
-SignalAction *SignalAction::duplicate() const
-{
-    return new SignalAction(_target, _slot);
-}
-
 } // namespace de
diff --git a/doomsday/libappfw/src/widgets/buttonwidget.cpp b/doomsday/libappfw/src/widgets/buttonwidget.cpp
@@ -29,21 +29,28 @@ DENG2_OBSERVES(Action, Triggered)
 {
     State state;
     DotPath hoverTextColor;
-    QScopedPointer<Action> action;
+    Action *action;
     Animation scale;
     Animation frameOpacity;
     bool animating;
 
     Instance(Public *i)
         : Base(i)
         , state(Up)
+        , action(0)
         , scale(1.f)
         , frameOpacity(.08f, Animation::Linear)
         , animating(false)
     {
         setDefaultBackground();
     }
 
+    ~Instance()
+    {
+        if(action) action->audienceForTriggered -= this;
+        releaseRef(action);
+    }
+
     void setState(State st)
     {
         if(state == st) return;
@@ -151,29 +158,24 @@ void ButtonWidget::setHoverTextColor(DotPath const &hoverTextId)
     d->hoverTextColor = hoverTextId;
 }
 
-void ButtonWidget::setAction(Action *action)
+void ButtonWidget::setAction(RefArg<Action> action)
 {
-    if(!d->action.isNull())
+    if(d->action)
     {
         d->action->audienceForTriggered -= d;
     }
 
-    d->action.reset(action);
+    changeRef(d->action, action);
 
     if(action)
     {
         action->audienceForTriggered += d;
     }
 }
 
-Action *ButtonWidget::action() const
+Action const *ButtonWidget::action() const
 {
-    return d->action.data();
-}
-
-Action *ButtonWidget::takeAction()
-{
-    return d->action.take();
+    return d->action;
 }
 
 ButtonWidget::State ButtonWidget::state() const
@@ -206,12 +208,15 @@ bool ButtonWidget::handleEvent(Event const &event)
                 d->updateHover(mouse.pos());
                 if(hitTest(mouse.pos()))
                 {
+                    // Hold an extra ref so the action isn't deleted by triggering.
+                    AutoRef<Action> held = holdRef(d->action);
+
                     // Notify.
                     DENG2_FOR_AUDIENCE(Press, i) i->buttonPressed(*this);
 
-                    if(!d->action.isNull())
+                    if(held)
                     {
-                        d->action->trigger();
+                        held->trigger();
                     }
                 }
                 return true;

diff --git a/doomsday/libappfw/src/widgets/choicewidget.cpp b/doomsday/libappfw/src/widgets/choicewidget.cpp
@@ -52,12 +52,6 @@ DENG2_OBSERVES(ChildWidgetOrganizer, WidgetUpdate)
 
             emit d->self.selectionChangedByUser(d->selected);
         }
-
-        Action *duplicate() const
-        {
-            DENG2_ASSERT(false); // not needed
-            return 0;
-        }
     };
 
     PopupMenuWidget *choices;

diff --git a/doomsday/libappfw/src/widgets/dialogwidget.cpp b/doomsday/libappfw/src/widgets/dialogwidget.cpp
@@ -520,7 +520,7 @@ bool DialogWidget::handleEvent(Event const &event)
                 ButtonWidget const &but = d->buttonWidget(*defaultAction);
                 if(but.action())
                 {
-                    but.action()->trigger();
+                    const_cast<de::Action *>(but.action())->trigger();
                 }
             }
             return true;
@@ -626,15 +626,15 @@ DialogWidget::ButtonItem::ButtonItem(RoleFlags flags, String const &label)
     : ui::ActionItem(label, 0), _role(flags)
 {}
 
-DialogWidget::ButtonItem::ButtonItem(RoleFlags flags, String const &label, de::Action *action)
+DialogWidget::ButtonItem::ButtonItem(RoleFlags flags, String const &label, RefArg<de::Action> action)
     : ui::ActionItem(label, action), _role(flags)
 {}
 
-DialogWidget::ButtonItem::ButtonItem(RoleFlags flags, Image const &image, de::Action *action)
+DialogWidget::ButtonItem::ButtonItem(RoleFlags flags, Image const &image, RefArg<de::Action> action)
     : ui::ActionItem(image, "", action), _role(flags)
 {}
 
-DialogWidget::ButtonItem::ButtonItem(RoleFlags flags, Image const &image, String const &label, de::Action *action)
+DialogWidget::ButtonItem::ButtonItem(RoleFlags flags, Image const &image, String const &label, RefArg<de::Action> action)
     : ui::ActionItem(image, label, action), _role(flags)
 {}
 

diff --git a/doomsday/libappfw/src/widgets/menuwidget.cpp b/doomsday/libappfw/src/widgets/menuwidget.cpp
@@ -94,12 +94,6 @@ DENG2_PIMPL(MenuWidget)
             _widget->open();
         }
 
-        Action *duplicate() const
-        {
-            DENG2_ASSERT(false); // not needed
-            return 0;
-        }
-
     protected:
         Instance *d;
         ui::Item const &_parentItem;
@@ -284,7 +278,7 @@ DENG2_PIMPL(MenuWidget)
                 b.setText(act->label());
                 if(act->action())
                 {
-                    b.setAction(act->action()->duplicate());
+                    b.setAction(*act->action());
                 }
             }
             else if(item.semantics().testFlag(Item::ShownAsLabel))
@@ -297,7 +291,7 @@ DENG2_PIMPL(MenuWidget)
                 t.setText(act->label());
                 if(act->action())
                 {
-                    t.setAction(act->action()->duplicate());
+                    t.setAction(*act->action());
                 }
             }
         }