LibJS+LibWeb: Make Console, ConsoleClient & subclasses GC-allocated

These objects had confusing ownership semantics. Let's just throw them
all on the GC heap and stop worrying about it.
This commit is contained in:
Andreas Kling 2024-04-20 21:19:51 +02:00
parent 44659f2f2a
commit 4db1712f90
Notes: sideshowbarker 2024-07-17 06:20:50 +09:00
13 changed files with 95 additions and 34 deletions

View file

@ -18,11 +18,23 @@
namespace JS { namespace JS {
JS_DEFINE_ALLOCATOR(Console);
JS_DEFINE_ALLOCATOR(ConsoleClient);
Console::Console(Realm& realm) Console::Console(Realm& realm)
: m_realm(realm) : m_realm(realm)
{ {
} }
Console::~Console() = default;
void Console::visit_edges(Visitor& visitor)
{
Base::visit_edges(visitor);
visitor.visit(m_realm);
visitor.visit(m_client);
}
// 1.1.1. assert(condition, ...data), https://console.spec.whatwg.org/#assert // 1.1.1. assert(condition, ...data), https://console.spec.whatwg.org/#assert
ThrowCompletionOr<Value> Console::assert_() ThrowCompletionOr<Value> Console::assert_()
{ {
@ -543,10 +555,23 @@ ThrowCompletionOr<String> Console::format_time_since(Core::ElapsedTimer timer)
return MUST(builder.to_string()); return MUST(builder.to_string());
} }
ConsoleClient::ConsoleClient(Console& console)
: m_console(console)
{
}
ConsoleClient::~ConsoleClient() = default;
void ConsoleClient::visit_edges(Visitor& visitor)
{
Base::visit_edges(visitor);
visitor.visit(m_console);
}
// 2.1. Logger(logLevel, args), https://console.spec.whatwg.org/#logger // 2.1. Logger(logLevel, args), https://console.spec.whatwg.org/#logger
ThrowCompletionOr<Value> ConsoleClient::logger(Console::LogLevel log_level, MarkedVector<Value> const& args) ThrowCompletionOr<Value> ConsoleClient::logger(Console::LogLevel log_level, MarkedVector<Value> const& args)
{ {
auto& vm = m_console.realm().vm(); auto& vm = m_console->realm().vm();
// 1. If args is empty, return. // 1. If args is empty, return.
if (args.is_empty()) if (args.is_empty())
@ -578,7 +603,7 @@ ThrowCompletionOr<Value> ConsoleClient::logger(Console::LogLevel log_level, Mark
// 2.2. Formatter(args), https://console.spec.whatwg.org/#formatter // 2.2. Formatter(args), https://console.spec.whatwg.org/#formatter
ThrowCompletionOr<MarkedVector<Value>> ConsoleClient::formatter(MarkedVector<Value> const& args) ThrowCompletionOr<MarkedVector<Value>> ConsoleClient::formatter(MarkedVector<Value> const& args)
{ {
auto& realm = m_console.realm(); auto& realm = m_console->realm();
auto& vm = realm.vm(); auto& vm = realm.vm();
// 1. If argss size is 1, return args. // 1. If argss size is 1, return args.
@ -691,7 +716,7 @@ ThrowCompletionOr<MarkedVector<Value>> ConsoleClient::formatter(MarkedVector<Val
ThrowCompletionOr<String> ConsoleClient::generically_format_values(MarkedVector<Value> const& values) ThrowCompletionOr<String> ConsoleClient::generically_format_values(MarkedVector<Value> const& values)
{ {
AllocatingMemoryStream stream; AllocatingMemoryStream stream;
auto& vm = m_console.realm().vm(); auto& vm = m_console->realm().vm();
PrintContext ctx { vm, stream, true }; PrintContext ctx { vm, stream, true };
bool first = true; bool first = true;
for (auto const& value : values) { for (auto const& value : values) {

View file

@ -14,6 +14,8 @@
#include <AK/Vector.h> #include <AK/Vector.h>
#include <LibCore/ElapsedTimer.h> #include <LibCore/ElapsedTimer.h>
#include <LibJS/Forward.h> #include <LibJS/Forward.h>
#include <LibJS/Heap/Cell.h>
#include <LibJS/Heap/CellAllocator.h>
#include <LibJS/Runtime/Value.h> #include <LibJS/Runtime/Value.h>
namespace JS { namespace JS {
@ -21,11 +23,13 @@ namespace JS {
class ConsoleClient; class ConsoleClient;
// https://console.spec.whatwg.org // https://console.spec.whatwg.org
class Console { class Console : public Cell {
AK_MAKE_NONCOPYABLE(Console); JS_CELL(Console, Cell);
AK_MAKE_NONMOVABLE(Console); JS_DECLARE_ALLOCATOR(Console);
public: public:
virtual ~Console() override;
// These are not really levels, but that's the term used in the spec. // These are not really levels, but that's the term used in the spec.
enum class LogLevel { enum class LogLevel {
Assert, Assert,
@ -54,8 +58,6 @@ public:
Vector<String> stack; Vector<String> stack;
}; };
explicit Console(Realm&);
void set_client(ConsoleClient& client) { m_client = &client; } void set_client(ConsoleClient& client) { m_client = &client; }
Realm& realm() const { return m_realm; } Realm& realm() const { return m_realm; }
@ -87,24 +89,26 @@ public:
void report_exception(JS::Error const&, bool) const; void report_exception(JS::Error const&, bool) const;
private: private:
explicit Console(Realm&);
virtual void visit_edges(Visitor&) override;
ThrowCompletionOr<String> value_vector_to_string(MarkedVector<Value> const&); ThrowCompletionOr<String> value_vector_to_string(MarkedVector<Value> const&);
ThrowCompletionOr<String> format_time_since(Core::ElapsedTimer timer); ThrowCompletionOr<String> format_time_since(Core::ElapsedTimer timer);
NonnullGCPtr<Realm> m_realm; NonnullGCPtr<Realm> m_realm;
ConsoleClient* m_client { nullptr }; GCPtr<ConsoleClient> m_client;
HashMap<String, unsigned> m_counters; HashMap<String, unsigned> m_counters;
HashMap<String, Core::ElapsedTimer> m_timer_table; HashMap<String, Core::ElapsedTimer> m_timer_table;
Vector<Group> m_group_stack; Vector<Group> m_group_stack;
}; };
class ConsoleClient { class ConsoleClient : public Cell {
public: JS_CELL(ConsoleClient, Cell);
explicit ConsoleClient(Console& console) JS_DECLARE_ALLOCATOR(ConsoleClient);
: m_console(console)
{
}
public:
using PrinterArguments = Variant<Console::Group, Console::Trace, MarkedVector<Value>>; using PrinterArguments = Variant<Console::Group, Console::Trace, MarkedVector<Value>>;
ThrowCompletionOr<Value> logger(Console::LogLevel log_level, MarkedVector<Value> const& args); ThrowCompletionOr<Value> logger(Console::LogLevel log_level, MarkedVector<Value> const& args);
@ -120,9 +124,11 @@ public:
ThrowCompletionOr<String> generically_format_values(MarkedVector<Value> const&); ThrowCompletionOr<String> generically_format_values(MarkedVector<Value> const&);
protected: protected:
virtual ~ConsoleClient() = default; explicit ConsoleClient(Console&);
virtual ~ConsoleClient() override;
virtual void visit_edges(Visitor& visitor) override;
Console& m_console; NonnullGCPtr<Console> m_console;
}; };
} }

View file

@ -16,14 +16,22 @@ JS_DEFINE_ALLOCATOR(ConsoleObject);
ConsoleObject::ConsoleObject(Realm& realm) ConsoleObject::ConsoleObject(Realm& realm)
: Object(ConstructWithPrototypeTag::Tag, realm.intrinsics().object_prototype()) : Object(ConstructWithPrototypeTag::Tag, realm.intrinsics().object_prototype())
, m_console(make<Console>(realm))
{ {
} }
ConsoleObject::~ConsoleObject() = default;
void ConsoleObject::visit_edges(Visitor& visitor)
{
Base::visit_edges(visitor);
visitor.visit(m_console);
}
void ConsoleObject::initialize(Realm& realm) void ConsoleObject::initialize(Realm& realm)
{ {
auto& vm = this->vm(); auto& vm = this->vm();
Base::initialize(realm); Base::initialize(realm);
m_console = vm.heap().allocate<Console>(realm, realm);
u8 attr = Attribute::Writable | Attribute::Enumerable | Attribute::Configurable; u8 attr = Attribute::Writable | Attribute::Enumerable | Attribute::Configurable;
define_native_function(realm, vm.names.assert, assert_, 0, attr); define_native_function(realm, vm.names.assert, assert_, 0, attr);
define_native_function(realm, vm.names.clear, clear, 0, attr); define_native_function(realm, vm.names.clear, clear, 0, attr);

View file

@ -16,12 +16,13 @@ class ConsoleObject final : public Object {
public: public:
virtual void initialize(Realm&) override; virtual void initialize(Realm&) override;
virtual ~ConsoleObject() override = default; virtual ~ConsoleObject() override;
Console& console() { return *m_console; } Console& console() { return *m_console; }
private: private:
explicit ConsoleObject(Realm&); explicit ConsoleObject(Realm&);
virtual void visit_edges(Visitor&) override;
JS_DECLARE_NATIVE_FUNCTION(assert_); JS_DECLARE_NATIVE_FUNCTION(assert_);
JS_DECLARE_NATIVE_FUNCTION(clear); JS_DECLARE_NATIVE_FUNCTION(clear);
@ -41,7 +42,7 @@ private:
JS_DECLARE_NATIVE_FUNCTION(time_log); JS_DECLARE_NATIVE_FUNCTION(time_log);
JS_DECLARE_NATIVE_FUNCTION(time_end); JS_DECLARE_NATIVE_FUNCTION(time_end);
NonnullOwnPtr<Console> m_console; GCPtr<Console> m_console;
}; };
} }

View file

@ -13,6 +13,8 @@
namespace Web::HTML { namespace Web::HTML {
JS_DEFINE_ALLOCATOR(WorkerDebugConsoleClient);
WorkerDebugConsoleClient::WorkerDebugConsoleClient(JS::Console& console) WorkerDebugConsoleClient::WorkerDebugConsoleClient(JS::Console& console)
: ConsoleClient(console) : ConsoleClient(console)
{ {
@ -34,7 +36,7 @@ void WorkerDebugConsoleClient::end_group()
// 2.3. Printer(logLevel, args[, options]), https://console.spec.whatwg.org/#printer // 2.3. Printer(logLevel, args[, options]), https://console.spec.whatwg.org/#printer
JS::ThrowCompletionOr<JS::Value> WorkerDebugConsoleClient::printer(JS::Console::LogLevel log_level, PrinterArguments arguments) JS::ThrowCompletionOr<JS::Value> WorkerDebugConsoleClient::printer(JS::Console::LogLevel log_level, PrinterArguments arguments)
{ {
auto& vm = m_console.realm().vm(); auto& vm = m_console->realm().vm();
auto indent = TRY_OR_THROW_OOM(vm, String::repeated(' ', m_group_stack_depth * 2)); auto indent = TRY_OR_THROW_OOM(vm, String::repeated(' ', m_group_stack_depth * 2));
@ -59,7 +61,7 @@ JS::ThrowCompletionOr<JS::Value> WorkerDebugConsoleClient::printer(JS::Console::
} }
auto output = TRY(generically_format_values(arguments.get<JS::MarkedVector<JS::Value>>())); auto output = TRY(generically_format_values(arguments.get<JS::MarkedVector<JS::Value>>()));
m_console.output_debug_message(log_level, output); m_console->output_debug_message(log_level, output);
switch (log_level) { switch (log_level) {
case JS::Console::LogLevel::Debug: case JS::Console::LogLevel::Debug:

View file

@ -16,16 +16,18 @@ namespace Web::HTML {
class WorkerDebugConsoleClient final class WorkerDebugConsoleClient final
: public JS::ConsoleClient : public JS::ConsoleClient
, public RefCounted<WorkerDebugConsoleClient>
, public Weakable<WorkerDebugConsoleClient> { , public Weakable<WorkerDebugConsoleClient> {
public: JS_CELL(WorkerDebugConsoleClient, JS::ConsoleClient);
WorkerDebugConsoleClient(JS::Console& console); JS_DECLARE_ALLOCATOR(WorkerDebugConsoleClient);
public:
virtual void clear() override; virtual void clear() override;
virtual void end_group() override; virtual void end_group() override;
virtual JS::ThrowCompletionOr<JS::Value> printer(JS::Console::LogLevel log_level, PrinterArguments arguments) override; virtual JS::ThrowCompletionOr<JS::Value> printer(JS::Console::LogLevel log_level, PrinterArguments arguments) override;
private: private:
WorkerDebugConsoleClient(JS::Console& console);
int m_group_stack_depth { 0 }; int m_group_stack_depth { 0 };
}; };

View file

@ -680,10 +680,10 @@ void PageClient::initialize_js_console(Web::DOM::Document& document)
{ {
auto& realm = document.realm(); auto& realm = document.realm();
auto console_object = realm.intrinsics().console_object(); auto console_object = realm.intrinsics().console_object();
auto console_client = make<WebContentConsoleClient>(console_object->console(), document.realm(), *this); auto console_client = heap().allocate_without_realm<WebContentConsoleClient>(console_object->console(), document.realm(), *this);
console_object->console().set_client(*console_client); console_object->console().set_client(*console_client);
m_console_clients.set(document, move(console_client)); m_console_clients.set(document, console_client);
} }
void PageClient::destroy_js_console(Web::DOM::Document& document) void PageClient::destroy_js_console(Web::DOM::Document& document)

View file

@ -196,7 +196,7 @@ private:
BackingStores m_backing_stores; BackingStores m_backing_stores;
// NOTE: These documents are not visited, but manually removed from the map on document finalization. // NOTE: These documents are not visited, but manually removed from the map on document finalization.
HashMap<JS::RawGCPtr<Web::DOM::Document>, NonnullOwnPtr<WebContentConsoleClient>> m_console_clients; HashMap<JS::RawGCPtr<Web::DOM::Document>, JS::NonnullGCPtr<WebContentConsoleClient>> m_console_clients;
WeakPtr<WebContentConsoleClient> m_top_level_document_console_client; WeakPtr<WebContentConsoleClient> m_top_level_document_console_client;
JS::Handle<JS::GlobalObject> m_console_global_object; JS::Handle<JS::GlobalObject> m_console_global_object;

View file

@ -24,6 +24,8 @@
namespace WebContent { namespace WebContent {
JS_DEFINE_ALLOCATOR(WebContentConsoleClient);
WebContentConsoleClient::WebContentConsoleClient(JS::Console& console, JS::Realm& realm, PageClient& client) WebContentConsoleClient::WebContentConsoleClient(JS::Console& console, JS::Realm& realm, PageClient& client)
: ConsoleClient(console) : ConsoleClient(console)
, m_client(client) , m_client(client)
@ -32,6 +34,15 @@ WebContentConsoleClient::WebContentConsoleClient(JS::Console& console, JS::Realm
m_console_global_environment_extensions = realm.heap().allocate<ConsoleGlobalEnvironmentExtensions>(realm, realm, window); m_console_global_environment_extensions = realm.heap().allocate<ConsoleGlobalEnvironmentExtensions>(realm, realm, window);
} }
WebContentConsoleClient::~WebContentConsoleClient() = default;
void WebContentConsoleClient::visit_edges(JS::Cell::Visitor& visitor)
{
Base::visit_edges(visitor);
visitor.visit(m_client);
visitor.visit(m_console_global_environment_extensions);
}
void WebContentConsoleClient::handle_input(ByteString const& js_source) void WebContentConsoleClient::handle_input(ByteString const& js_source)
{ {
if (!m_console_global_environment_extensions) if (!m_console_global_environment_extensions)
@ -158,7 +169,7 @@ JS::ThrowCompletionOr<JS::Value> WebContentConsoleClient::printer(JS::Console::L
} }
auto output = TRY(generically_format_values(arguments.get<JS::MarkedVector<JS::Value>>())); auto output = TRY(generically_format_values(arguments.get<JS::MarkedVector<JS::Value>>()));
m_console.output_debug_message(log_level, output); m_console->output_debug_message(log_level, output);
StringBuilder html; StringBuilder html;
switch (log_level) { switch (log_level) {

View file

@ -20,14 +20,20 @@ namespace WebContent {
class WebContentConsoleClient final : public JS::ConsoleClient class WebContentConsoleClient final : public JS::ConsoleClient
, public Weakable<WebContentConsoleClient> { , public Weakable<WebContentConsoleClient> {
JS_CELL(WebContentConsoleClient, JS::ConsoleClient);
JS_DECLARE_ALLOCATOR(WebContentConsoleClient);
public: public:
WebContentConsoleClient(JS::Console&, JS::Realm&, PageClient&); virtual ~WebContentConsoleClient() override;
void handle_input(ByteString const& js_source); void handle_input(ByteString const& js_source);
void send_messages(i32 start_index); void send_messages(i32 start_index);
void report_exception(JS::Error const&, bool) override; void report_exception(JS::Error const&, bool) override;
private: private:
WebContentConsoleClient(JS::Console&, JS::Realm&, PageClient&);
virtual void visit_edges(JS::Cell::Visitor&) override;
virtual void clear() override; virtual void clear() override;
virtual JS::ThrowCompletionOr<JS::Value> printer(JS::Console::LogLevel log_level, PrinterArguments) override; virtual JS::ThrowCompletionOr<JS::Value> printer(JS::Console::LogLevel log_level, PrinterArguments) override;
@ -38,7 +44,7 @@ private:
} }
JS::NonnullGCPtr<PageClient> m_client; JS::NonnullGCPtr<PageClient> m_client;
JS::Handle<ConsoleGlobalEnvironmentExtensions> m_console_global_environment_extensions; JS::GCPtr<ConsoleGlobalEnvironmentExtensions> m_console_global_environment_extensions;
void clear_output(); void clear_output();
void print_html(ByteString const& line); void print_html(ByteString const& line);

View file

@ -55,7 +55,7 @@ void DedicatedWorkerHost::run(JS::NonnullGCPtr<Web::Page> page, Web::HTML::Trans
auto inner_settings = Web::HTML::WorkerEnvironmentSettingsObject::setup(page, move(realm_execution_context)); auto inner_settings = Web::HTML::WorkerEnvironmentSettingsObject::setup(page, move(realm_execution_context));
auto& console_object = *inner_settings->realm().intrinsics().console_object(); auto& console_object = *inner_settings->realm().intrinsics().console_object();
m_console = adopt_ref(*new Web::HTML::WorkerDebugConsoleClient(console_object.console())); m_console = console_object.heap().allocate_without_realm<Web::HTML::WorkerDebugConsoleClient>(console_object.console());
VERIFY(m_console); VERIFY(m_console);
console_object.console().set_client(*m_console); console_object.console().set_client(*m_console);

View file

@ -23,7 +23,7 @@ public:
void run(JS::NonnullGCPtr<Web::Page>, Web::HTML::TransferDataHolder message_port_data, Web::HTML::SerializedEnvironmentSettingsObject const&); void run(JS::NonnullGCPtr<Web::Page>, Web::HTML::TransferDataHolder message_port_data, Web::HTML::SerializedEnvironmentSettingsObject const&);
private: private:
RefPtr<Web::HTML::WorkerDebugConsoleClient> m_console; JS::Handle<Web::HTML::WorkerDebugConsoleClient> m_console;
URL::URL m_url; URL::URL m_url;
String m_type; String m_type;

View file

@ -497,7 +497,7 @@ public:
auto output = TRY(generically_format_values(arguments.get<JS::MarkedVector<JS::Value>>())); auto output = TRY(generically_format_values(arguments.get<JS::MarkedVector<JS::Value>>()));
#ifdef AK_OS_SERENITY #ifdef AK_OS_SERENITY
m_console.output_debug_message(log_level, output); m_console->output_debug_message(log_level, output);
#endif #endif
switch (log_level) { switch (log_level) {