Closed Bug 1881229 Opened 3 months ago Closed 5 days ago

[Linux] Rework nsDragService module

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
128 Branch
Tracking Status
firefox128 --- fixed

People

(Reporter: stransky, Assigned: stransky)

References

(Blocks 1 open bug)

Details

Crash Data

Attachments

(10 files, 14 obsolete files)

48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review

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.

Flags: needinfo?(stransky)
Flags: needinfo?(stransky)
  • Rename GetDragFlavors to GetAvailableDragFlavors
  • Rename dragFlavors to availableDragFlavors
Assignee: nobody → stransky
Status: NEW → ASSIGNED
  • Rename gdkFlavor to requestedFlavor
  • Rename nsDragService::GetTargetDragData() arguments

Depends on D207246

Depends on D207248

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

Depends on D207251

Pushed by stransky@redhat.com:
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
Regressions: 1891707

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.

Crash Signature: [@ DragData::HasURIs] → [@ DragData::HasURIs] [@ DragData::GetURIs]

Backed out for causing linux drag and drop crashes.

Status: RESOLVED → REOPENED
Flags: needinfo?(stransky)
Resolution: FIXED → ---
Target Milestone: 127 Branch → ---
Duplicate of this bug: 1891707

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

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

Depends on D207719

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.

Keywords: topcrash
Attachment #9397114 - Attachment is obsolete: true
Attachment #9397115 - Attachment is obsolete: true
Attachment #9397116 - Attachment is obsolete: true
Attachment #9397139 - Attachment is obsolete: true
Attachment #9397140 - Attachment is obsolete: true
Attachment #9397141 - Attachment is obsolete: true
Attachment #9397142 - Attachment is obsolete: true
Summary: [Linux] nsDragService module uses too many gdk_atom_intern() conversions → [Linux] Rework nsDragService module
Flags: needinfo?(stransky)
Attachment #9396378 - Attachment is obsolete: true
Attachment #9396189 - Attachment is obsolete: true
Attachment #9396188 - Attachment is obsolete: true
Attachment #9396187 - Attachment is obsolete: true
Attachment #9396186 - Attachment is obsolete: true
Attachment #9396185 - Attachment is obsolete: true
Attachment #9396184 - Attachment is obsolete: true
Status: REOPENED → ASSIGNED
Flags: needinfo?(stransky)
Keywords: topcrashleave-open
  • 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

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

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

Attachment #9399950 - Attachment description: Bug 1881229 [Linux] Indend D&D log according to event loop depth r?emilio → Bug 1881229 [Linux] Indent D&D log according to event loop depth r?emilio
Pushed by stransky@redhat.com:
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
Regressions: 1896680
Blocks: 1885400
No longer duplicate of this bug: 1891707

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

Flags: needinfo?(stransky)

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.

(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.

Status: ASSIGNED → RESOLVED
Closed: 1 month ago5 days ago
Flags: needinfo?(stransky)
Keywords: leave-open
Resolution: --- → FIXED
Version: unspecified → Firefox 128
Target Milestone: --- → 128 Branch
Version: Firefox 128 → unspecified
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: