Forward kWebAppEnableLinkCapturing flag into Blink as WebAppLinkCapturing flag

This CL simplifies the declarative link capturing flag situation by
making WebAppLinkCapturing inherit the value of
kWebAppEnableLinkCapturing. This means Blink only needs to read one flag
to determine whether this feature is enabled.

This is required for using implied_by: ['WebAppLinkCapturing'] in
runtime_enabled_features.json5 in a future CL.


Bug: 1163398
Change-Id: Ia0593cf9cf572b865a9c86497fb9e49fe033e58a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2966419
Reviewed-by: Nate Chapin <[email protected]>
Reviewed-by: Dominick Ng <[email protected]>
Commit-Queue: Alan Cutter <[email protected]>
Cr-Commit-Position: refs/heads/master@{#893685}

Author: alancutter

content/child/runtime_features.cc

@@ -397,6 +397,7 @@ void SetRuntimeFeaturesFromChromiumFeatures() {
                kThrottleDisplayNoneAndVisibilityHiddenCrossOriginIframes},
           {"TrustedDOMTypes", features::kTrustedDOMTypes},
           {"UserAgentClientHint", features::kUserAgentClientHint},
+          {"WebAppLinkCapturing", blink::features::kWebAppEnableLinkCapturing},
           {"WebAppTabStrip", features::kDesktopPWAsTabStrip},
           {"WebAppWindowControlsOverlay",
            features::kWebAppWindowControlsOverlay},

third_party/blink/renderer/modules/manifest/manifest_parser.cc

@@ -1334,12 +1334,8 @@ String ManifestParser::ParseGCMSenderID(const JSONObject* object) {
 
 mojom::blink::CaptureLinks ManifestParser::ParseCaptureLinks(
     const JSONObject* object) {
-  // Parse if either the command line flag is passed (for about:flags) or the
-  // runtime enabled feature is turned on (for origin trial).
-  if (!base::FeatureList::IsEnabled(features::kWebAppEnableLinkCapturing) &&
-      !RuntimeEnabledFeatures::WebAppLinkCapturingEnabled(feature_context_)) {
+  if (!RuntimeEnabledFeatures::WebAppLinkCapturingEnabled(feature_context_))
     return mojom::blink::CaptureLinks::kUndefined;
-  }
 
   String capture_links_string;
   if (object->GetString("capture_links", &capture_links_string)) {

third_party/blink/renderer/modules/manifest/manifest_parser_unittest.cc

@@ -4402,15 +4402,6 @@ TEST_F(ManifestParserTest, GCMSenderIDParseRules) {
   }
 }
 
-TEST_F(ManifestParserTest, CaptureLinksParseRules) {
-  // Feature not enabled, should not be parsed.
-  {
-    auto& manifest = ParseManifest(R"({ "capture_links": "none" })");
-    EXPECT_EQ(manifest->capture_links, mojom::blink::CaptureLinks::kUndefined);
-    EXPECT_EQ(0u, GetErrorCount());
-  }
-}
-
 TEST_F(ManifestParserTest, StorageIsolationDisabled) {
   // Feature not enabled, should not be parsed.
   {
@@ -4447,110 +4438,118 @@ TEST_F(ManifestParserTest, StorageIsolationBadScope) {
   }
 }
 
-class ManifestCaptureLinksParserTest : public ManifestParserTest {
- public:
-  ManifestCaptureLinksParserTest() {
-    scoped_feature_list_.InitAndEnableFeature(
-        features::kWebAppEnableLinkCapturing);
-  }
-
- private:
-  base::test::ScopedFeatureList scoped_feature_list_;
-};
-
-TEST_F(ManifestCaptureLinksParserTest, CaptureLinksParseRules) {
-  // Smoke test.
-  {
-    auto& manifest = ParseManifest(R"({ "capture_links": "none" })");
-    EXPECT_EQ(manifest->capture_links, mojom::blink::CaptureLinks::kNone);
-    EXPECT_EQ(0u, GetErrorCount());
-  }
+TEST_F(ManifestParserTest, CaptureLinksParseRules) {
   {
-    auto& manifest = ParseManifest(R"({ "capture_links": ["new-client"] })");
-    EXPECT_EQ(manifest->capture_links, mojom::blink::CaptureLinks::kNewClient);
-    EXPECT_EQ(0u, GetErrorCount());
-  }
+    ScopedWebAppLinkCapturingForTest feature(false);
 
-  // Empty array is fine.
-  {
-    auto& manifest = ParseManifest(R"({ "capture_links": [] })");
+    // Feature not enabled, should not be parsed.
+    auto& manifest = ParseManifest(R"({ "capture_links": "none" })");
     EXPECT_EQ(manifest->capture_links, mojom::blink::CaptureLinks::kUndefined);
     EXPECT_EQ(0u, GetErrorCount());
   }
 
-  // Unknown single string.
-  {
-    auto& manifest = ParseManifest(R"({ "capture_links": "unknown" })");
-    EXPECT_EQ(manifest->capture_links, mojom::blink::CaptureLinks::kUndefined);
-    EXPECT_EQ(1u, GetErrorCount());
-    EXPECT_EQ("capture_links value 'unknown' ignored, unknown value.",
-              errors()[0]);
-  }
-
-  // First known value in array is used.
   {
-    auto& manifest = ParseManifest(
-        R"({ "capture_links": ["none", "existing-client-navigate"] })");
-    EXPECT_EQ(manifest->capture_links, mojom::blink::CaptureLinks::kNone);
-    EXPECT_EQ(0u, GetErrorCount());
-  }
-  {
-    auto& manifest = ParseManifest(R"({
-      "capture_links": [
-        "unknown",
-        "existing-client-navigate",
-        "also-unknown",
-        "none"
-      ]
-    })");
-    EXPECT_EQ(manifest->capture_links,
-              mojom::blink::CaptureLinks::kExistingClientNavigate);
-    EXPECT_EQ(1u, GetErrorCount());
-    EXPECT_EQ("capture_links value 'unknown' ignored, unknown value.",
-              errors()[0]);
-  }
-  {
-    auto& manifest = ParseManifest(R"({
-      "capture_links": [
-        1234,
-        "new-client",
-        null,
-        "none"
-      ]
-    })");
-    EXPECT_EQ(manifest->capture_links, mojom::blink::CaptureLinks::kNewClient);
-    EXPECT_EQ(1u, GetErrorCount());
-    EXPECT_EQ("capture_links value '1234' ignored, string expected.",
-              errors()[0]);
-  }
-
-  // Don't parse if the property isn't a string or array of strings.
-  {
-    auto& manifest = ParseManifest(R"({ "capture_links": null })");
-    EXPECT_EQ(manifest->capture_links, mojom::blink::CaptureLinks::kUndefined);
-    EXPECT_EQ(1u, GetErrorCount());
-    EXPECT_EQ(
-        "property 'capture_links' ignored, type string or array of strings "
-        "expected.",
-        errors()[0]);
-  }
-  {
-    auto& manifest = ParseManifest(R"({ "capture_links": 1234 })");
-    EXPECT_EQ(manifest->capture_links, mojom::blink::CaptureLinks::kUndefined);
-    EXPECT_EQ(1u, GetErrorCount());
-    EXPECT_EQ(
-        "property 'capture_links' ignored, type string or array of strings "
-        "expected.",
-        errors()[0]);
-  }
-  {
-    auto& manifest = ParseManifest(R"({ "capture_links": [12, 34] })");
-    EXPECT_EQ(manifest->capture_links, mojom::blink::CaptureLinks::kUndefined);
-    EXPECT_EQ(2u, GetErrorCount());
-    EXPECT_EQ("capture_links value '12' ignored, string expected.",
-              errors()[0]);
-    EXPECT_EQ("capture_links value '34' ignored, string expected.",
-              errors()[1]);
+    ScopedWebAppLinkCapturingForTest feature(true);
+    // Smoke test.
+    {
+      auto& manifest = ParseManifest(R"({ "capture_links": "none" })");
+      EXPECT_EQ(manifest->capture_links, mojom::blink::CaptureLinks::kNone);
+      EXPECT_EQ(0u, GetErrorCount());
+    }
+    {
+      auto& manifest = ParseManifest(R"({ "capture_links": ["new-client"] })");
+      EXPECT_EQ(manifest->capture_links,
+                mojom::blink::CaptureLinks::kNewClient);
+      EXPECT_EQ(0u, GetErrorCount());
+    }
+
+    // Empty array is fine.
+    {
+      auto& manifest = ParseManifest(R"({ "capture_links": [] })");
+      EXPECT_EQ(manifest->capture_links,
+                mojom::blink::CaptureLinks::kUndefined);
+      EXPECT_EQ(0u, GetErrorCount());
+    }
+
+    // Unknown single string.
+    {
+      auto& manifest = ParseManifest(R"({ "capture_links": "unknown" })");
+      EXPECT_EQ(manifest->capture_links,
+                mojom::blink::CaptureLinks::kUndefined);
+      EXPECT_EQ(1u, GetErrorCount());
+      EXPECT_EQ("capture_links value 'unknown' ignored, unknown value.",
+                errors()[0]);
+    }
+
+    // First known value in array is used.
+    {
+      auto& manifest = ParseManifest(
+          R"({ "capture_links": ["none", "existing-client-navigate"] })");
+      EXPECT_EQ(manifest->capture_links, mojom::blink::CaptureLinks::kNone);
+      EXPECT_EQ(0u, GetErrorCount());
+    }
+    {
+      auto& manifest = ParseManifest(R"({
+        "capture_links": [
+          "unknown",
+          "existing-client-navigate",
+          "also-unknown",
+          "none"
+        ]
+      })");
+      EXPECT_EQ(manifest->capture_links,
+                mojom::blink::CaptureLinks::kExistingClientNavigate);
+      EXPECT_EQ(1u, GetErrorCount());
+      EXPECT_EQ("capture_links value 'unknown' ignored, unknown value.",
+                errors()[0]);
+    }
+    {
+      auto& manifest = ParseManifest(R"({
+        "capture_links": [
+          1234,
+          "new-client",
+          null,
+          "none"
+        ]
+      })");
+      EXPECT_EQ(manifest->capture_links,
+                mojom::blink::CaptureLinks::kNewClient);
+      EXPECT_EQ(1u, GetErrorCount());
+      EXPECT_EQ("capture_links value '1234' ignored, string expected.",
+                errors()[0]);
+    }
+
+    // Don't parse if the property isn't a string or array of strings.
+    {
+      auto& manifest = ParseManifest(R"({ "capture_links": null })");
+      EXPECT_EQ(manifest->capture_links,
+                mojom::blink::CaptureLinks::kUndefined);
+      EXPECT_EQ(1u, GetErrorCount());
+      EXPECT_EQ(
+          "property 'capture_links' ignored, type string or array of strings "
+          "expected.",
+          errors()[0]);
+    }
+    {
+      auto& manifest = ParseManifest(R"({ "capture_links": 1234 })");
+      EXPECT_EQ(manifest->capture_links,
+                mojom::blink::CaptureLinks::kUndefined);
+      EXPECT_EQ(1u, GetErrorCount());
+      EXPECT_EQ(
+          "property 'capture_links' ignored, type string or array of strings "
+          "expected.",
+          errors()[0]);
+    }
+    {
+      auto& manifest = ParseManifest(R"({ "capture_links": [12, 34] })");
+      EXPECT_EQ(manifest->capture_links,
+                mojom::blink::CaptureLinks::kUndefined);
+      EXPECT_EQ(2u, GetErrorCount());
+      EXPECT_EQ("capture_links value '12' ignored, string expected.",
+                errors()[0]);
+      EXPECT_EQ("capture_links value '34' ignored, string expected.",
+                errors()[1]);
+    }
   }
 }