Registering a custom URI scheme handler does not allow setting HTTP response headers, as there is no API in WebKitURISchemeRequest which would allow such a thing. Supporting this would allow, for example, setting `Access-Control-Allow-Origin` to allow CORS requests in pages generated by a handler.
Created attachment 381610 [details] WIP Patch
Motivation for this API addition: https://lists.webkit.org/pipermail/webkit-wpe/2019-May/000156.html
Created attachment 381614 [details] WIP Patch v2
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 on attachment 381614 [details] WIP Patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=381614&action=review How does this solves the problem of allowing CORS? This should include a unit test for that case. My main concern here is that this introduces HTTP headers for a non-HTTP load, it's weird. > Source/WebKit/UIProcess/API/glib/WebKitURISchemeRequest.cpp:69 > + GRefPtr<SoupMessageHeaders> headers { adoptGRef(soup_message_headers_new(SOUP_MESSAGE_HEADERS_RESPONSE)) }; SoupMessageHeaders is not a GObject, you need to include <WebCore/GUniquePtrSoup.h> and use GUniquePtr instead. I prefer to use this initialization here for simple fixed values, so I would move this to webkitURISchemeRequestCreate(). > Source/WebKit/UIProcess/API/glib/WebKitURISchemeRequest.cpp:165 > + * webkit_uri_scheme_request_get_headers: request_get_headers sounds like it's returning the request headers. I would use webkit_uri_scheme_request_get_response_headers(). > Source/WebKit/UIProcess/API/glib/WebKitURISchemeRequest.cpp:227 > + * Note that passing values for @stream_length and @content_type other than -1 > + * and %NULL will override the corresponding headers which may have been > + * previously set in the #SoupMessageHeaders object returned by > + * webkit_uri_scheme_request_get_headers(). This is what makes me doubt about this API. Maybe it would be better to add webkit_uri_scheme_request_finish_with_headers() or something similar.
Thanks for reviewing the WIP patch, Carlos! The main intention was to have an initial pass at it to start discussing how to make the functionality fit in the public API and I haven't had the time yet to test it O:-) (In reply to Carlos Garcia Campos from comment #5) > Comment on attachment 381614 [details] > WIP Patch v2 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=381614&action=review > > How does this solves the problem of allowing CORS? This should include a > unit test for that case. My main concern here is that this introduces HTTP > headers for a non-HTTP load, it's weird. Allowing to set the “sent” response headers allows defining the “Access-Control-Allow-Origin”, so for example resources loaded a custom scheme “a://” can provide: Access-Control-Allow-Origin: b://foo/ and then resources loaded from “b://foo/…” could access resources from “a://”. One thought , though: probably we would want to also allow inspecting the *request* headers and HTTP method inside the handler as well, so it can handle CORS preflight requests (which are done using the OPTIONS method). There are a few other uses which would be enabled by having access to the request/response headers that I can think of: - Handlers can set the “Location” header to cause redirects (though we would probably need to expose a way to set the HTTP status code for this to fully work). - Handlers can set “Content-Disposition” in the response headers to indicate that the generated content should be saved to a file instead of displayed inline. - While in HTML one can use <meta http-equiv=…> to set a few of the response headers, not all headers can be set that way. If the handler does not generate HTML, the only option is to set response headers. - For handlers which are computationally expensive, having access to request headers would allow checking for “If-Modified-Since” and friends. - Returning localized content by checking “Accept-Language” in the request headers. - Using ranged requests and responses (“Content-Range”, “Range”, “Accept-Ranges”). > > Source/WebKit/UIProcess/API/glib/WebKitURISchemeRequest.cpp:69 > > + GRefPtr<SoupMessageHeaders> headers { adoptGRef(soup_message_headers_new(SOUP_MESSAGE_HEADERS_RESPONSE)) }; > > SoupMessageHeaders is not a GObject, you need to include > <WebCore/GUniquePtrSoup.h> and use GUniquePtr instead. I prefer to use this > initialization here for simple fixed values, so I would move this to > webkitURISchemeRequestCreate(). Good catch, thanks. > > Source/WebKit/UIProcess/API/glib/WebKitURISchemeRequest.cpp:165 > > + * webkit_uri_scheme_request_get_headers: > > request_get_headers sounds like it's returning the request headers. I would > use webkit_uri_scheme_request_get_response_headers(). > > > Source/WebKit/UIProcess/API/glib/WebKitURISchemeRequest.cpp:227 > > + * Note that passing values for @stream_length and @content_type other than -1 > > + * and %NULL will override the corresponding headers which may have been > > + * previously set in the #SoupMessageHeaders object returned by > > + * webkit_uri_scheme_request_get_headers(). > > This is what makes me doubt about this API. Maybe it would be better to add > webkit_uri_scheme_request_finish_with_headers() or something similar. Adding a new _finish_<foo>() method also crossed my mind. I went with the getter just in case at some point we would decide to also allow setting the HTTP status code, to avoid having to add one more method like _finish_with_headers_and_status() with its name getting longer and longer. Also, it seemed like a good idea to avoid that the user of the API manually had to create the SoupMessageHeaders—what if they don't create it with SOUP_MESSAGE_HEADERS_RESPONSE? After thinking a bit more today about how to shape the API, the following sounds best in my head: * webkit_uri_scheme_request_finish_with_headers(request, stream, status, headers): Allows setting the HTTP status code and headers for the response, the content type and length have to be set beforehand in “headers”. * webkit_uri_scheme_request_get_method(): Returns the HTTP method. * webkit_uri_scheme_request_get_headers(): Returns the *request* headers. Additionally, there's a related issue, which probably can be better handled as a separate bug: it's not possible for URI scheme handlers to inspect the request URI parameters, only the path and scheme. Maybe the following would make sense: * Add webkit_uri_scheme_get_uri(): Returns a SoupURI for the requested URI. * Deprecate webkit_uri_scheme_get_path() and webkit_uri_scheme_get_scheme(), which are unneeded because the requested URI already carries the scheme and path. Thoughts?
The main problem is that custom uri loads are not HTTP, so all those HTTP features don't really fit here. I would need to understand the initial CORS issue that motivated this, because maybe it can be solved by using the current WebKitSecurityManager API, maybe registering the scheme as local and then allow access to file URIs or something like that.
(In reply to Carlos Garcia Campos from comment #7) > The main problem is that custom uri loads are not HTTP, so all those HTTP > features don't really fit here. I would need to understand the initial CORS > issue that motivated this, because maybe it can be solved by using the > current WebKitSecurityManager API, maybe registering the scheme as local and > then allow access to file URIs or something like that. I disagree with this sentiment. The whole purpose of URI scheme handlers is to be able to provide content to a web view as if it came from the network, and to make that content behave (and believe) that it has been fetched from the network. Content loaded through an URI scheme handler most of the time is regular web content (HTML, CSS, JS, etc.) that happens to be provided to the web view using a transport other than HTTP(S)—be it local files, or any other transport—and as such is developed using the same tools and libraries used for all other web content… which means some HTTP-isms are to be expected. I think there is no way around this. While I can agree that the current API provided by the GTK and WPE ports covers a good amount of use cases while exposing a simple API, I reckon that it has its flaws and that it would be good to also provide a more full-featured API. For an example of subtle breakage, bug #222471 shows that even a minor deviation from setting HTTP-like status codes for the response affects any JS library that uses the Fetch API, which will no longer work as advertised in its specification.
The Cocoa API has “-[WKURLSchemeTask didReceiveResponse:(NSURLResponse*)]” and the internal WebCore::ResourceResponse object creation checks whether the instance passed is a NSHTTPURLResponse, and in that case it proceeds to pick the HTTP status and headers from the passed object. Using a delegate fits an Objective-C style API nicely, but not so much our GLib style. I will try to give this some thought and try to come up with something that feels idiomatic and complements the API that we already have ^_^
*** Bug 232653 has been marked as a duplicate of this bug. ***
Created attachment 443262 [details] Patch
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See https://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
*** Bug 231564 has been marked as a duplicate of this bug. ***
Comment on attachment 443262 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=443262&action=review > Source/WebKit/UIProcess/API/glib/WebKitURISchemeRequest.cpp:202 > + auto headers = soup_message_headers_new(SOUP_MESSAGE_HEADERS_REQUEST); > + request->priv->task->request().updateSoupMessageHeaders(headers); > + request->priv->headers = headers; You're leaking the SoupMessageHeaders here. You have a ref, then assign it to the GRefPtr, which itself takes a ref. Now there are two refs, but only one is owned and the other is lost. When creating a new GObject, you almost always need to use adoptGRef(): request->priv->headers = adoptGRef(soup_message_headers_new(SOUP_MESSAGE_HEADERS_REQUEST)); request->priv->task->request().updateSoupMessageHeaders(request->priv->headers.get()); In general, avoid ever holding ownership in a raw pointer, always store ownership in a smart pointer -- even for short periods of time -- and use raw pointers only for unowned access. This makes it much harder to make mistakes. One more thing: our code style is to write auto* rather than plain auto, to make more clear that you're working with a raw pointer. > Source/WebKit/UIProcess/API/glib/WebKitURISchemeRequest.cpp:241 > + SoupMessageHeaders* headers = WebKitURISchemeResponseGetHeaders(resp); > + if (headers) > + response.updateFromSoupMessageHeaders(headers); if (auto* headers = webKitURISchemeResponseGetHeaders(resp)) response.updateFromSoupMessageHeaders(headers); Also note: lowercase 'w'... > Source/WebKit/UIProcess/API/glib/WebKitURISchemeResponse.cpp:153 > +SoupMessageHeaders* WebKitURISchemeResponseGetHeaders(WebKitURISchemeResponse* response) Ooops, we missed this in the previous rounds of review, but these private functions are supposed to follow WebKit coding style, which uses camelCase like this. So it should be named webkitURISchemeResponseGetHeaders with an initial lowercase w. If WebKit were a more normal software project that followed generally-accepted commit granularity rules, I would ask you to fix these all in a separate commit, but in WebKit it is actually typical to land small mildly-related fixes all in one large patch. So you can go ahead and fix them all as part of this patch. > Source/WebKit/UIProcess/API/glib/WebKitURISchemeResponse.cpp:203 > +void webkit_uri_scheme_response_set_http_headers(WebKitURISchemeResponse* response, SoupMessageHeaders* headers) I'm not sure about the name of this function. Problem is the name "set" implies the previous value will be replaced with the new value. But you're not doing a replacement, you're just appending. I think I would call this webkit_uri_scheme_response_append_http_headers(). I would also add a warning that it is an error to append a header that already exists if it is not a list-valued header, same as when using soup_message_headers_append() directly. Do we actually need this at all? Is allowing access to the SoupMessageHeaders not enough for applications to do what they want with it? > Source/WebKit/UIProcess/API/glib/WebKitURISchemeResponse.cpp:209 > + auto headers = soup_message_headers_new(SOUP_MESSAGE_HEADERS_RESPONSE); > + response->priv->headers = headers; Leaking here too. Fix it with adoptGRef: response->priv->headers = adoptGRef(soup_message_headers_new(SOUP_MESSAGE_HEADERS_RESPONSE)); > Source/WebKit/UIProcess/API/glib/WebKitURISchemeResponse.cpp:212 > + // iterate over headers and append them to our response's headers' "headers" is not possessive, so remove the stray apostrophe. Also, we like to write comments as sentences, so we'd start with a capital I and end with a period. That said, we have one more rule: we generally use comments to explain *why* the code is doing things, not to explain *what* it is that the code is doing (unless the code is necessarily unusually complex or confusing, which is not the case here). So actually we would not use a comment here at all. You can just remove it. > Source/WebKit/UIProcess/API/glib/WebKitURISchemeResponse.cpp:213 > + soup_message_headers_foreach(headers, [](const char *name, const char *value, gpointer user_data) { Remember the * hangs on the type: const char* name, const char* value And remember everything has to use camelCase with the exception of the public API functions that use underscores, so user_data -> userData. > Source/WebKit/UIProcess/API/gtk/WebKitURISchemeRequest.h:28 > +#include <libsoup/soup-message-headers.h> I think a forward declaration would be enough: typedef struct _SoupMessageHeaders SoupMessageHeaders; Does that work? If not, add it alphabetically to the list of includes rather than adding it at the bottom. > Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitWebContext.cpp:133 > + auto headers = webkit_uri_scheme_request_get_http_headers(request); auto* headers = > Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitWebContext.cpp:348 > + static const char* headersHTML = "<html><body><script>fetch('headers:data', {headers: {'X-Test': 'A'}})</script></body></html>"; > + test->registerURISchemeHandler("headers", nullptr, 0, "application/json", 204); > + test->m_loadEvents.clear(); > + test->loadHtml(headersHTML, "headers:form"); > + test->waitUntilLoadFinished(); > + g_assert_false(test->m_loadEvents.contains(LoadTrackingTest::ProvisionalLoadFailed)); > + g_assert_false(test->m_loadEvents.contains(LoadTrackingTest::LoadFailed)); OK, this is a good start at a test. I see that we cannot test what happens when you add a header that already exists, because that behavior is undefined. And we cannot test removing or replacing headers, because your API does not allow that (should it?). We could test list-valued headers, though, to ensure that you're able to properly construct a list using multiple headers. (See documentation of soup_message_headers_get_list().) The other little thing we need to test is... webkit_uri_scheme_response_set_http_headers()! Did you forget about that? Oops. :)
Created attachment 443381 [details] Patch
Created attachment 443383 [details] Patch
Looks like your new API test is crashing?
(In reply to Michael Catanzaro from comment #17) > Looks like your new API test is crashing? I can't seem to reproduce this crash on my system. I will try some other distribution + GLib version combinations tomorrow.
Created attachment 443797 [details] Patch
Created attachment 443866 [details] Patch
Created attachment 443903 [details] Patch
Comment on attachment 443903 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=443903&action=review > Source/WebKit/UIProcess/API/glib/WebKitURISchemeRequest.cpp:73 > + GUniquePtr<SoupMessageHeaders> headers; This will cause the same issue, you have to include <WebCore/GUniquePtrSoup.h> in this file. See below. > Source/WebKit/UIProcess/API/glib/WebKitURISchemeRequest.cpp:202 > + // Unfortunately, SoupMessageHeaders is not a GObject. So we need to use GUniquePtr instead of GRefPtr. I don't see why it's unfortunate that SoupMessageHeaders is not a GObject, so better remove this comment. We use unique_ptr for compatibility with soup2, once soup2 code is removed we can use GRefPtr instead, even when it's not a GObject. We can avoid the local variable: request->priv->headers.reset(soup_message_headers_new(SOUP_MESSAGE_HEADERS_REQUEST)); request->priv->task->request().updateSoupMessageHeaders(request->priv->headers.get()); > Source/WebKit/UIProcess/API/glib/WebKitURISchemeResponse.cpp:26 > +#include <wtf/glib/GUniquePtr.h> This should be #include <WebCore/GUniquePtrSoup.h> > Source/WebKit/UIProcess/API/glib/WebKitURISchemeResponse.cpp:65 > +namespace WTF { > +#if USE(SOUP2) > +WTF_DEFINE_GPTR_DELETER(SoupMessageHeaders, soup_message_headers_free); > +#else > +WTF_DEFINE_GPTR_DELETER(SoupMessageHeaders, soup_message_headers_unref); > +#endif > +} This is already defined in GUniquePtrSoup.h > Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitWebContext.cpp:25 > +#include <libsoup/soup-message-headers.h> I'm surprised this works, you should only include the soup main header.
(In reply to Carlos Garcia Campos from comment #22) > > Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitWebContext.cpp:25 > > +#include <libsoup/soup-message-headers.h> > > I'm surprised this works, you should only include the soup main header. I was surprised too, but libsoup really does not have guards to block inclusion of random headers, so it seems like this is the intended way to use the library. If libsoup doesn't want us doing this, it should add guards in libsoup3.
Created attachment 444020 [details] Patch
Comment on attachment 444020 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=444020&action=review > Source/WebKit/UIProcess/API/gtk/WebKitURISchemeRequest.h:47 > +#ifndef SOUP_TYPE_MESSAGE_HEADERS > +typedef struct _SoupMessageHeaders SoupMessageHeaders; > +#endif I don't think you need the #ifndef, should be safe to always do the typedef. I think. Was there really a build failure without this?
(In reply to Michael Catanzaro from comment #25) > Comment on attachment 444020 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=444020&action=review > > > Source/WebKit/UIProcess/API/gtk/WebKitURISchemeRequest.h:47 > > +#ifndef SOUP_TYPE_MESSAGE_HEADERS > > +typedef struct _SoupMessageHeaders SoupMessageHeaders; > > +#endif > > I don't think you need the #ifndef, should be safe to always do the typedef. > I think. Was there really a build failure without this? Yes. If you build WebKit2Gtk with libsoup 2.4, without this #ifdef, it will fail with redefinition errors.
The EWS failures do not seem to be related to my patch. My patch did not touch any layout system and should not affect the macOS code.
You can ignore unrelated EWS failures, don't worry about them.
Comment on attachment 444020 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=444020&action=review >>> Source/WebKit/UIProcess/API/gtk/WebKitURISchemeRequest.h:47 >>> +#endif >> >> I don't think you need the #ifndef, should be safe to always do the typedef. I think. Was there really a build failure without this? > > Yes. If you build WebKit2Gtk with libsoup 2.4, without this #ifdef, it will fail with redefinition errors. I think it's fine to include <libsoup/soup.h> in this header.
(In reply to Carlos Garcia Campos from comment #29) > Comment on attachment 444020 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=444020&action=review > > >>> Source/WebKit/UIProcess/API/gtk/WebKitURISchemeRequest.h:47 > >>> +#endif > >> > >> I don't think you need the #ifndef, should be safe to always do the typedef. I think. Was there really a build failure without this? > > > > Yes. If you build WebKit2Gtk with libsoup 2.4, without this #ifdef, it will fail with redefinition errors. > > I think it's fine to include <libsoup/soup.h> in this header. Understood. So I will remove the forward declaration in this file (and the WPE one) and include <libsoup/soup.h>. By the way, I have checked the documentation, the documentation recommends including <libsoup/soup.h> but did not forbid you to include the individual header from the `libsoup` folder.
Created attachment 444224 [details] Patch
Reported https://gitlab.gnome.org/GNOME/libsoup/-/issues/251
Comment on attachment 444224 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=444224&action=review > Source/WebKit/UIProcess/API/gtk/docs/webkit2gtk-4.0-sections.txt:1297 > +SoupMessageHeaders Why do we need this? > Source/WebKit/UIProcess/API/gtk/docs/webkit2gtk-4.0-sections.txt:1320 > +SoupMessageHeaders Ditto. > Source/WebKit/UIProcess/API/wpe/docs/wpe-1.0-sections.txt:1234 > +SoupMessageHeaders Ditto. > Source/WebKit/UIProcess/API/wpe/docs/wpe-1.0-sections.txt:1257 > +SoupMessageHeaders Ditto.
(In reply to Carlos Garcia Campos from comment #33) > Comment on attachment 444224 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=444224&action=review > > > Source/WebKit/UIProcess/API/gtk/docs/webkit2gtk-4.0-sections.txt:1297 > > +SoupMessageHeaders > > Why do we need this? > > > Source/WebKit/UIProcess/API/gtk/docs/webkit2gtk-4.0-sections.txt:1320 > > +SoupMessageHeaders > > Ditto. > > > Source/WebKit/UIProcess/API/wpe/docs/wpe-1.0-sections.txt:1234 > > +SoupMessageHeaders > > Ditto. > > > Source/WebKit/UIProcess/API/wpe/docs/wpe-1.0-sections.txt:1257 > > +SoupMessageHeaders > > Ditto. > Why do we need this? This is required by gtk-doc generator. I was forced to add SoupMessageHeaders here because otherwise, gtk-doc will complain there is no documentation section for SoupMessageHeaders, even if I included the `libsoup/soup.h` header. You can find the historical CI logs that led me to this solution: https://ews-build.webkit.org/#/builders/36/builds/50682/steps/11/logs/stdio
(In reply to liushuyu011 from comment #34) > (In reply to Carlos Garcia Campos from comment #33) > > Comment on attachment 444224 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=444224&action=review > > > > > Source/WebKit/UIProcess/API/gtk/docs/webkit2gtk-4.0-sections.txt:1297 > > > +SoupMessageHeaders > > > > Why do we need this? > > > > > Source/WebKit/UIProcess/API/gtk/docs/webkit2gtk-4.0-sections.txt:1320 > > > +SoupMessageHeaders > > > > Ditto. > > > > > Source/WebKit/UIProcess/API/wpe/docs/wpe-1.0-sections.txt:1234 > > > +SoupMessageHeaders > > > > Ditto. > > > > > Source/WebKit/UIProcess/API/wpe/docs/wpe-1.0-sections.txt:1257 > > > +SoupMessageHeaders > > > > Ditto. > > > Why do we need this? > > This is required by gtk-doc generator. I was forced to add > SoupMessageHeaders here because otherwise, gtk-doc will complain there is no > documentation section for SoupMessageHeaders, even if I included the > `libsoup/soup.h` header. > > You can find the historical CI logs that led me to this solution: > https://ews-build.webkit.org/#/builders/36/builds/50682/steps/11/logs/stdio That's weird, it's not in WebKit namespace. We are already exposing SoupMessageHeaders in SoupURIRequest/Response without this problem. Maybe that log was when the forward declaration was used and that confused gtkdoc somehow?
(In reply to Carlos Garcia Campos from comment #35) > (In reply to liushuyu011 from comment #34) > > (In reply to Carlos Garcia Campos from comment #33) > > > Comment on attachment 444224 [details] > > > Patch > > > > > > View in context: > > > https://bugs.webkit.org/attachment.cgi?id=444224&action=review > > > > > > > Source/WebKit/UIProcess/API/gtk/docs/webkit2gtk-4.0-sections.txt:1297 > > > > +SoupMessageHeaders > > > > > > Why do we need this? > > > > > > > Source/WebKit/UIProcess/API/gtk/docs/webkit2gtk-4.0-sections.txt:1320 > > > > +SoupMessageHeaders > > > > > > Ditto. > > > > > > > Source/WebKit/UIProcess/API/wpe/docs/wpe-1.0-sections.txt:1234 > > > > +SoupMessageHeaders > > > > > > Ditto. > > > > > > > Source/WebKit/UIProcess/API/wpe/docs/wpe-1.0-sections.txt:1257 > > > > +SoupMessageHeaders > > > > > > Ditto. > > > > > Why do we need this? > > > > This is required by gtk-doc generator. I was forced to add > > SoupMessageHeaders here because otherwise, gtk-doc will complain there is no > > documentation section for SoupMessageHeaders, even if I included the > > `libsoup/soup.h` header. > > > > You can find the historical CI logs that led me to this solution: > > https://ews-build.webkit.org/#/builders/36/builds/50682/steps/11/logs/stdio > > That's weird, it's not in WebKit namespace. We are already exposing > SoupMessageHeaders in SoupURIRequest/Response without this problem. Maybe > that log was when the forward declaration was used and that confused gtkdoc > somehow? I see. Yes, that log was made when the forward declaration was used. This patch also included that solution: https://bugs.webkit.org/attachment.cgi?id=381614&action=prettypatch. I will try again on my machine without the forward declaration.
Created attachment 444360 [details] Patch
Created attachment 444361 [details] Patch
(In reply to liushuyu011 from comment #38) > Created attachment 444361 [details] > Patch I have replaced all the libsoup related forward-declarations with include's. And it worked this time.
Committed r285919 (244328@main): <https://commits.webkit.org/244328@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 444361 [details].
<rdar://problem/85494189>