Bug 48375 - Need delegate calls in PageLoaderClient to indicate if we have loaded insecure content
Summary: Need delegate calls in PageLoaderClient to indicate if we have loaded insecur...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2010-10-26 14:33 PDT by Alexey Proskuryakov
Modified: 2010-10-26 16:25 PDT (History)
2 users (show)

See Also:


Attachments
proposed patch (13.16 KB, patch)
2010-10-26 14:43 PDT, Alexey Proskuryakov
sam: review-
Details | Formatted Diff | Diff
pass userData (23.07 KB, patch)
2010-10-26 15:49 PDT, Alexey Proskuryakov
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2010-10-26 14:33:16 PDT
Like these WebKit1 Windows ones:

IWebFrameLoadDelegatePrivate2::didDisplayInsecureContent
IWebFrameLoadDelegatePrivate2::didRunInsecureContent
Comment 1 Alexey Proskuryakov 2010-10-26 14:33:51 PDT
<rdar://problem/8392724>
Comment 2 Alexey Proskuryakov 2010-10-26 14:43:21 PDT
Created attachment 71940 [details]
proposed patch
Comment 3 WebKit Review Bot 2010-10-26 14:45:36 PDT
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.
Comment 4 Early Warning System Bot 2010-10-26 14:51:51 PDT
Attachment 71940 [details] did not build on qt:
Build output: http://queues.webkit.org/results/4828036
Comment 5 Sam Weinig 2010-10-26 14:56:22 PDT
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.
Comment 6 Alexey Proskuryakov 2010-10-26 15:49:09 PDT
Created attachment 71954 [details]
pass userData
Comment 7 WebKit Review Bot 2010-10-26 15:52:26 PDT
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.
Comment 8 Alexey Proskuryakov 2010-10-26 16:01:37 PDT
Committed <http://trac.webkit.org/changeset/70584>.
Comment 9 Early Warning System Bot 2010-10-26 16:08:59 PDT
Attachment 71954 [details] did not build on qt:
Build output: http://queues.webkit.org/results/4857026
Comment 10 Alexey Proskuryakov 2010-10-26 16:25:37 PDT
Landed build fix in <http://trac.webkit.org/changeset/70587>.