SpaceAnalyzer: Make TreeMapWidget responsible for filesystem analysis

Also, the Tree object was only ever used by the TreeMapWidget, so its
creation can happen inside `analyze()`, fixing the memory leak issue.
Plus, it doesn't have to be RefCounted.

Userland/Applications/SpaceAnalyzer/Tree.h

@@ -9,7 +9,6 @@
 #include <AK/DeprecatedString.h>
 #include <AK/Forward.h>
 #include <AK/OwnPtr.h>
-#include <AK/RefCounted.h>
 #include <AK/Vector.h>
 
 struct MountInfo {
@@ -44,16 +43,21 @@ private:
     OwnPtr<Vector<TreeNode>> m_children;
 };
 
-class Tree : public RefCounted<Tree> {
+class Tree {
 public:
-    Tree(DeprecatedString root_name)
-        : m_root(move(root_name)) {};
+    static ErrorOr<NonnullOwnPtr<Tree>> create(DeprecatedString root_name)
+    {
+        return adopt_nonnull_own_or_enomem(new (nothrow) Tree(move(root_name)));
+    }
     ~Tree() {};
+
     TreeNode& root()
     {
         return m_root;
     };
 
 private:
+    Tree(DeprecatedString root_name)
+        : m_root(move(root_name)) {};
     TreeNode m_root;
 };

Userland/Applications/SpaceAnalyzer/TreeMapWidget.cpp

@@ -1,17 +1,19 @@
 /*
  * Copyright (c) 2021-2022, the SerenityOS developers.
- * Copyright (c) 2022, Sam Atkins <atkinssj@serenityos.org>
+ * Copyright (c) 2022-2023, Sam Atkins <atkinssj@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
 #include "TreeMapWidget.h"
+#include "ProgressWindow.h"
 #include "Tree.h"
 #include <AK/Array.h>
 #include <AK/DeprecatedString.h>
 #include <AK/NumberFormat.h>
 #include <LibGUI/ConnectionToWindowServer.h>
 #include <LibGUI/Painter.h>
+#include <LibGUI/Statusbar.h>
 #include <LibGfx/Font/Font.h>
 #include <WindowServer/WindowManager.h>
 
@@ -373,15 +375,79 @@ void TreeMapWidget::recalculate_path_for_new_tree()
         m_viewpoint = new_path_length - 1;
 }
 
-void TreeMapWidget::set_tree(RefPtr<Tree> tree)
+static ErrorOr<void> fill_mounts(Vector<MountInfo>& output)
 {
-    m_tree = tree;
+    // Output info about currently mounted filesystems.
+    auto file = TRY(Core::Stream::File::open("/sys/kernel/df"sv, Core::Stream::OpenMode::Read));
+
+    auto content = TRY(file->read_until_eof());
+    auto json = TRY(JsonValue::from_string(content));
+
+    TRY(json.as_array().try_for_each([&output](JsonValue const& value) -> ErrorOr<void> {
+        auto& filesystem_object = value.as_object();
+        MountInfo mount_info;
+        mount_info.mount_point = filesystem_object.get_deprecated_string("mount_point"sv).value_or({});
+        mount_info.source = filesystem_object.get_deprecated_string("source"sv).value_or("none");
+        TRY(output.try_append(mount_info));
+        return {};
+    }));
+
+    return {};
+}
+
+ErrorOr<void> TreeMapWidget::analyze(GUI::Statusbar& statusbar)
+{
+    statusbar.set_text("");
+    auto progress_window = TRY(ProgressWindow::try_create("Space Analyzer"sv));
+    progress_window->show();
+
+    // Build an in-memory tree mirroring the filesystem and for each node
+    // calculate the sum of the file size for all its descendants.
+    auto tree = TRY(Tree::create(""));
+    Vector<MountInfo> mounts;
+    TRY(fill_mounts(mounts));
+    auto errors = tree->root().populate_filesize_tree(mounts, [&](size_t processed_file_count) {
+        progress_window->update_progress_label(processed_file_count);
+    });
+
+    progress_window->close();
+
+    // Display an error summary in the statusbar.
+    if (!errors.is_empty()) {
+        StringBuilder builder;
+        bool first = true;
+        builder.append("Some directories were not analyzed: "sv);
+        for (auto& key : errors.keys()) {
+            if (!first) {
+                builder.append(", "sv);
+            }
+            auto const* error = strerror(key);
+            builder.append({ error, strlen(error) });
+            builder.append(" ("sv);
+            int value = errors.get(key).value();
+            builder.append(DeprecatedString::number(value));
+            if (value == 1) {
+                builder.append(" time"sv);
+            } else {
+                builder.append(" times"sv);
+            }
+            builder.append(')');
+            first = false;
+        }
+        statusbar.set_text(builder.to_deprecated_string());
+    } else {
+        statusbar.set_text("No errors");
+    }
+
+    m_tree = move(tree);
     recalculate_path_for_new_tree();
 
     if (on_path_change) {
         on_path_change();
     }
     update();
+
+    return {};
 }
 
 void TreeMapWidget::set_viewpoint(size_t viewpoint)

Userland/Applications/SpaceAnalyzer/TreeMapWidget.h

@@ -24,7 +24,7 @@ public:
     TreeNode const* path_node(size_t n) const;
     size_t viewpoint() const;
     void set_viewpoint(size_t);
-    void set_tree(RefPtr<Tree> tree);
+    ErrorOr<void> analyze(GUI::Statusbar&);
 
 private:
     TreeMapWidget() = default;
@@ -53,7 +53,7 @@ private:
     Vector<DeprecatedString> path_to_position(Gfx::IntPoint);
     void recalculate_path_for_new_tree();
 
-    RefPtr<Tree> m_tree;
+    OwnPtr<Tree> m_tree;
     Vector<DeprecatedString> m_path;
     size_t m_viewpoint { 0 };
     void const* m_selected_node_cache;

Userland/Applications/SpaceAnalyzer/main.cpp

@@ -5,20 +5,13 @@
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
-#include "ProgressWindow.h"
 #include "Tree.h"
 #include "TreeMapWidget.h"
-#include <AK/Error.h>
 #include <AK/LexicalPath.h>
-#include <AK/Queue.h>
-#include <AK/QuickSort.h>
 #include <AK/String.h>
-#include <AK/StringView.h>
 #include <AK/URL.h>
 #include <Applications/SpaceAnalyzer/SpaceAnalyzerGML.h>
 #include <LibCore/File.h>
-#include <LibCore/IODevice.h>
-#include <LibCore/Stream.h>
 #include <LibDesktop/Launcher.h>
 #include <LibGUI/Application.h>
 #include <LibGUI/BoxLayout.h>
@@ -26,7 +19,6 @@
 #include <LibGUI/Clipboard.h>
 #include <LibGUI/FileIconProvider.h>
 #include <LibGUI/Icon.h>
-#include <LibGUI/Label.h>
 #include <LibGUI/Menu.h>
 #include <LibGUI/Menubar.h>
 #include <LibGUI/MessageBox.h>
@@ -36,73 +28,6 @@
 
 static constexpr auto APP_NAME = "Space Analyzer"sv;
 
-static ErrorOr<void> fill_mounts(Vector<MountInfo>& output)
-{
-    // Output info about currently mounted filesystems.
-    auto file = TRY(Core::Stream::File::open("/sys/kernel/df"sv, Core::Stream::OpenMode::Read));
-
-    auto content = TRY(file->read_until_eof());
-    auto json = TRY(JsonValue::from_string(content));
-
-    TRY(json.as_array().try_for_each([&output](JsonValue const& value) -> ErrorOr<void> {
-        auto& filesystem_object = value.as_object();
-        MountInfo mount_info;
-        mount_info.mount_point = filesystem_object.get_deprecated_string("mount_point"sv).value_or({});
-        mount_info.source = filesystem_object.get_deprecated_string("source"sv).value_or("none");
-        TRY(output.try_append(mount_info));
-        return {};
-    }));
-
-    return {};
-}
-
-static ErrorOr<void> analyze(RefPtr<Tree> tree, SpaceAnalyzer::TreeMapWidget& treemapwidget, GUI::Statusbar& statusbar)
-{
-    statusbar.set_text("");
-    auto progress_window = TRY(ProgressWindow::try_create(APP_NAME));
-    progress_window->show();
-
-    // Build an in-memory tree mirroring the filesystem and for each node
-    // calculate the sum of the file size for all its descendants.
-    Vector<MountInfo> mounts;
-    TRY(fill_mounts(mounts));
-    auto errors = tree->root().populate_filesize_tree(mounts, [&](size_t processed_file_count) {
-        progress_window->update_progress_label(processed_file_count);
-    });
-
-    progress_window->close();
-
-    // Display an error summary in the statusbar.
-    if (!errors.is_empty()) {
-        StringBuilder builder;
-        bool first = true;
-        builder.append("Some directories were not analyzed: "sv);
-        for (auto& key : errors.keys()) {
-            if (!first) {
-                builder.append(", "sv);
-            }
-            auto const* error = strerror(key);
-            builder.append({ error, strlen(error) });
-            builder.append(" ("sv);
-            int value = errors.get(key).value();
-            builder.append(DeprecatedString::number(value));
-            if (value == 1) {
-                builder.append(" time"sv);
-            } else {
-                builder.append(" times"sv);
-            }
-            builder.append(')');
-            first = false;
-        }
-        statusbar.set_text(builder.to_deprecated_string());
-    } else {
-        statusbar.set_text("No errors");
-    }
-    treemapwidget.set_tree(tree);
-
-    return {};
-}
-
 static DeprecatedString get_absolute_path_to_selected_node(SpaceAnalyzer::TreeMapWidget const& treemapwidget, bool include_last_node = true)
 {
     StringBuilder path_builder;
@@ -120,8 +45,6 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
 {
     auto app = TRY(GUI::Application::try_create(arguments));
 
-    RefPtr<Tree> tree = adopt_ref(*new Tree(""));
-
     // Configure application window.
     auto app_icon = GUI::Icon::default_icon("app-space-analyzer"sv);
     auto window = TRY(GUI::Window::try_create());
@@ -141,9 +64,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
     auto file_menu = TRY(window->try_add_menu("&File"));
     TRY(file_menu->try_add_action(GUI::Action::create("&Analyze", [&](auto&) {
         // FIXME: Just modify the tree in memory instead of traversing the entire file system
-        // FIXME: Dispose of the old tree
-        auto new_tree = adopt_ref(*new Tree(""));
-        if (auto result = analyze(new_tree, treemapwidget, statusbar); result.is_error()) {
+        if (auto result = treemapwidget.analyze(statusbar); result.is_error()) {
             GUI::MessageBox::show_error(window, DeprecatedString::formatted("{}", result.error()));
         }
     })));
@@ -197,9 +118,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
             }
         }
 
-        // FIXME: Dispose of the old tree
-        auto new_tree = adopt_ref(*new Tree(""));
-        if (auto result = analyze(new_tree, treemapwidget, statusbar); result.is_error()) {
+        if (auto result = treemapwidget.analyze(statusbar); result.is_error()) {
             GUI::MessageBox::show_error(window, DeprecatedString::formatted("{}", result.error()));
         }
     });
@@ -257,7 +176,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
     };
 
     // At startup automatically do an analysis of root.
-    TRY(analyze(tree, treemapwidget, statusbar));
+    TRY(treemapwidget.analyze(statusbar));
 
     window->show();
     return app->exec();