Teach SSLHostStateDelegate about subresources with cert errors

chrome/browser/ssl/chrome_ssl_host_state_delegate_test.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ssl/chrome_ssl_host_state_delegate.h"
 
 #include <stdint.h>
 #include <utility>
 
 #include "base/command_line.h"
@@ skipping 178 matching lines @@
 // DidHostRunInsecureContent unit tests the expected behavior of calling
 // DidHostRunInsecureContent as well as HostRanInsecureContent to check if
 // insecure content has been run and to mark it as such.
 IN_PROC_BROWSER_TEST_F(ChromeSSLHostStateDelegateTest,
                        DidHostRunInsecureContent) {
   content::WebContents* tab =
       browser()->tab_strip_model()->GetActiveWebContents();
   Profile* profile = Profile::FromBrowserContext(tab->GetBrowserContext());
   content::SSLHostStateDelegate* state = profile->GetSSLHostStateDelegate();
 
-  EXPECT_FALSE(state->DidHostRunInsecureContent("www.google.com", 42));
-  EXPECT_FALSE(state->DidHostRunInsecureContent("www.google.com", 191));
-  EXPECT_FALSE(state->DidHostRunInsecureContent("example.com", 42));
+  EXPECT_FALSE(state->DidHostRunInsecureContent(
+      "www.google.com", 42, content::SSLHostStateDelegate::MIXED_CONTENT));
+  EXPECT_FALSE(state->DidHostRunInsecureContent(
+      "www.google.com", 191, content::SSLHostStateDelegate::MIXED_CONTENT));
+  EXPECT_FALSE(state->DidHostRunInsecureContent(
+      "example.com", 42, content::SSLHostStateDelegate::MIXED_CONTENT));
 
-  state->HostRanInsecureContent("www.google.com", 42);
+  state->HostRanInsecureContent("www.google.com", 42,
+                                content::SSLHostStateDelegate::MIXED_CONTENT);
 
-  EXPECT_TRUE(state->DidHostRunInsecureContent("www.google.com", 42));
-  EXPECT_FALSE(state->DidHostRunInsecureContent("www.google.com", 191));
-  EXPECT_FALSE(state->DidHostRunInsecureContent("example.com", 42));
+  EXPECT_TRUE(state->DidHostRunInsecureContent(
+      "www.google.com", 42, content::SSLHostStateDelegate::MIXED_CONTENT));
+  EXPECT_FALSE(state->DidHostRunInsecureContent(
+      "www.google.com", 191, content::SSLHostStateDelegate::MIXED_CONTENT));
+  EXPECT_FALSE(state->DidHostRunInsecureContent(
+      "example.com", 42, content::SSLHostStateDelegate::MIXED_CONTENT));
 
-  state->HostRanInsecureContent("example.com", 42);
+  state->HostRanInsecureContent("example.com", 42,
+                                content::SSLHostStateDelegate::MIXED_CONTENT);
 
-  EXPECT_TRUE(state->DidHostRunInsecureContent("www.google.com", 42));
-  EXPECT_FALSE(state->DidHostRunInsecureContent("www.google.com", 191));
-  EXPECT_TRUE(state->DidHostRunInsecureContent("example.com", 42));
+  EXPECT_TRUE(state->DidHostRunInsecureContent(
+      "www.google.com", 42, content::SSLHostStateDelegate::MIXED_CONTENT));
+  EXPECT_FALSE(state->DidHostRunInsecureContent(
+      "www.google.com", 191, content::SSLHostStateDelegate::MIXED_CONTENT));
+  EXPECT_TRUE(state->DidHostRunInsecureContent(
+      "example.com", 42, content::SSLHostStateDelegate::MIXED_CONTENT));
+
+  state->HostRanInsecureContent(
+      "example.com", 42, content::SSLHostStateDelegate::CERT_ERRORS_CONTENT);
+  EXPECT_FALSE(state->DidHostRunInsecureContent(
+      "www.google.com", 191, content::SSLHostStateDelegate::MIXED_CONTENT));
+  EXPECT_TRUE(state->DidHostRunInsecureContent(
+      "example.com", 42, content::SSLHostStateDelegate::MIXED_CONTENT));
+  EXPECT_TRUE(state->DidHostRunInsecureContent(
+      "example.com", 42, content::SSLHostStateDelegate::CERT_ERRORS_CONTENT));
+
+  state->HostRanInsecureContent(

[jww, 2016/08/11 18:50:15]

This looks identical to the set of tests right above it (starting at line 226).
Did you mean for this to be the reverse, where you mark a host as having run
mixed content, and verify that it's not marking it as having run cert error
content?

Even if I'm missing something about how this is different, you should still add
the test case I just mentioned anyway.

[estark, 2016/08/11 20:58:38]

As best as I can interpret past-Emily's intentions, I think the test case on
line 226 was supposed to be marking a MIXED_CONTENT site as CERT_ERRORS_CONTENT,
and this test case was supposed to be marking a non-MIXED_CONTENT site as
CERT_ERRORS_CONTENT.

I added some comments to each block to explain what it's doing, and added some
expectations to the test cases above to test the case you suggested (marking a
MIXED_CONTENT site should not marked it as CERT_ERRORS_CONTENT).

+      "www.google.com", 191,
+      content::SSLHostStateDelegate::CERT_ERRORS_CONTENT);
+  EXPECT_TRUE(state->DidHostRunInsecureContent(
+      "www.google.com", 191,
+      content::SSLHostStateDelegate::CERT_ERRORS_CONTENT));
+  EXPECT_FALSE(state->DidHostRunInsecureContent(
+      "www.google.com", 191, content::SSLHostStateDelegate::MIXED_CONTENT));
 }
 
 // Test the migration code needed as a result of changing how the content
 // setting is stored. We used to map the settings dictionary to the pattern
 // pair <origin, origin> but now we map it to <origin, wildcard>.
 IN_PROC_BROWSER_TEST_F(ChromeSSLHostStateDelegateTest, Migrate) {
   scoped_refptr<net::X509Certificate> cert = GetOkCert();
   content::WebContents* tab =
       browser()->tab_strip_model()->GetActiveWebContents();
   Profile* profile = Profile::FromBrowserContext(tab->GetBrowserContext());
@@ skipping 400 matching lines @@
   EXPECT_EQ(
       content::SSLHostStateDelegate::ALLOWED,
       state->QueryPolicy("localhost", *cert,
                          net::CERT_STATUS_COMMON_NAME_INVALID, &unused_value));
 
   EXPECT_EQ(
       content::SSLHostStateDelegate::ALLOWED,
       state->QueryPolicy("127.0.0.1", *cert,
                          net::CERT_STATUS_COMMON_NAME_INVALID, &unused_value));
 }