CrashReporter: Move progressbar into main window

Previously we would create a temporary progress window to show a
progressbar while the coredump is processed. Since we're only waiting
on backtraces and CPU register states, we can move the progressbar
into the main window and show everything else immediately while the
slow parts are generated in a BackgroundAction.

2 files changed, 85 insertions, 101 deletions

diff --git a/Userland/Applications/CrashReporter/CrashReporterWindow.gml b/Userland/Applications/CrashReporter/CrashReporterWindow.gml
index 115367bc3c..529c390bda 100644
--- a/Userland/Applications/CrashReporter/CrashReporterWindow.gml
+++ b/Userland/Applications/CrashReporter/CrashReporterWindow.gml
@@ -73,8 +73,14 @@
         }
     }
 
+    @GUI::Progressbar {
+        name: "progressbar"
+        text: "Generating crash report: "
+    }
+
     @GUI::TabWidget {
         name: "tab_widget"
+        visible: false
     }
 
     @GUI::Widget {
diff --git a/Userland/Applications/CrashReporter/main.cpp b/Userland/Applications/CrashReporter/main.cpp
index a2dcc7dfbe..89d73a87bc 100644
--- a/Userland/Applications/CrashReporter/main.cpp
+++ b/Userland/Applications/CrashReporter/main.cpp
@@ -34,6 +34,7 @@
 #include <LibGUI/Widget.h>
 #include <LibGUI/Window.h>
 #include <LibMain/Main.h>
+#include <LibThreading/BackgroundAction.h>
 #include <string.h>
 #include <unistd.h>
 
@@ -42,52 +43,15 @@ struct TitleAndText {
     String text;
 };
 
-static NonnullRefPtr<GUI::Window> create_progress_window()
-{
-    auto window = GUI::Window::construct();
-
-    window->set_title("CrashReporter");
-    window->set_resizable(false);
-    window->resize(240, 64);
-    window->center_on_screen();
-
-    auto& main_widget = window->set_main_widget<GUI::Widget>();
-    main_widget.set_fill_with_background_color(true);
-    main_widget.set_layout<GUI::VerticalBoxLayout>();
-
-    auto& label = main_widget.add<GUI::Label>("Generating crash report...");
-    label.set_fixed_height(30);
-
-    auto& progressbar = main_widget.add<GUI::Progressbar>();
-    progressbar.set_name("progressbar");
-    progressbar.set_fixed_width(150);
-    progressbar.set_fixed_height(22);
-
-    window->on_close = [&]() {
-        if (progressbar.value() != progressbar.max())
-            exit(0);
-    };
-
-    return window;
-}
+struct ThreadBacktracesAndCpuRegisters {
+    Vector<TitleAndText> thread_backtraces;
+    Vector<TitleAndText> thread_cpu_registers;
+};
 
-static TitleAndText build_backtrace(Coredump::Reader const& coredump, ELF::Core::ThreadInfo const& thread_info, size_t thread_index)
+static TitleAndText build_backtrace(Coredump::Reader const& coredump, ELF::Core::ThreadInfo const& thread_info, size_t thread_index, Function<void(size_t, size_t)> on_progress)
 {
-    // Show a very simple progress window ASAP to make crashing feel more responsive.
-    // FIXME: This is not the most beautifully factored thing.
-    auto progress_window = create_progress_window();
-    progress_window->show();
-
-    auto& progressbar = *progress_window->main_widget()->find_descendant_of_type_named<GUI::Progressbar>("progressbar");
-
     auto timer = Core::ElapsedTimer::start_new();
-    Coredump::Backtrace backtrace(coredump, thread_info, [&](size_t frame_index, size_t frame_count) {
-        progress_window->set_progress(100.0f * (float)(frame_index + 1) / (float)frame_count);
-        progressbar.set_value(frame_index + 1);
-        progressbar.set_max(frame_count);
-        Core::EventLoop::current().pump(Core::EventLoop::WaitMode::PollForEvents);
-    });
-    progress_window->close();
+    Coredump::Backtrace backtrace(coredump, thread_info, move(on_progress));
 
     auto metadata = coredump.metadata();
 
@@ -168,7 +132,7 @@ static void unlink_coredump(StringView const& coredump_path)
 
 ErrorOr<int> serenity_main(Main::Arguments arguments)
 {
-    TRY(Core::System::pledge("stdio recvfd sendfd cpath rpath unix proc exec"));
+    TRY(Core::System::pledge("stdio recvfd sendfd cpath rpath unix proc exec thread"));
 
     auto app = TRY(GUI::Application::try_create(arguments));
 
@@ -181,51 +145,36 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
     args_parser.add_option(unlink_on_exit, "Delete the coredump after its parsed", "unlink", 0);
     args_parser.parse(arguments);
 
-    Vector<TitleAndText> thread_backtraces;
-    Vector<TitleAndText> thread_cpu_registers;
+    auto coredump = Coredump::Reader::create(coredump_path);
+    if (!coredump) {
+        warnln("Could not open coredump '{}'", coredump_path);
+        return 1;
+    }
 
-    String executable_path;
-    Vector<String> crashed_process_arguments;
-    Vector<String> environment;
     Vector<String> memory_regions;
-    int pid { 0 };
-    u8 termination_signal { 0 };
-
-    {
-        auto coredump = Coredump::Reader::create(coredump_path);
-        if (!coredump) {
-            warnln("Could not open coredump '{}'", coredump_path);
-            return 1;
-        }
-
-        size_t thread_index = 0;
-        coredump->for_each_thread_info([&](auto& thread_info) {
-            thread_backtraces.append(build_backtrace(*coredump, thread_info, thread_index));
-            thread_cpu_registers.append(build_cpu_registers(thread_info, thread_index));
-            ++thread_index;
-            return IterationDecision::Continue;
-        });
-
-        coredump->for_each_memory_region_info([&](auto& memory_region_info) {
-            memory_regions.append(String::formatted("{:p} - {:p}: {}", memory_region_info.region_start, memory_region_info.region_end, (const char*)memory_region_info.region_name));
-            return IterationDecision::Continue;
-        });
-
-        executable_path = coredump->process_executable_path();
-        crashed_process_arguments = coredump->process_arguments();
-        environment = coredump->process_environment();
-        pid = coredump->process_pid();
-        termination_signal = coredump->process_termination_signal();
-    }
+    coredump->for_each_memory_region_info([&](auto& memory_region_info) {
+        memory_regions.append(String::formatted("{:p} - {:p}: {}", memory_region_info.region_start, memory_region_info.region_end, (const char*)memory_region_info.region_name));
+        return IterationDecision::Continue;
+    });
 
-    TRY(Core::System::unveil(executable_path.characters(), "r"));
-    TRY(Core::System::unveil("/res", "r"));
-    TRY(Core::System::unveil("/tmp/portal/launch", "rw"));
+    auto executable_path = coredump->process_executable_path();
+    auto crashed_process_arguments = coredump->process_arguments();
+    auto environment = coredump->process_environment();
+    auto pid = coredump->process_pid();
+    auto termination_signal = coredump->process_termination_signal();
 
     if (unlink_on_exit)
         TRY(Core::System::unveil(coredump_path, "c"));
-
+    TRY(Core::System::unveil(executable_path.characters(), "r"));
     TRY(Core::System::unveil("/bin/HackStudio", "rx"));
+    TRY(Core::System::unveil("/res", "r"));
+    TRY(Core::System::unveil("/tmp/portal/launch", "rw"));
+    TRY(Core::System::unveil("/usr/lib", "r"));
+    coredump->for_each_library([](auto library_info) {
+        // FIXME: Make for_each_library propagate ErrorOr values so we can use TRY.
+        if (library_info.path.starts_with('/'))
+            MUST(Core::System::unveil(library_info.path, "r"));
+    });
     TRY(Core::System::unveil(nullptr, nullptr));
 
     auto app_icon = GUI::Icon::default_icon("app-crash-reporter");
@@ -233,7 +182,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
     auto window = TRY(GUI::Window::try_create());
     window->set_title("Crash Reporter");
     window->set_icon(app_icon.bitmap_for_size(16));
-    window->resize(460, 340);
+    window->resize(460, 190);
     window->center_on_screen();
     window->on_close = [unlink_on_exit, &coredump_path]() {
         if (unlink_on_exit)
@@ -271,6 +220,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
     auto& arguments_label = *widget->find_descendant_of_type_named<GUI::Label>("arguments_label");
     arguments_label.set_text(String::join(" ", crashed_process_arguments));
 
+    auto& progressbar = *widget->find_descendant_of_type_named<GUI::Progressbar>("progressbar");
     auto& tab_widget = *widget->find_descendant_of_type_named<GUI::TabWidget>("tab_widget");
 
     auto backtrace_tab = TRY(tab_widget.try_add_tab<GUI::Widget>("Backtrace"));
@@ -283,15 +233,6 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
 
     auto backtrace_tab_widget = TRY(backtrace_tab->try_add<GUI::TabWidget>());
     backtrace_tab_widget->set_tab_position(GUI::TabWidget::TabPosition::Bottom);
-    for (auto& backtrace : thread_backtraces) {
-        auto container = TRY(backtrace_tab_widget->try_add_tab<GUI::Widget>(backtrace.title));
-        (void)TRY(container->try_set_layout<GUI::VerticalBoxLayout>());
-        container->layout()->set_margins(4);
-        auto backtrace_text_editor = TRY(container->try_add<GUI::TextEditor>());
-        backtrace_text_editor->set_text(backtrace.text);
-        backtrace_text_editor->set_mode(GUI::TextEditor::Mode::ReadOnly);
-        backtrace_text_editor->set_should_hide_unnecessary_scrollbars(true);
-    }
 
     auto cpu_registers_tab = TRY(tab_widget.try_add_tab<GUI::Widget>("CPU Registers"));
     cpu_registers_tab->set_layout<GUI::VerticalBoxLayout>();
@@ -303,15 +244,6 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
 
     auto cpu_registers_tab_widget = TRY(cpu_registers_tab->try_add<GUI::TabWidget>());
     cpu_registers_tab_widget->set_tab_position(GUI::TabWidget::TabPosition::Bottom);
-    for (auto& cpu_registers : thread_cpu_registers) {
-        auto container = TRY(cpu_registers_tab_widget->try_add_tab<GUI::Widget>(cpu_registers.title));
-        (void)TRY(container->try_set_layout<GUI::VerticalBoxLayout>());
-        container->layout()->set_margins(4);
-        auto cpu_registers_text_editor = TRY(container->try_add<GUI::TextEditor>());
-        cpu_registers_text_editor->set_text(cpu_registers.text);
-        cpu_registers_text_editor->set_mode(GUI::TextEditor::Mode::ReadOnly);
-        cpu_registers_text_editor->set_should_hide_unnecessary_scrollbars(true);
-    }
 
     auto environment_tab = TRY(tab_widget.try_add_tab<GUI::Widget>("Environment"));
     (void)TRY(environment_tab->try_set_layout<GUI::VerticalBoxLayout>());
@@ -352,6 +284,52 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
         }
     };
 
+    (void)Threading::BackgroundAction<ThreadBacktracesAndCpuRegisters>::construct(
+        [&, coredump = move(coredump)](auto&) {
+            ThreadBacktracesAndCpuRegisters results;
+            size_t thread_index = 0;
+            coredump->for_each_thread_info([&](auto& thread_info) {
+                results.thread_backtraces.append(build_backtrace(*coredump, thread_info, thread_index, [&](size_t frame_index, size_t frame_count) {
+                    window->set_progress(100.0f * (float)(frame_index + 1) / (float)frame_count);
+                    progressbar.set_value(frame_index + 1);
+                    progressbar.set_max(frame_count);
+                    Core::EventLoop::wake();
+                }));
+                results.thread_cpu_registers.append(build_cpu_registers(thread_info, thread_index));
+                ++thread_index;
+                return IterationDecision::Continue;
+            });
+            return results;
+        },
+        [&](auto results) {
+            // FIXME: Make BackgroundAction propagate ErrorOr values so we can replace these MUSTs with TRYs.
+
+            for (auto& backtrace : results.thread_backtraces) {
+                auto container = MUST(backtrace_tab_widget->try_add_tab<GUI::Widget>(backtrace.title));
+                (void)MUST(container->template try_set_layout<GUI::VerticalBoxLayout>());
+                container->layout()->set_margins(4);
+                auto backtrace_text_editor = MUST(container->template try_add<GUI::TextEditor>());
+                backtrace_text_editor->set_text(backtrace.text);
+                backtrace_text_editor->set_mode(GUI::TextEditor::Mode::ReadOnly);
+                backtrace_text_editor->set_should_hide_unnecessary_scrollbars(true);
+            }
+
+            for (auto& cpu_registers : results.thread_cpu_registers) {
+                auto container = MUST(cpu_registers_tab_widget->try_add_tab<GUI::Widget>(cpu_registers.title));
+                (void)MUST(container->template try_set_layout<GUI::VerticalBoxLayout>());
+                container->layout()->set_margins(4);
+                auto cpu_registers_text_editor = MUST(container->template try_add<GUI::TextEditor>());
+                cpu_registers_text_editor->set_text(cpu_registers.text);
+                cpu_registers_text_editor->set_mode(GUI::TextEditor::Mode::ReadOnly);
+                cpu_registers_text_editor->set_should_hide_unnecessary_scrollbars(true);
+            }
+
+            progressbar.set_visible(false);
+            tab_widget.set_visible(true);
+            window->resize(window->width(), max(340, window->height()));
+            window->set_progress(0);
+        });
+
     window->show();
 
     return app->exec();