Adapt MixedContentChecker for remote frames

content/browser/site_per_process_browsertest.cc

 // Copyright (c) 2012 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 "content/browser/site_per_process_browsertest.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
@@ skipping 536 matching lines @@
   SitePerProcessHighDPIBrowserTest() {}
 
  protected:
   void SetUpCommandLine(base::CommandLine* command_line) override {
     SitePerProcessBrowserTest::SetUpCommandLine(command_line);
     command_line->AppendSwitchASCII(switches::kForceDeviceScaleFactor,
                                     base::StringPrintf("2"));
   }
 };
 
+// SitePerProcessIgnoreCertErrorsBrowserTest
+
+class SitePerProcessIgnoreCertErrorsBrowserTest
+    : public SitePerProcessBrowserTest {
+ public:
+  SitePerProcessIgnoreCertErrorsBrowserTest() {}
+
+ protected:
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    SitePerProcessBrowserTest::SetUpCommandLine(command_line);
+    command_line->AppendSwitch(switches::kIgnoreCertificateErrors);
+  }
+};
+
 // Ensure that navigating subframes in --site-per-process mode works and the
 // correct documents are committed.
 IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, CrossSiteIframe) {
   GURL main_url(embedded_test_server()->GetURL(
       "a.com", "/cross_site_iframe_factory.html?a(a,a(a,a(a)))"));
   NavigateToURL(shell(), main_url);
 
   // It is safe to obtain the root frame tree node here, as it doesn't change.
   FrameTreeNode* root =
       static_cast<WebContentsImpl*>(shell()->web_contents())->
@@ skipping 4087 matching lines @@
   // RenderFrame will be properly created and there will be no crash.
   // Therefore, navigate the remaining subframe to completely different site,
   // which will cause the original process to exit cleanly.
   NavigateFrameToURL(
       web_contents->GetFrameTree()->root()->child_at(0),
       embedded_test_server()->GetURL("d.com", "/title3.html"));
   watcher.Wait();
   EXPECT_TRUE(watcher.did_exit_normally());
 }
 
+IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, PassiveMixedContent) {

[alexmos, 2016/01/16 00:49:50]

Hmm, it doesn't look like this test exercises any OOPIF functionality, though it
does seem useful to have.  I'd suggest moving it to something like
web_contents_impl_browsertest.cc, except that it'd be nice to keep all of these
tests together.  Actually, do you think the other tests would be useful to run
without --site-per-process as well?  If so, something like
web_contents_impl_browsertest or render_frame_host_manager_browsertest (or even
a new test file) might be a good home for them.  These would run without
--site-per-process normally, and with --site-per-process on our FYI and try bots
(which should be enabled the CQ some time soon).

[estark, 2016/01/20 05:56:22]

Hmm... actually, maybe we don't need this test at all, since as you say, it
doesn't really test OOPIFs, and mixed content in general is covered pretty well
by layout tests. So maybe I should just remove this one? For the rest of the
tests I added here, it certainly wouldn't hurt to run them without
--site-per-process, but I do think we already have non-site-per-process test
coverage (in the form of layout tests) for this same functionality. So, maybe
they should stay here and only run for --site-per-process? What do you think?

[alexmos, 2016/01/20 22:43:55]

That sounds good to me.  Let's remove this one and keep the others in this file.

[estark, 2016/01/21 04:40:13]

Done.

+  net::EmbeddedTestServer https_server(net::EmbeddedTestServer::TYPE_HTTPS);
+  https_server.ServeFilesFromSourceDirectory("content/test/data");
+  ASSERT_TRUE(https_server.Start());
+
+  GURL url(https_server.GetURL("/mixed-content/basic-passive.html"));
+  NavigateToURL(shell(), url);
+  EXPECT_TRUE(shell()->web_contents()->DisplayedInsecureContent());
+}
+
+IN_PROC_BROWSER_TEST_F(SitePerProcessIgnoreCertErrorsBrowserTest,

[alexmos, 2016/01/16 00:49:50]

A comment about why these tests need to ignore cert errors would be nice.

[estark, 2016/01/20 05:56:22]

Done.

+                       PassiveMixedContentInIframe) {

[alexmos, 2016/01/20 22:43:55]

Do you plan to add similar tests for active mixed content?  Seems like that
might be worth having.

[estark, 2016/01/21 04:40:13]

I should have done that earlier, sorry. I just added a couple, but the only way
I could figure out to do them is to put them in
chrome_site_per_process_browsertest.cc so that I could use the
|kAllowRunningInsecureContent| switch in chrome/. It's a bummer to have the
passive mixed content ones here and the active somewhere else... What do you
think of that? Do you know of any way to set a webkit setting from a content
browser test?

[alexmos, 2016/01/21 18:13:28]

What do you think about flipping things to instead test that the active mixed
content gets blocked correctly by default?  That would better cover the bug we
originally had with failing open in 486936, and if you use an <iframe> instead
of <script>, you could just check whether an additional FrameTreeNode was
created or not.  This won't require specifying kAllowRunningInsecureContent so
you could do it from content.

For covering the kAllowRunningInsecureContent path, it seems we may already some
tests (e.g., ChromeSecurityStateModelClientTest.MixedContent), though I didn't
check if they end up using OOPIFs for their frames when run with
--site-per-process.

[alexmos, 2016/01/21 18:21:51]

Also, if you did want to override that setting from content/, maybe you could do
something like this?

  WebPreferences prefs =
      GetWebContents()->GetRenderViewHost()->GetWebkitPreferences();
  prefs.allow_running_insecure_content = true;
  GetWebContents()->GetRenderViewHost()->UpdateWebkitPreferences(prefs);

No idea whether that'd work, but BrowserPluginGuest::InitInternal appears to use
something like this
(https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/br...)

+  net::EmbeddedTestServer https_server(net::EmbeddedTestServer::TYPE_HTTPS);
+  https_server.ServeFilesFromSourceDirectory("content/test/data");
+  ASSERT_TRUE(https_server.Start());
+
+  GURL iframe_url(
+      https_server.GetURL("/mixed-content/basic-passive-in-iframe.html"));
+  NavigateToURL(shell(), iframe_url);
+  EXPECT_TRUE(shell()->web_contents()->DisplayedInsecureContent());

[alexmos, 2016/01/16 00:49:50]

Does navigating the subframe to another (secure) cross-site URL clear
DisplayedInsecureContent?  What about navigating the main frame?  This may be
worth checking in the test.

[estark, 2016/01/20 05:56:22]

Done.

+}
+
+IN_PROC_BROWSER_TEST_F(SitePerProcessIgnoreCertErrorsBrowserTest,
+                       PassiveMixedContentInIframeWithStrictBlocking) {
+  net::EmbeddedTestServer https_server(net::EmbeddedTestServer::TYPE_HTTPS);
+  https_server.ServeFilesFromSourceDirectory("content/test/data");
+  ASSERT_TRUE(https_server.Start());
+
+  GURL iframe_url_with_strict_blocking(https_server.GetURL(
+      "/mixed-content/basic-passive-in-iframe-with-strict-blocking.html"));
+  NavigateToURL(shell(), iframe_url_with_strict_blocking);
+  EXPECT_FALSE(shell()->web_contents()->DisplayedInsecureContent());

[alexmos, 2016/01/16 00:49:50]

Maybe also check the strict mixed content flags stored on the FrameTreeNodes in
current_replication_state()?

[estark, 2016/01/20 05:56:22]

Done.

+}
+
 }  // namespace content