Bug 51969 – WebKit2: Should be able to call into injected bundle to ask if we should allow resource loads

• Component: WebKit2
• Status: RESOLVED FIXED

[Brian Weinstein]

WebKit2 needs a way to call into the injected bundle to ask whether or not we should load resources, and if the injected bundle says we should, we should cancel the resource load.

[Brian Weinstein]

Created attachment 78069 [details]
[PATCH] Fix

[WebKit Review Bot]

Attachment 78069 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'WebKit2/ChangeLog', u'WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePage.h', u'WebKit2/WebProcess/InjectedBundle/InjectedBundlePageLoaderClient.cpp', u'WebKit2/WebProcess/InjectedBundle/InjectedBundlePageLoaderClient.h', u'WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp']" exit_code: 1
WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePage.h:90:  The parameter name "page" adds no information, so it should be removed.  [readability/parameter_name] [5]
WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePage.h:90:  The parameter name "frame" adds no information, so it should be removed.  [readability/parameter_name] [5]
WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePage.h:90:  The parameter name "url" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 3 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Early Warning System Bot]

Attachment 78069 [details] did not build on qt:
Build output: http://queues.webkit.org/results/7289428

[Brian Weinstein]

Created attachment 78080 [details]
[PATCH] Fix + WebKitTestRunner

[Ada Chan]

Comment on attachment 78080 [details]
[PATCH] Fix + WebKitTestRunner

Typo: 
// FIXME: We should problably ...

[Alexey Proskuryakov]

+bool InjectedBundlePageLoaderClient::shouldLoadResourceForFrame(WebPage* page, WebFrame* frame, const String& subResourceURL)
+{
+    if (!m_client.shouldLoadResourceForFrame)
+        return true;
+
+    return m_client.shouldLoadResourceForFrame(toAPI(page), toAPI(frame), toURLRef(subResourceURL.impl()), m_client.clientInfo);

Is it subResourceURL, or just resourceURL?

[Brian Weinstein]

Landed in r75174.

[Brian Weinstein]

(In reply to comment #6)
> +bool InjectedBundlePageLoaderClient::shouldLoadResourceForFrame(WebPage* page, WebFrame* frame, const String& subResourceURL)
> +{
> +    if (!m_client.shouldLoadResourceForFrame)
> +        return true;
> +
> +    return m_client.shouldLoadResourceForFrame(toAPI(page), toAPI(frame), toURLRef(subResourceURL.impl()), m_client.clientInfo);
> 
> Is it subResourceURL, or just resourceURL?

It should be resourceURL. I will fix this in a follow-up.