Remove legacy handling for Android C++ -> Java base::Feature extractor
Bug: 1364289
Change-Id: I559c3bee3c7d7ae191d3bbe58fef8e8a8ecb81cd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3931362
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Egor Pasko <pasko@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1054166}
diff --git a/build/android/gyp/java_cpp_features.py b/build/android/gyp/java_cpp_features.py
index 5aeb219..451acb6 100755
--- a/build/android/gyp/java_cpp_features.py
+++ b/build/android/gyp/java_cpp_features.py
@@ -20,19 +20,10 @@
# ExtractConstantName() -> 'ConstantName'
# ExtractValue() -> '"StringNameOfTheFeature"'
FEATURE_RE = re.compile(r'BASE_FEATURE\(k([^,]+),')
- # Ex. 'const base::Feature kConstantName{"StringNameOfTheFeature", ...};'
- # would parse as:
- # ExtractConstantName() -> 'ConstantName'
- # ExtractValue() -> '"StringNameOfTheFeature"'
- LEGACY_FEATURE_RE = re.compile(
- r'\s*const(?:\s+\w+_EXPORT)? (?:base::)?Feature\s+k(\w+)\s*(?:=\s*)?')
VALUE_RE = re.compile(r'\s*("(?:\"|[^"])*")\s*,')
def ExtractConstantName(self, line):
match = FeatureParserDelegate.FEATURE_RE.match(line)
- if match:
- return match.group(1)
- match = FeatureParserDelegate.LEGACY_FEATURE_RE.match(line)
return match.group(1) if match else None
def ExtractValue(self, line):
diff --git a/build/android/gyp/java_cpp_features_tests.py b/build/android/gyp/java_cpp_features_tests.py
index 7242ba2f..3053955 100755
--- a/build/android/gyp/java_cpp_features_tests.py
+++ b/build/android/gyp/java_cpp_features_tests.py
@@ -27,14 +27,14 @@
// Comment followed by unrelated code.
int foo() { return 3; }
-// Real comment.
-const base::Feature kSomeFeature{"SomeFeature",
- base::FEATURE_DISABLED_BY_DEFAULT};
+// Real comment. base::Feature intentionally split across two lines.
+BASE_FEATURE(kSomeFeature, "SomeFeature",
+ base::FEATURE_DISABLED_BY_DEFAULT);
// Real comment that spans
// multiple lines.
-const base::Feature kSomeOtherFeature{"SomeOtherFeature",
- base::FEATURE_ENABLED_BY_DEFAULT};
+BASE_FEATURE(kSomeOtherFeature, "SomeOtherFeature",
+ base::FEATURE_ENABLED_BY_DEFAULT);
// Comment followed by nothing.
""".split('\n')
@@ -52,18 +52,18 @@
def testWhitespace(self):
test_data = """
// 1 line
-const base::Feature kShort{"Short", base::FEATURE_DISABLED_BY_DEFAULT};
+BASE_FEATURE(kShort, "Short", base::FEATURE_DISABLED_BY_DEFAULT);
// 2 lines
-const base::Feature kTwoLineFeatureA{"TwoLineFeatureA",
- base::FEATURE_DISABLED_BY_DEFAULT};
-const base::Feature kTwoLineFeatureB{
- "TwoLineFeatureB", base::FEATURE_DISABLED_BY_DEFAULT};
+BASE_FEATURE(kTwoLineFeatureA, "TwoLineFeatureA",
+ base::FEATURE_DISABLED_BY_DEFAULT);
+BASE_FEATURE(kTwoLineFeatureB,
+ "TwoLineFeatureB", base::FEATURE_DISABLED_BY_DEFAULT);
// 3 lines
-const base::Feature kFeatureWithAVeryLongNameThatWillHaveToWrap{
+BASE_FEATURE(kFeatureWithAVeryLongNameThatWillHaveToWrap,
"FeatureWithAVeryLongNameThatWillHaveToWrap",
- base::FEATURE_DISABLED_BY_DEFAULT};
+ base::FEATURE_DISABLED_BY_DEFAULT);
""".split('\n')
feature_file_parser = java_cpp_utils.CppConstantParser(
java_cpp_features.FeatureParserDelegate(), test_data)
@@ -83,64 +83,59 @@
def testCppSyntax(self):
test_data = """
// Mismatched name
-const base::Feature kMismatchedFeature{"MismatchedName",
- base::FEATURE_DISABLED_BY_DEFAULT};
+BASE_FEATURE(kMismatchedFeature, "MismatchedName",
+ base::FEATURE_DISABLED_BY_DEFAULT);
namespace myfeature {
// In a namespace
-const base::Feature kSomeFeature{"SomeFeature",
- base::FEATURE_DISABLED_BY_DEFAULT};
+BASE_FEATURE(kSomeFeature, "SomeFeature",
+ base::FEATURE_DISABLED_BY_DEFAULT);
}
-// Defined with equals sign
-const base::Feature kFoo = {"Foo", base::FEATURE_DISABLED_BY_DEFAULT};
-
// Build config-specific base::Feature
#if BUILDFLAG(IS_ANDROID)
-const base::Feature kAndroidOnlyFeature{"AndroidOnlyFeature",
- base::FEATURE_DISABLED_BY_DEFAULT};
+BASE_FEATURE(kAndroidOnlyFeature, "AndroidOnlyFeature",
+ base::FEATURE_DISABLED_BY_DEFAULT);
#endif
// Value depends on build config
-const base::Feature kMaybeEnabled{"MaybeEnabled",
+BASE_FEATURE(kMaybeEnabled, "MaybeEnabled",
#if BUILDFLAG(IS_ANDROID)
base::FEATURE_DISABLED_BY_DEFAULT
#else
base::FEATURE_ENABLED_BY_DEFAULT
#endif
-};
+);
""".split('\n')
feature_file_parser = java_cpp_utils.CppConstantParser(
java_cpp_features.FeatureParserDelegate(), test_data)
features = feature_file_parser.Parse()
- self.assertEqual(5, len(features))
+ self.assertEqual(4, len(features))
self.assertEqual('MISMATCHED_FEATURE', features[0].name)
self.assertEqual('"MismatchedName"', features[0].value)
self.assertEqual('SOME_FEATURE', features[1].name)
self.assertEqual('"SomeFeature"', features[1].value)
- self.assertEqual('FOO', features[2].name)
- self.assertEqual('"Foo"', features[2].value)
- self.assertEqual('ANDROID_ONLY_FEATURE', features[3].name)
- self.assertEqual('"AndroidOnlyFeature"', features[3].value)
- self.assertEqual('MAYBE_ENABLED', features[4].name)
- self.assertEqual('"MaybeEnabled"', features[4].value)
+ self.assertEqual('ANDROID_ONLY_FEATURE', features[2].name)
+ self.assertEqual('"AndroidOnlyFeature"', features[2].value)
+ self.assertEqual('MAYBE_ENABLED', features[3].name)
+ self.assertEqual('"MaybeEnabled"', features[3].value)
def testNotYetSupported(self):
# Negative test for cases we don't yet support, to ensure we don't misparse
# these until we intentionally add proper support.
test_data = """
// Not currently supported: name depends on C++ directive
-const base::Feature kNameDependsOnOs{
+BASE_FEATURE(kNameDependsOnOs,
#if BUILDFLAG(IS_ANDROID)
"MaybeName1",
#else
"MaybeName2",
#endif
- base::FEATURE_DISABLED_BY_DEFAULT};
+ base::FEATURE_DISABLED_BY_DEFAULT);
// Not currently supported: feature named with a constant instead of literal
-const base::Feature kNamedAfterConstant{kNamedStringConstant,
- base::FEATURE_DISABLED_BY_DEFAULT};
+BASE_FEATURE(kNamedAfterConstant, kNamedStringConstant,
+ base::FEATURE_DISABLED_BY_DEFAULT};
""".split('\n')
feature_file_parser = java_cpp_utils.CppConstantParser(
java_cpp_features.FeatureParserDelegate(), test_data)
@@ -149,13 +144,13 @@
def testTreatWebViewLikeOneWord(self):
test_data = """
-const base::Feature kSomeWebViewFeature{"SomeWebViewFeature",
- base::FEATURE_DISABLED_BY_DEFAULT};
-const base::Feature kWebViewOtherFeature{"WebViewOtherFeature",
- base::FEATURE_ENABLED_BY_DEFAULT};
-const base::Feature kFeatureWithPluralWebViews{
+BASE_FEATURE(kSomeWebViewFeature, "SomeWebViewFeature",
+ base::FEATURE_DISABLED_BY_DEFAULT);
+BASE_FEATURE(kWebViewOtherFeature, "WebViewOtherFeature",
+ base::FEATURE_ENABLED_BY_DEFAULT);
+BASE_FEATURE(kFeatureWithPluralWebViews,
"FeatureWithPluralWebViews",
- base::FEATURE_ENABLED_BY_DEFAULT};
+ base::FEATURE_ENABLED_BY_DEFAULT);
""".split('\n')
feature_file_parser = java_cpp_utils.CppConstantParser(
java_cpp_features.FeatureParserDelegate(), test_data)
@@ -169,11 +164,11 @@
def testSpecialCharacters(self):
test_data = r"""
-const base::Feature kFeatureWithEscapes{"Weird\tfeature\"name\n",
- base::FEATURE_DISABLED_BY_DEFAULT};
-const base::Feature kFeatureWithEscapes2{
+BASE_FEATURE(kFeatureWithEscapes, "Weird\tfeature\"name\n",
+ base::FEATURE_DISABLED_BY_DEFAULT);
+BASE_FEATURE(kFeatureWithEscapes2,
"Weird\tfeature\"name\n",
- base::FEATURE_ENABLED_BY_DEFAULT};
+ base::FEATURE_ENABLED_BY_DEFAULT);
""".split('\n')
feature_file_parser = java_cpp_utils.CppConstantParser(
java_cpp_features.FeatureParserDelegate(), test_data)
@@ -183,16 +178,6 @@
self.assertEqual('FEATURE_WITH_ESCAPES2', features[1].name)
self.assertEqual(r'"Weird\tfeature\"name\n"', features[1].value)
- def testNoBaseNamespacePrefix(self):
- test_data = """
-const Feature kSomeFeature{"SomeFeature", FEATURE_DISABLED_BY_DEFAULT};
-""".split('\n')
- feature_file_parser = java_cpp_utils.CppConstantParser(
- java_cpp_features.FeatureParserDelegate(), test_data)
- features = feature_file_parser.Parse()
- self.assertEqual('SOME_FEATURE', features[0].name)
- self.assertEqual('"SomeFeature"', features[0].value)
-
if __name__ == '__main__':
unittest.main()
diff --git a/build/config/android/rules.gni b/build/config/android/rules.gni
index 2a1cfec..bd8bb18 100644
--- a/build/config/android/rules.gni
+++ b/build/config/android/rules.gni
@@ -618,8 +618,8 @@
# foo_features.cc:
#
# // A feature.
- # const base::Feature kSomeFeature{"SomeFeature",
- # base::FEATURE_DISABLED_BY_DEFAULT};
+ # BASE_FEATURE(kSomeFeature, "SomeFeature",
+ # base::FEATURE_DISABLED_BY_DEFAULT);
#
# FooFeatures.java.tmpl
#
