Prevent GtkBuilder crashes

Up until now - we've been implementing workarounds to detect XML tree
that would make GtkBuilder crash. This was very fragile and did not
support the many ways a widget can crash at init time.

Remove the workarorunds and build the XML in a separate process to
assert that it won't crash the main process.

Fixes #170

Author: sonnyp

data/app.metainfo.xml

@@ -49,8 +49,12 @@
     <release version="43.2" date="2022-12-xx">
       <description>
         <ul>
-          <li>Add support for JavaScript diagnostics</li>
+          <li>Switching between XML and Blueprint will now convert between the two</li>
+          <li>Add support for JavaScript diagnostics and linting</li>
+          <li>Optimize Blueprint preview</li>
           <li>Blueprint 0.6.0</li>
+          <li>Fix preview of non GtkBuildable</li>
+          <li>Prevent all GtkBuilder related crash</li>
         </ul>
       </description>
     </release>

src/PanelCode.js

@@ -54,7 +54,7 @@ export default function PanelCode({
     panel.language = dropdown_code_lang.selected_item.string;
     stack_code.visible_child_name = panel.language;
     previewer.useInternal();
-    previewer.update();
+    previewer.update().catch(logError);
   }
   switchLanguage();
 

src/PanelUI.js

@@ -131,9 +131,8 @@ export default function PanelUI({
   function start() {
     stop();
     lang = getLanguage(dropdown_ui_lang.selected_item.string);
-    handler_id_xml = code_view_xml.connect(
-      "changed",
-      () => code_view_xml.buffer.text,
+    handler_id_xml = code_view_xml.connect("changed", () =>
+      onXML(code_view_xml.buffer.text),
     );
     handler_id_blueprint = code_view_blueprint.connect("changed", onBlueprint);
     blueprint.lspc.connect(

src/Previewer/Previewer.js

@@ -6,13 +6,13 @@ import Gio from "gi://Gio";
 import * as xml from "../langs/xml/xml.js";
 import * as postcss from "../lib/postcss.js";
 
-import { settings } from "../util.js";
+import { settings, unstack } from "../util.js";
 
 import Internal from "./Internal.js";
 import External from "./External.js";
 import { getClassNameType } from "../overrides.js";
 
-import { assertBuildable, isPreviewable } from "./utils.js";
+import { isBuilderable, isPreviewable } from "./utils.js";
 
 // Workbench always defaults to in-process preview now if Vala is selected.
 // Workbench will switch to out-of-process preview when Vala is run
@@ -101,10 +101,10 @@ export default function Previewer({
   function start() {
     stop();
     if (handler_id_ui === null) {
-      handler_id_ui = panel_ui.connect("updated", update);
+      handler_id_ui = panel_ui.connect("updated", schedule_update);
     }
     if (handler_id_css === null) {
-      handler_id_css = code_view_css.connect("changed", update);
+      handler_id_css = code_view_css.connect("changed", schedule_update);
     }
   }
 
@@ -145,11 +145,7 @@ export default function Previewer({
   );
 
   let symbols = null;
-  function update() {
-    const builder = new Gtk.Builder();
-    const scope = new BuilderScope();
-    builder.set_scope(scope);
-
+  async function update() {
     let text = panel_ui.xml.trim();
     let target_id;
     let tree;
@@ -168,32 +164,29 @@ export default function Previewer({
 
     if (!target_id) return;
 
-    try {
-      assertBuildable(tree);
-    } catch (err) {
-      console.error(err);
-      return;
-    }
+    console.time("builderable");
+    if (!(await isBuilderable(text))) return;
+    console.timeEnd("builderable");
+
+    const builder = new Gtk.Builder();
+    const scope = new BuilderScope();
+    builder.set_scope(scope);
 
     registerSignals({ tree, scope, symbols, template });
 
+    term_console.clear();
+
     try {
-      // For some reason this log warnings twice
       builder.add_from_string(text, -1);
     } catch (err) {
-      // The following while being obviously invalid
-      // does no produce an error - so we will need to strictly validate the XML
-      // before constructing the builder
-      // prettier-xml throws but doesn't give a stack trace
-      // <style>
-      //   <class name="title-1"
-      // </style>
+      if (err instanceof GLib.MarkupError || err instanceof Gtk.BuilderError) {
+        console.warn(err.message);
+        return;
+      }
       logError(err);
       return;
     }
 
-    term_console.clear();
-
     const object_preview = builder.get_object(target_id);
     if (!object_preview) return;
 
@@ -215,6 +208,8 @@ export default function Previewer({
     symbols = null;
   }
 
+  const schedule_update = unstack(update, logError);
+
   function useExternal() {
     if (current === external) return;
     stack.set_visible_child_name("open_window");

src/Previewer/crasher.vala

@@ -0,0 +1,11 @@
+public void main (string[] args) {
+	  Adw.init ();
+
+    var builder = new Gtk.Builder();
+
+	  try {
+	    builder.add_from_string(args[1], -1);
+    // We are only interested in crashes here
+    // Previewer.js will handle errors
+	  } catch {}
+}

src/Previewer/meson.build

@@ -0,0 +1,13 @@
+executable('workbench-vala-previewer',
+  'previewer.vala',
+  dependencies: [ dependency('gio-2.0'), dependency('gmodule-2.0'), dependency('libadwaita-1') ],
+  # vala_args: [ '--gresourcesdir=' + meson.current_build_dir() ],
+  install: true,
+)
+
+executable('workbench-crasher',
+  'crasher.vala',
+  dependencies: [ dependency('gio-2.0'), dependency('gmodule-2.0'), dependency('libadwaita-1') ],
+  # vala_args: [ '--gresourcesdir=' + meson.current_build_dir() ],
+  install: true,
+)

src/Previewer/vala-previewer/previewer.vala renamed to src/Previewer/previewer.vala

@@ -1,5 +1,7 @@
 import GObject from "gi://GObject";
 import Gtk from "gi://Gtk";
+import Gio from "gi://Gio";
+import { promiseTask } from "../../troll/src/util.js";
 
 export function getObjectClass(class_name) {
   const split = class_name.split(/(?=[A-Z])/);
@@ -13,33 +15,17 @@ export function isPreviewable(class_name) {
   const klass = getObjectClass(class_name);
   if (!klass) return false;
 
-  return GObject.type_is_a(klass, Gtk.Widget);
-}
+  // GLib-GObject-ERROR: cannot create instance of abstract (non-instantiatable) type 'GtkWidget'
+  if (GObject.type_test_flags(klass, GObject.TypeFlags.ABSTRACT)) return false;
 
-// TODO: GTK Builder shouldn't crash when encountering a non buildable parent
-// https://github.com/sonnyp/Workbench/issues/49
-export function assertBuildable(el) {
-  for (const el_object of el.getChildren("object")) {
-    assertObjectBuildable(el_object);
-  }
+  return GObject.type_is_a(klass, Gtk.Widget);
 }
 
-function assertObjectBuildable(el_object) {
-  const class_name = el_object.attrs.class;
-  if (class_name) {
-    const klass = getObjectClass(class_name);
-    // GLib-GObject-ERROR: cannot create instance of abstract (non-instantiatable) type 'GtkWidget'
-    if (klass && GObject.type_test_flags(klass, GObject.TypeFlags.ABSTRACT)) {
-      throw new Error(`${class_name} is an abstract type`);
-    }
-    if (klass && !GObject.type_is_a(klass, Gtk.Buildable)) {
-      throw new Error(`${class_name} is not a GtkBuildable`);
-    }
-  }
-
-  for (const el_child of el_object.getChildren("child")) {
-    for (const el of el_child.getChildren("object")) {
-      assertObjectBuildable(el);
-    }
-  }
+export async function isBuilderable(str) {
+  const flags =
+    Gio.SubprocessFlags.STDOUT_SILENCE | Gio.SubprocessFlags.STDERR_SILENCE;
+  const proc = Gio.Subprocess.new(["workbench-crasher", str], flags);
+  await promiseTask(proc, "wait_async", "wait_finish", null);
+  if (!proc.get_if_exited()) return false;
+  return proc.get_exit_status() === 0;
 }

src/Previewer/utils.js

@@ -129,12 +129,10 @@ export default class LSPClient {
   _start_process() {
     let flags =
       Gio.SubprocessFlags.STDIN_PIPE | Gio.SubprocessFlags.STDOUT_PIPE;
-    // vala-language-server emit lots of criticals so we disable this on release
+    // vala-language-server and blueprint are very verbose
     // https://github.com/vala-lang/vala-language-server/issues/274
-    // blueprint logs everything on stderr and is also quite verbose
-    if (!__DEV__) {
-      flags = flags | Gio.SubprocessFlags.STDERR_SILENCE;
-    }
+    // comment this to debug LSP
+    flags = flags | Gio.SubprocessFlags.STDERR_SILENCE;
 
     this.proc = Gio.Subprocess.new(this.argv, flags);
     this.proc.wait_async(null, (_self, res) => {