Fix crashes using MaskByColor command after we cannot lock the sprite

If ContextWriter thrown an exception (i.e. we cannot lock the sprite
for writing purposes), the "Mask by Color" ui::Window would persist
with its signal slots, and it creates an unstable scenario where color
change signals are generated from sliders and crashes the program
because we were using a deleted ContextReader variable which was
stored in the stack in MaskByColorCommand::onExecute().
This commit is contained in:
David Capello 2020-02-26 12:49:23 -03:00
parent 5cd23b8522
commit f0e60d2068

View File

@ -27,6 +27,7 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/chrono.h" #include "base/chrono.h"
#include "base/convert_to.h" #include "base/convert_to.h"
#include "base/scoped_value.h"
#include "doc/image.h" #include "doc/image.h"
#include "doc/mask.h" #include "doc/mask.h"
#include "doc/sprite.h" #include "doc/sprite.h"
@ -70,11 +71,11 @@ private:
} }
}; };
Window* m_window; // TODO we cannot use a std::unique_ptr because clone() needs a copy ctor Window* m_window = nullptr;
ColorButton* m_buttonColor; ColorButton* m_buttonColor = nullptr;
CheckBox* m_checkPreview; CheckBox* m_checkPreview = nullptr;
Slider* m_sliderTolerance; Slider* m_sliderTolerance = nullptr;
SelModeField* m_selMode; SelModeField* m_selMode = nullptr;
}; };
MaskByColorCommand::MaskByColorCommand() MaskByColorCommand::MaskByColorCommand()
@ -91,6 +92,8 @@ bool MaskByColorCommand::onEnabled(Context* context)
void MaskByColorCommand::onExecute(Context* context) void MaskByColorCommand::onExecute(Context* context)
{ {
ASSERT(!m_window);
const ContextReader reader(context); const ContextReader reader(context);
const Sprite* sprite = reader.sprite(); const Sprite* sprite = reader.sprite();
@ -102,7 +105,8 @@ void MaskByColorCommand::onExecute(Context* context)
if (!image) if (!image)
return; return;
m_window = new Window(Window::WithTitleBar, "Mask by Color"); std::unique_ptr<Window> win(new Window(Window::WithTitleBar, "Mask by Color"));
base::ScopedValue<Window*> setWindow(m_window, win.get(), nullptr);
TooltipManager* tooltipManager = new TooltipManager(); TooltipManager* tooltipManager = new TooltipManager();
m_window->addChild(tooltipManager); m_window->addChild(tooltipManager);
auto box1 = new Box(VERTICAL); auto box1 = new Box(VERTICAL);
@ -135,7 +139,6 @@ void MaskByColorCommand::onExecute(Context* context)
button_ok->Click.connect(base::Bind<void>(&Window::closeWindow, m_window, button_ok)); button_ok->Click.connect(base::Bind<void>(&Window::closeWindow, m_window, button_ok));
button_cancel->Click.connect(base::Bind<void>(&Window::closeWindow, m_window, button_cancel)); button_cancel->Click.connect(base::Bind<void>(&Window::closeWindow, m_window, button_cancel));
m_buttonColor->Change.connect(base::Bind<void>(&MaskByColorCommand::maskPreview, this, base::Ref(reader))); m_buttonColor->Change.connect(base::Bind<void>(&MaskByColorCommand::maskPreview, this, base::Ref(reader)));
m_sliderTolerance->Change.connect(base::Bind<void>(&MaskByColorCommand::maskPreview, this, base::Ref(reader))); m_sliderTolerance->Change.connect(base::Bind<void>(&MaskByColorCommand::maskPreview, this, base::Ref(reader)));
m_checkPreview->Click.connect(base::Bind<void>(&MaskByColorCommand::maskPreview, this, base::Ref(reader))); m_checkPreview->Click.connect(base::Bind<void>(&MaskByColorCommand::maskPreview, this, base::Ref(reader)));
@ -198,7 +201,6 @@ void MaskByColorCommand::onExecute(Context* context)
// Save window configuration. // Save window configuration.
save_window_pos(m_window, "MaskColor"); save_window_pos(m_window, "MaskColor");
delete m_window;
} }
Mask* MaskByColorCommand::generateMask(const Mask& origMask, Mask* MaskByColorCommand::generateMask(const Mask& origMask,
@ -245,7 +247,8 @@ Mask* MaskByColorCommand::generateMask(const Mask& origMask,
void MaskByColorCommand::maskPreview(const ContextReader& reader) void MaskByColorCommand::maskPreview(const ContextReader& reader)
{ {
if (m_checkPreview->isSelected()) { ASSERT(m_window);
if (m_window && m_checkPreview->isSelected()) {
int xpos, ypos; int xpos, ypos;
const Image* image = reader.image(&xpos, &ypos); const Image* image = reader.image(&xpos, &ypos);
std::unique_ptr<Mask> mask(generateMask(*reader.document()->mask(), std::unique_ptr<Mask> mask(generateMask(*reader.document()->mask(),