Fix crash when loading an extension with invalid dithering matrices a…

…nd the console must be shown (fix aseprite#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
@@ -92,6 +92,9 @@ MainWindow::MainWindow()
 {
   m_tooltipManager = new TooltipManager();
   m_menuBar = new MainMenuBar();
+  m_statusBar = new StatusBar(m_tooltipManager);
+  m_colorBar = new ColorBar(colorBarPlaceholder()->align(),
+                            m_tooltipManager);
 
   // Register commands to load menus+shortcuts for these commands
   Editor::registerCommands();
@@ -102,10 +105,24 @@ MainWindow::MainWindow()
   // Setup the main menubar
   m_menuBar->setMenu(AppMenus::instance()->getRootMenu());
 
+  // configure all widgets to expansives
+  m_menuBar->setExpansive(true);
+  m_statusBar->setExpansive(true);
+  m_colorBar->setExpansive(true);
+}
+
+// This 'initialize' function is a way of splitting the creation of
+// the MainWindow. First a minimal instance of MainWindow is made, then
+// all UI components that can trigger the console to report
+// any unexpected errors/warnings are initialized.
+// Prior to this split, Aseprite could crash due to errors during launch.
+// Along the MainWindow creation if any error occurs, whatever it is,
+// Console was called, its constructor tried to create a connection to
+// the MainWindow "Close" command, but resulted in a crash since
+// such an instance did not yet exist.
+// Refer to https://github.com/aseprite/aseprite/issues/3914
+void MainWindow::initialize() {
   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);
@@ -168,34 +185,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();