Mixed content errors should contain stack traces when relevant.

[Mike West]

Currently, the mixed content errors we generate are of the form "The page at <url> displayed/ran insecure content from <url>." When relevant, we should attach a line number or stack trace to give some context.

[Mike West]

Created attachment 169624 [details]
WIP

[Mike West]

Created attachment 169673 [details]
WIP

[Mike West]

Hello Inspector folks. :)

I'm having a hard time writing a test for this patch. It needs to run under HTTPS in order to generate a mixed content warning, but the simple redirect that works for other tests I've seen doesn't seem to work here. The test simply times out after redirecting.

Are there any HTTPS/inspector test limitations that you know of? Or am I just doing something obviously wrong in the attached patch? I've tried a few variations on the theme, but haven't yet been very successful. :/

Thanks!

[Pavel Feldman]

Comment on attachment 169673 [details]
WIP

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

Note that inspector harness is very sensitive wrt navigation. You should use InspectorTest.navigate from within inspector to navigate.

> LayoutTests/http/tests/inspector/mixedcontent-warning-contains-stacktrace.html:17
> +        <script src="/inspector/resources/csp-test.js"></script>

What is csp-test?

[Mike West]

Created attachment 169768 [details]
Patch

[Mike West]

(In reply to comment #5)
> Created an attachment (id=169768) [details]
> Patch

This patch's test should work correctly on mac. I'll have to wait till I'm back at the office to generate the Chromium linux baseline, as the inspector tests don't seem to work on Chromium mac. :(

[WebKit Review Bot]

Comment on attachment 169768 [details]
Patch

Attachment 169768 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14457977

New failing tests:
http/tests/inspector/stacktraces/mixedcontent-warning-contains-stacktrace.html

[Mike West]

Created attachment 169850 [details]
Patch

[Mike West]

(In reply to comment #8)
> Created an attachment (id=169850) [details]
> Patch

This patch updates the Chromium expectations. Ideally, it'll make all the bots happy.

[Pavel Feldman]

Comment on attachment 169850 [details]
Patch

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

> Source/WebCore/loader/MixedContentChecker.cpp:113
> +    console->addMessage(NetworkMessageSource, LogMessageType, WarningMessageLevel, message, m_frame->document()->url().string(), 0, callStack);

Why did you switch from HTML to network source?

> LayoutTests/http/tests/inspector/stacktraces/mixedcontent-warning-contains-stacktrace.html:8
> +    function test() {

Please place { on the next line. We also don't indent top-level functions.

> LayoutTests/http/tests/inspector/stacktraces/mixedcontent-warning-contains-stacktrace.html:11
> +        InspectorTest.navigate("https://127.0.0.1:8443/inspector/stacktraces/resources/mixedcontent-page.html", navigateBack);

Navigation is extremely fragile in the inspector harness, please open iframes whenever possible.

[Mike West]

(In reply to comment #10)
> (From update of attachment 169850 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=169850&action=review
> 
> > Source/WebCore/loader/MixedContentChecker.cpp:113
> > +    console->addMessage(NetworkMessageSource, LogMessageType, WarningMessageLevel, message, m_frame->document()->url().string(), 0, callStack);
> 
> Why did you switch from HTML to network source?

Mistake.That's leftover from earlier experimentation. I'll remove it when I respin the patch. 

> > LayoutTests/http/tests/inspector/stacktraces/mixedcontent-warning-contains-stacktrace.html:8
> > +    function test() {
> 
> Please place { on the next line. We also don't indent top-level functions.
> 
> > LayoutTests/http/tests/inspector/stacktraces/mixedcontent-warning-contains-stacktrace.html:11
> > +        InspectorTest.navigate("https://127.0.0.1:8443/inspector/stacktraces/resources/mixedcontent-page.html", navigateBack);
> 
> Navigation is extremely fragile in the inspector harness, please open iframes whenever possible.

I wasn't able to get this test working in an iframe. :/ I'll try again, but probably won't have time to fiddle until later this week.

[Mike West]

Closing this in favor of 100650, where I'm doing the same work in a more general way.

*** This bug has been marked as a duplicate of bug 100650 ***

[Eric Seidel (no email)]

Comment on attachment 169850 [details]
Patch

Cleared review? from attachment 169850 [details] so that this bug does not appear in http://webkit.org/pending-review.  If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).