Reenable feature policy control over fullscreen

content/browser/site_per_process_browsertest.cc

Index: content/browser/site_per_process_browsertest.cc
diff --git a/content/browser/site_per_process_browsertest.cc b/content/browser/site_per_process_browsertest.cc
index e1594717613ba57a1b265b6e2e9b51753b1dc8e8..c5d0b8c58ba7e11b2600fed50d463ba2c34b7470 100644
--- a/content/browser/site_per_process_browsertest.cc
+++ b/content/browser/site_per_process_browsertest.cc
@@ -752,6 +752,21 @@ class SitePerProcessFeaturePolicyBrowserTest
   }
 };
 
+// SitePerProcessFeaturePolicyDisabledBrowserTest
+
+class SitePerProcessFeaturePolicyDisabledBrowserTest
+    : public SitePerProcessBrowserTest {
+ public:
+  SitePerProcessFeaturePolicyDisabledBrowserTest() {}
+
+ protected:
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    SitePerProcessBrowserTest::SetUpCommandLine(command_line);
+    command_line->AppendSwitchASCII(switches::kDisableBlinkFeatures,
+                                    "FeaturePolicy");
+  }
+};
+
 IN_PROC_BROWSER_TEST_F(SitePerProcessHighDPIBrowserTest,
                        SubframeLoadsWithCorrectDeviceScaleFactor) {
   GURL main_url(embedded_test_server()->GetURL(
@@ -8902,10 +8917,10 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
 #endif
 
 // Check that out-of-process frames correctly calculate their ability to enter
-// fullscreen. A frame is allowed enter fullscreen if the allowFullscreen
-// attribute is present in all of its ancestor <iframe> elements.  For OOPIF,
-// when a parent frame changes this attribute, the change is replicated to the
-// child frame and its proxies.
+// fullscreen when Feature Policy is disabled. A frame is allowed to enter
+// fullscreen if the allowFullscreen attribute is present in all of its ancestor
+// <iframe> elements.  For OOPIF, when a parent frame changes this attribute,
+// the change is replicated to the child frame and its proxies.

[alexmos, 2017/08/16 22:12:16]

It might be useful to say explicitly that this is now testing deprecated
behavior that will be eventually removed once Feature Policy launches, and give
pointers to bugs that explain changes in the new path.

Once FP launches, this test will presumably need to be refactored to not assume
that dynamic changes to allowfullscreen take place immediately?  Are there any
other changes that will break this test?

[iclelland, 2017/08/17 20:01:56]

There are two changes that could break tests -- the fact that allowfullscreen is
no longer dynamic, and the fact that fullscreen is allowed by default in
same-origin frames -- this test only relies on the first. If we want to keep the
test, we'll have to update it to look more like
SitePerProcessFeaturePolicyBrowserTest.ContainerPolicyDynamic.

There are a number of other tests (in the SitePerProcessFeaturePolicyBrowserTest
class) which test things like policy replication, and we have unit tests that
ensure that the correct policy is constructed in the first place, so we may not
need this special-case-test for allowfullscreen at all once the old paths have
been removed.

 //
 // The test checks the following cases:
 //
@@ -8913,7 +8928,8 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
 // 2. Attribute injected dynamically via JavaScript
 // 3. Multiple levels of nesting (A-embed-B-embed-C)
 // 4. Cross-site subframe navigation
-IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, AllowFullscreen) {
+IN_PROC_BROWSER_TEST_F(SitePerProcessFeaturePolicyDisabledBrowserTest,
+                       AllowFullscreen) {
   // Load a page with a cross-site <iframe allowFullscreen>.
   GURL url_1(embedded_test_server()->GetURL(
       "a.com", "/page_with_allowfullscreen_frame.html"));