Use FeaturePolicy to implement iframe[allowfullscreen]

third_party/WebKit/Source/core/dom/Fullscreen.cpp

Index: third_party/WebKit/Source/core/dom/Fullscreen.cpp
diff --git a/third_party/WebKit/Source/core/dom/Fullscreen.cpp b/third_party/WebKit/Source/core/dom/Fullscreen.cpp
index 757f18f4fbced805d3946606551a02f88394704f..9a301da51091460d976068814c44f68ab3f22635 100644
--- a/third_party/WebKit/Source/core/dom/Fullscreen.cpp
+++ b/third_party/WebKit/Source/core/dom/Fullscreen.cpp
@@ -62,19 +62,52 @@ bool allowedToUseFullscreen(const Frame* frame) {
   if (!frame)
     return false;
 
-  // 2. If |document|'s browsing context is a top-level browsing context, then
-  // return true.
-  if (frame->isMainFrame())
+  if (!RuntimeEnabledFeatures::featurePolicyEnabled()) {
+    // 2. If |document|'s browsing context is a top-level browsing context, then
+    // return true.
+    if (frame->isMainFrame())
+      return true;
+
+    // 3. If |document|'s browsing context has a browsing context container that
+    // is an iframe element with an |allowattribute| attribute specified, and
+    // whose node document is allowed to use the feature indicated by
+    // |allowattribute|, then return true.
+    if (frame->owner() && frame->owner()->allowFullscreen())
+      return allowedToUseFullscreen(frame->tree().parent());
+
+    // 4. Return false.
+    return false;
+  }
+
+  // If Feature Policy is enabled, then we need this hack to support it, until
+  // we have proper support for <iframe allowfullscreen> in FP:
+
+  // 1. If FP, by itself, enables fullscreen in this document, then fullscreen
+  // is allowed.
+  if (frame->securityContext()->getFeaturePolicy()->isFeatureEnabled(
+          kFullscreenFeature)) {
     return true;
+  }
 
-  // 3. If |document|'s browsing context has a browsing context container that
-  // is an iframe element with an |allowattribute| attribute specified, and
-  // whose node document is allowed to use the feature indicated by
-  // |allowattribute|, then return true.
-  if (frame->owner() && frame->owner()->allowFullscreen())
-    return allowedToUseFullscreen(frame->tree().parent());
+  // 2. Otherwise, if the embedding frame's document is allowed to use

[foolip, 2017/01/19 17:14:18]

Can this part be turned into a step 4 before the return false, conditional on FP
being enabled? The duplication of the allowedToUseFullscreen calls looks
unfortunate and can perhaps be avoided.

[iclelland, 2017/01/19 19:32:15]

I was trying to maintain the integrity of the original algorithm as much as
possible, by just wrapping it in "if(!fp-enabled)". We could certainly avoid
duplicating the recursive call, but I thought it would make the whole method
less clear.

There are two things coming that will allow us to make this code simpler: 
- I've proposed the change to fullscreen to allow it in same-origin-subframes
(if allowed in the parent). There's been no I2S for that, though. If we could
ship that, then the two sides of this method will be almost identical, and we
could easily just wrap the call to isFeatureEnabled like you say.
- The other thing that we want to get done very soon (won't be for M57, but
definitely for M58) is supporting iframe attributes directly in the feature
policy framework. Once we have *that*, then the FP side becomes literally
"return isFeatureEnabled(fullscreen)" and the rest of the hack goes away.

+  // fullscreen (either through FP or otherwise), and either:
+  //   a) this is a same-origin embedded document, or
+  //   b) this document's iframe has the allowfullscreen attribute set,
+  // then fullscreen is allowed.
+  if (!frame->isMainFrame()) {
+    if (allowedToUseFullscreen(frame->tree().parent())) {
+      return (frame->owner() && frame->owner()->allowFullscreen()) ||
+             frame->tree()
+                 .parent()
+                 ->securityContext()
+                 ->getSecurityOrigin()
+                 ->isSameSchemeHostPortAndSuborigin(
+                     frame->securityContext()->getSecurityOrigin());
+    }
+  }
 
-  // 4. Return false.
+  // Otherwise, fullscreen is not allowed. (If we reach here and this is the
+  // main frame, then fullscreen must have been disabled by FP.)
   return false;
 }
 
@@ -103,43 +136,14 @@ bool allowedToRequestFullscreen(Document& document) {
 }
 
 // https://fullscreen.spec.whatwg.org/#fullscreen-is-supported
-// TODO(lunalu): update the placement of the feature policy code once it is in
-// https://fullscreen.spec.whatwg.org/.
-bool fullscreenIsSupported(Document& document) {
+bool fullscreenIsSupported(const Document& document) {
   LocalFrame* frame = document.frame();
   if (!frame)
     return false;
 
   // Fullscreen is supported if there is no previously-established user
   // preference, security risk, or platform limitation.
-  bool fullscreenSupported =
-      !document.settings() || document.settings()->fullscreenSupported();
-
-  if (!RuntimeEnabledFeatures::featurePolicyEnabled()) {
-    return fullscreenSupported;
-  }
-
-  // TODO(lunalu): clean all of this up once iframe attributes are supported
-  // for feature policy.
-  if (Frame* parent = frame->tree().parent()) {
-    // If FeaturePolicy is enabled, check the fullscreen is not disabled by
-    // policy in the parent frame.
-    if (fullscreenSupported &&
-        parent->securityContext()->getFeaturePolicy()->isFeatureEnabled(
-            kFullscreenFeature)) {
-      return true;
-    }
-  }
-  // Even if the iframe allowfullscreen attribute is not present, allow
-  // fullscreen to be enabled by feature policy.
-  else if (isFeatureEnabledInFrame(kFullscreenFeature, frame)) {
-    return true;
-  }
-
-  document.addConsoleMessage(ConsoleMessage::create(
-      JSMessageSource, WarningMessageLevel,

[lunalu1, 2017/01/10 18:14:57]

Do we want to be more implicit when fullscreen is disabled by FP? In that case,
we need to be able to return different console messages when
allowedToUseFullscreen returns false. 

-      "Fullscreen API is disabled by feature policy for this frame"));
-  return false;
+  return !document.settings() || document.settings()->fullscreenSupported();
 }
 
 // https://fullscreen.spec.whatwg.org/#fullscreen-element-ready-check