Browse Source

LibWeb: Change DOM::Position to be GC-allocated

Aliaksandr Kalenik 1 năm trước cách đây
mục cha
commit
46254101f7

+ 3 - 3
Userland/Libraries/LibWeb/DOM/Position.cpp

@@ -13,8 +13,8 @@
 
 namespace Web::DOM {
 
-Position::Position(Node& node, unsigned offset)
-    : m_node(JS::make_handle(node))
+Position::Position(JS::GCPtr<Node> node, unsigned offset)
+    : m_node(node)
     , m_offset(offset)
 {
 }
@@ -23,7 +23,7 @@ ErrorOr<String> Position::to_string() const
 {
     if (!node())
         return String::formatted("DOM::Position(nullptr, {})", offset());
-    return String::formatted("DOM::Position({} ({})), {})", node()->node_name(), node(), offset());
+    return String::formatted("DOM::Position({} ({})), {})", node()->node_name(), node().ptr(), offset());
 }
 
 bool Position::increment_offset()

+ 15 - 11
Userland/Libraries/LibWeb/DOM/Position.h

@@ -10,21 +10,23 @@
 #include <AK/Error.h>
 #include <AK/RefPtr.h>
 #include <AK/String.h>
-#include <LibJS/Heap/Handle.h>
+#include <LibJS/Heap/Heap.h>
 #include <LibWeb/DOM/Node.h>
 #include <LibWeb/Forward.h>
 
 namespace Web::DOM {
 
-class Position {
-public:
-    Position() = default;
-    Position(Node&, unsigned offset);
+class Position final : public JS::Cell {
+    JS_CELL(Position, JS::Cell);
 
-    bool is_valid() const { return m_node.ptr(); }
+public:
+    [[nodiscard]] static JS::NonnullGCPtr<Position> create(JS::Realm& realm, JS::NonnullGCPtr<Node> node, unsigned offset)
+    {
+        return realm.heap().allocate<Position>(realm, node, offset);
+    }
 
-    Node* node() { return m_node.cell(); }
-    Node const* node() const { return m_node.cell(); }
+    JS::GCPtr<Node> node() { return m_node; }
+    JS::GCPtr<Node const> node() const { return m_node; }
 
     unsigned offset() const { return m_offset; }
     bool offset_is_at_end_of_node() const;
@@ -32,15 +34,17 @@ public:
     bool increment_offset();
     bool decrement_offset();
 
-    bool operator==(Position const& other) const
+    bool equals(JS::NonnullGCPtr<Position> other) const
     {
-        return m_node.ptr() == other.m_node.ptr() && m_offset == other.m_offset;
+        return m_node.ptr() == other->m_node.ptr() && m_offset == other->m_offset;
     }
 
     ErrorOr<String> to_string() const;
 
 private:
-    JS::Handle<Node> m_node;
+    Position(JS::GCPtr<Node>, unsigned offset);
+
+    JS::GCPtr<Node> m_node;
     unsigned m_offset { 0 };
 };
 

+ 16 - 15
Userland/Libraries/LibWeb/HTML/BrowsingContext.cpp

@@ -269,9 +269,9 @@ BrowsingContext::BrowsingContext(Page& page, HTML::NavigableContainer* container
     m_cursor_blink_timer = Core::Timer::create_repeating(500, [this] {
         if (!is_focused_context())
             return;
-        if (m_cursor_position.node() && m_cursor_position.node()->layout_node()) {
+        if (m_cursor_position && m_cursor_position->node()->layout_node()) {
             m_cursor_blink_state = !m_cursor_blink_state;
-            m_cursor_position.node()->layout_node()->set_needs_display();
+            m_cursor_position->node()->layout_node()->set_needs_display();
         }
     }).release_value_but_fixme_should_propagate_errors();
 }
@@ -285,6 +285,7 @@ void BrowsingContext::visit_edges(Cell::Visitor& visitor)
     for (auto& entry : m_session_history)
         visitor.visit(entry);
     visitor.visit(m_container);
+    visitor.visit(m_cursor_position);
     visitor.visit(m_window_proxy);
     visitor.visit(m_opener_browsing_context);
     visitor.visit(m_group);
@@ -309,8 +310,8 @@ void BrowsingContext::did_edit(Badge<EditEventHandler>)
 {
     reset_cursor_blink_cycle();
 
-    if (m_cursor_position.node() && is<DOM::Text>(*m_cursor_position.node())) {
-        auto& text_node = static_cast<DOM::Text&>(*m_cursor_position.node());
+    if (m_cursor_position && is<DOM::Text>(*m_cursor_position->node())) {
+        auto& text_node = static_cast<DOM::Text&>(*m_cursor_position->node());
         if (auto* text_node_owner = text_node.editable_text_node_owner())
             text_node_owner->did_edit_text_node({});
     }
@@ -320,8 +321,8 @@ void BrowsingContext::reset_cursor_blink_cycle()
 {
     m_cursor_blink_state = true;
     m_cursor_blink_timer->restart();
-    if (m_cursor_position.is_valid() && m_cursor_position.node()->layout_node())
-        m_cursor_position.node()->layout_node()->set_needs_display();
+    if (m_cursor_position && m_cursor_position->node()->layout_node())
+        m_cursor_position->node()->layout_node()->set_needs_display();
 }
 
 // https://html.spec.whatwg.org/multipage/browsers.html#top-level-browsing-context
@@ -392,18 +393,18 @@ CSSPixelPoint BrowsingContext::to_top_level_position(CSSPixelPoint a_position)
     return position;
 }
 
-void BrowsingContext::set_cursor_position(DOM::Position position)
+void BrowsingContext::set_cursor_position(JS::NonnullGCPtr<DOM::Position> position)
 {
-    if (m_cursor_position == position)
+    if (m_cursor_position && m_cursor_position->equals(position))
         return;
 
-    if (m_cursor_position.node() && m_cursor_position.node()->layout_node())
-        m_cursor_position.node()->layout_node()->set_needs_display();
+    if (m_cursor_position && m_cursor_position->node()->layout_node())
+        m_cursor_position->node()->layout_node()->set_needs_display();
 
-    m_cursor_position = move(position);
+    m_cursor_position = position;
 
-    if (m_cursor_position.node() && m_cursor_position.node()->layout_node())
-        m_cursor_position.node()->layout_node()->set_needs_display();
+    if (m_cursor_position && m_cursor_position->node()->layout_node())
+        m_cursor_position->node()->layout_node()->set_needs_display();
 
     reset_cursor_blink_cycle();
 }
@@ -461,7 +462,7 @@ void BrowsingContext::select_all()
 
 bool BrowsingContext::increment_cursor_position_offset()
 {
-    if (!m_cursor_position.increment_offset())
+    if (!m_cursor_position->increment_offset())
         return false;
     reset_cursor_blink_cycle();
     return true;
@@ -469,7 +470,7 @@ bool BrowsingContext::increment_cursor_position_offset()
 
 bool BrowsingContext::decrement_cursor_position_offset()
 {
-    if (!m_cursor_position.decrement_offset())
+    if (!m_cursor_position->decrement_offset())
         return false;
     reset_cursor_blink_cycle();
     return true;

+ 3 - 3
Userland/Libraries/LibWeb/HTML/BrowsingContext.h

@@ -144,8 +144,8 @@ public:
     CSSPixelPoint to_top_level_position(CSSPixelPoint);
     CSSPixelRect to_top_level_rect(CSSPixelRect const&);
 
-    DOM::Position const& cursor_position() const { return m_cursor_position; }
-    void set_cursor_position(DOM::Position);
+    JS::GCPtr<DOM::Position> cursor_position() const { return m_cursor_position; }
+    void set_cursor_position(JS::NonnullGCPtr<DOM::Position>);
     bool increment_cursor_position_offset();
     bool decrement_cursor_position_offset();
 
@@ -218,7 +218,7 @@ private:
     // https://html.spec.whatwg.org/multipage/browsers.html#browsing-context
     JS::GCPtr<HTML::WindowProxy> m_window_proxy;
 
-    DOM::Position m_cursor_position;
+    JS::GCPtr<DOM::Position> m_cursor_position;
     RefPtr<Core::Timer> m_cursor_blink_timer;
     bool m_cursor_blink_state { false };
 

+ 1 - 1
Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp

@@ -537,7 +537,7 @@ void HTMLInputElement::did_receive_focus()
         return;
     if (!m_text_node)
         return;
-    browsing_context->set_cursor_position(DOM::Position { *m_text_node, 0 });
+    browsing_context->set_cursor_position(DOM::Position::create(realm(), *m_text_node, 0));
 }
 
 void HTMLInputElement::did_lose_focus()

+ 1 - 1
Userland/Libraries/LibWeb/HTML/HTMLTextAreaElement.cpp

@@ -56,7 +56,7 @@ void HTMLTextAreaElement::did_receive_focus()
         return;
     if (!m_text_node)
         return;
-    browsing_context->set_cursor_position(DOM::Position { *m_text_node, 0 });
+    browsing_context->set_cursor_position(DOM::Position::create(*vm().current_realm(), *m_text_node, 0));
 }
 
 void HTMLTextAreaElement::did_lose_focus()

+ 9 - 9
Userland/Libraries/LibWeb/Page/EditEventHandler.cpp

@@ -17,19 +17,19 @@
 
 namespace Web {
 
-void EditEventHandler::handle_delete_character_after(DOM::Position const& cursor_position)
+void EditEventHandler::handle_delete_character_after(JS::NonnullGCPtr<DOM::Position> cursor_position)
 {
-    auto& node = *static_cast<DOM::Text*>(const_cast<DOM::Node*>(cursor_position.node()));
+    auto& node = verify_cast<DOM::Text>(*cursor_position->node());
     auto& text = node.data();
 
-    auto next_grapheme_offset = Unicode::next_grapheme_segmentation_boundary(Utf8View { text }, cursor_position.offset());
+    auto next_grapheme_offset = Unicode::next_grapheme_segmentation_boundary(Utf8View { text }, cursor_position->offset());
     if (!next_grapheme_offset.has_value()) {
         // FIXME: Move to the next node and delete the first character there.
         return;
     }
 
     StringBuilder builder;
-    builder.append(text.bytes_as_string_view().substring_view(0, cursor_position.offset()));
+    builder.append(text.bytes_as_string_view().substring_view(0, cursor_position->offset()));
     builder.append(text.bytes_as_string_view().substring_view(*next_grapheme_offset));
     node.set_data(MUST(builder.to_string()));
 
@@ -102,15 +102,15 @@ void EditEventHandler::handle_delete(DOM::Range& range)
     m_browsing_context->did_edit({});
 }
 
-void EditEventHandler::handle_insert(DOM::Position position, u32 code_point)
+void EditEventHandler::handle_insert(JS::NonnullGCPtr<DOM::Position> position, u32 code_point)
 {
-    if (is<DOM::Text>(*position.node())) {
-        auto& node = verify_cast<DOM::Text>(*position.node());
+    if (is<DOM::Text>(*position->node())) {
+        auto& node = verify_cast<DOM::Text>(*position->node());
 
         StringBuilder builder;
-        builder.append(node.data().bytes_as_string_view().substring_view(0, position.offset()));
+        builder.append(node.data().bytes_as_string_view().substring_view(0, position->offset()));
         builder.append_code_point(code_point);
-        builder.append(node.data().bytes_as_string_view().substring_view(position.offset()));
+        builder.append(node.data().bytes_as_string_view().substring_view(position->offset()));
         node.set_data(MUST(builder.to_string()));
 
         node.invalidate_style();

+ 2 - 2
Userland/Libraries/LibWeb/Page/EditEventHandler.h

@@ -20,9 +20,9 @@ public:
 
     virtual ~EditEventHandler() = default;
 
-    virtual void handle_delete_character_after(DOM::Position const&);
+    virtual void handle_delete_character_after(JS::NonnullGCPtr<DOM::Position>);
     virtual void handle_delete(DOM::Range&);
-    virtual void handle_insert(DOM::Position, u32 code_point);
+    virtual void handle_insert(JS::NonnullGCPtr<DOM::Position>, u32 code_point);
 
 private:
     JS::NonnullGCPtr<HTML::BrowsingContext> m_browsing_context;

+ 19 - 14
Userland/Libraries/LibWeb/Page/EventHandler.cpp

@@ -398,7 +398,8 @@ bool EventHandler::handle_mousedown(CSSPixelPoint position, CSSPixelPoint screen
                 // If we didn't focus anything, place the document text cursor at the mouse position.
                 // FIXME: This is all rather strange. Find a better solution.
                 if (!did_focus_something) {
-                    m_browsing_context->set_cursor_position(DOM::Position(*paintable->dom_node(), result->index_in_node));
+                    auto& realm = document->realm();
+                    m_browsing_context->set_cursor_position(DOM::Position::create(realm, *paintable->dom_node(), result->index_in_node));
                     if (auto selection = document->get_selection()) {
                         (void)selection->set_base_and_extent(*paintable->dom_node(), result->index_in_node, *paintable->dom_node(), result->index_in_node);
                     }
@@ -419,6 +420,7 @@ bool EventHandler::handle_mousemove(CSSPixelPoint position, CSSPixelPoint screen
         return false;
 
     auto& document = *m_browsing_context->active_document();
+    auto& realm = document.realm();
 
     bool hovered_node_changed = false;
     bool is_hovering_link = false;
@@ -495,7 +497,7 @@ bool EventHandler::handle_mousemove(CSSPixelPoint position, CSSPixelPoint screen
         if (m_in_mouse_selection) {
             auto hit = paint_root()->hit_test(position, Painting::HitTestType::TextCursor);
             if (start_index.has_value() && hit.has_value() && hit->dom_node()) {
-                m_browsing_context->set_cursor_position(DOM::Position(*hit->dom_node(), *start_index));
+                m_browsing_context->set_cursor_position(DOM::Position::create(realm, *hit->dom_node(), *start_index));
                 if (auto selection = document.get_selection()) {
                     auto anchor_node = selection->anchor_node();
                     if (anchor_node)
@@ -611,7 +613,8 @@ bool EventHandler::handle_doubleclick(CSSPixelPoint position, CSSPixelPoint scre
                 return text_for_rendering.length();
             }();
 
-            m_browsing_context->set_cursor_position(DOM::Position(hit_dom_node, first_word_break_after));
+            auto& realm = node->document().realm();
+            m_browsing_context->set_cursor_position(DOM::Position::create(realm, hit_dom_node, first_word_break_after));
             if (auto selection = node->document().get_selection()) {
                 (void)selection->set_base_and_extent(hit_dom_node, first_word_break_before, hit_dom_node, first_word_break_after);
             }
@@ -709,13 +712,15 @@ bool EventHandler::handle_keydown(KeyCode key, unsigned modifiers, u32 code_poin
         return focus_next_element();
     }
 
+    auto& realm = document->realm();
+
     if (auto selection = document->get_selection()) {
         auto range = selection->range();
         if (range && range->start_container()->is_editable()) {
             selection->remove_all_ranges();
 
             // FIXME: This doesn't work for some reason?
-            m_browsing_context->set_cursor_position({ *range->start_container(), range->start_offset() });
+            m_browsing_context->set_cursor_position(DOM::Position::create(realm, *range->start_container(), range->start_offset()));
 
             if (key == KeyCode::Key_Backspace || key == KeyCode::Key_Delete) {
                 m_edit_event_handler->handle_delete(*range);
@@ -723,7 +728,7 @@ bool EventHandler::handle_keydown(KeyCode key, unsigned modifiers, u32 code_poin
             }
             if (!should_ignore_keydown_event(code_point)) {
                 m_edit_event_handler->handle_delete(*range);
-                m_edit_event_handler->handle_insert(m_browsing_context->cursor_position(), code_point);
+                m_edit_event_handler->handle_insert(JS::NonnullGCPtr { *m_browsing_context->cursor_position() }, code_point);
                 m_browsing_context->increment_cursor_position_offset();
                 return true;
             }
@@ -735,22 +740,22 @@ bool EventHandler::handle_keydown(KeyCode key, unsigned modifiers, u32 code_poin
         media_element.handle_keydown({}, key).release_value_but_fixme_should_propagate_errors();
     }
 
-    if (m_browsing_context->cursor_position().is_valid() && m_browsing_context->cursor_position().node()->is_editable()) {
+    if (m_browsing_context->cursor_position() && m_browsing_context->cursor_position()->node()->is_editable()) {
         if (key == KeyCode::Key_Backspace) {
             if (!m_browsing_context->decrement_cursor_position_offset()) {
                 // FIXME: Move to the previous node and delete the last character there.
                 return true;
             }
 
-            m_edit_event_handler->handle_delete_character_after(m_browsing_context->cursor_position());
+            m_edit_event_handler->handle_delete_character_after(*m_browsing_context->cursor_position());
             return true;
         }
         if (key == KeyCode::Key_Delete) {
-            if (m_browsing_context->cursor_position().offset_is_at_end_of_node()) {
+            if (m_browsing_context->cursor_position()->offset_is_at_end_of_node()) {
                 // FIXME: Move to the next node and delete the first character there.
                 return true;
             }
-            m_edit_event_handler->handle_delete_character_after(m_browsing_context->cursor_position());
+            m_edit_event_handler->handle_delete_character_after(*m_browsing_context->cursor_position());
             return true;
         }
         if (key == KeyCode::Key_Right) {
@@ -766,17 +771,17 @@ bool EventHandler::handle_keydown(KeyCode key, unsigned modifiers, u32 code_poin
             return true;
         }
         if (key == KeyCode::Key_Home) {
-            auto& node = *static_cast<DOM::Text*>(const_cast<DOM::Node*>(m_browsing_context->cursor_position().node()));
-            m_browsing_context->set_cursor_position(DOM::Position { node, 0 });
+            auto& node = verify_cast<DOM::Text>(*m_browsing_context->cursor_position()->node());
+            m_browsing_context->set_cursor_position(DOM::Position::create(realm, node, 0));
             return true;
         }
         if (key == KeyCode::Key_End) {
-            auto& node = *static_cast<DOM::Text*>(const_cast<DOM::Node*>(m_browsing_context->cursor_position().node()));
-            m_browsing_context->set_cursor_position(DOM::Position { node, (unsigned)node.data().bytes().size() });
+            auto& node = verify_cast<DOM::Text>(*m_browsing_context->cursor_position()->node());
+            m_browsing_context->set_cursor_position(DOM::Position::create(realm, node, (unsigned)node.data().bytes().size()));
             return true;
         }
         if (!should_ignore_keydown_event(code_point)) {
-            m_edit_event_handler->handle_insert(m_browsing_context->cursor_position(), code_point);
+            m_edit_event_handler->handle_insert(JS::NonnullGCPtr { *m_browsing_context->cursor_position() }, code_point);
             m_browsing_context->increment_cursor_position_offset();
             return true;
         }

+ 3 - 3
Userland/Libraries/LibWeb/Painting/PaintableBox.cpp

@@ -520,11 +520,11 @@ static void paint_cursor_if_needed(PaintContext& context, Layout::TextNode const
     if (!browsing_context.cursor_blink_state())
         return;
 
-    if (browsing_context.cursor_position().node() != &text_node.dom_node())
+    if (browsing_context.cursor_position()->node() != &text_node.dom_node())
         return;
 
     // NOTE: This checks if the cursor is before the start or after the end of the fragment. If it is at the end, after all text, it should still be painted.
-    if (browsing_context.cursor_position().offset() < (unsigned)fragment.start() || browsing_context.cursor_position().offset() > (unsigned)(fragment.start() + fragment.length()))
+    if (browsing_context.cursor_position()->offset() < (unsigned)fragment.start() || browsing_context.cursor_position()->offset() > (unsigned)(fragment.start() + fragment.length()))
         return;
 
     if (!fragment.layout_node().dom_node() || !fragment.layout_node().dom_node()->is_editable())
@@ -533,7 +533,7 @@ static void paint_cursor_if_needed(PaintContext& context, Layout::TextNode const
     auto fragment_rect = fragment.absolute_rect();
 
     CSSPixelRect cursor_rect {
-        fragment_rect.x() + CSSPixels::nearest_value_for(text_node.font().width(fragment.text().substring_view(0, text_node.browsing_context().cursor_position().offset() - fragment.start()))),
+        fragment_rect.x() + CSSPixels::nearest_value_for(text_node.font().width(fragment.text().substring_view(0, text_node.browsing_context().cursor_position()->offset() - fragment.start()))),
         fragment_rect.top(),
         1,
         fragment_rect.height()