98002 2012-10-01 01:51:11 -0700 WebPrintOperationGtk destructor should be virtual 2012-10-02 14:24:16 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 0 berto webkit-unassigned cgarcia mrobinson webkit.review.bot oldest_to_newest 731487 0 berto 2012-10-01 01:51:11 -0700 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] 731496 1 166429 berto 2012-10-01 02:05:31 -0700 Created attachment 166429 Patch 731550 2 166429 cgarcia 2012-10-01 04:01:31 -0700 Comment on attachment 166429 Patch Thanks! 731559 3 166429 webkit.review.bot 2012-10-01 04:12:01 -0700 Comment on attachment 166429 Patch Clearing flags on attachment: 166429 Committed r130028: <http://trac.webkit.org/changeset/130028> 731560 4 webkit.review.bot 2012-10-01 04:12:04 -0700 All reviewed patches have been landed. Closing bug. 732670 5 cgarcia 2012-10-02 08:35:52 -0700 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 ..... 732972 6 berto 2012-10-02 12:50:51 -0700 (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. 733074 7 berto 2012-10-02 14:24:16 -0700 (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. 166429 2012-10-01 02:05:31 -0700 2012-10-01 04:12:01 -0700 Patch virtual.diff text/plain 1507 berto ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCA3ZGY2MjY4Li44MThkMDVjIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcg QEAKKzIwMTItMTAtMDEgIEFsYmVydG8gR2FyY2lhICA8YWdhcmNpYUBpZ2FsaWEuY29tPgorCisg ICAgICAgIFdlYlByaW50T3BlcmF0aW9uR3RrIGRlc3RydWN0b3Igc2hvdWxkIGJlIHZpcnR1YWwK KyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTk4MDAyCisK KyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgV2ViUHJpbnRP cGVyYXRpb25HdGsgaXMgYW4gYWJzdHJhY3QgY2xhc3Mgd2hpY2ggY2FuIGJlIGRlbGV0ZWQKKyAg ICAgICAgdXNpbmcgYSBwb2ludGVyIHRvIGl0c2VsZiwgeWV0IGl0cyBkZXN0cnVjdG9yIGlzIG5v dCB2aXJ0dWFsLgorICAgICAgICBGaXhlcyAtV2RlbGV0ZS1ub24tdmlydHVhbC1kdG9yLgorCisg ICAgICAgICogV2ViUHJvY2Vzcy9XZWJQYWdlL2d0ay9XZWJQcmludE9wZXJhdGlvbkd0ay5oOgor ICAgICAgICAoV2ViUHJpbnRPcGVyYXRpb25HdGspOgorCiAyMDEyLTA5LTMwICBNYXJpbyBTYW5j aGV6IFByYWRhICA8bXNhbmNoZXpAaWdhbGlhLmNvbT4KIAogICAgICAgICBbV0syXVtHVEtdIEFk ZCBBUEkgdG8gZ2V0IHRoZSBmYXZpY29uIGZvciBhIFdlYktpdFdlYlZpZXcKZGlmZiAtLWdpdCBh L1NvdXJjZS9XZWJLaXQyL1dlYlByb2Nlc3MvV2ViUGFnZS9ndGsvV2ViUHJpbnRPcGVyYXRpb25H dGsuaCBiL1NvdXJjZS9XZWJLaXQyL1dlYlByb2Nlc3MvV2ViUGFnZS9ndGsvV2ViUHJpbnRPcGVy YXRpb25HdGsuaAppbmRleCAzZDFmMTA4Li4zOTU5MzA1IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2Vi S2l0Mi9XZWJQcm9jZXNzL1dlYlBhZ2UvZ3RrL1dlYlByaW50T3BlcmF0aW9uR3RrLmgKKysrIGIv U291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9XZWJQYWdlL2d0ay9XZWJQcmludE9wZXJhdGlvbkd0 ay5oCkBAIC00OCw3ICs0OCw3IEBAIGNsYXNzIFdlYlBhZ2U7CiBjbGFzcyBXZWJQcmludE9wZXJh dGlvbkd0ayA6IHB1YmxpYyBSZWZDb3VudGVkPFdlYlByaW50T3BlcmF0aW9uR3RrPiB7CiBwdWJs aWM6CiAgICAgc3RhdGljIFBhc3NSZWZQdHI8V2ViUHJpbnRPcGVyYXRpb25HdGs+IGNyZWF0ZShX ZWJQYWdlKiwgY29uc3QgUHJpbnRJbmZvJik7Ci0gICAgfldlYlByaW50T3BlcmF0aW9uR3RrKCk7 CisgICAgdmlydHVhbCB+V2ViUHJpbnRPcGVyYXRpb25HdGsoKTsKIAogICAgIFdlYkNvcmU6OlBy aW50Q29udGV4dCogcHJpbnRDb250ZXh0KCkgY29uc3QgeyByZXR1cm4gbV9wcmludENvbnRleHQ7 IH0KICAgICBHdGtQcmludFNldHRpbmdzKiBwcmludFNldHRpbmdzKCkgY29uc3QgeyByZXR1cm4g bV9wcmludFNldHRpbmdzLmdldCgpOyB9Cg==