[gimp] app: keep track of number of errors and traces in GimpCriticalDialog.



commit 34fe992f44972c24481541d2fb0d02c6c28a7f07
Author: Jehan <jehan girinstud io>
Date:   Mon Feb 12 01:59:08 2018 +0100

    app: keep track of number of errors and traces in GimpCriticalDialog.
    
    We don't want an infinite number of traces because it takes some time to
    get. Until now I was keeping track of traces in app/errors.c, but that
    was very sucky because then I was limiting traces per session. Instead
    save them as a variable of a GimpCriticalDialog instance. Therefore only
    generate the traces for WARNING/CRITICAL at the last second, when
    calling the dialog.
    When too many traces are displayed, just fallback to just add error
    messages only. But then even errors without traces can be time-consuming
    (if you have dozens of thousands of errors in a few seconds, as I had
    the other day, updating the dialog for all of them would just freeze the
    whole application for a long time).
    So also keep track of errors as well and as last fallback, just send the
    remaining errors to the stderr.

 app/core/gimp-gui.c              |    9 +---
 app/core/gimp-gui.h              |    6 +--
 app/core/gimp.c                  |    4 +-
 app/errors.c                     |   48 +++-------------------
 app/gui/gui-message.c            |   84 ++++++++++++++++++++-----------------
 app/gui/gui-message.h            |    3 +-
 app/pdb/message-cmds.c           |    2 +-
 app/widgets/gimpcriticaldialog.c |   35 ++++++++++++++--
 app/widgets/gimpcriticaldialog.h |    5 ++
 pdb/groups/message.pdb           |    2 +-
 10 files changed, 97 insertions(+), 101 deletions(-)
---
diff --git a/app/core/gimp-gui.c b/app/core/gimp-gui.c
index 4c3842b..1c899db 100644
--- a/app/core/gimp-gui.c
+++ b/app/core/gimp-gui.c
@@ -159,10 +159,9 @@ gimp_show_message (Gimp                *gimp,
                    GObject             *handler,
                    GimpMessageSeverity  severity,
                    const gchar         *domain,
-                   const gchar         *message,
-                   const gchar         *trace)
+                   const gchar         *message)
 {
-  const gchar *desc = trace ? "Error" : "Message";
+  const gchar *desc = (severity == GIMP_MESSAGE_ERROR) ? "Error" : "Message";
 
   g_return_if_fail (GIMP_IS_GIMP (gimp));
   g_return_if_fail (handler == NULL || G_IS_OBJECT (handler));
@@ -176,7 +175,7 @@ gimp_show_message (Gimp                *gimp,
       if (gimp->gui.show_message)
         {
           gimp->gui.show_message (gimp, handler, severity,
-                                  domain, message, trace);
+                                  domain, message);
           return;
         }
       else if (GIMP_IS_PROGRESS (handler) &&
@@ -191,8 +190,6 @@ gimp_show_message (Gimp                *gimp,
   gimp_enum_get_value (GIMP_TYPE_MESSAGE_SEVERITY, severity,
                        NULL, NULL, &desc, NULL);
   g_printerr ("%s-%s: %s\n\n", domain, desc, message);
-  if (trace)
-    g_printerr ("Back trace:\n%s\n\n", trace);
 }
 
 void
diff --git a/app/core/gimp-gui.h b/app/core/gimp-gui.h
index e80516d..6118765 100644
--- a/app/core/gimp-gui.h
+++ b/app/core/gimp-gui.h
@@ -35,8 +35,7 @@ struct _GimpGui
                                              GObject             *handler,
                                              GimpMessageSeverity  severity,
                                              const gchar         *domain,
-                                             const gchar         *message,
-                                             const gchar         *trace);
+                                             const gchar         *message);
   void           (* help)                   (Gimp                *gimp,
                                              GimpProgress        *progress,
                                              const gchar         *help_domain,
@@ -145,8 +144,7 @@ void           gimp_show_message           (Gimp                *gimp,
                                             GObject             *handler,
                                             GimpMessageSeverity  severity,
                                             const gchar         *domain,
-                                            const gchar         *message,
-                                            const gchar         *trace);
+                                            const gchar         *message);
 void           gimp_help                   (Gimp                *gimp,
                                             GimpProgress        *progress,
                                             const gchar         *help_domain,
diff --git a/app/core/gimp.c b/app/core/gimp.c
index c2f1a04..ef3c933 100644
--- a/app/core/gimp.c
+++ b/app/core/gimp.c
@@ -1123,7 +1123,7 @@ gimp_message_valist (Gimp                *gimp,
 
   message = g_strdup_vprintf (format, args);
 
-  gimp_show_message (gimp, handler, severity, NULL, message, NULL);
+  gimp_show_message (gimp, handler, severity, NULL, message);
 
   g_free (message);
 }
@@ -1138,7 +1138,7 @@ gimp_message_literal (Gimp                *gimp,
   g_return_if_fail (handler == NULL || G_IS_OBJECT (handler));
   g_return_if_fail (message != NULL);
 
-  gimp_show_message (gimp, handler, severity, NULL, message, NULL);
+  gimp_show_message (gimp, handler, severity, NULL, message);
 }
 
 void
diff --git a/app/errors.c b/app/errors.c
index 62b326a..3bf83cc 100644
--- a/app/errors.c
+++ b/app/errors.c
@@ -40,8 +40,6 @@
 #include <windows.h>
 #endif
 
-#define MAX_TRACES 3
-
 /*  private variables  */
 
 static Gimp                *the_errors_gimp   = NULL;
@@ -183,7 +181,7 @@ gimp_third_party_message_log_func (const gchar    *log_domain,
        * messages.
        */
       gimp_show_message (gimp, NULL, GIMP_MESSAGE_WARNING,
-                         log_domain, message, NULL);
+                         log_domain, message);
     }
   else
     {
@@ -197,11 +195,8 @@ gimp_message_log_func (const gchar    *log_domain,
                        const gchar    *message,
                        gpointer        data)
 {
-  static gint          n_traces;
+  Gimp                *gimp     = data;
   GimpMessageSeverity  severity = GIMP_MESSAGE_WARNING;
-  Gimp                *gimp   = data;
-  gchar               *trace  = NULL;
-  const gchar         *reason;
 
   if (flags & (G_LOG_LEVEL_CRITICAL | G_LOG_LEVEL_WARNING))
     {
@@ -216,44 +211,18 @@ gimp_message_log_func (const gchar    *log_domain,
            (flags & G_LOG_LEVEL_CRITICAL)) ||
           debug_policy == GIMP_DEBUG_POLICY_WARNING)
         {
-          severity = (flags & G_LOG_LEVEL_CRITICAL) ?
-            GIMP_MESSAGE_ERROR : GIMP_MESSAGE_WARNING;
-
-          if (n_traces < MAX_TRACES)
-            {
-              /* Getting debug traces is time-expensive, and worse, some
-               * critical errors have the bad habit to create more errors
-               * (the first ones are therefore usually the most useful).
-               * This is why we keep track of how many times we made traces
-               * and stop doing them after a while.
-               * Hence when this happens, critical errors are simply processed as
-               * lower level errors.
-               */
-              gimp_print_stack_trace ((const gchar *) full_prog_name,
-                                      NULL, &trace);
-              n_traces++;
-            }
-        }
-      if (! trace)
-        {
-          /* Since we overrided glib default's WARNING and CRITICAL
-           * handler, if we decide not to handle this error in the end,
-           * let's just print it in terminal in a similar fashion as
-           * glib's default handler (though without the fancy terminal
-           * colors right now).
-           */
-          goto print_to_stderr;
+          severity = GIMP_MESSAGE_ERROR;
         }
     }
 
   if (gimp)
     {
-      gimp_show_message (gimp, NULL, severity,
-                         NULL, message, trace);
+      gimp_show_message (gimp, NULL, severity, NULL, message);
     }
   else
     {
-print_to_stderr:
+      const gchar *reason;
+
       switch (flags & G_LOG_LEVEL_MASK)
         {
         case G_LOG_LEVEL_WARNING:
@@ -270,12 +239,7 @@ print_to_stderr:
       g_printerr ("%s: %s-%s: %s\n",
                   gimp_filename_to_utf8 (full_prog_name),
                   log_domain, reason, message);
-      if (trace)
-        g_printerr ("Back trace:\n%s\n\n", trace);
     }
-
-  if (trace)
-    g_free (trace);
 }
 
 static void
diff --git a/app/gui/gui-message.c b/app/gui/gui-message.c
index 1f97423..bbaa3ca 100644
--- a/app/gui/gui-message.c
+++ b/app/gui/gui-message.c
@@ -56,7 +56,6 @@ typedef struct
   Gimp                *gimp;
   gchar               *domain;
   gchar               *message;
-  gchar               *trace;
   GObject             *handler;
   GimpMessageSeverity  severity;
 } GimpLogMessageData;
@@ -66,18 +65,15 @@ static gboolean  gui_message_idle          (gpointer             user_data);
 static gboolean  gui_message_error_console (Gimp                *gimp,
                                             GimpMessageSeverity  severity,
                                             const gchar         *domain,
-                                            const gchar         *message,
-                                            const gchar         *trace);
+                                            const gchar         *message);
 static gboolean  gui_message_error_dialog  (Gimp                *gimp,
                                             GObject             *handler,
                                             GimpMessageSeverity  severity,
                                             const gchar         *domain,
-                                            const gchar         *message,
-                                            const gchar         *trace);
+                                            const gchar         *message);
 static void      gui_message_console       (GimpMessageSeverity  severity,
                                             const gchar         *domain,
-                                            const gchar         *message,
-                                            const gchar         *trace);
+                                            const gchar         *message);
 static gchar *   gui_message_format        (GimpMessageSeverity  severity,
                                             const gchar         *domain,
                                             const gchar         *message);
@@ -88,13 +84,12 @@ gui_message (Gimp                *gimp,
              GObject             *handler,
              GimpMessageSeverity  severity,
              const gchar         *domain,
-             const gchar         *message,
-             const gchar         *trace)
+             const gchar         *message)
 {
   switch (gimp->message_handler)
     {
     case GIMP_ERROR_CONSOLE:
-      if (gui_message_error_console (gimp, severity, domain, message, trace))
+      if (gui_message_error_console (gimp, severity, domain, message))
         return;
 
       gimp->message_handler = GIMP_MESSAGE_BOX;
@@ -113,7 +108,6 @@ gui_message (Gimp                *gimp,
           data->gimp     = gimp;
           data->domain   = g_strdup (domain);
           data->message  = g_strdup (message);
-          data->trace    = trace ? g_strdup (trace) : NULL;
           data->handler  = handler? g_object_ref (handler) : NULL;
           data->severity = severity;
 
@@ -123,14 +117,14 @@ gui_message (Gimp                *gimp,
           return;
         }
       if (gui_message_error_dialog (gimp, handler, severity,
-                                    domain, message, trace))
+                                    domain, message))
         return;
 
       gimp->message_handler = GIMP_CONSOLE;
       /*  fallthru  */
 
     case GIMP_CONSOLE:
-      gui_message_console (severity, domain, message, trace);
+      gui_message_console (severity, domain, message);
       break;
     }
 }
@@ -144,18 +138,14 @@ gui_message_idle (gpointer user_data)
                                   data->handler,
                                   data->severity,
                                   data->domain,
-                                  data->message,
-                                  data->trace))
+                                  data->message))
     {
       gui_message_console (data->severity,
                            data->domain,
-                           data->message,
-                           data->trace);
+                           data->message);
     }
   g_free (data->domain);
   g_free (data->message);
-  if (data->trace)
-    g_free (data->trace);
   if (data->handler)
     g_object_unref (data->handler);
 
@@ -166,13 +156,10 @@ static gboolean
 gui_message_error_console (Gimp                *gimp,
                            GimpMessageSeverity  severity,
                            const gchar         *domain,
-                           const gchar         *message,
-                           const gchar         *trace)
+                           const gchar         *message)
 {
   GtkWidget *dockable;
 
-  /* TODO: right now the error console does nothing with the trace. */
-
   dockable = gimp_dialog_factory_find_widget (gimp_dialog_factory_get_singleton (),
                                               "gimp-error-console");
 
@@ -293,8 +280,7 @@ gui_message_error_dialog (Gimp                *gimp,
                           GObject             *handler,
                           GimpMessageSeverity  severity,
                           const gchar         *domain,
-                          const gchar         *message,
-                          const gchar         *trace)
+                          const gchar         *message)
 {
   GtkWidget      *dialog;
   GtkMessageType  type   = GTK_MESSAGE_ERROR;
@@ -306,23 +292,47 @@ gui_message_error_dialog (Gimp                *gimp,
     case GIMP_MESSAGE_ERROR:   type = GTK_MESSAGE_ERROR;   break;
     }
 
-  if (trace)
+  if (severity == GIMP_MESSAGE_ERROR)
     {
-      /* Process differently when traces are included.
+      /* Process differently errors.
        * The reason is that this will take significant place, and cannot
        * be processed as a progress message or in the global dialog. It
        * will require its own dedicated dialog which will encourage
        * people to report the bug.
        */
-      gchar *text;
-
       dialog = global_critical_dialog ();
-      text = gui_message_format (severity, domain, message);
-      gimp_critical_dialog_add (dialog, text, trace, FALSE, NULL, 0);
-      g_free (text);
-      gtk_widget_show (dialog);
 
-      return TRUE;
+      if (gimp_critical_dialog_want_errors (dialog))
+        {
+          gchar *text;
+          gchar *trace = NULL;
+
+          if (gimp_critical_dialog_want_traces (dialog))
+            gimp_print_stack_trace (NULL, NULL, &trace);
+
+          text = gui_message_format (severity, domain, message);
+          gimp_critical_dialog_add (dialog, text, trace, FALSE, NULL, 0);
+
+          gtk_widget_show (dialog);
+
+          g_free (text);
+          if (trace)
+            g_free (trace);
+
+          return TRUE;
+        }
+      else
+        {
+          /* Since we overrided glib default's WARNING and CRITICAL
+           * handler, if we decide not to handle this error in the end,
+           * let's just print it in terminal in a similar fashion as
+           * glib's default handler (though without the fancy terminal
+           * colors right now).
+           */
+          g_printerr ("%s-ERROR: %s\n", domain, message);
+
+          return TRUE;
+        }
     }
   else if (GIMP_IS_PROGRESS (handler))
     {
@@ -376,17 +386,13 @@ gui_message_error_dialog (Gimp                *gimp,
 static void
 gui_message_console (GimpMessageSeverity  severity,
                      const gchar         *domain,
-                     const gchar         *message,
-                     const gchar         *trace)
+                     const gchar         *message)
 {
   gchar *formatted_message;
 
   formatted_message = gui_message_format (severity, domain, message);
   g_printerr ("%s\n\n", formatted_message);
   g_free (formatted_message);
-
-  if (trace)
-    g_printerr ("Stack trace:\n%s\n\n", trace);
 }
 
 static gchar *
diff --git a/app/gui/gui-message.h b/app/gui/gui-message.h
index 805f266..45f4300 100644
--- a/app/gui/gui-message.h
+++ b/app/gui/gui-message.h
@@ -23,8 +23,7 @@ void gui_message (Gimp                *gimp,
                   GObject             *handler,
                   GimpMessageSeverity  severity,
                   const gchar         *domain,
-                  const gchar         *message,
-                  const gchar         *trace);
+                  const gchar         *message);
 
 
 #endif /* __GUI_VTABLE_H__ */
diff --git a/app/pdb/message-cmds.c b/app/pdb/message-cmds.c
index 7f2184e..9df0980 100644
--- a/app/pdb/message-cmds.c
+++ b/app/pdb/message-cmds.c
@@ -61,7 +61,7 @@ message_invoker (GimpProcedure         *procedure,
       if (gimp->plug_in_manager->current_plug_in)
         domain = gimp_plug_in_get_undo_desc (gimp->plug_in_manager->current_plug_in);
       gimp_show_message (gimp, G_OBJECT (progress), GIMP_MESSAGE_WARNING,
-                         domain, message, NULL);
+                         domain, message);
     }
 
   return gimp_procedure_get_return_values (procedure, success,
diff --git a/app/widgets/gimpcriticaldialog.c b/app/widgets/gimpcriticaldialog.c
index 38af5e5..5e989ca 100644
--- a/app/widgets/gimpcriticaldialog.c
+++ b/app/widgets/gimpcriticaldialog.c
@@ -54,6 +54,8 @@
 #define BUTTON1_TEXT _("Copy Bug Information")
 #define BUTTON2_TEXT _("Open Bug Tracker")
 
+#define MAX_TRACES 3
+#define MAX_ERRORS 10
 
 static void    gimp_critical_dialog_finalize (GObject     *object);
 static void    gimp_critical_dialog_response (GtkDialog   *dialog,
@@ -184,8 +186,10 @@ gimp_critical_dialog_init (GimpCriticalDialog *dialog)
   gtk_widget_show (dialog->details);
   gtk_container_add (GTK_CONTAINER (widget), dialog->details);
 
-  dialog->pid     = 0;
-  dialog->program = NULL;
+  dialog->pid      = 0;
+  dialog->program  = NULL;
+  dialog->n_errors = 0;
+  dialog->n_traces = 0;
 }
 
 static void
@@ -392,7 +396,7 @@ gimp_critical_dialog_add (GtkWidget   *dialog,
   GtkTextIter         end;
   gchar              *text;
 
-  if (! GIMP_IS_CRITICAL_DIALOG (dialog) || ! message || ! trace)
+  if (! GIMP_IS_CRITICAL_DIALOG (dialog) || ! message)
     {
       /* This is a bit hackish. We usually should use
        * g_return_if_fail(). But I don't want to end up in a critical
@@ -404,6 +408,14 @@ gimp_critical_dialog_add (GtkWidget   *dialog,
     }
   critical = GIMP_CRITICAL_DIALOG (dialog);
 
+  if (critical->n_errors > MAX_ERRORS ||
+      (trace && critical->n_traces > MAX_TRACES))
+    return;
+
+  critical->n_errors++;
+  if (trace)
+    critical->n_traces++;
+
   /* The user text, which should be localized. */
   if (is_fatal)
     {
@@ -461,7 +473,10 @@ gimp_critical_dialog_add (GtkWidget   *dialog,
    */
   buffer = gtk_text_view_get_buffer (GTK_TEXT_VIEW (critical->details));
   gtk_text_buffer_get_iter_at_offset (buffer, &end, -1);
-  text = g_strdup_printf ("\n\n> %s\n\nStack trace:\n%s", message, trace);
+  if (trace)
+    text = g_strdup_printf ("\n> %s\n\nStack trace:\n%s", message, trace);
+  else
+    text = g_strdup_printf ("\n> %s\n", message);
   gtk_text_buffer_insert (buffer, &end, text, -1);
   g_free (text);
 
@@ -477,3 +492,15 @@ gimp_critical_dialog_add (GtkWidget   *dialog,
       critical->pid     = pid;
     }
 }
+
+gboolean
+gimp_critical_dialog_want_traces (GtkWidget *dialog)
+{
+  return (GIMP_CRITICAL_DIALOG (dialog)->n_traces <= MAX_TRACES);
+}
+
+gboolean
+gimp_critical_dialog_want_errors (GtkWidget *dialog)
+{
+  return (GIMP_CRITICAL_DIALOG (dialog)->n_errors <= MAX_ERRORS);
+}
diff --git a/app/widgets/gimpcriticaldialog.h b/app/widgets/gimpcriticaldialog.h
index 7e94ea2..5418e79 100644
--- a/app/widgets/gimpcriticaldialog.h
+++ b/app/widgets/gimpcriticaldialog.h
@@ -45,6 +45,9 @@ struct _GimpCriticalDialog
 
   gchar           *program;
   gint             pid;
+
+  gint             n_errors;
+  gint             n_traces;
 };
 
 struct _GimpCriticalDialogClass
@@ -62,6 +65,8 @@ void        gimp_critical_dialog_add      (GtkWidget          *dialog,
                                            gboolean            is_fatal,
                                            const gchar        *program,
                                            gint                pid);
+gboolean    gimp_critical_dialog_want_traces (GtkWidget       *dialog);
+gboolean    gimp_critical_dialog_want_errors (GtkWidget       *dialog);
 
 
 G_END_DECLS
diff --git a/pdb/groups/message.pdb b/pdb/groups/message.pdb
index c10660f..7ccbc34 100644
--- a/pdb/groups/message.pdb
+++ b/pdb/groups/message.pdb
@@ -39,7 +39,7 @@ HELP
   if (gimp->plug_in_manager->current_plug_in)
     domain = gimp_plug_in_get_undo_desc (gimp->plug_in_manager->current_plug_in);
   gimp_show_message (gimp, G_OBJECT (progress), GIMP_MESSAGE_WARNING,
-                     domain, message, NULL);
+                     domain, message);
 }
 CODE
     );


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]