Fix crash when loading an extension with invalid dithering matrices (fix #3914)

Before this fix, when installing dithering matrices if Aseprite
couldn't find the file of some matrix described in the json, Aseprite
would crash (this happened during the installation of an erroneous
dithering matrices extension, and after every reboot of Aseprite).

The cause of the crash was the absence of the MainWindow instance
during the ContextBar creation. When an error occurs, the console is
called, but since MainWindows is not yet available, aseprite crashes.

src/app/app.cpp

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2018-2022  Igara Studio S.A.
+// Copyright (C) 2018-2023  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -363,6 +363,7 @@ int App::initialize(const AppOptions& options)
 
     // Create the main window.
     m_mainWindow.reset(new MainWindow);
+    m_mainWindow->initialize();
     if (m_mod)
       m_mod->modMainWindow(m_mainWindow.get());
 

src/app/commands/cmd_change_pixel_format.cpp

@@ -14,6 +14,7 @@
 #include "app/cmd/set_pixel_format.h"
 #include "app/commands/command.h"
 #include "app/commands/params.h"
+#include "app/console.h"
 #include "app/context_access.h"
 #include "app/extensions.h"
 #include "app/i18n/strings.h"
@@ -533,8 +534,14 @@ void ChangePixelFormatCommand::onLoadParams(const Params& params)
     // Then, if the matrix doesn't exist we try to load it from a file
     else {
       render::DitheringMatrix ditMatrix;
-      if (!load_dithering_matrix_from_sprite(matrix, ditMatrix))
-        throw std::runtime_error("Invalid matrix name");
+      try {
+        load_dithering_matrix_from_sprite(matrix, ditMatrix);
+      }
+      catch (const std::exception& e) {
+        LOG(ERROR, "%s\n", e.what());
+        Console::showException(e);
+      }
+
       m_dithering.matrix(ditMatrix);
     }
   }

src/app/extensions.cpp

@@ -219,8 +219,8 @@ Extension::DitheringMatrixInfo::DitheringMatrixInfo(const std::string& path,
 const render::DitheringMatrix& Extension::DitheringMatrixInfo::matrix() const
 {
   if (!m_loaded) {
-    m_loaded = true;
     load_dithering_matrix_from_sprite(m_path, m_matrix);
+    m_loaded = true;
   }
   return m_matrix;
 }

src/app/load_matrix.cpp

@@ -1,4 +1,5 @@
 // Aseprite
+// Copyright (C) 2023 Igara Studio S.A.
 // Copyright (C) 2017-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -15,17 +16,19 @@
 #include "app/file/file.h"
 #include "doc/layer.h"
 #include "doc/sprite.h"
+#include "fmt/format.h"
 #include "render/dithering_matrix.h"
 
 namespace app {
 
-bool load_dithering_matrix_from_sprite(
+void load_dithering_matrix_from_sprite(
   const std::string& filename,
   render::DitheringMatrix& matrix)
 {
   std::unique_ptr<Doc> doc(load_document(nullptr, filename));
   if (!doc)
-    return false;
+    throw std::runtime_error(
+      fmt::format("The dithering matrix file {} doesn't exist", filename));
 
   doc::Sprite* spr = doc->sprite();
   const doc::Layer* lay = (spr && spr->root() ? spr->root()->firstLayer():
@@ -45,8 +48,6 @@ bool load_dithering_matrix_from_sprite(
   else {
     matrix = render::DitheringMatrix();
   }
-
-  return true;
 }
 
 } // namespace app

src/app/load_matrix.h

@@ -1,4 +1,5 @@
 // Aseprite
+// Copyright (C) 2023  Igara Studio S.A.
 // Copyright (C) 2017  David Capello
 //
 // This program is distributed under the terms of
@@ -16,7 +17,7 @@ namespace render {
 
 namespace app {
 
-  bool load_dithering_matrix_from_sprite(
+  void load_dithering_matrix_from_sprite(
     const std::string& filename,
     render::DitheringMatrix& matrix);
 

src/app/ui/dithering_selector.cpp

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2019-2022  Igara Studio S.A.
+// Copyright (C) 2019-2023  Igara Studio S.A.
 // Copyright (C) 2017  David Capello
 //
 // This program is distributed under the terms of
@@ -12,6 +12,7 @@
 #include "app/ui/dithering_selector.h"
 
 #include "app/app.h"
+#include "app/console.h"
 #include "app/extensions.h"
 #include "app/i18n/strings.h"
 #include "app/modules/palettes.h"
@@ -210,17 +211,29 @@ void DitheringSelector::regenerate()
                              render::DitheringMatrix(),
                              Strings::dithering_selector_no_dithering()));
       for (const auto& it : ditheringMatrices) {
-        addItem(new DitherItem(
-          render::DitheringAlgorithm::Ordered,
-          it.matrix(),
-          Strings::dithering_selector_ordered_dithering() + it.name()));
+        try {
+          addItem(new DitherItem(
+            render::DitheringAlgorithm::Ordered,
+            it.matrix(),
+            Strings::dithering_selector_ordered_dithering() + it.name()));
+        }
+        catch (const std::exception& e) {
+          LOG(ERROR, "%s\n", e.what());
+          Console::showException(e);
+        }
       }
       for (const auto& it : ditheringMatrices) {
-        addItem(
-          new DitherItem(
-            render::DitheringAlgorithm::Old,
-            it.matrix(),
-            Strings::dithering_selector_old_dithering() + it.name()));
+        try {
+          addItem(
+            new DitherItem(
+              render::DitheringAlgorithm::Old,
+              it.matrix(),
+              Strings::dithering_selector_old_dithering() + it.name()));
+        }
+        catch (const std::exception& e) {
+          LOG(ERROR, "%s\n", e.what());
+          Console::showException(e);
+        }
       }
       addItem(
         new DitherItem(
@@ -231,8 +244,15 @@ void DitheringSelector::regenerate()
     case SelectMatrix:
       addItem(new DitherItem(render::DitheringMatrix(),
                              Strings::dithering_selector_no_dithering()));
-      for (auto& it : ditheringMatrices)
-        addItem(new DitherItem(it.matrix(), it.name()));
+      for (auto& it : ditheringMatrices) {
+        try {
+          addItem(new DitherItem(it.matrix(), it.name()));
+        }
+        catch (const std::exception& e) {
+          LOG(ERROR, "%s\n", e.what());
+          Console::showException(e);
+        }
+      }
       break;
   }
 

src/app/ui/main_window.cpp

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2018-2022  Igara Studio S.A.
+// Copyright (C) 2018-2023  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -89,6 +89,18 @@ MainWindow::MainWindow()
 #ifdef ENABLE_SCRIPTING
   , m_devConsoleView(nullptr)
 #endif
+{
+}
+
+// This 'initialize' function is a way to split the creation of the
+// MainWindow. First a minimal instance of MainWindow is created, then
+// all UI components that can trigger the Console to report any
+// unexpected errors/warnings in the initialization. Prior to this,
+// Aseprite could fail in the same constructor, and the Console didn't
+// have access to the App::instance()->mainWindow() pointer.
+//
+// Refer to https://github.com/aseprite/aseprite/issues/3914
+void MainWindow::initialize()
 {
   m_tooltipManager = new TooltipManager();
   m_menuBar = new MainMenuBar();
@@ -104,13 +116,13 @@ MainWindow::MainWindow()
 
   m_notifications = new Notifications();
   m_statusBar = new StatusBar(m_tooltipManager);
-  m_colorBar = new ColorBar(colorBarPlaceholder()->align(),
-                            m_tooltipManager);
-  m_contextBar = new ContextBar(m_tooltipManager, m_colorBar);
   m_toolBar = new ToolBar();
   m_tabsBar = new WorkspaceTabs(this);
   m_workspace = new Workspace();
   m_previewEditor = new PreviewEditorWindow();
+  m_colorBar = new ColorBar(colorBarPlaceholder()->align(),
+                            m_tooltipManager);
+  m_contextBar = new ContextBar(m_tooltipManager, m_colorBar);
 
   // The timeline (AniControls) tooltips will use the keyboard
   // shortcuts loaded above.
@@ -168,34 +180,38 @@ MainWindow::~MainWindow()
 
 #ifdef ENABLE_SCRIPTING
   if (m_devConsoleView) {
-    if (m_devConsoleView->parent())
+    if (m_devConsoleView->parent() && m_workspace)
       m_workspace->removeView(m_devConsoleView);
     delete m_devConsoleView;
   }
 #endif
 
   if (m_browserView) {
-    if (m_browserView->parent())
+    if (m_browserView->parent() && m_workspace)
       m_workspace->removeView(m_browserView);
     delete m_browserView;
   }
 
   if (m_homeView) {
-    if (m_homeView->parent())
+    if (m_homeView->parent() && m_workspace)
       m_workspace->removeView(m_homeView);
     delete m_homeView;
   }
-  delete m_contextBar;
-  delete m_previewEditor;
+  if (m_contextBar)
+    delete m_contextBar;
+  if (m_previewEditor)
+    delete m_previewEditor;
 
   // Destroy the workspace first so ~Editor can dettach slots from
   // ColorBar. TODO this is a terrible hack for slot/signal stuff,
   // connections should be handle in a better/safer way.
-  delete m_workspace;
+  if (m_workspace)
+    delete m_workspace;
 
   // Remove the root-menu from the menu-bar (because the rootmenu
   // module should destroy it).
-  m_menuBar->setMenu(NULL);
+  if (m_menuBar)
+    m_menuBar->setMenu(NULL);
 }
 
 void MainWindow::onLanguageChange()

src/app/ui/main_window.h

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2018-2022  Igara Studio S.A.
+// Copyright (C) 2018-2023  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -70,6 +70,7 @@ namespace app {
     void updateConsentCheckbox();
 #endif
 
+    void initialize();
     void start();
     void showNotification(INotificationDelegate* del);
     void showHomeOnOpen();