Bug 48375 – Need delegate calls in PageLoaderClient to indicate if we have loaded insecure content

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Alexey Proskuryakov]

Like these WebKit1 Windows ones:

IWebFrameLoadDelegatePrivate2::didDisplayInsecureContent
IWebFrameLoadDelegatePrivate2::didRunInsecureContent

[Alexey Proskuryakov]

<rdar://problem/8392724>

[Alexey Proskuryakov]

Created attachment 71940 [details]
proposed patch

[WebKit Review Bot]

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

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit2/UIProcess/API/C/WKPage.h:91:  Extra space between WKPageDidFirstLayoutForFrameCallback and didFirstLayoutForFrame  [whitespace/declaration] [3]
WebKit2/UIProcess/API/C/WKPage.h:92:  Extra space between WKPageDidFirstVisuallyNonEmptyLayoutForFrameCallback and didFirstVisuallyNonEmptyLayoutForFrame  [whitespace/declaration] [3]
WebKit2/UIProcess/API/C/WKPage.h:94:  Extra space between WKPageDidDisplayInsecureContentForFrameCallback and didDisplayInsecureContentForFrame  [whitespace/declaration] [3]
WebKit2/UIProcess/API/C/WKPage.h:95:  Extra space between WKPageDidRunInsecureContentForFrameCallback and didRunInsecureContentForFrame  [whitespace/declaration] [3]
Total errors found: 4 in 9 files


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

[Early Warning System Bot]

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

[Sam Weinig]

Comment on attachment 71940 [details]
proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=71940&action=review

> WebKit2/ChangeLog:11
> +        Added the delegate. Just like the bundle version, it misses WebOrigin parameter that Mac
> +        delegate call used to have. It doesn't seem necessary for clients.
> +

I believe the origin is necessary to implement origin tainting (eg. if a page in one origin runs insecure content, we need to taint all other pages in that origin).  This tracking may be something we should do in WebCore in WebKit instead of in the app though.

> WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:785
> +
> +    RefPtr<APIObject> userData;
> +    WebProcess::shared().connection()->send(Messages::WebPageProxy::DidDisplayInsecureContentForFrame(m_frame->frameID(), InjectedBundleUserMessageEncoder(userData.get())), webPage->pageID());
>  }

This is just passing a null userData to the UIProcess.  We either need to pass the userData to the bundle callback or not pass it to the UIProcess.

> WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:796
>      webPage->injectedBundleLoaderClient().didRunInsecureContentForFrame(webPage, m_frame);
> +
> +    RefPtr<APIObject> userData;
> +    WebProcess::shared().connection()->send(Messages::WebPageProxy::DidRunInsecureContentForFrame(m_frame->frameID(), InjectedBundleUserMessageEncoder(userData.get())), webPage->pageID());

Here too.

[Alexey Proskuryakov]

Created attachment 71954 [details]
pass userData

[WebKit Review Bot]

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

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit2/UIProcess/API/C/WKPage.h:91:  Extra space between WKPageDidFirstLayoutForFrameCallback and didFirstLayoutForFrame  [whitespace/declaration] [3]
WebKit2/UIProcess/API/C/WKPage.h:92:  Extra space between WKPageDidFirstVisuallyNonEmptyLayoutForFrameCallback and didFirstVisuallyNonEmptyLayoutForFrame  [whitespace/declaration] [3]
WebKit2/UIProcess/API/C/WKPage.h:94:  Extra space between WKPageDidDisplayInsecureContentForFrameCallback and didDisplayInsecureContentForFrame  [whitespace/declaration] [3]
WebKit2/UIProcess/API/C/WKPage.h:95:  Extra space between WKPageDidRunInsecureContentForFrameCallback and didRunInsecureContentForFrame  [whitespace/declaration] [3]
Total errors found: 4 in 17 files


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

[Alexey Proskuryakov]

Committed <http://trac.webkit.org/changeset/70584>.

[Early Warning System Bot]

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

[Alexey Proskuryakov]

Landed build fix in <http://trac.webkit.org/changeset/70587>.