198564 2019-06-05 07:21:57 -0700 [WPE][GTK] Clean up use of initiatingPageID in implementation of webkit_uri_scheme_request_get_web_view() 2019-06-06 06:51:21 -0700 1 1 1 Unclassified WebKit WebKitGTK WebKit Nightly Build PC Linux RESOLVED FIXED P2 Normal --- 1 mcatanzaro mcatanzaro berto bugs-noreply cgarcia commit-queue ews-watchlist gustavo mcatanzaro oldest_to_newest 1541965 0 mcatanzaro 2019-06-05 07:21:57 -0700 webkit_uri_scheme_request_get_web_view() currently has illegal code to return NULL: WebKitWebView* webkit_uri_scheme_request_get_web_view(WebKitURISchemeRequest* request) { g_return_val_if_fail(WEBKIT_IS_URI_SCHEME_REQUEST(request), 0); // FIXME: initiatingPage is now always null, we need to re-implement this somehow. return request->priv->initiatingPage ? webkitWebContextGetWebViewForPage(request->priv->webContext, request->priv->initiatingPage.get()) : nullptr; } This is illegal because the return value is not nullable. Since bug #198485, this code can no longer be reached, because for initiatingPage to be unset, we would have to dereference a disengaged Optional resourceRequest.initiatingPageID() in webkitURISchemeRequestCreate. But Chris points out that it was probably already unreachable because, prior to bug #198485, the code would have called webPage() with 0, which "would have crashed since it would try and look up 0 in a HashMap whose key type is uint64_t." So it seems the page ID is guaranteed to be valid at this point, even though the FIXME comment indicates it is never valid. Probably makes sense to replace all this with an ASSERT. I don't plan to investigate this further. 1541971 1 371394 mcatanzaro 2019-06-05 07:52:41 -0700 Created attachment 371394 Patch 1541973 2 ews-watchlist 2019-06-05 07:54:22 -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 1542384 3 371394 commit-queue 2019-06-06 06:51:19 -0700 Comment on attachment 371394 Patch Clearing flags on attachment: 371394 Committed r246152: <https://trac.webkit.org/changeset/246152> 1542385 4 commit-queue 2019-06-06 06:51:21 -0700 All reviewed patches have been landed. Closing bug. 371394 2019-06-05 07:52:41 -0700 2019-06-06 06:51:19 -0700 Patch bug-198564-20190605095240.patch text/plain 2587 mcatanzaro U3VidmVyc2lvbiBSZXZpc2lvbjogMjQ2MTA0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IDQ0ZWI3N2ZhYTJhZjhmNjUw M2ZkNzFkNTA1YjczZmMyYTUxNzc2YTcuLjI0MzZlNDVhZTVhYmM5ZjgwNjZhNDc5NTM4ZTljZDRh YTRhOGJiNzIgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTktMDYtMDUgIE1pY2hhZWwg Q2F0YW56YXJvICA8bWNhdGFuemFyb0BpZ2FsaWEuY29tPgorCisgICAgICAgIFtXUEVdW0dUS10g Q2xlYW4gdXAgdXNlIG9mIGluaXRpYXRpbmdQYWdlSUQgaW4gaW1wbGVtZW50YXRpb24gb2Ygd2Vi a2l0X3VyaV9zY2hlbWVfcmVxdWVzdF9nZXRfd2ViX3ZpZXcoKQorICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTk4NTY0CisKKyAgICAgICAgUmV2aWV3ZWQg YnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTm90ZSB0aGF0IHdlIGhhdmUgdG8gdXNlIFJl ZlB0ciByYXRoZXIgdGhhbiBSZWYgaGVyZSBiZWNhdXNlIHRoZSBwcml2IHN0cnVjdCBoYXMgdG8g cmVtYWluCisgICAgICAgIGRlZmF1bHQtY29uc3RydWN0aWJsZS4KKworICAgICAgICAqIFVJUHJv Y2Vzcy9BUEkvZ2xpYi9XZWJLaXRVUklTY2hlbWVSZXF1ZXN0LmNwcDoKKyAgICAgICAgKHdlYmtp dFVSSVNjaGVtZVJlcXVlc3RDcmVhdGUpOgorICAgICAgICAod2Via2l0X3VyaV9zY2hlbWVfcmVx dWVzdF9nZXRfd2ViX3ZpZXcpOgorCiAyMDE5LTA2LTAxICBBbnRvaW5lIFF1aW50ICA8Z3Jhb3V0 c0BhcHBsZS5jb20+CiAKICAgICAgICAgW1BvaW50ZXIgRXZlbnRzXSBBZGQgc3VwcG9ydCBmb3Ig Y2hvcmRlZCBidXR0b24gaW50ZXJhY3Rpb25zCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L1VJ UHJvY2Vzcy9BUEkvZ2xpYi9XZWJLaXRVUklTY2hlbWVSZXF1ZXN0LmNwcCBiL1NvdXJjZS9XZWJL aXQvVUlQcm9jZXNzL0FQSS9nbGliL1dlYktpdFVSSVNjaGVtZVJlcXVlc3QuY3BwCmluZGV4IGRl YmI4ZWU2ZTQ1YTE4ZmJiMTk5Zjg1NmIwMjU5MzA2M2JhZmNjOTcuLjU0YTI0MDUyNGI5MGUzYmUw ODRjYzU5ZDJjNTJhNjIwN2E2NTliNmEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvVUlQcm9j ZXNzL0FQSS9nbGliL1dlYktpdFVSSVNjaGVtZVJlcXVlc3QuY3BwCisrKyBiL1NvdXJjZS9XZWJL aXQvVUlQcm9jZXNzL0FQSS9nbGliL1dlYktpdFVSSVNjaGVtZVJlcXVlc3QuY3BwCkBAIC04NCw4 ICs4NCwxMiBAQCBXZWJLaXRVUklTY2hlbWVSZXF1ZXN0KiB3ZWJraXRVUklTY2hlbWVSZXF1ZXN0 Q3JlYXRlKHVpbnQ2NF90IHJlcXVlc3RJRCwgV2ViS2l0VwogICAgIHJlcXVlc3QtPnByaXYtPndl YkNvbnRleHQgPSB3ZWJDb250ZXh0OwogICAgIHJlcXVlc3QtPnByaXYtPm1hbmFnZXIgPSAmbWFu YWdlcjsKICAgICByZXF1ZXN0LT5wcml2LT51cmkgPSByZXNvdXJjZVJlcXVlc3QudXJsKCkuc3Ry aW5nKCkudXRmOCgpOwotICAgIHJlcXVlc3QtPnByaXYtPmluaXRpYXRpbmdQYWdlID0gV2ViUHJv Y2Vzc1Byb3h5Ojp3ZWJQYWdlKCpyZXNvdXJjZVJlcXVlc3QuaW5pdGlhdGluZ1BhZ2VJRCgpKTsK ICAgICByZXF1ZXN0LT5wcml2LT5yZXF1ZXN0SUQgPSByZXF1ZXN0SUQ7CisKKyAgICBBU1NFUlQo cmVzb3VyY2VSZXF1ZXN0LmluaXRpYXRpbmdQYWdlSUQoKSk7CisgICAgcmVxdWVzdC0+cHJpdi0+ aW5pdGlhdGluZ1BhZ2UgPSBXZWJQcm9jZXNzUHJveHk6OndlYlBhZ2UoKnJlc291cmNlUmVxdWVz dC5pbml0aWF0aW5nUGFnZUlEKCkpOworICAgIEFTU0VSVChyZXF1ZXN0LT5wcml2LT5pbml0aWF0 aW5nUGFnZSk7CisKICAgICByZXR1cm4gcmVxdWVzdDsKIH0KIApAQCAtMTY2LDggKzE3MCw3IEBA IFdlYktpdFdlYlZpZXcqIHdlYmtpdF91cmlfc2NoZW1lX3JlcXVlc3RfZ2V0X3dlYl92aWV3KFdl YktpdFVSSVNjaGVtZVJlcXVlc3QqIHJlCiB7CiAgICAgZ19yZXR1cm5fdmFsX2lmX2ZhaWwoV0VC S0lUX0lTX1VSSV9TQ0hFTUVfUkVRVUVTVChyZXF1ZXN0KSwgMCk7CiAKLSAgICAvLyBGSVhNRTog aW5pdGlhdGluZ1BhZ2UgaXMgbm93IGFsd2F5cyBudWxsLCB3ZSBuZWVkIHRvIHJlLWltcGxlbWVu dCB0aGlzIHNvbWVob3cuCi0gICAgcmV0dXJuIHJlcXVlc3QtPnByaXYtPmluaXRpYXRpbmdQYWdl ID8gd2Via2l0V2ViQ29udGV4dEdldFdlYlZpZXdGb3JQYWdlKHJlcXVlc3QtPnByaXYtPndlYkNv bnRleHQsIHJlcXVlc3QtPnByaXYtPmluaXRpYXRpbmdQYWdlLmdldCgpKSA6IG51bGxwdHI7Cisg ICAgcmV0dXJuIHdlYmtpdFdlYkNvbnRleHRHZXRXZWJWaWV3Rm9yUGFnZShyZXF1ZXN0LT5wcml2 LT53ZWJDb250ZXh0LCByZXF1ZXN0LT5wcml2LT5pbml0aWF0aW5nUGFnZS5nZXQoKSk7CiB9CiAK IHN0YXRpYyB2b2lkIHdlYmtpdFVSSVNjaGVtZVJlcXVlc3RSZWFkQ2FsbGJhY2soR0lucHV0U3Ry ZWFtKiBpbnB1dFN0cmVhbSwgR0FzeW5jUmVzdWx0KiByZXN1bHQsIFdlYktpdFVSSVNjaGVtZVJl cXVlc3QqIHNjaGVtZVJlcXVlc3QpCg==