Closed Bug 1730533 Opened 3 years ago Closed 3 years ago

[Linux] Use D&D popup for tab drops

Categories

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

enhancement

Tracking

()

VERIFIED FIXED
96 Branch
Tracking Status
firefox96 --- verified

People

(Reporter: stransky, Assigned: stransky)

References

(Blocks 1 open bug, Regressed 1 open bug)

Details

Crash Data

Attachments

(6 files, 3 obsolete files)

59.31 KB, patch
Details | Diff | Splinter 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

Use D&D popup for tab drops.

Attached patch WIP patchSplinter Review

There's a WIP patch for it. It's more complicated that I expected but it's also shed light to GPU process on Wayland and simplifies remote rendering setup.

I will submit it as individual patches for next devel cycle (FF95).

Depends on: 1733055
Depends on: 1733057

When GdkWindow is unmapped (during reparent, when GtkWindow is hidden) we need to clear Window handle at GtkCompositorWidget as it's no longer valid.
A new GdkWindow/Window is created when we're mapped again.

Make sure we don't crash when WindowSurfaceProvider does not have valid X Window handle (when we're hidden for instance).

Depends on D126890

This patch does:

  • Track nsWindow real visibility by widget_map_cb / widget_unrealize_cb callbacks and set mIsMapped attribute properly.
  • Clear mGdkWindow attribute when mGdkWindow is not visible and we can't paint into it.
  • Implement and use ConfigureGdkWindow() to set up mGdkWindow when it's visible, start compositor, VSync.
  • Implement and use ReleaseGdkWindow() to clear up mGdkWindow when it's not visible, stop compositor, Vsync.
  • Make sure nsWindow works when mGdkWindow is null.
  • Configure Drag popup accordingly.

Depends on D126894

See Also: → 1693513

There's still one issue which needs to be addressed:

[komat@localhost bin]$ MOZ_ENABLE_WAYLAND=1 ./firefox -P
[Parent 74175, Main Thread] WARNING: dependent window created without a parent: file /raid/src/toolkit/components/startup/nsAppStartup.cpp:744
Initializing context 0x7f25203c67b0 surface (nil) on display 0x7f253d7ea000
[Parent 74175, Renderer] WARNING: robust_buffer_access_behavior marked as unsupported: file /raid/src/gfx/gl/GLContextFeatures.cpp:632
[Socket 74339, Main Thread] WARNING: 'NS_FAILED(rv)', file /raid/src/netwerk/protocol/http/nsHttpHandler.cpp:372
[Socket 74339, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, kKnownEsrVersion) failed with result 0x80004002 (NS_NOINTERFACE): file /raid/src/toolkit/components/resistfingerprinting/nsRFPService.cpp:560
[Parent 74175, Main Thread] WARNING: NS_ENSURE_TRUE(rootFrame) failed: file /raid/src/dom/base/nsGlobalWindowOuter.cpp:4240
[Parent 74175, Main Thread] WARNING: NS_ENSURE_TRUE(root) failed: file /raid/src/layout/base/nsDocumentViewer.cpp:2613
[2021-10-05T07:14:15Z WARN webrender::device::gl] Missing optimized shader source for gpu_cache_update
Crash Annotation GraphicsCriticalError: |[0][GFX1-]: window is null (t=0.584913) [GFX1-]: window is null
Crash Annotation GraphicsCriticalError: |[0][GFX1-]: window is null (t=0.584913) |[1][GFX1-]: Failed to create EGLSurface (t=0.584971) [GFX1-]: Failed to create EGLSurface
Crash Annotation GraphicsCriticalError: |[0][GFX1-]: window is null (t=0.584913) |[1][GFX1-]: Failed to create EGLSurface (t=0.584971) |[2][GFX1-]: Fallback WR to SW-WR (t=0.613179) [GFX1-]: Fallback WR to SW-WR

  • When GtkCompositorWidget is created, make suspended rendering state as initial one as we can't render to widget window.
    Enable rendering when underlying window is visible (nsWindow::ConfigureGdkWindow) or GtkCompositorWidget is assigned (nsWindow::SetCompositorWidgetDelegate).
  • Use moz_container_wayland_add_initial_draw_callback() at nsWindow::ConfigureGdkWindow() to set Wayland EGL window as it was before.
  • Make moz_container_wayland_add_initial_draw_callback() to run callbacks instantly when mozcontainer is already visible.
  • Implement and use moz_container_wayland_clear_initial_draw_callback() to clear mozcontainer callbacks when it becomes hidden.
  • Rename COMPOSITOR_PAUSED_MISSING_EGL_WINDOW to COMPOSITOR_PAUSED_MISSING_WINDOW as it's used for GLX too.
  • Initially pause rendering for GLX too.

Depends on D126895

Type: defect → enhancement
Depends on: 1737065
Depends on: 1737068

Comment on attachment 9243399 [details]
Bug 1730533 [Linux] Enable/Disable rendering to GdkWindow when we're mapped/unmapped r?rmader

Revision D126895 was moved to bug 1737068. Setting attachment 9243399 [details] to obsolete.

Attachment #9243399 - Attachment is obsolete: true

Comment on attachment 9244344 [details]
Bug 1730533 [Linux] Postpone compositor rendering until widget window is ready, r?rmader

Revision D127542 was moved to bug 1737068. Setting attachment 9244344 [details] to obsolete.

Attachment #9244344 - Attachment is obsolete: true
Attachment #9243396 - Attachment is obsolete: true
Pushed by stransky@redhat.com:
https://hg.mozilla.org/integration/autoland/rev/811ddc8525f3
[Linux] Use Drag and Drop popups on Gtk >= 3.24 r=rmader
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 95 Branch
Regressions: 1738482
Regressions: 1738639

Please backout D129494 due to regressions (Bug 1738639, Bug 1738482).
Thanks.

Flags: needinfo?(sheriffs)

Martin, is it ok to backout from autoland?

Flags: needinfo?(stransky)
Status: RESOLVED → REOPENED
Flags: needinfo?(sheriffs)
Resolution: FIXED → ---
Target Milestone: 95 Branch → ---
Backout by abutkovits@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/770627d2ad07
Backed out changeset 811ddc8525f3 for causing regressions (Bug 1738639, Bug 1738482). CLOSED TREE

(In reply to Atila Butkovits from comment #20)

Martin, is it ok to backout from autoland?

I don't know what it means. I just don't want to ship that in FF95.
Thanks.

Flags: needinfo?(stransky)

Backed out from beta (version 95):

https://hg.mozilla.org/releases/mozilla-beta/rev/c1ca9614aee590e1355074f7227ef87a2785c58b

(In reply to Martin Stránský [:stransky] (ni? me) from comment #23)

(In reply to Atila Butkovits from comment #20)

Martin, is it ok to backout from autoland?

I don't know what it means. I just don't want to ship that in FF95.

This is the question if it's urgent to have such a change in mozilla-central which gets used to builds Nightlies etc. or if it can progress like normal patches which get merged 2-4x times per day to mozilla-central when they have been proven as not being affected by permanent build or test failures.

Thanks for explanation, it's ok to merge as a normal patch.
Thanks.

Pushed by stransky@redhat.com:
https://hg.mozilla.org/integration/autoland/rev/1a493142d313
[Linux] Use Drag and Drop popups on Gtk >= 3.24 r=rmader
Status: REOPENED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 96 Branch
Crash Signature: [@ mozilla::widget::WlCrashHandler]

Hello! Are there any steps that manual QA could use in order to verify this? Thank you!

Flags: needinfo?(stransky)

Yes, when you re-arrange or move browser tab with mouse, you should see a picture of page screenshot (instead of an D&D icon).

Flags: needinfo?(stransky)

(In reply to Martin Stránský [:stransky] (ni? me) from comment #31)

Yes, when you re-arrange or move browser tab with mouse, you should see a picture of page screenshot (instead of an D&D icon).

Thank you! I will mark this as qe+ then.

Flags: qe-verify+

Verified fixed with Firefox 96.0b4 (20211212185725) on Ubuntu 21.1 and 20.04. The tab picture preview is successfully displayed when moving tabs.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Regressions: 1844373
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: