Don't enable portals in the renderer without the browser's knowledge

Currently, using
--enable-blink-features=Portals
or
--enable-blink-test-features
causes renderer kills because the renderer enables portals for itself
with runtime_enabled_features, however the browser only considers the
blink::features::kPortals feature.

We have the renderer respect the value of blink::features::kPortals.
We now ignore --enable-blink-features=Portals when the browser does not
have portals enabled and only set portals' experimental/test status if
the blink::features::kPortals default allows for it.

Since now portals are not exposed for the blink testing config for web
tests, we remove their interface listing expectations.

Bug: None
Change-Id: I02fe241e2ddd17d86eda896b2b2fa21c36bcb682
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2518427
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825114}

content/browser/portal/portal_browsertest.cc

@@ -5,6 +5,7 @@
 #include <memory>
 #include <tuple>
 
+#include "base/base_switches.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
@@ -2363,6 +2364,65 @@ IN_PROC_BROWSER_TEST_F(PortalBrowserTest, CallActivateTwice) {
 }
 #endif
 
+// Tests that various ways of enabling features via the command line produce a
+// valid configuration. That is, a configuration where we don't have the
+// renderer thinking that portals are enabled when the browser thinks portals
+// are disabled.
+class PortalsValidConfigurationBrowserTest
+    : public ContentBrowserTest,
+      public ::testing::WithParamInterface<
+          std::pair<const char*, const char*>> {
+ protected:
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    ContentBrowserTest::SetUpCommandLine(command_line);
+
+    std::string switch_name;
+    std::string switch_value;
+    std::tie(switch_name, switch_value) = GetParam();
+    if (switch_name == switches::kEnableFeatures) {
+      scoped_feature_list_.InitFromCommandLine(switch_value, "");
+    } else {
+      command_line->AppendSwitchASCII(switch_name, switch_value);
+    }
+  }
+
+  void SetUpOnMainThread() override {
+    ContentBrowserTest::SetUpOnMainThread();
+    host_resolver()->AddRule("*", "127.0.0.1");
+    ASSERT_TRUE(embedded_test_server()->Start());
+  }
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+namespace {
+const std::pair<const char*, const char*> kEnablingFlags[] = {
+    {switches::kEnableFeatures, blink::features::kPortals.name},
+    {switches::kEnableBlinkTestFeatures, ""},
+    {switches::kEnableExperimentalWebPlatformFeatures, ""},
+    {switches::kEnableBlinkFeatures, "Portals"}};
+}
+
+INSTANTIATE_TEST_SUITE_P(All,
+                         PortalsValidConfigurationBrowserTest,
+                         ::testing::ValuesIn(kEnablingFlags));
+
+IN_PROC_BROWSER_TEST_P(PortalsValidConfigurationBrowserTest,
+                       ConfigurationIsValid) {
+  ASSERT_TRUE(NavigateToURL(
+      shell(), embedded_test_server()->GetURL("portal.test", "/title1.html")));
+  WebContentsImpl* web_contents_impl =
+      static_cast<WebContentsImpl*>(shell()->web_contents());
+  RenderFrameHostImpl* possible_portal_host = web_contents_impl->GetMainFrame();
+
+  bool html_portal_element_exposed =
+      EvalJs(possible_portal_host, "'HTMLPortalElement' in self").ExtractBool();
+
+  if (html_portal_element_exposed)
+    EXPECT_TRUE(Portal::IsEnabled());
+}
+
 namespace {
 
 static constexpr struct {
@@ -2463,8 +2523,11 @@ IN_PROC_BROWSER_TEST_F(PortalOriginTrialBrowserTest, WithTrialToken) {
       static_cast<WebContentsImpl*>(shell()->web_contents());
   ASSERT_TRUE(NavigateToURL(web_contents_impl,
                             GURL("https://portal.test/?origintrial=portals")));
-  EXPECT_EQ(PlatformSupportsPortalsOriginTrial(),
-            EvalJs(web_contents_impl, "'HTMLPortalElement' in self"));
+  bool html_portal_element_exposed =
+      EvalJs(web_contents_impl, "'HTMLPortalElement' in self").ExtractBool();
+  EXPECT_EQ(PlatformSupportsPortalsOriginTrial(), html_portal_element_exposed);
+  if (html_portal_element_exposed)
+    EXPECT_TRUE(Portal::IsEnabled());
 }
 
 class PortalPixelBrowserTest : public PortalBrowserTest {

content/child/runtime_features.cc

@@ -6,6 +6,7 @@
 
 #include <vector>
 
+#include "base/base_switches.h"
 #include "base/command_line.h"
 #include "base/feature_list.h"
 #include "base/metrics/field_trial.h"
@@ -592,6 +593,19 @@ void SetCustomizedRuntimeFeaturesFromCombinedArgs(
   }
 }
 
+// Ensures that the various ways of enabling/disabling features do not produce
+// an invalid configuration.
+void ResolveInvalidConfigurations() {
+  // Portals cannot be enabled without the support of the browser process.
+  if (!base::FeatureList::IsEnabled(blink::features::kPortals)) {
+    LOG_IF(WARNING, WebRuntimeFeatures::IsPortalsEnabled())
+        << "Portals cannot be enabled in this configuration. Use --"
+        << switches::kEnableFeatures << "=" << blink::features::kPortals.name
+        << " instead.";
+    WebRuntimeFeatures::EnablePortals(false);
+  }
+}
+
 }  // namespace
 
 namespace content {
@@ -641,6 +655,8 @@ void SetRuntimeFeaturesDefaultsAndUpdateFromArgs(
        FeaturesFromSwitch(command_line, switches::kDisableBlinkFeatures)) {
     WebRuntimeFeatures::EnableFeatureFromString(feature, false);
   }
+
+  ResolveInvalidConfigurations();
 }
 
 }  // namespace content

third_party/blink/common/features.cc

@@ -114,8 +114,9 @@ const base::Feature kPlzDedicatedWorker{"PlzDedicatedWorker",
                                         base::FEATURE_DISABLED_BY_DEFAULT};
 
 // Enable Portals. https://crbug.com/865123.
-// For the current origin trial (https://crbug.com/1040212), this is enabled on
-// Android only.
+// Note that default enabling this does not expose the portal
+// element on its own, but does allow its use with an origin trial. This was the
+// case for the M85 Android only origin trial (https://crbug.com/1040212).
 const base::Feature kPortals {
   "Portals",
 #if defined(OS_ANDROID)

third_party/blink/public/platform/web_runtime_features.h

@@ -149,6 +149,7 @@ class WebRuntimeFeatures {
   BLINK_PLATFORM_EXPORT static void EnablePictureInPictureAPI(bool);
   BLINK_PLATFORM_EXPORT static void EnablePointerLockOptions(bool);
   BLINK_PLATFORM_EXPORT static void EnablePortals(bool);
+  BLINK_PLATFORM_EXPORT static bool IsPortalsEnabled();
   BLINK_PLATFORM_EXPORT static void EnablePreciseMemoryInfo(bool);
   BLINK_PLATFORM_EXPORT static void EnablePresentationAPI(bool);
   BLINK_PLATFORM_EXPORT static void EnablePushMessaging(bool);

third_party/blink/renderer/platform/exported/web_runtime_features.cc

@@ -365,6 +365,10 @@ void WebRuntimeFeatures::EnablePortals(bool enable) {
   RuntimeEnabledFeatures::SetPortalsEnabled(enable);
 }
 
+bool WebRuntimeFeatures::IsPortalsEnabled() {
+  return RuntimeEnabledFeatures::PortalsEnabledByRuntimeFlag();
+}
+
 void WebRuntimeFeatures::EnableScriptedSpeechRecognition(bool enable) {
   RuntimeEnabledFeatures::SetScriptedSpeechRecognitionEnabled(enable);
 }

third_party/blink/renderer/platform/runtime_enabled_features.json5

@@ -1558,8 +1558,14 @@
     },
     {
       name: "Portals",
-      status: "test",
+      // Portals must be enabled by blink::features::kPortals as we require the
+      // support of the browser process to enable the feature. Enabling this
+      // feature specifically within blink has no effect. An experimental/test
+      // status can only be set here if the default of blink::features::kPortals
+      // allows for it.
+      status: {"Android": "test"},
       origin_trial_feature_name: "Portals",
+      origin_trial_os: ["android"],
     },
     {
       name: "PreciseMemoryInfo",

third_party/blink/web_tests/fast/dom/Window/property-access-on-cached-window-after-frame-navigated-expected.txt

@@ -148,7 +148,6 @@ PASS oldChildWindow.onpointerover is newChildWindow.onpointerover
 PASS oldChildWindow.onpointerrawupdate is newChildWindow.onpointerrawupdate
 PASS oldChildWindow.onpointerup is newChildWindow.onpointerup
 PASS oldChildWindow.onpopstate is newChildWindow.onpopstate
-PASS oldChildWindow.onportalactivate is newChildWindow.onportalactivate
 PASS oldChildWindow.onprogress is newChildWindow.onprogress
 PASS oldChildWindow.onratechange is newChildWindow.onratechange
 PASS oldChildWindow.onrejectionhandled is newChildWindow.onrejectionhandled
@@ -220,7 +219,6 @@ PASS oldChildWindow.performance.timing.secureConnectionStart is newChildWindow.p
 PASS oldChildWindow.performance.timing.unloadEventEnd is newChildWindow.performance.timing.unloadEventEnd
 PASS oldChildWindow.performance.timing.unloadEventStart is newChildWindow.performance.timing.unloadEventStart
 PASS oldChildWindow.personalbar.visible is newChildWindow.personalbar.visible
-PASS oldChildWindow.portalHost is newChildWindow.portalHost
 PASS oldChildWindow.scheduler.currentTaskSignal.aborted is newChildWindow.scheduler.currentTaskSignal.aborted
 PASS oldChildWindow.scheduler.currentTaskSignal.onabort is newChildWindow.scheduler.currentTaskSignal.onabort
 PASS oldChildWindow.scheduler.currentTaskSignal.onprioritychange is newChildWindow.scheduler.currentTaskSignal.onprioritychange

third_party/blink/web_tests/fast/dom/Window/property-access-on-cached-window-after-frame-removed-and-gced-expected.txt

@@ -113,7 +113,6 @@ PASS childWindow.onpointerover is null
 PASS childWindow.onpointerrawupdate is null
 PASS childWindow.onpointerup is null
 PASS childWindow.onpopstate is null
-PASS childWindow.onportalactivate is null
 PASS childWindow.onprogress is null
 PASS childWindow.onratechange is null
 PASS childWindow.onrejectionhandled is null
@@ -161,7 +160,6 @@ PASS childWindow.pageXOffset is 0
 PASS childWindow.pageYOffset is 0
 PASS childWindow.performance.onresourcetimingbufferfull is null
 PASS childWindow.personalbar.visible is false
-PASS childWindow.portalHost is null
 PASS childWindow.scheduler.currentTaskSignal.aborted is false
 PASS childWindow.scheduler.currentTaskSignal.onabort is null
 PASS childWindow.scheduler.currentTaskSignal.onprioritychange is null

third_party/blink/web_tests/fast/dom/Window/property-access-on-cached-window-after-frame-removed-expected.txt

@@ -113,7 +113,6 @@ PASS childWindow.onpointerover is null
 PASS childWindow.onpointerrawupdate is null
 PASS childWindow.onpointerup is null
 PASS childWindow.onpopstate is null
-PASS childWindow.onportalactivate is null
 PASS childWindow.onprogress is null
 PASS childWindow.onratechange is null
 PASS childWindow.onrejectionhandled is null
@@ -161,7 +160,6 @@ PASS childWindow.pageXOffset is 0
 PASS childWindow.pageYOffset is 0
 PASS childWindow.performance.onresourcetimingbufferfull is null
 PASS childWindow.personalbar.visible is false
-PASS childWindow.portalHost is null
 PASS childWindow.scheduler.currentTaskSignal.aborted is false
 PASS childWindow.scheduler.currentTaskSignal.onabort is null
 PASS childWindow.scheduler.currentTaskSignal.onprioritychange is null

third_party/blink/web_tests/webexposed/element-instance-property-listing-expected.txt

@@ -463,7 +463,6 @@ html element body
     property onpagehide
     property onpageshow
     property onpopstate
-    property onportalactivate
     property onrejectionhandled
     property onstorage
     property ontimezonechange
@@ -616,7 +615,6 @@ html element frameset
     property onpagehide
     property onpageshow
     property onpopstate
-    property onportalactivate
     property onrejectionhandled
     property onstorage
     property ontimezonechange
@@ -904,13 +902,6 @@ html element param
     property valueType
 html element picture
 html element plaintext
-html element portal
-    property activate
-    property onmessage
-    property onmessageerror
-    property postMessage
-    property referrerPolicy
-    property src
 html element pre
     property width
 html element progress

third_party/blink/web_tests/webexposed/global-interface-listing-expected.txt

@@ -2781,7 +2781,6 @@ interface HTMLBodyElement : HTMLElement
     getter onpagehide
     getter onpageshow
     getter onpopstate
-    getter onportalactivate
     getter onrejectionhandled
     getter onresize
     getter onscroll
@@ -2812,7 +2811,6 @@ interface HTMLBodyElement : HTMLElement
     setter onpagehide
     setter onpageshow
     setter onpopstate
-    setter onportalactivate
     setter onrejectionhandled
     setter onresize
     setter onscroll
@@ -3276,7 +3274,6 @@ interface HTMLFrameSetElement : HTMLElement
     getter onpagehide
     getter onpageshow
     getter onpopstate
-    getter onportalactivate
     getter onrejectionhandled
     getter onresize
     getter onscroll
@@ -3303,7 +3300,6 @@ interface HTMLFrameSetElement : HTMLElement
     setter onpagehide
     setter onpageshow
     setter onpopstate
-    setter onportalactivate
     setter onrejectionhandled
     setter onresize
     setter onscroll
@@ -3878,19 +3874,6 @@ interface HTMLParamElement : HTMLElement
 interface HTMLPictureElement : HTMLElement
     attribute @@toStringTag
     method constructor
-interface HTMLPortalElement : HTMLElement
-    attribute @@toStringTag
-    getter onmessage
-    getter onmessageerror
-    getter referrerPolicy
-    getter src
-    method activate
-    method constructor
-    method postMessage
-    setter onmessage
-    setter onmessageerror
-    setter referrerPolicy
-    setter src
 interface HTMLPreElement : HTMLElement
     attribute @@toStringTag
     getter width
@@ -6140,19 +6123,6 @@ interface PopStateEvent : Event
     attribute @@toStringTag
     getter state
     method constructor
-interface PortalActivateEvent : Event
-    attribute @@toStringTag
-    getter data
-    method adoptPredecessor
-    method constructor
-interface PortalHost : EventTarget
-    attribute @@toStringTag
-    getter onmessage
-    getter onmessageerror
-    method constructor
-    method postMessage
-    setter onmessage
-    setter onmessageerror
 interface Presentation
     attribute @@toStringTag
     getter defaultRequest
@@ -11012,7 +10982,6 @@ namespace CSS
     getter onpointerrawupdate
     getter onpointerup
     getter onpopstate
-    getter onportalactivate
     getter onprogress
     getter onratechange
     getter onrejectionhandled
@@ -11062,7 +11031,6 @@ namespace CSS
     getter parent
     getter performance
     getter personalbar
-    getter portalHost
     getter scheduler
     getter screen
     getter screenLeft
@@ -11227,7 +11195,6 @@ namespace CSS
     setter onpointerrawupdate
     setter onpointerup
     setter onpopstate
-    setter onportalactivate
     setter onprogress
     setter onratechange
     setter onrejectionhandled