Reenable feature policy control over fullscreen

content/browser/site_per_process_browsertest.cc

 // Copyright (c) 2012 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.
 
 #include "content/browser/site_per_process_browsertest.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
@@ skipping 734 matching lines @@
 
   ParsedFeaturePolicyHeader CreateFPHeaderMatchesAll(
       blink::WebFeaturePolicyFeature feature) {
     ParsedFeaturePolicyHeader result(1);
     result[0].feature = feature;
     result[0].matches_all_origins = true;
     return result;
   }
 };
 
+// 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(
       "a.com", "/cross_site_iframe_factory.html?a(b)"));
   EXPECT_TRUE(NavigateToURL(shell(), main_url));
 
   // On Android forcing device scale factor does not work for tests, therefore
   // we ensure that make frame and iframe have the same DIP scale there, but
   // not necessarily kDeviceScaleFactor.
   const double expected_dip_scale =
@@ skipping 8130 matching lines @@
   EXPECT_FALSE(base::ContainsKey(
       web_contents()->pending_widget_views_,
       std::make_pair(process1->GetID(), filter1->routing_id())));
   EXPECT_FALSE(base::ContainsKey(
       web_contents()->pending_widget_views_,
       std::make_pair(process2->GetID(), filter2->routing_id())));
 }
 #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:
 //
 // 1. Static attribute (<iframe allowfullscreen>)
 // 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"));
   EXPECT_TRUE(NavigateToURL(shell(), url_1));
 
   WebContentsImpl* contents = web_contents();
   FrameTreeNode* root = contents->GetFrameTree()->root();
 
   // Helper to check if a frame is allowed to go fullscreen on the renderer
   // side.
@@ skipping 2353 matching lines @@
   SimpleTap(gfx::Point(point_outside_iframe.x(), point_outside_iframe.y()));
   selection_controller_client->Wait();
 
   EXPECT_EQ(ui::TouchSelectionController::INACTIVE,
             parent_view->touch_selection_controller()->active_status());
 }
 
 #endif  // defined(OS_ANDROID)
 
 }  // namespace content