RESOLVED FIXED 32900
[GTK] Crashes cleaning clipboard contents
https://bugs.webkit.org/show_bug.cgi?id=32900
Summary [GTK] Crashes cleaning clipboard contents
Gustavo Noronha (kov)
Reported 2009-12-23 06:45:02 PST
It looks like, with page cache enabled, the destruction of the EditorClient being delayed may end up with it crashing. That is because a callback is being called late - after the destruction of the client.
Attachments
stack trace (12.60 KB, text/plain)
2009-12-23 07:01 PST, Gustavo Noronha (kov)
no flags
Make EditorClientGtk refcounted, and use explict refs to protect it for the callbacks (4.55 KB, patch)
2009-12-23 07:03 PST, Gustavo Noronha (kov)
gustavo: commit-queue-
Null-check the focus controller, because it may be null when the callback is called, leading to crash (1.86 KB, patch)
2010-01-14 17:09 PST, Gustavo Noronha (kov)
gustavo: commit-queue-
Fix crash by passing WebView object to the clipboard functions instead of Page (5.30 KB, patch)
2010-01-16 16:28 PST, Gustavo Noronha (kov)
gustavo: commit-queue-
Fix crash by passing WebView instead of Page (4.87 KB, patch)
2010-01-17 16:51 PST, Gustavo Noronha (kov)
xan.lopez: review+
gustavo: commit-queue-
Gustavo Noronha (kov)
Comment 1 2009-12-23 07:01:51 PST
Created attachment 45436 [details] stack trace How I reproduce it: I run Epiphany, go to a page, select some text, open a new tab, close the first tab, select some new text, rinse and repeat until you get a crash.
Gustavo Noronha (kov)
Comment 2 2009-12-23 07:03:43 PST
Created attachment 45437 [details] Make EditorClientGtk refcounted, and use explict refs to protect it for the callbacks
WebKit Review Bot
Comment 3 2009-12-23 07:08:32 PST
style-queue ran check-webkit-style on attachment 45437 [details] without any errors.
Estêvão Samuel Procópio Amaral
Comment 4 2009-12-23 08:31:35 PST
Having the same problem here. Waiting for the patch to be reviewed. Anyone?!?
Duy Nguyen
Comment 5 2009-12-27 21:59:02 PST
With Gustavo's patch, I got two stack traces (probably not reproducible). The latter may due to some other bug. (gdb) bt #0 0xb719af2b in WebKit::clipboard_clear_contents_cb () from /home/pclouds/opt/webkit/lib/libwebkit-1.0.so.2 #1 0xb6156843 in clipboard_unset () from /usr/lib/libgtk-x11-2.0.so.0 #2 0xb61568d1 in selection_clear_event_cb () from /usr/lib/libgtk-x11-2.0.so.0 #3 0xb603284b in _gtk_marshal_BOOLEAN__BOXED () from /usr/lib/libgtk-x11-2.0.so.0 #4 0xb53bb2eb in g_closure_invoke () from /usr/lib/libgobject-2.0.so.0 #5 0xb53cc388 in signal_emit_unlocked_R () from /usr/lib/libgobject-2.0.so.0 #6 0xb53cd5ac in g_signal_emit_valist () from /usr/lib/libgobject-2.0.so.0 #7 0xb53cdb8f in g_signal_emit () from /usr/lib/libgobject-2.0.so.0 #8 0xb613b2b8 in gtk_widget_event_internal () from /usr/lib/libgtk-x11-2.0.so.0 #9 0xb602d304 in gtk_main_do_event () from /usr/lib/libgtk-x11-2.0.so.0 #10 0xb5ec75dd in gdk_event_dispatch () from /usr/lib/libgdk-x11-2.0.so.0 #11 0xb52ec3bf in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0 #12 0xb52eccb6 in g_main_context_iterate () from /usr/lib/libglib-2.0.so.0 #13 0xb52eceb1 in g_main_loop_run () from /usr/lib/libglib-2.0.so.0 #14 0xb602d5dc in gtk_main () from /usr/lib/libgtk-x11-2.0.so.0 #15 0x08072109 in main (argc=-2082109099, argv=0xbfc50984) at ephy-main.c:778 (gdb) bt #0 0x00610065 in ?? () #1 0xb751a138 in WebCore::Editor::clientIsEditable () from /home/pclouds/opt/webkit/lib/libwebkit-1.0.so.2 #2 0xb76b7288 in WebCore::Frame::isContentEditable () from /home/pclouds/opt/webkit/lib/libwebkit-1.0.so.2 #3 0xb75a9138 in WebCore::HTMLElement::isContentEditable () from /home/pclouds/opt/webkit/lib/libwebkit-1.0.so.2 #4 0xb74bfded in WebCore::Node::isContentEditable () from /home/pclouds/opt/webkit/lib/libwebkit-1.0.so.2 #5 0xb74c0942 in WebCore::Node::rootEditableElement () from /home/pclouds/opt/webkit/lib/libwebkit-1.0.so.2 #6 0xb7780b89 in WebCore::RenderBlock::isSelectionRoot () from /home/pclouds/opt/webkit/lib/libwebkit-1.0.so.2 #7 0xb7780c68 in WebCore::RenderBlock::shouldPaintSelectionGaps () from /home/pclouds/opt/webkit/lib/libwebkit-1.0.so.2 #8 0xb778cfda in WebCore::RenderBlock::paintSelection () from /home/pclouds/opt/webkit/lib/libwebkit-1.0.so.2 #9 0xb778d2fe in WebCore::RenderBlock::paintObject () from /home/pclouds/opt/webkit/lib/libwebkit-1.0.so.2 #10 0xb777fbbd in WebCore::RenderBlock::paint () from /home/pclouds/opt/webkit/lib/libwebkit-1.0.so.2 #11 0xb778688d in WebCore::RenderBlock::paintChildren () from /home/pclouds/opt/webkit/lib/libwebkit-1.0.so.2 #12 0xb7786a26 in WebCore::RenderBlock::paintContents () from /home/pclouds/opt/webkit/lib/libwebkit-1.0.so.2 #13 0xb778d1f3 in WebCore::RenderBlock::paintObject () from /home/pclouds/opt/webkit/lib/libwebkit-1.0.so.2 #14 0xb777fbbd in WebCore::RenderBlock::paint () from /home/pclouds/opt/webkit/lib/libwebkit-1.0.so.2 #15 0xb778688d in WebCore::RenderBlock::paintChildren () from /home/pclouds/opt/webkit/lib/libwebkit-1.0.so.2 #16 0xb7786a26 in WebCore::RenderBlock::paintContents () from /home/pclouds/opt/webkit/lib/libwebkit-1.0.so.2 #17 0xb778d1f3 in WebCore::RenderBlock::paintObject () from /home/pclouds/opt/webkit/lib/libwebkit-1.0.so.2 #18 0xb777fbbd in WebCore::RenderBlock::paint () from /home/pclouds/opt/webkit/lib/libwebkit-1.0.so.2 #19 0xb77d781c in WebCore::RenderLayer::paintLayer () from /home/pclouds/opt/webkit/lib/libwebkit-1.0.so.2 #20 0xb77d7921 in WebCore::RenderLayer::paintLayer () from /home/pclouds/opt/webkit/lib/libwebkit-1.0.so.2 #21 0xb77d81cb in WebCore::RenderLayer::paint () from /home/pclouds/opt/webkit/lib/libwebkit-1.0.so.2 #22 0xb76c6265 in WebCore::FrameView::paintContents () from /home/pclouds/opt/webkit/lib/libwebkit-1.0.so.2 #23 0xb770bbde in WebCore::ScrollView::paint () from /home/pclouds/opt/webkit/lib/libwebkit-1.0.so.2 #24 0xb71b780e in webkit_web_view_expose_event () from /home/pclouds/opt/webkit/lib/libwebkit-1.0.so.2 #25 0xb602c84b in _gtk_marshal_BOOLEAN__BOXED () from /usr/lib/libgtk-x11-2.0.so.0 #26 0xb53b4495 in g_type_class_meta_marshal () from /usr/lib/libgobject-2.0.so.0 #27 0xb53b62eb in g_closure_invoke () from /usr/lib/libgobject-2.0.so.0 #28 0xb53c7737 in signal_emit_unlocked_R () from /usr/lib/libgobject-2.0.so.0 #29 0xb53c85ac in g_signal_emit_valist () from /usr/lib/libgobject-2.0.so.0 #30 0xb53c8b8f in g_signal_emit () from /usr/lib/libgobject-2.0.so.0 #31 0xb61352b8 in gtk_widget_event_internal () from /usr/lib/libgtk-x11-2.0.so.0 #32 0xb60273a9 in gtk_main_do_event () from /usr/lib/libgtk-x11-2.0.so.0 #33 0xb5eaa41c in _gdk_window_process_updates_recurse () from /usr/lib/libgdk-x11-2.0.so.0 #34 0xb5eaa3ce in _gdk_window_process_updates_recurse () from /usr/lib/libgdk-x11-2.0.so.0 #35 0xb5ecf944 in _gdk_windowing_window_process_updates_recurse () from /usr/lib/libgdk-x11-2.0.so.0 #36 0xb5ea6580 in gdk_window_process_updates_internal () from /usr/lib/libgdk-x11-2.0.so.0 #37 0xb5ea8745 in gdk_window_process_all_updates () from /usr/lib/libgdk-x11-2.0.so.0 #38 0xb5ea8764 in gdk_window_update_idle () from /usr/lib/libgdk-x11-2.0.so.0 #39 0xb5e87e1a in gdk_threads_dispatch () from /usr/lib/libgdk-x11-2.0.so.0 #40 0xb52e30c2 in g_idle_dispatch () from /usr/lib/libglib-2.0.so.0 #41 0xb52e73bf in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0 #42 0xb52e7cb6 in g_main_context_iterate () from /usr/lib/libglib-2.0.so.0 #43 0xb52e7eb1 in g_main_loop_run () from /usr/lib/libglib-2.0.so.0 #44 0xb60275dc in gtk_main () from /usr/lib/libgtk-x11-2.0.so.0 #45 0x08072109 in main (argc=159697152, argv=0xbf8dd694) at ephy-main.c:778
Gustavo Noronha (kov)
Comment 6 2009-12-29 11:30:24 PST
Just a heads up - Martin Robinson has a patch to improve DND, and it essentially replaces/moves the code I am touching in my patch. See https://bugs.webkit.org/show_bug.cgi?id=30623
Eric Seidel (no email)
Comment 7 2010-01-05 13:45:01 PST
Should this still be up for review then?
Gustavo Noronha (kov)
Comment 8 2010-01-06 04:47:01 PST
Comment on attachment 45437 [details] Make EditorClientGtk refcounted, and use explict refs to protect it for the callbacks Removed from review queue for now.
Diego Escalante Urrelo
Comment 9 2010-01-13 19:52:00 PST
Happens also if you select text somewhere else (other apps). Fun :)
Gustavo Noronha (kov)
Comment 10 2010-01-14 17:09:19 PST
Created attachment 46621 [details] Null-check the focus controller, because it may be null when the callback is called, leading to crash ok, I found what's wrong with Martin's approach, it seems; wasn't able to crash Epiphany anymore with this
Eric Seidel (no email)
Comment 11 2010-01-14 17:11:53 PST
Comment on attachment 46621 [details] Null-check the focus controller, because it may be null when the callback is called, leading to crash OK.
Gustavo Noronha (kov)
Comment 12 2010-01-14 17:27:32 PST
Patch landed as 53304.
Gustavo Noronha (kov)
Comment 13 2010-01-15 07:52:58 PST
*** Bug 33387 has been marked as a duplicate of this bug. ***
Sebastian Dröge (slomo)
Comment 14 2010-01-16 05:01:55 PST
Unfortunately this is not enough, still crashes for me with latest trunk of epiphany and webkit: #0 0x00007ffff4fa2353 in WebKit::clearClipboardContentsCallback(_GtkClipboard*, void*) () from /usr/local/lib/libwebkit-1.0.so.2 #1 0x00007ffff3f9438a in clipboard_unset (clipboard=0x813ba0) at /gtk+2.0-2.19.3/gtk/gtkclipboard.c:693 #2 0x00007ffff3f943fc in selection_clear_event_cb ( widget=<value optimized out>, event=<value optimized out>) at /gtk+2.0-2.19.3/gtk/gtkclipboard.c:349 #3 0x00007ffff3e483d3 in _gtk_marshal_BOOLEAN__BOXED (closure=0x85fa40, return_value=0x7fffffffcbe0, n_param_values=<value optimized out>, param_values=0xef8840, invocation_hint=<value optimized out>, marshal_data=0x7ffff3f943e0) at /gtk+2.0-2.19.3/gtk/gtkmarshalers.c:84 #4 0x00007ffff1e717ce in IA__g_closure_invoke (closure=0x85fa40, return_value=0x7fffffffcbe0, n_param_values=2, param_values=0xef8840, invocation_hint=0x7fffffffcba0) at /build/buildd/glib2.0-2.23.1/gobject/gclosure.c:767 #5 0x00007ffff1e86776 in signal_emit_unlocked_R (node=0x73f600, detail=<value optimized out>, instance=<value optimized out>, emission_return=<value optimized out>, instance_and_params=<value optimized out>) at /build/buildd/glib2.0-2.23.1/gobject/gsignal.c:3247 #6 0x00007ffff1e87a7e in IA__g_signal_emit_valist (instance=0x82f070, signal_id=<value optimized out>, detail=0, var_args=0x7fffffffcd90) ---Type <return> to continue, or q <return> to quit--- at /build/buildd/glib2.0-2.23.1/gobject/gsignal.c:2990 #7 0x00007ffff1e88453 in IA__g_signal_emit (instance=0x3f00000040, signal_id=4158854264, detail=3792310656) at /build/buildd/glib2.0-2.23.1/gobject/gsignal.c:3037 #8 0x00007ffff3f75d0f in gtk_widget_event_internal (widget=0x82f070, event=0x12f8920) at /gtk+2.0-2.19.3/gtk/gtkwidget.c:4941 #9 0x00007ffff3e403a5 in IA__gtk_main_do_event (event=0x12f8920) at /gtk+2.0-2.19.3/gtk/gtkmain.c:1601 #10 0x00007ffff384142c in gdk_event_dispatch (source=<value optimized out>, callback=<value optimized out>, user_data=<value optimized out>) at /gtk+2.0-2.19.3/gdk/x11/gdkevents-x11.c:2372 #11 0x00007ffff13c2a7e in g_main_dispatch (context=0x71b770) at /build/buildd/glib2.0-2.23.1/glib/gmain.c:1960 #12 IA__g_main_context_dispatch (context=0x71b770) at /build/buildd/glib2.0-2.23.1/glib/gmain.c:2513 #13 0x00007ffff13c6438 in g_main_context_iterate (context=0x71b770, block=<value optimized out>, dispatch=<value optimized out>, self=<value optimized out>) at /build/buildd/glib2.0-2.23.1/glib/gmain.c:2591 #14 0x00007ffff13c6895 in IA__g_main_loop_run (loop=0x7a54e0) at /build/buildd/glib2.0-2.23.1/glib/gmain.c:2799 #15 0x00007ffff3e40777 in IA__gtk_main () at /gtk+2.0-2.19.3/gtk/gtkmain.c:1219 #16 0x0000000000435b33 in main (argc=1, argv=0x7fffffffe2c8) at ephy-main.c:739
Estêvão Samuel Procópio Amaral
Comment 15 2010-01-16 10:09:02 PST
Well... sure it's more stable with this patch. But I'm still getting some random segfaults and I think it's still related to clipboard. Here is a full bt: Program received signal SIGSEGV, Segmentation fault. 0xb5d609ff in WTF::RefCountedBase::derefBase (this=0x670072) at ./JavaScriptCore/wtf/RefCounted.h:67 67 ASSERT(!m_deletionHasBegun); Current language: auto The current source language is "auto; currently c++". (gdb) bt full #0 0xb5d609ff in WTF::RefCountedBase::derefBase (this=0x670072) at ./JavaScriptCore/wtf/RefCounted.h:67 __PRETTY_FUNCTION__ = "bool WTF::RefCountedBase::derefBase()" #1 0xb5d744d7 in WTF::RefCounted<WebCore::Range>::deref (this=0x670072) at ./JavaScriptCore/wtf/RefCounted.h:108 No locals. #2 0xb660bfa8 in WTF::RefPtr<WebCore::Range>::operator= (this=0xbcd777c, optr=0x0) at ./JavaScriptCore/wtf/RefPtr.h:132 ptr = 0x670072 #3 0xb660a058 in WebKit::clipboard_clear_contents_cb (clipboard=0x82304b8, data=0xbcd7748) at WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp:209 client = 0xbcd7748 #4 0xb5a9dfef in ?? () from /usr/lib/libgtk-x11-2.0.so.0 No symbol table info available. #5 0xb5a9e046 in ?? () from /usr/lib/libgtk-x11-2.0.so.0 No symbol table info available. #6 0xb593b322 in ?? () from /usr/lib/libgtk-x11-2.0.so.0 No symbol table info available. #7 0xb5447f62 in g_closure_invoke () from /usr/lib/libgobject-2.0.so.0 No symbol table info available. #8 0xb545c3a8 in ?? () from /usr/lib/libgobject-2.0.so.0 No symbol table info available. #9 0xb545d5b8 in g_signal_emit_valist () from /usr/lib/libgobject-2.0.so.0 No symbol table info available. #10 0xb545dba6 in g_signal_emit () from /usr/lib/libgobject-2.0.so.0 No symbol table info available. #11 0xb5a7d1be in ?? () from /usr/lib/libgtk-x11-2.0.so.0 No symbol table info available. #12 0xb593408f in gtk_main_do_event () from /usr/lib/libgtk-x11-2.0.so.0 No symbol table info available. #13 0xb5757b2a in ?? () from /usr/lib/libgdk-x11-2.0.so.0 No symbol table info available. #14 0xb53a7b38 in g_main_context_dispatch () from /lib/libglib-2.0.so.0 No symbol table info available. #15 0xb53ab3d0 in ?? () from /lib/libglib-2.0.so.0 No symbol table info available. #16 0xb53ab83f in g_main_loop_run () from /lib/libglib-2.0.so.0 No symbol table info available. #17 0xb5934539 in gtk_main () from /usr/lib/libgtk-x11-2.0.so.0 No symbol table info available. #18 0x0806f7bb in main (argc=1, argv=0xbfffefd4) at ephy-main.c:739 option_context = <value optimized out> option_group = <value optimized out> proxy = <value optimized out> error = 0x0 user_time = 65083348
Danilo Šegan
Comment 16 2010-01-16 10:36:54 PST
Yeah, it's definitely more stable. But I still get a different stacktrace myself with webkit r53331 from yesterday or so: #0 clearClipboardContentsCallback (clipboard=<value optimized out>, data=0x7f5b62f03b40) at ../WebKit/gtk/WebCoreSupport/PasteboardHelperGtk.cpp:137 frame = 0x3f00000040 dataObject = <value optimized out> #1 0x00007f5b85a5d8ea in ?? () from /usr/lib/libgtk-x11-2.0.so.0 No symbol table info available. #2 0x00007f5b85a5eecd in ?? () from /usr/lib/libgtk-x11-2.0.so.0 No symbol table info available. #3 0x00007f5b85a5f0b8 in gtk_clipboard_set_with_owner () from /usr/lib/libgtk-x11-2.0.so.0 No symbol table info available. #4 0x00007f5b858bde9c in ?? () from /usr/lib/libgtk-x11-2.0.so.0 No symbol table info available. #5 0x00000000004840ea in ephy_location_entry_activate (entry=<value optimized out>) at /build/buildd/epiphany-browser-2.29.5/lib/widgets/ephy-location-entry.c:1510 priv = 0x15a0af0 toplevel = 0x213a480 #6 0x000000000043b061 in ephy_toolbar_activate_location (toolbar=0x1c11180) at /build/buildd/epiphany-browser-2.29.5/src/ephy-toolbar.c:455 proxies = <value optimized out> entry = <value optimized out> visible = 1 #7 0x00007f5b8472d5ae in g_closure_invoke () from /usr/lib/libgobject-2.0.so.0 No symbol table info available. #8 0x00007f5b84742983 in ?? () from /usr/lib/libgobject-2.0.so.0 No symbol table info available. #9 0x00007f5b84743d39 in g_signal_emit_valist () from /usr/lib/libgobject-2.0.so.0 No symbol table info available. #10 0x00007f5b84744283 in g_signal_emit () from /usr/lib/libgobject-2.0.so.0 No symbol table info available. #11 0x00007f5b85874d13 in ?? () from /usr/lib/libgtk-x11-2.0.so.0 No symbol table info available. #12 0x00007f5b858772e9 in ?? () from /usr/lib/libgtk-x11-2.0.so.0 No symbol table info available. #13 0x00007f5b8472d5ae in g_closure_invoke () from /usr/lib/libgobject-2.0.so.0 No symbol table info available. #14 0x00007f5b84742983 in ?? () from /usr/lib/libgobject-2.0.so.0 No symbol table info available. #15 0x00007f5b84743bcc in g_signal_emit_valist () from /usr/lib/libgobject-2.0.so.0 No symbol table info available. #16 0x00007f5b84744283 in g_signal_emit () from /usr/lib/libgobject-2.0.so.0 No symbol table info available. #17 0x00007f5b85871094 in gtk_accel_group_activate () from /usr/lib/libgtk-x11-2.0.so.0 No symbol table info available. #18 0x00007f5b8587119d in gtk_accel_groups_activate () from /usr/lib/libgtk-x11-2.0.so.0 No symbol table info available. #19 0x00007f5b85a59113 in gtk_window_activate_key () from /usr/lib/libgtk-x11-2.0.so.0 No symbol table info available. #20 0x00007f5b85a59199 in ?? () from /usr/lib/libgtk-x11-2.0.so.0 No symbol table info available. #21 0x00007f5b8593c728 in ?? () from /usr/lib/libgtk-x11-2.0.so.0 No symbol table info available. #22 0x00007f5b8472d5ae in g_closure_invoke () from /usr/lib/libgobject-2.0.so.0 No symbol table info available. #23 0x00007f5b8474264d in ?? () from /usr/lib/libgobject-2.0.so.0 No symbol table info available. #24 0x00007f5b84743bcc in g_signal_emit_valist () from /usr/lib/libgobject-2.0.so.0 No symbol table info available. #25 0x00007f5b84744283 in g_signal_emit () from /usr/lib/libgobject-2.0.so.0 No symbol table info available. #26 0x00007f5b85a4371f in ?? () from /usr/lib/libgtk-x11-2.0.so.0 No symbol table info available. #27 0x00007f5b85934da4 in gtk_propagate_event () from /usr/lib/libgtk-x11-2.0.so.0 No symbol table info available. #28 0x00007f5b85935ca3 in gtk_main_do_event () from /usr/lib/libgtk-x11-2.0.so.0 No symbol table info available. #29 0x00007f5b85355cdc in ?? () from /usr/lib/libgdk-x11-2.0.so.0 No symbol table info available. #30 0x00007f5b8408bbce in g_main_context_dispatch () from /lib/libglib-2.0.so.0 No symbol table info available. #31 0x00007f5b8408f598 in ?? () from /lib/libglib-2.0.so.0 No symbol table info available. #32 0x00007f5b8408f9f5 in g_main_loop_run () from /lib/libglib-2.0.so.0 No symbol table info available. #33 0x00007f5b85936177 in gtk_main () from /usr/lib/libgtk-x11-2.0.so.0 No symbol table info available. #34 0x0000000000435203 in main (argc=1, argv=0x7fff6214c698) at /build/buildd/epiphany-browser-2.29.5/src/ephy-main.c:741 option_context = <value optimized out> option_group = <value optimized out> proxy = <value optimized out> error = 0x0 user_time = 196643610 (excuse my lack of symbols in gtk+ and glib libraries)
Gustavo Noronha (kov)
Comment 17 2010-01-16 13:03:05 PST
I posted a patch I think fixes these crashes to https://bugs.webkit.org/show_bug.cgi?id=33746.
Gustavo Noronha (kov)
Comment 18 2010-01-16 16:25:08 PST
*** Bug 33746 has been marked as a duplicate of this bug. ***
Gustavo Noronha (kov)
Comment 19 2010-01-16 16:28:28 PST
Created attachment 46753 [details] Fix crash by passing WebView object to the clipboard functions instead of Page This is the same patch I posted to 33746 (which I marked as duplicate to this one) with a couple changes: 1. adopting the recommendation of mrobinson about checking if data is NULL, because in the near future it might be (for non-X11 primary clipboards) 2. check the return of set_with_data, and unref the webView if the call failed, to avoid leaking Martin also raised the issue of the webView being unrefed in the get function - my original patch did that as well, and I couldn't see how it would be called twice, or in addition to clear yet (by looking at the GTK+ code), but I may be wrong.
Sebastian Dröge (slomo)
Comment 20 2010-01-17 00:11:09 PST
Works good so far but I guess it needs some longer testing if it really fixes all clipboard issues :)
Martin Robinson
Comment 21 2010-01-17 09:16:53 PST
(In reply to comment #19) > Martin also raised the issue of the webView being unrefed in the get function - > my original patch did that as well, and I couldn't see how it would be called > twice, or in addition to clear yet (by looking at the GTK+ code), but I may be > wrong. The get function is actually called every time you middle button paste the contents of the clipboard. So if you middle button paste a bunch of times you'll actually see this cause critical warnings like: (GtkLauncher:13194): GLib-GObject-CRITICAL **: g_object_unref: assertion `G_IS_OBJECT (object)' failed The only other change I would make to this patch is to change the order of these lines in PasteboardHelperGtk.cpp: > ASSERT(dataObject); > > // This will be true for clipboards other than X11 primary. > if (!data) > return; > > dataObject->clear(); In the future, we'll want dataObject->clear() to happen regardless of whether or not this is an X11 primary clipboard (data != NULL). Looks great otherwise!
Gustavo Noronha (kov)
Comment 22 2010-01-17 16:48:26 PST
(In reply to comment #21) > (In reply to comment #19) > > Martin also raised the issue of the webView being unrefed in the get function - > > my original patch did that as well, and I couldn't see how it would be called > > twice, or in addition to clear yet (by looking at the GTK+ code), but I may be > > wrong. > > The get function is actually called every time you middle button paste the > contents of the clipboard. So if you middle button paste a bunch of times > you'll actually see this cause critical warnings like: > > (GtkLauncher:13194): GLib-GObject-CRITICAL **: g_object_unref: assertion > `G_IS_OBJECT (object)' failed I didn't get those here. Is it certain that clear will happen, though? > > ASSERT(dataObject); > > > > // This will be true for clipboards other than X11 primary. > > if (!data) > > return; > > > > dataObject->clear(); > > In the future, we'll want dataObject->clear() to happen regardless of whether > or not this is an X11 primary clipboard (data != NULL). > > Looks great otherwise! OK!
Gustavo Noronha (kov)
Comment 23 2010-01-17 16:51:10 PST
Created attachment 46772 [details] Fix crash by passing WebView instead of Page OK, this addresses the comments Martin made about the patch.
Sebastian Dröge (slomo)
Comment 24 2010-01-17 22:25:13 PST
(In reply to comment #22) > (In reply to comment #21) > > (In reply to comment #19) > > > Martin also raised the issue of the webView being unrefed in the get function - > > > my original patch did that as well, and I couldn't see how it would be called > > > twice, or in addition to clear yet (by looking at the GTK+ code), but I may be > > > wrong. > > > > The get function is actually called every time you middle button paste the > > contents of the clipboard. So if you middle button paste a bunch of times > > you'll actually see this cause critical warnings like: > > > > (GtkLauncher:13194): GLib-GObject-CRITICAL **: g_object_unref: assertion > > `G_IS_OBJECT (object)' failed > > I didn't get those here. Is it certain that clear will happen, though? From my understanding of the docs the clear function will only ever by called once and will always be called if the clipboard content changes. The get function can (and will) be called multiple times.
Sebastian Dröge (slomo)
Comment 25 2010-01-19 02:54:01 PST
*** Bug 33422 has been marked as a duplicate of this bug. ***
Sebastian Dröge (slomo)
Comment 26 2010-01-19 02:54:11 PST
*** Bug 33421 has been marked as a duplicate of this bug. ***
Xan Lopez
Comment 27 2010-01-19 04:40:08 PST
Comment on attachment 46772 [details] Fix crash by passing WebView instead of Page I think the cool thing to do these days would be to use a GOwnPtr in the clear callback for the view? Looks fine to me otherwise, feel free to commit either way.
Gustavo Noronha (kov)
Comment 28 2010-01-19 11:11:58 PST
Landed as r53477.
Note You need to log in before you can comment on or make changes to this bug.