Language detection feature flag for experimental rollout.

bumping language detection switch expiry to m87.

AX-Relnotes: Language detection experiment.

Change-Id: I3c836fe0feca9e564afb50eeea9173096443f3de
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2355655
Commit-Queue: Chris Hall <chrishall@chromium.org>
Reviewed-by: Alice Boxhall <aboxhall@chromium.org>
Reviewed-by: Nektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800026}

chrome/browser/flag-metadata.json

@@ -1470,12 +1470,12 @@
   {
     "name": "enable-experimental-accessibility-language-detection",
     "owners": [ "chrishall", "//ui/accessibility/OWNERS" ],
-    "expiry_milestone": 86
+    "expiry_milestone": 87
   },
   {
     "name": "enable-experimental-accessibility-language-detection-dynamic",
     "owners": [ "chrishall", "//ui/accessibility/OWNERS" ],
-    "expiry_milestone": 86
+    "expiry_milestone": 87
   },
   {
     "name": "enable-experimental-accessibility-switch-access",

ui/accessibility/accessibility_features.cc

@@ -37,6 +37,16 @@ bool IsAccessibilityExposeHTMLElementEnabled() {
       ::features::kEnableAccessibilityExposeHTMLElement);
 }
 
+// Enable language detection to determine language used in page text, exposed
+// on the browser process AXTree.
+const base::Feature kEnableAccessibilityLanguageDetection{
+    "AccessibilityLanguageDetection", base::FEATURE_DISABLED_BY_DEFAULT};
+
+bool IsAccessibilityLanguageDetectionEnabled() {
+  return base::FeatureList::IsEnabled(
+      ::features::kEnableAccessibilityLanguageDetection);
+}
+
 // Serializes accessibility information from the Views tree and deserializes it
 // into an AXTree in the browser process.
 const base::Feature kEnableAccessibilityTreeForViews{

ui/accessibility/accessibility_features.h

@@ -30,6 +30,12 @@ AX_BASE_EXPORT extern const base::Feature kEnableAccessibilityExposeHTMLElement;
 // browser process AXTree (as an ignored node).
 AX_BASE_EXPORT bool IsAccessibilityExposeHTMLElementEnabled();
 
+AX_BASE_EXPORT extern const base::Feature kEnableAccessibilityLanguageDetection;
+
+// Return true if language detection should be used to determine the language
+// of text content in page and exposed to the browser process AXTree.
+AX_BASE_EXPORT bool IsAccessibilityLanguageDetectionEnabled();
+
 // Serializes accessibility information from the Views tree and deserializes it
 // into an AXTree in the browser process.
 AX_BASE_EXPORT extern const base::Feature kEnableAccessibilityTreeForViews;

ui/accessibility/ax_language_detection.cc

@@ -12,6 +12,7 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/trace_event/trace_event.h"
+#include "ui/accessibility/accessibility_features.h"
 #include "ui/accessibility/accessibility_switches.h"
 #include "ui/accessibility/ax_enums.mojom.h"
 #include "ui/accessibility/ax_tree.h"
@@ -208,10 +209,28 @@ AXLanguageDetectionManager::AXLanguageDetectionManager(AXTree* tree)
 
 AXLanguageDetectionManager::~AXLanguageDetectionManager() = default;
 
+bool AXLanguageDetectionManager::IsStaticLanguageDetectionEnabled() {
+  // Static language detection can be enabled by either:
+  //  1) The general language detection feature flag which gates both static and
+  //     dynamic language detection (feature flag for experiment), or
+  //  2) The Static specific flag (user controlled switch).
+  return features::IsAccessibilityLanguageDetectionEnabled() ||
+         ::switches::IsExperimentalAccessibilityLanguageDetectionEnabled();
+}
+
+bool AXLanguageDetectionManager::IsDynamicLanguageDetectionEnabled() {
+  // Dynamic language detection can be enabled by either:
+  //  1) The general language detection feature flag which gates both static and
+  //     dynamic language detection (feature flag for experiment), or
+  //  2) The Dynamic specific flag (user controlled switch).
+  return features::IsAccessibilityLanguageDetectionEnabled() ||
+         ::switches::
+             IsExperimentalAccessibilityLanguageDetectionDynamicEnabled();
+}
+
 void AXLanguageDetectionManager::RegisterLanguageDetectionObserver() {
-  // If the dynamic feature flag is not enabled then do nothing.
-  if (!::switches::
-          IsExperimentalAccessibilityLanguageDetectionDynamicEnabled()) {
+  // Do not perform dynamic language detection unless explicitly enabled.
+  if (!IsDynamicLanguageDetectionEnabled()) {
     return;
   }
 
@@ -223,7 +242,8 @@ void AXLanguageDetectionManager::RegisterLanguageDetectionObserver() {
 // Detect languages for each node.
 void AXLanguageDetectionManager::DetectLanguages() {
   TRACE_EVENT0("accessibility", "AXLanguageInfo::DetectLanguages");
-  if (!::switches::IsExperimentalAccessibilityLanguageDetectionEnabled()) {
+
+  if (!IsStaticLanguageDetectionEnabled()) {
     return;
   }
 
@@ -308,7 +328,7 @@ void AXLanguageDetectionManager::DetectLanguagesForNode(AXNode* node) {
 void AXLanguageDetectionManager::LabelLanguages() {
   TRACE_EVENT0("accessibility", "AXLanguageInfo::LabelLanguages");
 
-  if (!::switches::IsExperimentalAccessibilityLanguageDetectionEnabled()) {
+  if (!IsStaticLanguageDetectionEnabled()) {
     return;
   }
 
@@ -443,8 +463,7 @@ AXLanguageDetectionObserver::AXLanguageDetectionObserver(AXTree* tree)
   // We expect the feature flag to have be checked before this Observer is
   // constructed, this should have been checked by
   // RegisterLanguageDetectionObserver.
-  DCHECK(
-      ::switches::IsExperimentalAccessibilityLanguageDetectionDynamicEnabled());
+  DCHECK(AXLanguageDetectionManager::IsDynamicLanguageDetectionEnabled());
 
   tree_->AddObserver(this);
 }

ui/accessibility/ax_language_detection.h

@@ -281,6 +281,10 @@ class AX_EXPORT AXLanguageDetectionManager {
   // Allow access from a fixture only used in testing.
   friend class AXLanguageDetectionTestFixture;
 
+  // Helper methods to test if language detection features are enabled.
+  static bool IsStaticLanguageDetectionEnabled();
+  static bool IsDynamicLanguageDetectionEnabled();
+
   // Perform detection for subtree rooted at subtree_root.
   void DetectLanguagesForSubtree(AXNode* subtree_root);
   // Perform detection for node. Will not descend into children.

ui/accessibility/ax_language_detection_unittest.cc

@@ -11,7 +11,9 @@
 
 #include "base/command_line.h"
 #include "base/test/metrics/histogram_tester.h"
+#include "base/test/scoped_feature_list.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/accessibility/accessibility_features.h"
 #include "ui/accessibility/accessibility_switches.h"
 #include "ui/accessibility/ax_enums.mojom.h"
 #include "ui/accessibility/ax_node.h"
@@ -56,6 +58,14 @@ class AXLanguageDetectionTestFixture : public testing::Test {
       const AXLanguageDetectionTestFixture&) = delete;
 
  protected:
+  bool IsStaticLanguageDetectionEnabled() {
+    return AXLanguageDetectionManager::IsStaticLanguageDetectionEnabled();
+  }
+
+  bool IsDynamicLanguageDetectionEnabled() {
+    return AXLanguageDetectionManager::IsDynamicLanguageDetectionEnabled();
+  }
+
   AXLanguageDetectionObserver* getObserver(AXTree& tree) {
     return tree.language_detection_manager->language_detection_observer_.get();
   }
@@ -138,28 +148,45 @@ class AXLanguageDetectionTestDynamicContent
   }
 };
 
-TEST(AXLanguageDetectionTest, StaticContentFeatureFlag) {
+TEST_F(AXLanguageDetectionTestFixture, StaticContentFeatureFlag) {
   // TODO(crbug/889370): Remove this test once this feature is stable
   EXPECT_FALSE(
       ::switches::IsExperimentalAccessibilityLanguageDetectionEnabled());
+  EXPECT_FALSE(IsStaticLanguageDetectionEnabled());
 
   base::CommandLine::ForCurrentProcess()->AppendSwitch(
       ::switches::kEnableExperimentalAccessibilityLanguageDetection);
 
   EXPECT_TRUE(
       ::switches::IsExperimentalAccessibilityLanguageDetectionEnabled());
+  EXPECT_TRUE(IsStaticLanguageDetectionEnabled());
 }
 
-TEST(AXLanguageDetectionTest, DynamicContentFeatureFlag) {
+TEST_F(AXLanguageDetectionTestFixture, DynamicContentFeatureFlag) {
   // TODO(crbug/889370): Remove this test once this feature is stable
   EXPECT_FALSE(
       ::switches::IsExperimentalAccessibilityLanguageDetectionDynamicEnabled());
+  EXPECT_FALSE(IsDynamicLanguageDetectionEnabled());
 
   base::CommandLine::ForCurrentProcess()->AppendSwitch(
       ::switches::kEnableExperimentalAccessibilityLanguageDetectionDynamic);
 
   EXPECT_TRUE(
       ::switches::IsExperimentalAccessibilityLanguageDetectionDynamicEnabled());
+  EXPECT_TRUE(IsDynamicLanguageDetectionEnabled());
+}
+
+TEST_F(AXLanguageDetectionTestFixture, FeatureFlag) {
+  // TODO(crbug/889370): Remove this test once this feature is stable
+  EXPECT_FALSE(IsStaticLanguageDetectionEnabled());
+  EXPECT_FALSE(IsDynamicLanguageDetectionEnabled());
+
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitWithFeatures(
+      {features::kEnableAccessibilityLanguageDetection}, {});
+
+  EXPECT_TRUE(IsStaticLanguageDetectionEnabled());
+  EXPECT_TRUE(IsDynamicLanguageDetectionEnabled());
 }
 
 TEST(AXLanguageDetectionTest, LangAttrInheritanceFeatureFlagOff) {
@@ -840,7 +867,8 @@ TEST_F(AXLanguageDetectionTestStaticContent, kLanguageUntouched) {
   }
 }
 
-// Test RegisterLanguageDetectionObserver correctly respects the feature flag.
+// Test RegisterLanguageDetectionObserver correctly respects the command line
+// flags.
 TEST_F(AXLanguageDetectionTestFixture, ObserverRegistrationObeysFlag) {
   // Enable only the flag controlling static language detection.
   base::CommandLine::ForCurrentProcess()->AppendSwitch(
@@ -871,6 +899,33 @@ TEST_F(AXLanguageDetectionTestFixture, ObserverRegistrationObeysFlag) {
   ASSERT_TRUE(tree.HasObserver(getObserver(tree)));
 }
 
+// Test RegisterLanguageDetectionObserver correctly respects the feature flag.
+TEST_F(AXLanguageDetectionTestFixture, ObserverRegistrationObeysFeatureFlag) {
+  // Construct empty tree and check initialisation.
+  AXTree tree;
+  ASSERT_NE(tree.language_detection_manager, nullptr);
+  ASSERT_EQ(getObserver(tree), nullptr);
+
+  // Try registration without enabling Dynamic feature flag, should be a no-op.
+  tree.language_detection_manager->RegisterLanguageDetectionObserver();
+
+  ASSERT_EQ(getObserver(tree), nullptr);
+
+  // Enable general feature flag which gates both Static and Dynamic features.
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitWithFeatures(
+      {features::kEnableAccessibilityLanguageDetection}, {});
+
+  // Try registration again, this should now construct and register an observer.
+  tree.language_detection_manager->RegisterLanguageDetectionObserver();
+
+  // Check our observer was constructed.
+  ASSERT_NE(getObserver(tree), nullptr);
+
+  // Check our observer was registered in our tree.
+  ASSERT_TRUE(tree.HasObserver(getObserver(tree)));
+}
+
 TEST_F(AXLanguageDetectionTestDynamicContent, Basic) {
   // Tree:
   //   1