1881229 - [Linux] Rework nsDragService module

Status: RESOLVED FIXED

(Core :: Widget: Gtk, enhancement, P3)

Description

[Martin Stránský [:stransky] (ni? me)]

nsDragService module uses too many gdk_atom_intern() conversions and text MIME type matches. We may flip MIME types from string to GtkAtoms and compare atoms instead of strings.

Comment 1

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1881229 [Linux] Rename GetDragFlavors to GetAvailableDragFlavors r?emilio

• Rename GetDragFlavors to GetAvailableDragFlavors
• Rename dragFlavors to availableDragFlavors

Comment 2

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1881229 [Linux] Rename flavor arguments to more explicit names r?emilio

• Rename gdkFlavor to requestedFlavor
• Rename nsDragService::GetTargetDragData() arguments

Depends on D207246

Comment 3

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1881229 [Linux] Rename GetTargetDragData to GetDragData r?emilio

Depends on D207247

Comment 4

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1881229 [Linux] Cache MIME type atoms r?emilio

Depends on D207248

Comment 5

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1881229 [Linux] D&D Use GdkAtom to check MIME types instead of string comparsions r?emilio

Depends on D207249

Comment 6

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1881229 [Linux] D&D factor out unused local variables r?emilio

Depends on D207250

Comment 7

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1881229 [Linux] Implement DragData to hold Drag & Drop data r?emilio

• Implement DragData to hold D&D data instead of separated nsDragService class members
• Implement data cache as DragData hash table

Depends on D207251

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/43ab3ccffa94
[Linux] Rename GetDragFlavors to GetAvailableDragFlavors r=emilio
https://hg.mozilla.org/integration/autoland/rev/e055b85d36fc
[Linux] Rename flavor arguments to more explicit names r=emilio
https://hg.mozilla.org/integration/autoland/rev/4a58c9a0cbd6
[Linux] Rename GetTargetDragData to GetDragData r=emilio
https://hg.mozilla.org/integration/autoland/rev/3f9f8f388f8d
[Linux] Cache MIME type atoms r=emilio
https://hg.mozilla.org/integration/autoland/rev/d4dfae34380d
[Linux] D&D Use GdkAtom to check MIME types instead of string comparsions r=emilio
https://hg.mozilla.org/integration/autoland/rev/4d5da5f79ce6
[Linux] D&D factor out unused local variables r=emilio
https://hg.mozilla.org/integration/autoland/rev/f98ce3bad10b
[Linux] Implement DragData to hold Drag & Drop data r=emilio

Comment 9

[Aron Cseh]

https://hg.mozilla.org/mozilla-central/rev/43ab3ccffa94
https://hg.mozilla.org/mozilla-central/rev/e055b85d36fc
https://hg.mozilla.org/mozilla-central/rev/4a58c9a0cbd6
https://hg.mozilla.org/mozilla-central/rev/3f9f8f388f8d
https://hg.mozilla.org/mozilla-central/rev/d4dfae34380d
https://hg.mozilla.org/mozilla-central/rev/4d5da5f79ce6
https://hg.mozilla.org/mozilla-central/rev/f98ce3bad10b

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

This causes frequent crashes with the signature [@ DragData::HasURIs], e.g. bp-27a0fa55-e4f6-46e3-b6e6-e7ece0240417, and will be backed out in the next Nightly.

Comment 11

[tszentpeteri]

Backed out for causing linux drag and drop crashes.

• Backout link

Comment 13

[Martin Stránský [:stransky] (ni? me)]

Updated, we didn't check return value of GetData().

Comment 14

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1881229 [Linux] Make nsDragService::GetDragData() to return bool r?emilio

Depends on D207333

Comment 15

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1881229 [Linux] Query URI list only if nsDragService::GetDragData() gets D&D data r?emilio

Depends on D207704

Comment 16

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1881229 [Linux] Query mDragData only if nsDragService::GetDragData() manages to get any r?emilio

Depends on D207705

Comment 17

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1881229 [Linux] Store data flavor in DragData r?emilio

Depends on D207706

Comment 18

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1881229 [Linux] Pass data flavour to DragData constructor r?emilio

Depends on D207718

Comment 19

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1881229 [Linux] Implement DragData::Export() to set data into nsITransferable r?emilio

• Cache images, kCustomTypesMime and kRTFMime atoms
• Add DragData::IsImageFlavor()
• Implement DragData::Export(). Do necessary conversions and pass drag data to nsITransferable.

Depends on D207719

Comment 20

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1881229 [Linux] Remove DragData::UpdateData, we don't want to fiddle with DragData internals r?emilio

Depends on D207720

Comment 21

[BugBot [:suhaib / :marco]]

The bug is linked to a topcrash signature, which matches the following criterion:

• Top 10 desktop browser crashes on nightly

For more information, please visit BugBot documentation.

Comment 22

[Intermittent Failures Robot]

16 failures in 4252 pushes (0.004 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• autoland: 16

Platform and build breakdown:

• linux64: 16
  • opt: 16

For more details, see:
https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1881229&startday=2024-04-15&endday=2024-04-21&tree=all

Comment 23

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1881229 [Linux] Increase D&D timeout to 5 seconds as D&D operations should not timeout even on high load r?emilio

Comment 24

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1881229 [Linux] Indent D&D log according to event loop depth r?emilio

Depends on D209341

Comment 25

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1881229 [Linux] Cache MIME type atoms r?emilio

Depends on D209342

Comment 26

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1881229 [Linux] Implement DragData class to hold received D&D data r?emilio

• Implement DragData class to hold received D&D data
• Implement D&D data conversions to various MIME types (nsIFile, x-moz-url, unicode text etc.)
• Implement D&D data export to nsITransferable

Depends on D209343

Comment 27

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1881229 [Linux] Add DragData and GdkAtom cache to hold already received data r?emilio

Depends on D209344

Comment 28

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1881229 [Linux] Implement nsDragService::GetDragData() r?emilio

Implement nsDragService::GetDragData(). It query Gtk for D&D data and returns them as ref counted DragData object
which is also cached.

It also updates nsDragService::TargetDataReceived() to work with DragData.

Depends on D209345

Comment 29

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1881229 [Linux] Implement nsDragService::GetNumDropItems() by DragData r?emilio

Use DragData to get number of items transfered by D&D. Add support for text/x-moz-url MIME type
which is used for internal Gecko URL transfers.

Depends on D209346

Comment 30

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1881229 [Linux] Use DragData / GetDragData() to implement nsDragService::GetData() r?emilio

Depends on D209347

Comment 31

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1881229 [Linux] Remove unused D&D parts r?emilio

Depends on D209348

Comment 32

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1881229 [Linux] Replace text MIME checks with GdkAtom ones r?emilio

Depends on D209349

Comment 33

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6b2409c9e7cf
[Linux] Increase D&D timeout to 5 seconds as D&D operations should not timeout even on high load r=emilio
https://hg.mozilla.org/integration/autoland/rev/7200da869ebe
[Linux] Indent D&D log according to event loop depth r=emilio
https://hg.mozilla.org/integration/autoland/rev/5af4a28aff29
[Linux] Cache MIME type atoms r=emilio
https://hg.mozilla.org/integration/autoland/rev/044c81dc2a45
[Linux] Implement DragData class to hold received D&D data r=emilio
https://hg.mozilla.org/integration/autoland/rev/a0aa9eb96d76
[Linux] Add DragData and GdkAtom cache to hold already received data r=emilio
https://hg.mozilla.org/integration/autoland/rev/615e6ec94f4c
[Linux] Implement nsDragService::GetDragData() r=emilio
https://hg.mozilla.org/integration/autoland/rev/004c60cdf817
[Linux] Implement nsDragService::GetNumDropItems() by DragData r=emilio
https://hg.mozilla.org/integration/autoland/rev/a599a81a1478
[Linux] Use DragData / GetDragData() to implement nsDragService::GetData() r=emilio
https://hg.mozilla.org/integration/autoland/rev/b6ddb782baf3
[Linux] Remove unused D&D parts r=emilio
https://hg.mozilla.org/integration/autoland/rev/68955533eb96
[Linux] Replace text MIME checks with GdkAtom ones r=emilio

Comment 34

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/6b2409c9e7cf
https://hg.mozilla.org/mozilla-central/rev/7200da869ebe
https://hg.mozilla.org/mozilla-central/rev/5af4a28aff29
https://hg.mozilla.org/mozilla-central/rev/044c81dc2a45
https://hg.mozilla.org/mozilla-central/rev/a0aa9eb96d76
https://hg.mozilla.org/mozilla-central/rev/615e6ec94f4c
https://hg.mozilla.org/mozilla-central/rev/004c60cdf817
https://hg.mozilla.org/mozilla-central/rev/a599a81a1478
https://hg.mozilla.org/mozilla-central/rev/b6ddb782baf3
https://hg.mozilla.org/mozilla-central/rev/68955533eb96

Comment 35

[Mathew Hodson]

This has the leave-open keyword, but it looks like all the patches landed. Can this be closed?

Comment 36

[zlice]

For the related issue I opened https://bugzilla.mozilla.org/show_bug.cgi?id=1888385

I still have issues on multiple machines. This has actually caused more problems for me.

Comment 37

[Martin Stránský [:stransky] (ni? me)]

(In reply to Mathew Hodson from comment #35)

This has the leave-open keyword, but it looks like all the patches landed. Can this be closed?

Yes, it can be closed now.