Browse Source

LibWeb: Use JS::HeapFunction for DocumentObserver callbacks

If GC-allocated object wants to own a function it should use
HeapFunction because using SafeFunction will almost always lead to a
leak.
Aliaksandr Kalenik 1 year ago
parent
commit
cad2d2c85b

+ 12 - 0
Userland/Libraries/LibWeb/DOM/DocumentObserver.cpp

@@ -21,6 +21,8 @@ void DocumentObserver::visit_edges(Cell::Visitor& visitor)
 {
     Base::visit_edges(visitor);
     visitor.visit(m_document);
+    visitor.visit(m_document_became_inactive);
+    visitor.visit(m_document_completely_loaded);
 }
 
 void DocumentObserver::finalize()
@@ -29,4 +31,14 @@ void DocumentObserver::finalize()
     m_document->unregister_document_observer({}, *this);
 }
 
+void DocumentObserver::set_document_became_inactive(Function<void()> callback)
+{
+    m_document_became_inactive = JS::create_heap_function(vm().heap(), move(callback));
+}
+
+void DocumentObserver::set_document_completely_loaded(Function<void()> callback)
+{
+    m_document_completely_loaded = JS::create_heap_function(vm().heap(), move(callback));
+}
+
 }

+ 8 - 2
Userland/Libraries/LibWeb/DOM/DocumentObserver.h

@@ -7,6 +7,7 @@
 #pragma once
 
 #include <LibJS/Forward.h>
+#include <LibJS/Heap/HeapFunction.h>
 #include <LibJS/SafeFunction.h>
 #include <LibWeb/Bindings/PlatformObject.h>
 #include <LibWeb/Forward.h>
@@ -17,8 +18,11 @@ class DocumentObserver final : public Bindings::PlatformObject {
     WEB_PLATFORM_OBJECT(DocumentObserver, Bindings::PlatformObject);
 
 public:
-    JS::SafeFunction<void()> document_became_inactive;
-    JS::SafeFunction<void()> document_completely_loaded;
+    [[nodiscard]] JS::GCPtr<JS::HeapFunction<void()>> document_became_inactive() const { return m_document_became_inactive; }
+    void set_document_became_inactive(Function<void()>);
+
+    [[nodiscard]] JS::GCPtr<JS::HeapFunction<void()>> document_completely_loaded() const { return m_document_completely_loaded; }
+    void set_document_completely_loaded(Function<void()>);
 
 private:
     explicit DocumentObserver(JS::Realm&, DOM::Document&);
@@ -27,6 +31,8 @@ private:
     virtual void finalize() override;
 
     JS::NonnullGCPtr<DOM::Document> m_document;
+    JS::GCPtr<JS::HeapFunction<void()>> m_document_became_inactive;
+    JS::GCPtr<JS::HeapFunction<void()>> m_document_completely_loaded;
 };
 
 }

+ 2 - 2
Userland/Libraries/LibWeb/HTML/HTMLMediaElement.cpp

@@ -57,11 +57,11 @@ void HTMLMediaElement::initialize(JS::Realm& realm)
     m_document_observer = realm.heap().allocate<DOM::DocumentObserver>(realm, realm, document());
 
     // https://html.spec.whatwg.org/multipage/media.html#playing-the-media-resource:media-element-82
-    m_document_observer->document_became_inactive = [this]() {
+    m_document_observer->set_document_became_inactive([this]() {
         // If the media element's node document stops being a fully active document, then the playback will stop until
         // the document is active again.
         pause_element().release_value_but_fixme_should_propagate_errors();
-    };
+    });
 }
 
 // https://html.spec.whatwg.org/multipage/media.html#queue-a-media-element-task

+ 2 - 2
Userland/Libraries/LibWeb/SVG/SVGUseElement.cpp

@@ -34,9 +34,9 @@ void SVGUseElement::initialize(JS::Realm& realm)
     set_shadow_root(shadow_root);
 
     m_document_observer = realm.heap().allocate<DOM::DocumentObserver>(realm, realm, document());
-    m_document_observer->document_completely_loaded = [this]() {
+    m_document_observer->set_document_completely_loaded([this]() {
         clone_element_tree_as_our_shadow_tree(referenced_element());
-    };
+    });
 }
 
 void SVGUseElement::visit_edges(Cell::Visitor& visitor)