PlzNavigate: Enforce 'frame-src' CSP on the browser.

content/browser/frame_host/csp_context_impl.h

Index: content/browser/frame_host/csp_context_impl.h
diff --git a/content/browser/frame_host/csp_context_impl.h b/content/browser/frame_host/csp_context_impl.h
new file mode 100644
index 0000000000000000000000000000000000000000..2fff17a16261222ad6bfa5213b47a4c5bd320176
--- /dev/null
+++ b/content/browser/frame_host/csp_context_impl.h
@@ -0,0 +1,30 @@
+// Copyright 2017 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 CONTENT_BROWSER_FRAME_HOST_CSP_CONTEXT_H_
+#define CONTENT_BROWSER_FRAME_HOST_CSP_CONTEXT_H_

[nasko, 2017/02/11 00:01:23]

Missing _IMPL_ part in the include guard.

[arthursonzogni, 2017/02/13 16:33:20]

Done.

+
+#include "content/common/content_security_policy/csp_context.h"
+
+namespace content {
+
+class FrameTreeNode;
+
+class CSPContextImpl : public CSPContext {
+ public:
+  CSPContextImpl(FrameTreeNode* frame_tree_node);
+  void LogToConsole(const std::string& message) override;
+
+  // Inform the renderer process that a navigation has been blocked by a content
+  // security policy.
+  void ReportViolation(const CSPViolationParams& violation_params) override;

[alexmos, 2017/02/10 22:59:53]

Just a heads-up that lukasza@ worked on some plumbing for CSP violations in
https://codereview.chromium.org/2190183002/.  This will probably supercede most
of that CL, but might be good to look over that CL to compare approaches and to
see if any of the discussion applies here.  For example, do the races Lukasz
discussed in RenderFrameProxyHost::OnForwardContentSecurityPolicyViolation no
longer apply here?

[arthursonzogni, 2017/02/13 16:33:20]

Thanks! I don't know yet if a race could happens, I will try to understand.

+
+ private:
+  bool SchemeShouldBypass(const base::StringPiece& scheme) override;

[alexmos, 2017/02/10 22:59:53]

Can this be static?  Doesn't look like it needs frame_tree_node_ for anything.

[arthursonzogni, 2017/02/13 16:33:20]

Yes, it doesn't need frame_tree_node_, but it's a virtual method, so it can't be
static.
In my previous patch:
https://codereview.chromium.org/2612793002/
The code in content/common/content_security_policy/* is in a way self-contained.
The behavior/state it depends on are implemented here in CSPContextImpl. It is
also implemented in a different way in tests.

[alexmos, 2017/02/14 06:57:19]

Acknowledged.

+
+  FrameTreeNode* frame_tree_node_;  // Never nullptr.

[nasko, 2017/02/11 00:01:23]

nit: Comments like that usually go above the member variable.

[arthursonzogni, 2017/02/13 16:33:20]

Done.

+};
+}  // namespace content

[nasko, 2017/02/11 00:01:23]

nit: Empty line before end of namespace.

[arthursonzogni, 2017/02/13 16:33:20]

Done.

+
+#endif  // CONTENT_BROWSER_FRAME_HOST_CSP_CONTEXT_H_ */

[nasko, 2017/02/11 00:01:23]

Why is there "*/" at the end?

[arthursonzogni, 2017/02/13 16:33:20]

Oops, vim-snipmate + wrong conversion to the google coding style.