Closed Bug 1562827 Opened 1 year ago Closed 1 year ago

[Wayland] Firefox on Wayland segmentation fault when developer tools are detached to separate window

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox69 --- wontfix
firefox70 --- fixed

People

(Reporter: stransky, Assigned: stransky)

References

(Blocks 1 open bug, Regression)

Details

Attachments

(1 file, 1 obsolete file)

[Wayland] Firefox on Wayland segmentation fault when developer tools are detached to separate window

Priority: -- → P2
Duplicate of this bug: 1563156

Martin, can you paste a stack trace here, please?
The output of call DumpJSStack() may also be useful, if this is triggered from JS.

Flags: needinfo?(stransky)
Duplicate of this bug: 1566646

(In reply to Karl Tomlinson (:karlt) from comment #3)

Martin, can you paste a stack trace here, please?

Sure, it crashes here:

15978 gtk_widget_unregister_window (GtkWidget *widget,
15979 GdkWindow *window)
15980 {
15981 GtkWidgetPrivate *priv;
15982 gpointer user_data;
15988
15989 gdk_window_get_user_data (window, &user_data);
15990 g_assert (user_data == widget); <<<<

#4 0x00007ffff683c9cc in gtk_widget_unregister_window (widget=0x7fffca9fc390, window=0x7fffca544a20) at gtkwidget.c:15990
#5 0x00007ffff6834faf in gtk_widget_real_unrealize (widget=0x7fffca9fc390) at gtkwidget.c:12477
[...]
#10 0x00007ffff6827e9c in gtk_widget_unrealize (widget=0x7fffca9fc390) at gtkwidget.c:5575
#11 0x00007ffff685202b in gtk_window_forall (container=0x7fffca490260, include_internals=1, callback=0x7ffff6827d41 <gtk_widget_unrealize>, callback_data=0x0) at gtkwindow.c:8592
#12 0x00007ffff6539f2d in gtk_container_forall (container=0x7fffca490260, callback=0x7ffff6827d41 <gtk_widget_unrealize>, callback_data=0x0) at gtkcontainer.c:2444
#13 0x00007ffff6834f85 in gtk_widget_real_unrealize (widget=0x7fffca490260) at gtkwidget.c:12471
#14 0x00007ffff68504e0 in gtk_window_unrealize (widget=0x7fffca490260) at gtkwindow.c:7699
[...]
#18 0x00007ffff6827e9c in gtk_widget_unrealize (widget=0x7fffca490260) at gtkwidget.c:5575
#19 0x00007ffff6834808 in gtk_widget_dispose (object=0x7fffca490260) at gtkwidget.c:12138
#20 0x00007ffff6847348 in gtk_window_dispose (object=0x7fffca490260) at gtkwindow.c:3164
#21 0x00007ffff5a1d496 in g_object_run_dispose () at /lib64/libgobject-2.0.so.0
#22 0x00007ffff6826559 in gtk_widget_destroy (widget=0x7fffca490260) at gtkwidget.c:4773
#23 0x00007ffff01a8097 in nsWindow::Destroy() (this=0x7fffca48fc00) at /home/komat/tmp676-trunk-gtk3/src-wayland/widget/gtk/nsWindow.cpp:711
#24 0x00007ffff0115c44 in DestroyWidgetRunnable::Run() (this=0x7fffc162b340) at /home/komat/tmp676-trunk-gtk3/src-wayland/view/nsView.cpp:118

The output of call DumpJSStack() may also be useful, if this is triggered from JS.
There's no JS content on stack.

Flags: needinfo?(stransky)

There's a backtrace from nsWindow::ReparentNativeWidget() which is more useful:

JS bt:

0 switchHost() ["resource://devtools/client/framework/toolbox-host-manager.js":239:14]
1 InterpretGeneratorResume(gen = [object AsyncFunctionGenerator], val = [object XULFrameElement], kind = "next") ["self-hosted":1300:7]
2 AsyncFunctionNext(val = [object XULFrameElement]) ["self-hosted":839:4]
this = [object AsyncFunctionGenerator]

c++ bt:

#0 0x00007ffff01a9b88 in nsWindow::ReparentNativeWidget(nsIWidget*) (this=0x7fffc3812000, aNewParent=0x7fffc287a800)
at /home/komat/tmp676-trunk-gtk3/src-wayland/widget/gtk/nsWindow.cpp:851
#1 0x00007ffff011d832 in nsViewManager::ReparentChildWidgets(nsView*, nsIWidget*) (this=0x7fffc37fe580, aView=0x7fffd564ab00, aNewWidget=0x7fffc287a800)
at /home/komat/tmp676-trunk-gtk3/src-wayland/view/nsViewManager.cpp:778
#2 0x00007ffff011d860 in nsViewManager::ReparentChildWidgets(nsView*, nsIWidget*) (this=0x7fffc37fe580, aView=0x7fffc58e7f80, aNewWidget=0x7fffc287a800)
at /home/komat/tmp676-trunk-gtk3/src-wayland/view/nsViewManager.cpp:787
#3 0x00007ffff011d9a0 in nsViewManager::ReparentWidgets(nsView*, nsView*) (this=0x7fffc37fe580, aView=0x7fffc58e7f80, aParent=0x7fffc29b4a80)
at /home/komat/tmp676-trunk-gtk3/src-wayland/view/nsViewManager.cpp:807
#4 0x00007ffff011dbb3 in nsViewManager::InsertChild(nsView*, nsView*, nsView*, bool) (this=0x7fffc37fe580, aParent=0x7fffc29b4a80, aChild=0x7fffc58e7f80, aSibling=0x0, aAfter=true)
at /home/komat/tmp676-trunk-gtk3/src-wayland/view/nsViewManager.cpp:833
#5 0x00007ffff07ac2cf in InsertViewsInReverseOrder(nsView*, nsView*) (aSibling=0x7fffc58e7f80, aParent=0x7fffc29b4a80)
at /home/komat/tmp676-trunk-gtk3/src-wayland/layout/generic/nsSubDocumentFrame.cpp:1110
#6 0x00007ffff07b0be7 in nsSubDocumentFrame::BeginSwapDocShells(nsIFrame*) (this=0x7fffcd098560, aOther=0x7fffc38f3798)
at /home/komat/tmp676-trunk-gtk3/src-wayland/layout/generic/nsSubDocumentFrame.cpp:1135
#7 0x00007fffed558da6 in nsFrameLoader::SwapWithOtherLoader(nsFrameLoader*, nsFrameLoaderOwner*, nsFrameLoaderOwner*)
(this=0x7fffc8d17230, aOther=0x7fffc29b5bb0, aThisOwner=0x7fffca091fe0, aOtherOwner=0x7fffd54ee7a0) at /home/komat/tmp676-trunk-gtk3/src-wayland/dom/base/nsFrameLoader.cpp:1593
#8 0x00007fffefdf1ce0 in mozilla::dom::XULFrameElement::SwapFrameLoaders(nsFrameLoaderOwner*, mozilla::ErrorResult&) (this=0x7fffca091f50, aOtherLoaderOwner=0x7fffd54ee7a0, rv=...)
at /home/komat/tmp676-trunk-gtk3/src-wayland/dom/xul/XULFrameElement.cpp:137
#9 0x00007fffefdf1b85 in mozilla::dom::XULFrameElement::SwapFrameLoaders(mozilla::dom::XULFrameElement&, mozilla::ErrorResult&) (this=0x7fffd54ee710, aOtherLoaderOwner=..., rv=...)
at /home/komat/tmp676-trunk-gtk3/src-wayland/dom/xul/XULFrameElement.cpp:120
#10 0x00007fffee3364ce in mozilla::dom::XULFrameElement_Binding::swapFrameLoaders(JSContext*, JS::Handle<JSObject*>, mozilla::dom::XULFrameElement*, JSJitMethodCallArgs const&)
(cx=0x7fffd6a2d000, obj=(JSObject * const) 0x126f5171fbe0 [object XULFrameElement], self=0x7fffd54ee710, args=...) at XULFrameElementBinding.cpp:345
#11 0x00007fffee7bfff7 in mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) (cx=0x7fffd6a2d000, argc=1, vp=0x7fffc529c150) at /home/komat/tmp676-trunk-gtk3/src-wayland/dom/bindings/BindingUtils.cpp:3181
#12 0x00007ffff295e89f in CallJSNative(JSContext*, bool ()(JSContext, unsigned int, JS::Value*), JS::CallArgs const&)
(cx=0x7fffd6a2d000, native=0x7fffee7bfd20 <mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*)>, args=...) at /home/komat/tmp676-trunk-gtk3/src-wayland/js/src/vm/Interpreter.cpp:448
#13 0x00007ffff295e2ea in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (cx=0x7fffd6a2d000, args=..., construct=js::NO_CONSTRUCT)
at /home/komat/tmp676-trunk-gtk3/src-wayland/js/src/vm/Interpreter.cpp:540

Attachment #9075307 - Attachment description: Bug 1562827 - [Wayland] When GdkWindow is moved between mozcontainer widgets, clear reference to GdkWindow from the former one, r=karlt → Bug 1562827 - [Linux] When GdkWindow is moved between mozcontainer widgets, clear reference to GdkWindow from the former one, r=karlt

The code in ReparentNativeWidgetInternal() derives from the SetParent() method. It was intended only for reparenting child windows, and there was a check to ensure that it was not used for toplevel windows. https://hg.mozilla.org/mozilla-central/rev/6bcfca597af9db7ffc2ff7440dd0de3d83e56fd6#l11.7

The purpose of nsWindow::ReparentNativeWidget() is not clearly documented, but based on the commit message at https://hg.mozilla.org/mozilla-central/rev/6bcfca597af9db7ffc2ff7440dd0de3d83e56fd6 and comments at the call site https://hg.mozilla.org/mozilla-central/rev/6bcfca597af9db7ffc2ff7440dd0de3d83e56fd6#l1.36, this is called only for toplevel widgets.

Toplevel GtkWidgets and GdkWindows do not have parents, and so the only widget adjustment required is setting the transient-for. Calling ReparentNativeWidgetInternal() is moving GdkWindows to a completely different toplevel, which is not what is intended. The GdkWindows should remain in the MozContainer for the nsIWidget (without adjustment).

To fix this, most of the changes made here to SetParent() should be reverted: https://hg.mozilla.org/mozilla-central/rev/6bcfca597af9db7ffc2ff7440dd0de3d83e56fd6#l11.31 and the call to ReparentNativeWidgetInternal() from ReparentNativeWidget() should be removed.

Regressed by: 449734
  • Make nsWindow::SetParent() to work on child windows only and use former ReparentNativeWidgetInternal() code there.
    Also update mToplevelParentWindow for Wayland to hold default toplevel window.

  • Make nsWindow::ReparentNativeWidget() to work on toplevel windows only and use only gtk_window_set_transient_for() to reparent
    a toplevel window. Also update mToplevelParentWindow for Wayland.

Attachment #9075307 - Attachment is obsolete: true

Updated, Thanks!

Hm, try does not seem to be happy, throws:
Assertion failure: mParent (nsWindow::ReparentNativeWidget() works on toplevel windows only.), at /builds/worker/workspace/build/src/widget/gtk/nsWindow.cpp:890

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=263825793&repo=try&lineNumber=10158
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7f0ababd15d42708920be3b08dc98f41275c369a&selectedJob=263825772

!mParent

(In reply to Karl Tomlinson (:karlt) from comment #11)

!mParent

Fixed, Thanks. There's a tray here:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0bffe2d9ee7ba592c902e7cf75e9cdfd7e9e9868

Keywords: checkin-needed

Pushed by btara@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/47561c3ad75b
[Linux] Merge nsWindow::ReparentNativeWidgetInternal() to nsWindow::SetParent() and use nsWindow::ReparentNativeWidget() for toplevel windows only, r=karlt

Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
You need to log in before you can comment on or make changes to this bug.