173421 2017-06-15 08:51:56 -0700 REGRESSION(r218325): [GTK] Runtime critical warnings running layout tests that show a popup menu 2017-06-15 23:03:29 -0700 1 1 1 Unclassified WebKit WebKitGTK WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED Gtk, Regression P2 Normal --- 1 cgarcia webkit-unassigned aperez berto bugs-noreply buildbot clopez gustavo mcatanzaro oldest_to_newest 1319587 0 cgarcia 2017-06-15 08:51:56 -0700 (WebKitTestRunner:15339): GLib-GObject-WARNING **: invalid cast from 'WebKitWebViewBase' to 'WebKitWebView' (WebKitTestRunner:15339): GLib-GObject-CRITICAL **: g_signal_emit_valist: assertion 'signal_id > 0' failed In r218325 I forgot to create a WebKitPopupMenu or WebPopupMenuProxyGtk depending on whether the view is a WebKitWebView or not. 1319588 1 cgarcia 2017-06-15 08:53:07 -0700 This only affect WTR. 1319589 2 312986 cgarcia 2017-06-15 08:54:22 -0700 Created attachment 312986 Patch 1319590 3 buildbot 2017-06-15 08:55:59 -0700 Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API 1319593 4 312986 aperez 2017-06-15 09:19:54 -0700 Comment on attachment 312986 Patch Informally reviewing: r+ Was this somehow not caught by the tests? From the ChangeLog description it sounds that the tests still pass despite the critical warnings. 1319626 5 312986 mcatanzaro 2017-06-15 11:23:43 -0700 Comment on attachment 312986 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=312986&action=review Any warning ought to cause tests to fail :/ but it probably doesn't work that way currently. > Source/WebKit2/UIProcess/API/gtk/PageClientImpl.cpp:211 > + if (WEBKIT_IS_WEB_VIEW(m_viewWidget)) > + return WebKitPopupMenu::create(m_viewWidget, page); > + return WebPopupMenuProxyGtk::create(m_viewWidget, page); It merits a comment to explain the difference, don't you think? 1319866 6 cgarcia 2017-06-15 23:03:29 -0700 Committed r218382: <http://trac.webkit.org/changeset/218382> 312986 2017-06-15 08:54:22 -0700 2017-06-15 09:32:07 -0700 Patch wk2-fix-popup-menu-wtr.diff text/plain 1717 cgarcia ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCBlYjE2MzcxZWU3NC4uODdlYTY3MDRkYWMgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJLaXQyL0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKQEAgLTEs NSArMSwxOCBAQAogMjAxNy0wNi0xNSAgQ2FybG9zIEdhcmNpYSBDYW1wb3MgIDxjZ2FyY2lhQGln YWxpYS5jb20+CiAKKyAgICAgICAgUkVHUkVTU0lPTihyMjE4MzI1KTogW0dUS10gUnVudGltZSBj cml0aWNhbCB3YXJuaW5ncyBydW5uaW5nIGxheW91dCB0ZXN0cyB0aGF0IHNob3cgYSBwb3B1cCBt ZW51CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNzM0 MjEKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBJbiBy MjE4MzI1IEkgZm9yZ290IHRvIGNyZWF0ZSBhIFdlYktpdFBvcHVwTWVudSBvciBXZWJQb3B1cE1l bnVQcm94eUd0ayBkZXBlbmRpbmcgb24gd2hldGhlciB0aGUgdmlldyBpcyBhCisgICAgICAgIFdl YktpdFdlYlZpZXcgb3Igbm90LgorCisgICAgICAgICogVUlQcm9jZXNzL0FQSS9ndGsvUGFnZUNs aWVudEltcGwuY3BwOgorICAgICAgICAoV2ViS2l0OjpQYWdlQ2xpZW50SW1wbDo6Y3JlYXRlUG9w dXBNZW51UHJveHkpOgorCisyMDE3LTA2LTE1ICBDYXJsb3MgR2FyY2lhIENhbXBvcyAgPGNnYXJj aWFAaWdhbGlhLmNvbT4KKwogICAgICAgICBbR1RLXSBDbGVhbnVwIGhlYWRlcnMgaW5jbHVkZXMg aW4gR1RLKyBBUEkgZmlsZXMKICAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTE3MzQxNQogCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3Mv QVBJL2d0ay9QYWdlQ2xpZW50SW1wbC5jcHAgYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvQVBJ L2d0ay9QYWdlQ2xpZW50SW1wbC5jcHAKaW5kZXggOTk4N2QzOWZkOWIuLjgzYjM5YTY5OWExIDEw MDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvQVBJL2d0ay9QYWdlQ2xpZW50SW1w bC5jcHAKKysrIGIvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL0FQSS9ndGsvUGFnZUNsaWVudElt cGwuY3BwCkBAIC0yMDYsNyArMjA2LDkgQEAgdm9pZCBQYWdlQ2xpZW50SW1wbDo6ZG9uZVdpdGhL ZXlFdmVudChjb25zdCBOYXRpdmVXZWJLZXlib2FyZEV2ZW50JiBldmVudCwgYm9vbAogCiBSZWZQ dHI8V2ViUG9wdXBNZW51UHJveHk+IFBhZ2VDbGllbnRJbXBsOjpjcmVhdGVQb3B1cE1lbnVQcm94 eShXZWJQYWdlUHJveHkmIHBhZ2UpCiB7Ci0gICAgcmV0dXJuIFdlYktpdFBvcHVwTWVudTo6Y3Jl YXRlKG1fdmlld1dpZGdldCwgcGFnZSk7CisgICAgaWYgKFdFQktJVF9JU19XRUJfVklFVyhtX3Zp ZXdXaWRnZXQpKQorICAgICAgICByZXR1cm4gV2ViS2l0UG9wdXBNZW51OjpjcmVhdGUobV92aWV3 V2lkZ2V0LCBwYWdlKTsKKyAgICByZXR1cm4gV2ViUG9wdXBNZW51UHJveHlHdGs6OmNyZWF0ZSht X3ZpZXdXaWRnZXQsIHBhZ2UpOwogfQogCiBSZWZQdHI8V2ViQ29udGV4dE1lbnVQcm94eT4gUGFn ZUNsaWVudEltcGw6OmNyZWF0ZUNvbnRleHRNZW51UHJveHkoV2ViUGFnZVByb3h5JiBwYWdlLCBj b25zdCBDb250ZXh0TWVudUNvbnRleHREYXRhJiBjb250ZXh0LCBjb25zdCBVc2VyRGF0YSYgdXNl ckRhdGEpCg==