LibWeb: Make the drag data store reference counted

Ownership of the drag data store is a bit weird. In a normal drag-and-
drop operation, the DragAndDropEventHandler owns the store. When events
are fired for the operation, the DataTransfer object assigned to those
events are "associated" with the store. We currently represent that with
an Optional<DragDataStore&>.

However, it's also possible to create DataTransfer objects from scripts.
Those objects create their own drag data store. This puts DataTransfer
in a weird situation where it may own a store or just reference one.

Rather than coming up with something like Variant<DDS, DDS&> or using
MaybeOwned<DDS> here, we can get by with just making the store reference
counted.
This commit is contained in:
Timothy Flynn 2024-08-21 06:39:02 -04:00 committed by Andreas Kling
commit f7c4165dde
Notes: github-actions[bot] 2024-08-22 12:23:00 +00:00
6 changed files with 24 additions and 15 deletions

View file

@ -76,7 +76,7 @@ void DataTransfer::set_effect_allowed(FlyString effect_allowed)
// On setting, if drag data store's mode is the read/write mode and the new value is one of "none", "copy", "copyLink", // On setting, if drag data store's mode is the read/write mode and the new value is one of "none", "copy", "copyLink",
// "copyMove", "link", "linkMove", "move", "all", or "uninitialized", then the attribute's current value must be set // "copyMove", "link", "linkMove", "move", "all", or "uninitialized", then the attribute's current value must be set
// to the new value. Otherwise, it must be left unchanged. // to the new value. Otherwise, it must be left unchanged.
if (m_associated_drag_data_store.has_value() && m_associated_drag_data_store->mode() == DragDataStore::Mode::ReadWrite) if (m_associated_drag_data_store && m_associated_drag_data_store->mode() == DragDataStore::Mode::ReadWrite)
set_effect_allowed_internal(move(effect_allowed)); set_effect_allowed_internal(move(effect_allowed));
} }
@ -109,7 +109,7 @@ ReadonlySpan<String> DataTransfer::types() const
String DataTransfer::get_data(String const& format_argument) const String DataTransfer::get_data(String const& format_argument) const
{ {
// 1. If the DataTransfer object is no longer associated with a drag data store, then return the empty string. // 1. If the DataTransfer object is no longer associated with a drag data store, then return the empty string.
if (!m_associated_drag_data_store.has_value()) if (!m_associated_drag_data_store)
return {}; return {};
// 2. If the drag data store's mode is the protected mode, then return the empty string. // 2. If the drag data store's mode is the protected mode, then return the empty string.
@ -165,7 +165,7 @@ JS::NonnullGCPtr<FileAPI::FileList> DataTransfer::files() const
// 2. If the DataTransfer object is no longer associated with a drag data store, the FileList is empty. Return // 2. If the DataTransfer object is no longer associated with a drag data store, the FileList is empty. Return
// the empty list L. // the empty list L.
if (!m_associated_drag_data_store.has_value()) if (!m_associated_drag_data_store)
return files; return files;
// 3. If the drag data store's mode is the protected mode, return the empty list L. // 3. If the drag data store's mode is the protected mode, return the empty list L.
@ -195,9 +195,9 @@ JS::NonnullGCPtr<FileAPI::FileList> DataTransfer::files() const
return files; return files;
} }
void DataTransfer::associate_with_drag_data_store(DragDataStore& drag_data_store) void DataTransfer::associate_with_drag_data_store(NonnullRefPtr<DragDataStore> drag_data_store)
{ {
m_associated_drag_data_store = drag_data_store; m_associated_drag_data_store = move(drag_data_store);
update_data_transfer_types_list(); update_data_transfer_types_list();
} }
@ -214,7 +214,7 @@ void DataTransfer::update_data_transfer_types_list()
Vector<String> types; Vector<String> types;
// 2. If the DataTransfer object is still associated with a drag data store, then: // 2. If the DataTransfer object is still associated with a drag data store, then:
if (m_associated_drag_data_store.has_value()) { if (m_associated_drag_data_store) {
bool contains_file = false; bool contains_file = false;
// 1. For each item in the DataTransfer object's drag data store item list whose kind is text, add an entry to L // 1. For each item in the DataTransfer object's drag data store item list whose kind is text, add an entry to L

View file

@ -55,7 +55,7 @@ public:
String get_data(String const& format) const; String get_data(String const& format) const;
JS::NonnullGCPtr<FileAPI::FileList> files() const; JS::NonnullGCPtr<FileAPI::FileList> files() const;
void associate_with_drag_data_store(DragDataStore& drag_data_store); void associate_with_drag_data_store(NonnullRefPtr<DragDataStore> drag_data_store);
void disassociate_with_drag_data_store(); void disassociate_with_drag_data_store();
private: private:
@ -79,7 +79,7 @@ private:
Vector<String> m_types; Vector<String> m_types;
// https://html.spec.whatwg.org/multipage/dnd.html#the-datatransfer-interface:drag-data-store-3 // https://html.spec.whatwg.org/multipage/dnd.html#the-datatransfer-interface:drag-data-store-3
Optional<DragDataStore&> m_associated_drag_data_store; RefPtr<DragDataStore> m_associated_drag_data_store;
}; };
} }

View file

@ -9,6 +9,11 @@
namespace Web::HTML { namespace Web::HTML {
NonnullRefPtr<DragDataStore> DragDataStore::create()
{
return adopt_ref(*new DragDataStore());
}
DragDataStore::DragDataStore() DragDataStore::DragDataStore()
: m_allowed_effects_state(DataTransferEffect::uninitialized) : m_allowed_effects_state(DataTransferEffect::uninitialized)
{ {

View file

@ -9,6 +9,8 @@
#include <AK/ByteBuffer.h> #include <AK/ByteBuffer.h>
#include <AK/ByteString.h> #include <AK/ByteString.h>
#include <AK/FlyString.h> #include <AK/FlyString.h>
#include <AK/NonnullRefPtr.h>
#include <AK/RefCounted.h>
#include <AK/String.h> #include <AK/String.h>
#include <AK/Vector.h> #include <AK/Vector.h>
#include <LibGfx/Bitmap.h> #include <LibGfx/Bitmap.h>
@ -33,7 +35,7 @@ struct DragDataStoreItem {
}; };
// https://html.spec.whatwg.org/multipage/dnd.html#drag-data-store // https://html.spec.whatwg.org/multipage/dnd.html#drag-data-store
class DragDataStore { class DragDataStore : public RefCounted<DragDataStore> {
public: public:
enum class Mode { enum class Mode {
ReadWrite, ReadWrite,
@ -41,7 +43,7 @@ public:
Protected, Protected,
}; };
DragDataStore(); static NonnullRefPtr<DragDataStore> create();
~DragDataStore(); ~DragDataStore();
void add_item(DragDataStoreItem item) { m_item_list.append(move(item)); } void add_item(DragDataStoreItem item) { m_item_list.append(move(item)); }
@ -55,6 +57,8 @@ public:
void set_allowed_effects_state(FlyString allowed_effects_state) { m_allowed_effects_state = move(allowed_effects_state); } void set_allowed_effects_state(FlyString allowed_effects_state) { m_allowed_effects_state = move(allowed_effects_state); }
private: private:
DragDataStore();
// https://html.spec.whatwg.org/multipage/dnd.html#drag-data-store-item-list // https://html.spec.whatwg.org/multipage/dnd.html#drag-data-store-item-list
Vector<DragDataStoreItem> m_item_list; Vector<DragDataStoreItem> m_item_list;

View file

@ -51,7 +51,7 @@ bool DragAndDropEventHandler::handle_drag_start(
// 2. Create a drag data store. All the DND events fired subsequently by the steps in this section must use this drag // 2. Create a drag data store. All the DND events fired subsequently by the steps in this section must use this drag
// data store. // data store.
m_drag_data_store.emplace(); m_drag_data_store = HTML::DragDataStore::create();
// 3. Establish which DOM node is the source node, as follows: // 3. Establish which DOM node is the source node, as follows:
// //
@ -185,7 +185,7 @@ bool DragAndDropEventHandler::handle_drag_move(
unsigned buttons, unsigned buttons,
unsigned modifiers) unsigned modifiers)
{ {
if (!m_drag_data_store.has_value()) if (!has_ongoing_drag_and_drop_operation())
return false; return false;
auto fire_a_drag_and_drop_event = [&](JS::GCPtr<DOM::EventTarget> target, FlyString const& name, JS::GCPtr<DOM::EventTarget> related_target = nullptr) { auto fire_a_drag_and_drop_event = [&](JS::GCPtr<DOM::EventTarget> target, FlyString const& name, JS::GCPtr<DOM::EventTarget> related_target = nullptr) {
@ -361,7 +361,7 @@ bool DragAndDropEventHandler::handle_drag_end(
unsigned buttons, unsigned buttons,
unsigned modifiers) unsigned modifiers)
{ {
if (!m_drag_data_store.has_value()) if (!has_ongoing_drag_and_drop_operation())
return false; return false;
auto fire_a_drag_and_drop_event = [&](JS::GCPtr<DOM::EventTarget> target, FlyString const& name, JS::GCPtr<DOM::EventTarget> related_target = nullptr) { auto fire_a_drag_and_drop_event = [&](JS::GCPtr<DOM::EventTarget> target, FlyString const& name, JS::GCPtr<DOM::EventTarget> related_target = nullptr) {

View file

@ -18,7 +18,7 @@ class DragAndDropEventHandler {
public: public:
void visit_edges(JS::Cell::Visitor& visitor) const; void visit_edges(JS::Cell::Visitor& visitor) const;
bool has_ongoing_drag_and_drop_operation() const { return m_drag_data_store.has_value(); } bool has_ongoing_drag_and_drop_operation() const { return !m_drag_data_store.is_null(); }
bool handle_drag_start(JS::Realm&, CSSPixelPoint screen_position, CSSPixelPoint page_offset, CSSPixelPoint client_offset, CSSPixelPoint offset, unsigned button, unsigned buttons, unsigned modifiers, Vector<HTML::SelectedFile> files); bool handle_drag_start(JS::Realm&, CSSPixelPoint screen_position, CSSPixelPoint page_offset, CSSPixelPoint client_offset, CSSPixelPoint offset, unsigned button, unsigned buttons, unsigned modifiers, Vector<HTML::SelectedFile> files);
bool handle_drag_move(JS::Realm&, JS::NonnullGCPtr<DOM::Document>, JS::NonnullGCPtr<DOM::Node>, CSSPixelPoint screen_position, CSSPixelPoint page_offset, CSSPixelPoint client_offset, CSSPixelPoint offset, unsigned button, unsigned buttons, unsigned modifiers); bool handle_drag_move(JS::Realm&, JS::NonnullGCPtr<DOM::Document>, JS::NonnullGCPtr<DOM::Node>, CSSPixelPoint screen_position, CSSPixelPoint page_offset, CSSPixelPoint client_offset, CSSPixelPoint offset, unsigned button, unsigned buttons, unsigned modifiers);
@ -49,7 +49,7 @@ private:
void reset(); void reset();
Optional<HTML::DragDataStore> m_drag_data_store; RefPtr<HTML::DragDataStore> m_drag_data_store;
// https://html.spec.whatwg.org/multipage/dnd.html#source-node // https://html.spec.whatwg.org/multipage/dnd.html#source-node
JS::GCPtr<DOM::EventTarget> m_source_node; JS::GCPtr<DOM::EventTarget> m_source_node;