1875369 - [Wayland] Minigate bogus moz_container_wayland_map_event callback

Status: RESOLVED FIXED

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

Description

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

It's possible under some condition that Gtk sends bogus moz_container_wayland_map_event() when mContainer is already unmapped:

[Parent 194459: Main Thread]: D/WidgetPopup [560213c45770]: nsWindow::OnUnmap
[Parent 194459: Main Thread]: D/WidgetPopup [560213c45770]: nsWindow::ReleaseGdkWindow()
[Parent 194459: Main Thread]: D/WidgetPopup [560213c45770]: nsWindow::SetCompositorWidgetDelegate 0 mIsMapped 0 mCompositorWidgetDelegate 5602140ac550
[Parent 194459: Main Thread]: D/WidgetPopup [560213c45770]: GtkCompositorWidget::~GtkCompositorWidget [560213c45770]
[Parent 194459: Main Thread]: D/WidgetPopup [560213c45770]: GtkCompositorWidget::DisableRendering [560213c45770]
[Parent 194459: Main Thread]: D/Widget moz_container_wayland_unmap [560213c45770]
[Parent 194459: Main Thread]: D/Widget moz_container_unmap() [560213c45770]
Widget unmap 0x560212e52ee0
[Parent 194459: Main Thread]: D/WidgetPopup [560213c45770]:   Clearing mMoveToRectPopupSize
[Parent 194459: Main Thread]: D/WidgetPopup [560213c45770]: nsWindow::OnWindowStateEvent for 56021376fb10 changed 0x81 new_window_state 0x80
[Parent 194459: Main Thread]: D/WidgetPopup [560213c45770]: nsWindow::SetHasMappedToplevel(1)
[Parent 194459: Main Thread]: D/WidgetPopup [560213c45770]:   quick return because IS_MOZ_CONTAINER(aWidget) is true
[Parent 194459: Main Thread]: D/Widget moz_container_wayland_map_event [560213c45770]
[Parent 194459: Main Thread]: D/WidgetWayland moz_container_wayland_surface_create_locked [560213c45770]
[Parent 194459: Main Thread]: D/WidgetWayland     gtk wl_surface 560214110fe0 ID 275
[Parent 194459: Main Thread]: D/WidgetWayland     commit to parent
[Parent 194459: Main Thread]: D/Widget moz_container_wayland_surface_set_scale_locked scale 2
[Parent 194459: Main Thread]: D/WidgetWayland moz_container_wayland_invalidate [560213c45770]
[Parent 194459: Main Thread]: D/Widget KeymapWrapper::SetFocusOut surface 56020e085f90 ID 48
[Parent 194459: Main Thread]: D/Widget [56020e1fb8c0]: enter notify (win=56020e64d7d0, sub=56020e64d7d0): 451.355469, 14.757813 mode 4, detail 0
[Parent 194459: Main Thread]: D/Widget moz_container_unrealize() [560213c45770] GdkWindow 560214745260

We should process moz_container_wayland_map_event() only if there's proper map event before and we're mapped.

Comment 1

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

Attachment: Bug 1875369 [Wayland] Don't mark popup as mapped in map-event r?emilio

Comment 2

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

Attachment: Bug 1875369 [Wayland] Handle unmap signals from MozContainer r?emilio

• Unmap MozContainer from it's unmap handler and don't use mShell handler for it
• Disable rendering to MozContainer before we unmap it

Depends on D199032

Comment 3

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

Attachment: Bug 1875369 [Wayland] Make sure MozContainer is unmapped before unrealize r?emilio

Depends on D199085

Comment 4

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

Attachment: Bug 1875369 [Wayland] Remove nsWindow::OnUnrealize handler as it's handled by MozContainer now r?emilio

Clear mGdkWindow from DisableRendering() handler and remove nsWindow::OnUnrealize().

Depends on D199086

Comment 5

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

Attachment: Bug 1875369 [Wayland] Hide window first before any resources release at nsWindow::Destroy() r?emilio

Depends on D199087

Comment 6

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

Attachment: Bug 1875369 [Linux] Don't set mGdkWindow at GetCompositorWidgetInitData() r?emilio

We can't set mGdkWindow at GetCompositorWidgetInitData() is window is unmapped. Rather we provide null XWindow and disable rendering and update XWindow afer OnMap event.

Depends on D199088

Comment 7

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

Attachment: Bug 1875369 [Wayland] Restring gtk_widget_get_window() usage r?emilio

• Remove gtk_widget_get_window() where it's possible and use mGdkWindow directly.
• Check gtk_widget_get_window() return values to ensure we don't use null GdkWindow.

Depends on D200268

Comment 8

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

Attachment: Bug 1875369 [Wayland] Create IMContextWrapper without GdkWindow and allow to update/delete GdkWindow r?emilio

nsWindow::mGdkWindow is set/reset when nsWindow becomes visible/invisible so we need to update GdkWindow in IMContextWrapper according to it.

Depends on D200269

Comment 9

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

Attachment: Bug 1875369 [Wayland] Set/unset IMContextWrapper GdkWindow when it's mapped/unmapped so we use valid GdkWindow r?emilio

Depends on D200270

Comment 10

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

Attachment: Bug 1875369 [Linux] Get renderer again after WillPaintWindow() r?emilio

WillPaintWindow() spins event loop which can lead to any action related to nsWindow - it can be hid, shown or destroyed.
If the nsWindow visibility is changed, compositor/renderer is deleted and we need to get it again.

Depends on D200271

Comment 11

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

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=70c3b39b2a6e78ea5c073cc182323f82df145f97

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9e55c666d554
[Wayland] Don't mark popup as mapped in map-event r=emilio
https://hg.mozilla.org/integration/autoland/rev/150d8b2ce3b3
[Wayland] Handle unmap signals from MozContainer r=emilio
https://hg.mozilla.org/integration/autoland/rev/302061ddbf3f
[Wayland] Make sure MozContainer is unmapped before unrealize r=emilio
https://hg.mozilla.org/integration/autoland/rev/91d4dc450ca6
[Wayland] Remove nsWindow::OnUnrealize handler as it's handled by MozContainer now r=emilio
https://hg.mozilla.org/integration/autoland/rev/e7d835c38205
[Wayland] Hide window first before any resources release at nsWindow::Destroy() r=emilio
https://hg.mozilla.org/integration/autoland/rev/a98d8031c4d6
[Linux] Don't set mGdkWindow at GetCompositorWidgetInitData() r=emilio
https://hg.mozilla.org/integration/autoland/rev/feeb46cf9436
[Wayland] Restring gtk_widget_get_window() usage r=emilio
https://hg.mozilla.org/integration/autoland/rev/282f24265efc
[Wayland] Create IMContextWrapper without GdkWindow and allow to update/delete GdkWindow r=emilio
https://hg.mozilla.org/integration/autoland/rev/a3452ece094d
[Wayland] Set/unset IMContextWrapper GdkWindow when it's mapped/unmapped so we use valid GdkWindow r=emilio
https://hg.mozilla.org/integration/autoland/rev/ae7ddca3f0bb
[Linux] Get renderer again after WillPaintWindow() r=emilio

Comment 13

[Cristian Tuns]

Backed out for causing reftest failures in 1453030.html

• Backout link
• Push with failures
• Failure Log
• Failure line: REFTEST PROCESS-CRASH | application crashed [@ libgdk-3.so.0 + 0x00000000000471f2] | dom/media/tests/crashtests/1453030.html

And also build bustages: https://treeherder.mozilla.org/logviewer?job_id=445678990&repo=autoland

Comment 14

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

Attachment: Bug 1875369 [Linux] Don't call gdk_window_get_toplevel() with nullptr GdkWindow r?emilio

Depends on D200272

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/80d023f90eef
[Wayland] Don't mark popup as mapped in map-event r=emilio
https://hg.mozilla.org/integration/autoland/rev/5cfd179606b1
[Wayland] Handle unmap signals from MozContainer r=emilio
https://hg.mozilla.org/integration/autoland/rev/df960c1bec65
[Wayland] Make sure MozContainer is unmapped before unrealize r=emilio
https://hg.mozilla.org/integration/autoland/rev/f382d71b3984
[Wayland] Remove nsWindow::OnUnrealize handler as it's handled by MozContainer now r=emilio
https://hg.mozilla.org/integration/autoland/rev/e13e6de0540c
[Wayland] Hide window first before any resources release at nsWindow::Destroy() r=emilio
https://hg.mozilla.org/integration/autoland/rev/508895d97d3d
[Linux] Don't set mGdkWindow at GetCompositorWidgetInitData() r=emilio
https://hg.mozilla.org/integration/autoland/rev/6259faab4a6b
[Wayland] Restring gtk_widget_get_window() usage r=emilio
https://hg.mozilla.org/integration/autoland/rev/c54685bca329
[Wayland] Create IMContextWrapper without GdkWindow and allow to update/delete GdkWindow r=emilio
https://hg.mozilla.org/integration/autoland/rev/84ef31f39d0c
[Wayland] Set/unset IMContextWrapper GdkWindow when it's mapped/unmapped so we use valid GdkWindow r=emilio
https://hg.mozilla.org/integration/autoland/rev/e0389b30f9e1
[Linux] Get renderer again after WillPaintWindow() r=emilio
https://hg.mozilla.org/integration/autoland/rev/039d034ac81d
[Linux] Don't call gdk_window_get_toplevel() with nullptr GdkWindow r=emilio

Comment 16

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/80d023f90eef
https://hg.mozilla.org/mozilla-central/rev/5cfd179606b1
https://hg.mozilla.org/mozilla-central/rev/df960c1bec65
https://hg.mozilla.org/mozilla-central/rev/f382d71b3984
https://hg.mozilla.org/mozilla-central/rev/e13e6de0540c
https://hg.mozilla.org/mozilla-central/rev/508895d97d3d
https://hg.mozilla.org/mozilla-central/rev/6259faab4a6b
https://hg.mozilla.org/mozilla-central/rev/c54685bca329
https://hg.mozilla.org/mozilla-central/rev/84ef31f39d0c
https://hg.mozilla.org/mozilla-central/rev/e0389b30f9e1
https://hg.mozilla.org/mozilla-central/rev/039d034ac81d