Bug 198564

Summary: [WPE][GTK] Clean up use of initiatingPageID in implementation of webkit_uri_scheme_request_get_web_view()
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: WebKitGTKAssignee: Michael Catanzaro <mcatanzaro>
Status: RESOLVED FIXED    
Severity: Normal CC: berto, bugs-noreply, cgarcia, commit-queue, ews-watchlist, gustavo, mcatanzaro
Priority: P2    
Version: WebKit Nightly Build   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Patch none

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]
Patch
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]
Patch

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.