Downgrade lock icon for broken-HTTPS subresources

third_party/WebKit/Source/core/loader/MixedContentChecker.cpp

Index: third_party/WebKit/Source/core/loader/MixedContentChecker.cpp
diff --git a/third_party/WebKit/Source/core/loader/MixedContentChecker.cpp b/third_party/WebKit/Source/core/loader/MixedContentChecker.cpp
index fdcc09b14a13bba18e331b1fa85bb1848942e2f5..389b4b7926d5f8c2e722eb11e6d98a42144b0dca 100644
--- a/third_party/WebKit/Source/core/loader/MixedContentChecker.cpp
+++ b/third_party/WebKit/Source/core/loader/MixedContentChecker.cpp
@@ -458,6 +458,22 @@ LocalFrame* MixedContentChecker::effectiveFrameForFrameType(LocalFrame* frame, W
     return effectiveFrame;
 }
 
+void MixedContentChecker::handleCertificateError(LocalFrame* frame, const KURL& url, WebURLRequest::RequestContext requestContext, WebURLRequest::FrameType frameType, const CString& securityInfo)

[jww, 2015/11/20 01:25:08]

Do we have any MixedContentChecker unit tests yet? I'd love see some unit tests
for this function to verify that inputs give you expected IPCs out of the
renderer. Maybe it would be easier to do that on the content/ side?

[estark, 2015/11/23 23:40:24]

Acknowledged, but haven't done this yet. MixedContentChecker unit tests seem
tricky because a.) I'm not sure there's a way to mock enough stuff in unit
tests, e.g. I can create a DummyPageHolder but I'm not sure if there's a way to
set the securityInfo() on the resulting document, and b.) they wouldn't actually
test what you're interested in testing (that the expected IPCs come out of the
renderer). On the other hand, there would be render_frame_impl_browsertest.cc,
but those seem pretty limited.

So, tl;dr I'm still looking into this.

+{
+    if (frameType == WebURLRequest::FrameTypeTopLevel || !frame)

[jww, 2015/11/20 01:25:08]

How can we can !frame? Can this be an ASSERT?

[estark, 2015/11/23 23:40:24]

I put this here because shouldBlockFetch() has a similar check; I'm not sure how
a request can be frameless but it does seem to be a possibility (see line 82).

[jww, 2015/11/25 19:24:02]

Hm, maybe Service Worker or something? Anyway, this is fine.

+        return;
+
+    FrameLoaderClient* client = frame->loader().client();
+    ContextType contextType = MixedContentChecker::contextTypeFromContext(requestContext, frame);
+    if (contextType == ContextTypeBlockable) {
+        SecurityOrigin* securityOrigin = frame->document()->securityOrigin();
+        client->didRunContentWithCertificateErrors(securityOrigin, url, securityInfo);
+    } else {
+        ASSERT(contextType != ContextTypeNotMixedContent);

[jww, 2015/11/20 01:25:08]

I guess this ASSERT exists because handleCertificateError should never be called
when there isn't mixed content? If so, can you add a comment with this invariant
above the function?

[estark, 2015/11/23 23:40:24]

Actually, it's because contextTypeFromContext() never returns anything but
Blockable/ShouldBeBlockable/OptionallyBlockable. I'll add a comment here.

+        client->didDisplayContentWithCertificateErrors(url, securityInfo);
+    }
+}
+
 MixedContentChecker::ContextType MixedContentChecker::contextTypeForInspector(LocalFrame* frame, const ResourceRequest& request)
 {
     LocalFrame* effectiveFrame = effectiveFrameForFrameType(frame, request.frameType());