Initial Implementation of Iframe Attribute for Feature Policy (Part 1)

third_party/WebKit/Source/core/html/HTMLIFrameElement.h

Index: third_party/WebKit/Source/core/html/HTMLIFrameElement.h
diff --git a/third_party/WebKit/Source/core/html/HTMLIFrameElement.h b/third_party/WebKit/Source/core/html/HTMLIFrameElement.h
index 6f1d0265f1c6c0ea00c17ad4363d88ef1982de28..4765d3fe3feddc0e44f4c5c43c9116fb2ad73393 100644
--- a/third_party/WebKit/Source/core/html/HTMLIFrameElement.h
+++ b/third_party/WebKit/Source/core/html/HTMLIFrameElement.h
@@ -26,9 +26,11 @@
 
 #include "core/CoreExport.h"
 #include "core/html/HTMLFrameElementBase.h"
+#include "core/html/HTMLIFrameElementAllow.h"
 #include "core/html/HTMLIFrameElementPermissions.h"
 #include "core/html/HTMLIFrameElementSandbox.h"
 #include "platform/Supplementable.h"
+#include "public/platform/WebFeaturePolicy.h"
 #include "public/platform/WebVector.h"
 #include "public/platform/modules/permissions/permission.mojom-blink.h"
 
@@ -46,9 +48,20 @@ class CORE_EXPORT HTMLIFrameElement final
   ~HTMLIFrameElement() override;
   DOMTokenList* sandbox() const;
   DOMTokenList* permissions() const;
+  DOMTokenList* allow() const;
 
   void sandboxValueWasSet();
   void permissionsValueWasSet();
+  void allowValueWasSet();
+
+  // Helper function used to process error message for sandbox, permissions, and
+  // allow.
+  static void addTokenError(StringBuilder& tokenErrors,

[Rick Byers, 2017/02/10 22:09:20]

I think hanging this off HTMLIFrameElement is fine (though I'd put the
implementation in the C++ file, and ideally make it a private instance method
instead of a public static).  But there's still a lot of redundant logic between
the different cases.

I was thinking something more like a template method over some enum T which
takes a DOMTokenList, an error-message substring like "feature names", a parsing
functor that attempts to convert a string to a 'T', and just returns the
Vector<T> with all the parsed enums (and generates the appropriate console
warning if any fail to parse).  But I haven't studied the three different cases
in as much detail as you have - perhaps there's not as much in common here as it
naively seems?  But the general concept of "turn a DOMTokenList into a set of
enums, generating an appropriate error if any are invalid" seems like a
reasonable abstraction to me.  WDYT?

BTW I'm not sure offhand what the best style is for the parsing functor - maybe
just a callback function, or maybe we have a common pattern we should copy.  

[lunalu1, 2017/02/10 23:04:12]

It is not as trivial as it seems.

parseSandboxPolicy returns an int (a bit map), and if you look at the code in
core/dom/SandboxFlags.cpp, there is no trivial parsing functor that will serve
this purpose.

I can sort of generalize parseDelegatedPermissions and parseAllowedFeatureNames,
but I will not only need a parsing functor (string -> enum), but also a
validating functor on the returend enum to verify if each string token was valid
or not.

I also changed the implementation for parseAllowedFeatureNames (I want a unique
set of feature names, so I have to sort the feature names first, which can't be
generalized).

We can write a base class for the three token lists, but the only methods can be
easily generalized are: constructor and destructor.

If we want to make the helper function a private method, then we have to move it
to DOMTokenList. HTMLIFrameElement owns the three DOMTokenList but there is no
easy way for the three token lists to access a private method of
HTMLIFrameElement.

But keeping the helper function in HTMLIFrameElement doesn't really make sense
to me. Even though it will cause duplicated code, I think it is still better to
write them out individually.

WDYT?

[Rick Byers, 2017/02/11 04:09:19]

Thanks for digging into this!  Ok, I see how this isn't easy, and this
duplicated logic is straight forward enough that I don't think it's worth any
more than the time we've already put in to try to unify it.  Sorry to waste your
time on it, thanks for exploring the options!

+                            const StringView& token) {
+    tokenErrors.append(tokenErrors.isEmpty() ? "'" : ", '");
+    tokenErrors.append(token);
+    tokenErrors.append("'");
+  }
 
  private:
   explicit HTMLIFrameElement(Document&);
@@ -91,8 +104,10 @@ class CORE_EXPORT HTMLIFrameElement final
   bool m_allowPaymentRequest;
   Member<HTMLIFrameElementSandbox> m_sandbox;
   Member<HTMLIFrameElementPermissions> m_permissions;
+  Member<HTMLIFrameElementAllow> m_allow;
 
   WebVector<mojom::blink::PermissionName> m_delegatedPermissions;
+  HashSet<WebFeaturePolicyFeature> m_allowedFeatureNames;
 
   ReferrerPolicy m_referrerPolicy;
 };