PlzNavigate: Move navigation-level mixed content checks to the browser.

chrome/browser/content_settings/mixed_content_settings.h

Index: chrome/browser/content_settings/mixed_content_settings.h
diff --git a/chrome/browser/content_settings/mixed_content_settings.h b/chrome/browser/content_settings/mixed_content_settings.h
new file mode 100644
index 0000000000000000000000000000000000000000..6918c4fb9ac7186fd942af82e358147fa1560943
--- /dev/null
+++ b/chrome/browser/content_settings/mixed_content_settings.h
@@ -0,0 +1,47 @@
+// Copyright (c) 2016 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.
+
+#ifndef CHROME_BROWSER_CONTENT_SETTINGS_MIXED_CONTENT_SETTINGS_H_
+#define CHROME_BROWSER_CONTENT_SETTINGS_MIXED_CONTENT_SETTINGS_H_
+
+#include "content/public/browser/web_contents_observer.h"
+#include "content/public/browser/web_contents_user_data.h"
+
+namespace content {
+class SiteInstance;
+}
+
+// Controls mixed content related settings for the associated WebContents.
+class MixedContentSettings

[nasko, 2017/01/12 18:32:37]

Shouldn't this class name include "Observer", since it is one? Or TabHelper,
since it is installed through TabHelpers.

[carlosk, 2017/01/21 02:54:58]

I added the TabHelper suffix as I think it clarifies the expected behavior. But
FYI many of the other instances registered by TabHelpers are not especially
named.

+    : public content::WebContentsObserver,
+      public content::WebContentsUserData<MixedContentSettings> {
+ public:
+  ~MixedContentSettings() override;
+
+  // Enables running mixed-content resources in the associated WebContents/tab.
+  // This state is reset to the default of not allowed when the main frame
+  // navigates to a different SiteInstance.

[nasko, 2017/01/12 18:32:37]

Why does the SiteInstance matter?

[carlosk, 2017/01/21 02:54:58]

It does not so I removed it.

It is now better explained as an implementation detail in the .cc file.

+  void AllowRunningOfInsecureContent();
+
+  bool insecure_content_allowed_running() {

[nasko, 2017/01/12 18:32:37]

nit: is_running_insecure_content_allowed

[carlosk, 2017/01/21 02:54:58]

Done.

+    return insecure_content_allowed_running_;
+  }
+
+ private:
+  friend class content::WebContentsUserData<MixedContentSettings>;
+
+  explicit MixedContentSettings(content::WebContents* tab);
+
+  void DidFinishNavigation(
+      content::NavigationHandle* navigation_handle) override;
+
+  // Members to control mixed-content settings for the tab. They will be reset
+  // on cross-site main frame navigations.

[nasko, 2017/01/12 18:32:37]

Why do they stay the same for same-site navigations? What's the difference?

[carlosk, 2017/01/21 02:54:58]

See:
https://codereview.chromium.org/1905033002/diff/1030001/chrome/browser/conten...

Do you think I should also not explain it here, keeping it as an implementation
detail (already explained in the .cc)? 

[nasko, 2017/01/23 22:32:37]

Thanks for the pointer. Also the description you've expanded on in the .cc file
is good, so no need to duplicate here.

+  content::SiteInstance* insecure_content_site_instance_;
+  bool insecure_content_allowed_running_;
+
+  DISALLOW_COPY_AND_ASSIGN(MixedContentSettings);
+};
+
+#endif  // CHROME_BROWSER_CONTENT_SETTINGS_MIXED_CONTENT_SETTINGS_H_