Bug 198564 - [WPE][GTK] Clean up use of initiatingPageID in implementation of webkit_uri_scheme_request_get_web_view()
Summary: [WPE][GTK] Clean up use of initiatingPageID in implementation of webkit_uri_s...
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: PC Linux
: P2 Normal
Assignee: Michael Catanzaro
Depends on:
Reported: 2019-06-05 07:21 PDT by Michael Catanzaro
Modified: 2019-06-06 06:51 PDT (History)
7 users (show)

See Also:

Patch (2.53 KB, patch)
2019-06-05 07:52 PDT, Michael Catanzaro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Catanzaro 2019-06-05 07:21:57 PDT
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.
Comment 1 Michael Catanzaro 2019-06-05 07:52:41 PDT
Created attachment 371394 [details]
Comment 2 EWS Watchlist 2019-06-05 07:54:22 PDT
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
Comment 3 WebKit Commit Bot 2019-06-06 06:51:19 PDT
Comment on attachment 371394 [details]

Clearing flags on attachment: 371394

Committed r246152: <https://trac.webkit.org/changeset/246152>
Comment 4 WebKit Commit Bot 2019-06-06 06:51:21 PDT
All reviewed patches have been landed.  Closing bug.