Bug 98002 - WebPrintOperationGtk destructor should be virtual
Summary: WebPrintOperationGtk destructor should be virtual
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-01 01:51 PDT by Alberto Garcia
Modified: 2012-10-02 14:24 PDT (History)
3 users (show)

See Also:


Attachments
Patch (1.47 KB, patch)
2012-10-01 02:05 PDT, Alberto Garcia
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alberto Garcia 2012-10-01 01:51:11 PDT
In file included from ../../Source/WebKit2/WebProcess/WebPage/gtk/WebPrintOperationGtk.h:32:0,
                 from ../../Source/WebKit2/WebProcess/WebPage/gtk/WebPrintOperationGtk.cpp:27:
../../Source/WTF/wtf/RefCounted.h: In instantiation of ‘void WTF::RefCounted<T>::deref() [with T = WebKit::WebPrintOperationGtk]’:
../../Source/WebKit2/WebProcess/WebPage/gtk/WebPrintOperationGtk.cpp:158:31:   required from here
../../Source/WTF/wtf/RefCounted.h:190:13: warning: deleting object of abstract class type ‘WebKit::WebPrintOperationGtk’ which has non-virtual destructor will cause undefined behaviour [-Wdelete-non-virtual-dtor]
Comment 1 Alberto Garcia 2012-10-01 02:05:31 PDT
Created attachment 166429 [details]
Patch
Comment 2 Carlos Garcia Campos 2012-10-01 04:01:31 PDT
Comment on attachment 166429 [details]
Patch

Thanks!
Comment 3 WebKit Review Bot 2012-10-01 04:12:01 PDT
Comment on attachment 166429 [details]
Patch

Clearing flags on attachment: 166429

Committed r130028: <http://trac.webkit.org/changeset/130028>
Comment 4 WebKit Review Bot 2012-10-01 04:12:04 PDT
All reviewed patches have been landed.  Closing bug.
Comment 5 Carlos Garcia Campos 2012-10-02 08:35:52 PDT
This patch broke unit tests, see

/webkit2/WebKitPrintOperation/printing-settings: OK
/webkit2/WebKitWebView/print: OK
/webkit2/WebKitPrintOperation/print: OK
/webkit2/WebKitPrintOperation/print-errors: 
(lt-WebKitWebProcess:19919): GLib-CRITICAL **: g_io_channel_write_chars: assertion `channel->is_writeable' failed

(lt-WebKitWebProcess:19919): GLib-CRITICAL **: g_io_channel_write_chars: assertion `channel->is_writeable' failed

(lt-WebKitWebProcess:19919): GLib-CRITICAL **: g_io_channel_write_chars: assertion `channel->is_writeable' failed

(lt-WebKitWebProcess:19919): GLib-CRITICAL **: g_io_channel_write_chars: assertion `channel->is_writeable' failed
.....
Comment 6 Alberto Garcia 2012-10-02 12:50:51 PDT
(In reply to comment #5)
> This patch broke unit tests, see
>
> /webkit2/WebKitPrintOperation/printing-settings: OK
> /webkit2/WebKitWebView/print: OK
> /webkit2/WebKitPrintOperation/print: OK
> /webkit2/WebKitPrintOperation/print-errors:
> (lt-WebKitWebProcess:19919): GLib-CRITICAL **: g_io_channel_write_chars: assertion `channel->is_writeable' failed

The difference now is that the destructor of the derived class is also
called, and that means in particular that
WebPrintOperationGtkUnix::m_printJob is now being destroyed, which is
probably the source of that critical.

I'll investigate what happens.
Comment 7 Alberto Garcia 2012-10-02 14:24:16 PDT
(In reply to comment #6)
> > (lt-WebKitWebProcess:19919): GLib-CRITICAL **: g_io_channel_write_chars: assertion `channel->is_writeable' failed

So this is triggered by the "print-errors" test, in particular the
last one (the one expecting WEBKIT_PRINT_ERROR_INVALID_PAGE_RANGE).

The test runs fine and it returns the expected value, but somehow
produces that critical message when the program exists.

My bet is that there's a bug somewhere in GtkPrintJob, which is the
object that gets destroyed now that the destructor is virtual.