Revert "[Ash] Simplify AccessibilityControllerImpl to remove duplications"

This reverts commit 2f2a33d5.

Reason for revert: <INSERT REASONING HERE>
It looks like this CL breaks some test case
https://ci.chromium.org/p/chromium/builders/ci/Linux%20ChromiumOS%20MSan%20Tests/18262?blamelist=1#blamelist-tab
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8885542762262570176/+/steps/exo_unittests/0/logs/Deterministic_failure:_All__x2f_SurfaceTest.AcquireFence__x2f_0__status_CRASH_/0


Original change's description:
> [Ash] Simplify AccessibilityControllerImpl to remove duplications
> 
> AccessibilityControllerImpl had a lot of duplications. Let's fix that.
> Also, now it controls fullscreen and docked magnifiers. Before that,
> we have to go AccessibilityDelegate and MagnificationManager to do
> this.
> 
> Change-Id: I6a5d827ded2a88a56eb173fed67e010aa002070d
> Bug: 1051892
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2050688
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
> Commit-Queue: Anatoliy Potapchuk <apotapchuk@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#751086}

TBR=xiyuan@chromium.org,dmazzoni@chromium.org,nektar@chromium.org,apotapchuk@chromium.org,burunduk@chromium.org,amraboelkher@google.com

Change-Id: Ifbd8e25594790d7417af06c84651d3837ad17b5d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1051892
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2107028Reviewed-by: Nancy Wang <nancylingwang@chromium.org>
Commit-Queue: Nancy Wang <nancylingwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#751181}

ash/accessibility/accessibility_controller_impl.cc

@@ -4,7 +4,6 @@
 
 #include "ash/accessibility/accessibility_controller_impl.h"
 
-#include <map>
 #include <memory>
 #include <set>
 #include <utility>
@@ -59,40 +58,6 @@ using session_manager::SessionState;
 namespace ash {
 namespace {
 
-using FeatureType = AccessibilityControllerImpl::FeatureType;
-
-// These classes are used to store the static configuration for a11y features.
-struct FeatureData {
-  FeatureType type;
-  const char* pref;
-  const gfx::VectorIcon* icon;
-  FeatureType conflicting_feature = FeatureType::kNoConflictingFeature;
-};
-
-struct FeatureDialogData {
-  AccessibilityControllerImpl::FeatureType type;
-  const char* pref;
-  int title;
-  int body;
-  bool mandatory;
-};
-
-// A static array describing each feature.
-const FeatureData kFeatures[] = {
-    {FeatureType::kAutoclick, prefs::kAccessibilityAutoclickEnabled,
-     &kSystemMenuAccessibilityAutoClickIcon},
-    {FeatureType::kCaretHighlight, prefs::kAccessibilityCaretHighlightEnabled,
-     nullptr},
-    {FeatureType::KCursorHighlight, prefs::kAccessibilityCursorHighlightEnabled,
-     nullptr}};
-
-// TODO(crbug.com/1051892): Remove this once all features are properly handled.
-const FeatureType kLastAddedFeature = FeatureType::KCursorHighlight;
-
-// An array describing the confirmation dialogs for the features which have
-// them.
-const FeatureDialogData kFeatureDialogs[] = {};
-
 constexpr char kNotificationId[] = "chrome://settings/accessibility";
 constexpr char kNotifierAccessibility[] = "ash.accessibility";
 
@@ -136,26 +101,6 @@ constexpr const char* const kCopiedOnSigninAccessibilityPrefs[]{
     prefs::kDisplayRotationAcceleratorDialogHasBeenAccepted2,
 };
 
-// Helper function that is used to verify the validity of kFeatures and
-// kFeatureDialogs.
-bool VerifyFeaturesData() {
-  // All feature prefs must be unique.
-  std::set<const char*> feature_prefs;
-  for (auto feature_data : kFeatures) {
-    if (feature_prefs.find(feature_data.pref) != feature_prefs.end())
-      return false;
-    feature_prefs.insert(feature_data.pref);
-  }
-
-  for (auto dialog_data : kFeatureDialogs) {
-    if (feature_prefs.find(dialog_data.pref) != feature_prefs.end())
-      return false;
-    feature_prefs.insert(dialog_data.pref);
-  }
-
-  return true;
-}
-
 // Returns true if |pref_service| is the one used for the signin screen.
 bool IsSigninPrefService(PrefService* pref_service) {
   const PrefService* signin_pref_service =
@@ -351,159 +296,14 @@ SwitchAccessCommandKeyCode UmaValueForKeyCode(int key_code) {
 
 }  // namespace
 
-AccessibilityControllerImpl::Feature::Feature(
-    FeatureType type,
-    const std::string& pref_name,
-    const gfx::VectorIcon* icon,
-    AccessibilityControllerImpl* controller)
-    : type_(type), pref_name_(pref_name), icon_(icon), owner_(controller) {}
-
-AccessibilityControllerImpl::Feature::~Feature() = default;
-
-void AccessibilityControllerImpl::Feature::SetEnabled(bool enabled) {
-  PrefService* prefs = owner_->active_user_prefs_;
-  if (!prefs)
-    return;
-  prefs->SetBoolean(pref_name_, enabled);
-  prefs->CommitPendingWrite();
-}
-
-bool AccessibilityControllerImpl::Feature::IsVisibleInTray() const {
-  return (conflicting_feature_ == kNoConflictingFeature ||
-          !owner_->GetFeature(conflicting_feature_).enabled()) &&
-         owner_->IsAccessibilityFeatureVisibleInTrayMenu(pref_name_);
-}
-
-bool AccessibilityControllerImpl::Feature::IsEnterpriseIconVisible() const {
-  return owner_->IsEnterpriseIconVisibleInTrayMenu(pref_name_);
-}
-
-const gfx::VectorIcon& AccessibilityControllerImpl::Feature::icon() const {
-  DCHECK(icon_);
-  if (icon_)
-    return *icon_;
-  return kPaletteTrayIconDefaultIcon;
-}
-
-void AccessibilityControllerImpl::Feature::UpdateFromPref() {
-  PrefService* prefs = owner_->active_user_prefs_;
-  DCHECK(prefs);
-
-  bool enabled = prefs->GetBoolean(pref_name_);
-
-  if (conflicting_feature_ != FeatureType::kNoConflictingFeature &&
-      owner_->GetFeature(conflicting_feature_).enabled()) {
-    enabled = false;
-  }
-
-  if (enabled == enabled_)
-    return;
-
-  enabled_ = enabled;
-  owner_->UpdateFeatureFromPref(type_);
-}
-
-void AccessibilityControllerImpl::Feature::SetConflictingFeature(
-    AccessibilityControllerImpl::FeatureType feature) {
-  DCHECK_EQ(conflicting_feature_, FeatureType::kNoConflictingFeature);
-  conflicting_feature_ = feature;
-}
-
-AccessibilityControllerImpl::FeatureWithDialog::FeatureWithDialog(
-    FeatureType type,
-    const std::string& pref_name,
-    const gfx::VectorIcon* icon,
-    const Dialog& dialog,
-    AccessibilityControllerImpl* controller)
-    : AccessibilityControllerImpl::Feature(type, pref_name, icon, controller),
-      dialog_(dialog) {}
-AccessibilityControllerImpl::FeatureWithDialog::~FeatureWithDialog() = default;
-
-void AccessibilityControllerImpl::FeatureWithDialog::SetDialogAccepted() {
-  PrefService* prefs = owner_->active_user_prefs_;
-  if (!prefs)
-    return;
-  prefs->SetBoolean(dialog_.pref_name, true);
-  prefs->CommitPendingWrite();
-}
-
-bool AccessibilityControllerImpl::FeatureWithDialog::WasDialogAccepted() const {
-  PrefService* prefs = owner_->active_user_prefs_;
-  DCHECK(prefs);
-  return prefs->GetBoolean(dialog_.pref_name);
-}
-
-void AccessibilityControllerImpl::FeatureWithDialog::SetEnabledWithDialog(
-    bool enabled,
-    base::OnceClosure completion_callback) {
-  PrefService* prefs = owner_->active_user_prefs_;
-  if (!prefs)
-    return;
-  // We should not show the dialog when the feature is already enabled.
-  if (enabled && !this->enabled() && !WasDialogAccepted()) {
-    Shell::Get()->accelerator_controller()->MaybeShowConfirmationDialog(
-        dialog_.title_resource_id, dialog_.body_resource_id,
-        // Callback for if the user accepts the dialog
-        base::BindOnce(
-            [](base::WeakPtr<AccessibilityControllerImpl> owner,
-               FeatureType type, base::OnceClosure completion_callback) {
-              if (!owner)
-                return;
-
-              static_cast<FeatureWithDialog&>(owner->GetFeature(type))
-                  .SetDialogAccepted();
-              // If they accept, try again to set value to true
-              owner->GetFeature(type).SetEnabled(true);
-              std::move(completion_callback).Run();
-            },
-            owner_->weak_ptr_factory_.GetWeakPtr(), type_,
-            std::move(completion_callback)),
-        base::DoNothing());
-
-    return;
-  }
-  Feature::SetEnabled(enabled);
-  std::move(completion_callback).Run();
-}
-
-void AccessibilityControllerImpl::FeatureWithDialog::SetEnabled(bool enabled) {
-  if (dialog_.mandatory)
-    SetEnabledWithDialog(enabled, base::DoNothing());
-  else
-    Feature::SetEnabled(enabled);
-}
-
 AccessibilityControllerImpl::AccessibilityControllerImpl()
     : autoclick_delay_(AutoclickController::GetDefaultAutoclickDelay()) {
   Shell::Get()->session_controller()->AddObserver(this);
   Shell::Get()->tablet_mode_controller()->AddObserver(this);
-  CreateAccessibilityFeatures();
 }
 
 AccessibilityControllerImpl::~AccessibilityControllerImpl() = default;
 
-void AccessibilityControllerImpl::CreateAccessibilityFeatures() {
-  DCHECK(VerifyFeaturesData());
-  // First, build all features with dialog.
-  std::map<FeatureType, Dialog> dialogs;
-  for (auto dialog_data : kFeatureDialogs) {
-    dialogs[dialog_data.type] = {dialog_data.pref, dialog_data.title,
-                                 dialog_data.body, dialog_data.mandatory};
-  }
-  for (auto feature_data : kFeatures) {
-    DCHECK(!features_[feature_data.type]);
-    auto it = dialogs.find(feature_data.type);
-    if (it == dialogs.end()) {
-      features_[feature_data.type] = std::make_unique<Feature>(
-          feature_data.type, feature_data.pref, feature_data.icon, this);
-    } else {
-      features_[feature_data.type] = std::make_unique<FeatureWithDialog>(
-          feature_data.type, feature_data.pref, feature_data.icon, it->second,
-          this);
-    }
-  }
-}
-
 // static
 void AccessibilityControllerImpl::RegisterProfilePrefs(
     PrefRegistrySimple* registry) {
@@ -727,37 +527,22 @@ void AccessibilityControllerImpl::RemoveObserver(
   observers_.RemoveObserver(observer);
 }
 
-AccessibilityControllerImpl::Feature& AccessibilityControllerImpl::GetFeature(
-    FeatureType type) const {
-  DCHECK(features_[type].get());
-  return *features_[type].get();
-}
-
-AccessibilityControllerImpl::Feature& AccessibilityControllerImpl::autoclick()
-    const {
-  return GetFeature(FeatureType::kAutoclick);
-}
-
-AccessibilityControllerImpl::Feature&
-AccessibilityControllerImpl::caret_highlight() const {
-  return GetFeature(FeatureType::kCaretHighlight);
-}
-
-AccessibilityControllerImpl::Feature&
-AccessibilityControllerImpl::cursor_highlight() const {
-  return GetFeature(FeatureType::KCursorHighlight);
-}
-
 void AccessibilityControllerImpl::SetAutoclickEnabled(bool enabled) {
-  autoclick().SetEnabled(enabled);
+  if (!active_user_prefs_)
+    return;
+  active_user_prefs_->SetBoolean(prefs::kAccessibilityAutoclickEnabled,
+                                 enabled);
+  active_user_prefs_->CommitPendingWrite();
 }
 
 bool AccessibilityControllerImpl::IsAutoclickSettingVisibleInTray() {
-  return autoclick().IsVisibleInTray();
+  return IsAccessibilityFeatureVisibleInTrayMenu(
+      prefs::kAccessibilityAutoclickEnabled);
 }
 
 bool AccessibilityControllerImpl::IsEnterpriseIconVisibleForAutoclick() {
-  return autoclick().IsEnterpriseIconVisible();
+  return IsEnterpriseIconVisibleInTrayMenu(
+      prefs::kAccessibilityAutoclickEnabled);
 }
 
 bool AccessibilityControllerImpl::IsPrimarySettingsViewVisibleInTray() {
@@ -789,27 +574,39 @@ bool AccessibilityControllerImpl::IsAdditionalSettingsSeparatorVisibleInTray() {
 }
 
 void AccessibilityControllerImpl::SetCaretHighlightEnabled(bool enabled) {
-  caret_highlight().SetEnabled(enabled);
+  if (!active_user_prefs_)
+    return;
+  active_user_prefs_->SetBoolean(prefs::kAccessibilityCaretHighlightEnabled,
+                                 enabled);
+  active_user_prefs_->CommitPendingWrite();
 }
 
 bool AccessibilityControllerImpl::IsCaretHighlightSettingVisibleInTray() {
-  return caret_highlight().IsVisibleInTray();
+  return IsAccessibilityFeatureVisibleInTrayMenu(
+      prefs::kAccessibilityCaretHighlightEnabled);
 }
 
 bool AccessibilityControllerImpl::IsEnterpriseIconVisibleForCaretHighlight() {
-  return caret_highlight().IsEnterpriseIconVisible();
+  return IsEnterpriseIconVisibleInTrayMenu(
+      prefs::kAccessibilityCaretHighlightEnabled);
 }
 
 void AccessibilityControllerImpl::SetCursorHighlightEnabled(bool enabled) {
-  cursor_highlight().SetEnabled(enabled);
+  if (!active_user_prefs_)
+    return;
+  active_user_prefs_->SetBoolean(prefs::kAccessibilityCursorHighlightEnabled,
+                                 enabled);
+  active_user_prefs_->CommitPendingWrite();
 }
 
 bool AccessibilityControllerImpl::IsCursorHighlightSettingVisibleInTray() {
-  return cursor_highlight().IsVisibleInTray();
+  return IsAccessibilityFeatureVisibleInTrayMenu(
+      prefs::kAccessibilityCursorHighlightEnabled);
 }
 
 bool AccessibilityControllerImpl::IsEnterpriseIconVisibleForCursorHighlight() {
-  return cursor_highlight().IsEnterpriseIconVisible();
+  return IsEnterpriseIconVisibleInTrayMenu(
+      prefs::kAccessibilityCursorHighlightEnabled);
 }
 
 void AccessibilityControllerImpl::SetDictationEnabled(bool enabled) {
@@ -1313,19 +1110,10 @@ void AccessibilityControllerImpl::ObservePrefs(PrefService* prefs) {
   // Watch for pref updates from webui settings and policy.
   pref_change_registrar_ = std::make_unique<PrefChangeRegistrar>();
   pref_change_registrar_->Init(prefs);
-
-  // It is safe to use base::Unreatined since we own pref_change_registrar.
-  for (int feature_id = 0; feature_id <= kLastAddedFeature; feature_id++) {
-    Feature* feature = features_[feature_id].get();
-    DCHECK(feature);
-    pref_change_registrar_->Add(
-        feature->pref_name(),
-        base::BindRepeating(
-            &AccessibilityControllerImpl::Feature::UpdateFromPref,
-            base::Unretained(feature)));
-    feature->UpdateFromPref();
-  }
-
+  pref_change_registrar_->Add(
+      prefs::kAccessibilityAutoclickEnabled,
+      base::BindRepeating(&AccessibilityControllerImpl::UpdateAutoclickFromPref,
+                          base::Unretained(this)));
   pref_change_registrar_->Add(
       prefs::kAccessibilityAutoclickDelayMs,
       base::BindRepeating(
@@ -1356,6 +1144,16 @@ void AccessibilityControllerImpl::ObservePrefs(PrefService* prefs) {
       base::BindRepeating(
           &AccessibilityControllerImpl::UpdateAutoclickMenuPositionFromPref,
           base::Unretained(this)));
+  pref_change_registrar_->Add(
+      prefs::kAccessibilityCaretHighlightEnabled,
+      base::BindRepeating(
+          &AccessibilityControllerImpl::UpdateCaretHighlightFromPref,
+          base::Unretained(this)));
+  pref_change_registrar_->Add(
+      prefs::kAccessibilityCursorHighlightEnabled,
+      base::BindRepeating(
+          &AccessibilityControllerImpl::UpdateCursorHighlightFromPref,
+          base::Unretained(this)));
   pref_change_registrar_->Add(
       prefs::kAccessibilityDictationEnabled,
       base::BindRepeating(&AccessibilityControllerImpl::UpdateDictationFromPref,
@@ -1451,16 +1249,15 @@ void AccessibilityControllerImpl::ObservePrefs(PrefService* prefs) {
                           base::Unretained(this)));
 
   // Load current state.
-  for (int feature_id = 0; feature_id <= kLastAddedFeature; feature_id++) {
-    features_[feature_id]->UpdateFromPref();
-  }
-
+  UpdateAutoclickFromPref();
   UpdateAutoclickDelayFromPref();
   UpdateAutoclickEventTypeFromPref();
   UpdateAutoclickRevertToLeftClickFromPref();
   UpdateAutoclickStabilizePositionFromPref();
   UpdateAutoclickMovementThresholdFromPref();
   UpdateAutoclickMenuPositionFromPref();
+  UpdateCaretHighlightFromPref();
+  UpdateCursorHighlightFromPref();
   UpdateDictationFromPref();
   UpdateFocusHighlightFromPref();
   UpdateHighContrastFromPref();
@@ -1475,6 +1272,22 @@ void AccessibilityControllerImpl::ObservePrefs(PrefService* prefs) {
   UpdateShortcutsEnabledFromPref();
 }
 
+void AccessibilityControllerImpl::UpdateAutoclickFromPref() {
+  DCHECK(active_user_prefs_);
+  const bool enabled =
+      active_user_prefs_->GetBoolean(prefs::kAccessibilityAutoclickEnabled);
+
+  if (autoclick_enabled_ == enabled)
+    return;
+
+  autoclick_enabled_ = enabled;
+
+  NotifyAccessibilityStatusChanged();
+
+  Shell::Get()->autoclick_controller()->SetEnabled(
+      enabled, true /* show confirmation dialog */);
+}
+
 void AccessibilityControllerImpl::UpdateAutoclickDelayFromPref() {
   DCHECK(active_user_prefs_);
   base::TimeDelta autoclick_delay = base::TimeDelta::FromMilliseconds(int64_t{
@@ -1572,6 +1385,34 @@ void AccessibilityControllerImpl::OnAutoclickScrollableBoundsFound(
       bounds_in_screen);
 }
 
+void AccessibilityControllerImpl::UpdateCaretHighlightFromPref() {
+  DCHECK(active_user_prefs_);
+  const bool enabled = active_user_prefs_->GetBoolean(
+      prefs::kAccessibilityCaretHighlightEnabled);
+
+  if (caret_highlight_enabled_ == enabled)
+    return;
+
+  caret_highlight_enabled_ = enabled;
+
+  NotifyAccessibilityStatusChanged();
+  UpdateAccessibilityHighlightingFromPrefs();
+}
+
+void AccessibilityControllerImpl::UpdateCursorHighlightFromPref() {
+  DCHECK(active_user_prefs_);
+  const bool enabled = active_user_prefs_->GetBoolean(
+      prefs::kAccessibilityCursorHighlightEnabled);
+
+  if (cursor_highlight_enabled_ == enabled)
+    return;
+
+  cursor_highlight_enabled_ = enabled;
+
+  NotifyAccessibilityStatusChanged();
+  UpdateAccessibilityHighlightingFromPrefs();
+}
+
 void AccessibilityControllerImpl::UpdateDictationFromPref() {
   DCHECK(active_user_prefs_);
   const bool enabled =
@@ -1679,7 +1520,7 @@ void AccessibilityControllerImpl::UpdateSpokenFeedbackFromPref() {
 }
 
 void AccessibilityControllerImpl::UpdateAccessibilityHighlightingFromPrefs() {
-  if (!caret_highlight().enabled() && !cursor_highlight().enabled() &&
+  if (!caret_highlight_enabled_ && !cursor_highlight_enabled_ &&
       !focus_highlight_enabled_) {
     accessibility_highlight_controller_.reset();
     return;
@@ -1690,10 +1531,9 @@ void AccessibilityControllerImpl::UpdateAccessibilityHighlightingFromPrefs() {
         std::make_unique<AccessibilityHighlightController>();
   }
 
-  accessibility_highlight_controller_->HighlightCaret(
-      caret_highlight().enabled());
+  accessibility_highlight_controller_->HighlightCaret(caret_highlight_enabled_);
   accessibility_highlight_controller_->HighlightCursor(
-      cursor_highlight().enabled());
+      cursor_highlight_enabled_);
   accessibility_highlight_controller_->HighlightFocus(focus_highlight_enabled_);
 }
 
@@ -1916,41 +1756,4 @@ bool AccessibilityControllerImpl::IsAccessibilityFeatureVisibleInTrayMenu(
   return true;
 }
 
-void AccessibilityControllerImpl::UpdateFeatureFromPref(FeatureType feature) {
-  bool enabled = features_[feature]->enabled();
-
-  switch (feature) {
-    case FeatureType::kAutoclick:
-      Shell::Get()->autoclick_controller()->SetEnabled(
-          enabled, true /* show confirmation dialog */);
-      break;
-    case FeatureType::kCaretHighlight:
-      UpdateAccessibilityHighlightingFromPrefs();
-      break;
-    case FeatureType::KCursorHighlight:
-      UpdateAccessibilityHighlightingFromPrefs();
-      break;
-    case FeatureType::kDictation:
-    case FeatureType::kFocusHighlight:
-    case FeatureType::kFullscreenMagnifier:
-    case FeatureType::kDockedMagnifier:
-    case FeatureType::kHighContrast:
-    case FeatureType::kLargeCursor:
-    case FeatureType::kMonoAudio:
-    case FeatureType::kSpokenFeedback:
-    case FeatureType::kSelectToSpeak:
-    case FeatureType::kStickyKeys:
-    case FeatureType::kSwitchAccess:
-    case FeatureType::kVirtualKeyboard:
-      // Do nothing for now.
-      // TODO(crbug.com/1051892): Add implementation for other features.
-      NOTIMPLEMENTED();
-      return;
-    case FeatureType::kFeatureCount:
-    case FeatureType::kNoConflictingFeature:
-      NOTREACHED();
-  }
-  NotifyAccessibilityStatusChanged();
-}
-
 }  // namespace ash

ash/accessibility/accessibility_controller_impl.h

@@ -13,7 +13,6 @@
 #include "ash/public/cpp/tablet_mode_observer.h"
 #include "ash/session/session_observer.h"
 #include "base/macros.h"
-#include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #include "base/time/time.h"
 
@@ -29,7 +28,6 @@ enum class Gesture;
 
 namespace gfx {
 class Point;
-struct VectorIcon;
 }  // namespace gfx
 
 namespace ash {
@@ -52,101 +50,6 @@ class ASH_EXPORT AccessibilityControllerImpl : public AccessibilityController,
                                                public SessionObserver,
                                                public TabletModeObserver {
  public:
-  enum FeatureType {
-    kAutoclick = 0,
-    kCaretHighlight,
-    KCursorHighlight,
-    kDictation,
-    kFocusHighlight,
-    kFullscreenMagnifier,
-    kDockedMagnifier,
-    kHighContrast,
-    kLargeCursor,
-    kMonoAudio,
-    kSpokenFeedback,
-    kSelectToSpeak,
-    kStickyKeys,
-    kSwitchAccess,
-    kVirtualKeyboard,
-
-    kFeatureCount,
-    kNoConflictingFeature
-  };
-
-  // Common interface for all features.
-  class Feature {
-   public:
-    Feature(FeatureType type,
-            const std::string& pref_name,
-            const gfx::VectorIcon* icon,
-            AccessibilityControllerImpl* controller);
-    Feature(const Feature&) = delete;
-    Feature& operator=(Feature const&) = delete;
-    virtual ~Feature();
-
-    FeatureType type() const { return type_; }
-    // Tries to set the feature to |enabled| by setting the user pref.
-    // Setting feature to be enabled can fail in following conditions:
-    // - there is a higher priority pref(managed), which overrides this value.
-    // - there is an other feature, which conflicts with the current one.
-    virtual void SetEnabled(bool enabled);
-    bool enabled() const { return enabled_; }
-    bool IsVisibleInTray() const;
-    bool IsEnterpriseIconVisible() const;
-    const std::string& pref_name() const { return pref_name_; }
-    const gfx::VectorIcon& icon() const;
-
-    void UpdateFromPref();
-    void SetConflictingFeature(FeatureType feature);
-
-   protected:
-    FeatureType type_;
-    // Some features cannot be enabled while others are on. When a conflicting
-    // feature is enabled, we cannot enable current feature.
-    FeatureType conflicting_feature_ = FeatureType::kNoConflictingFeature;
-    bool enabled_;
-    const std::string pref_name_;
-    const gfx::VectorIcon* icon_;
-    AccessibilityControllerImpl* owner_;
-  };
-
-  // Helper struct to store information about a11y dialog -- pref name, resource
-  // ids for title and body. Also stores the information whether this dialog is
-  // mandatory for every SetEnabled call.
-  struct Dialog {
-    std::string pref_name;
-    int title_resource_id;
-    int body_resource_id;
-    // Whether this dialog should be shown on every SetEnabled action.
-    bool mandatory;
-  };
-
-  // Some features have confirmation dialog associated with them.
-  // Dialog can be applied for all SetEnabled() actions, or only to ones
-  // associated with accelerators.
-  class FeatureWithDialog : public Feature {
-   public:
-    FeatureWithDialog(FeatureType type,
-                      const std::string& pref_name,
-                      const gfx::VectorIcon* icon,
-                      const Dialog& dialog,
-                      AccessibilityControllerImpl* controller);
-    ~FeatureWithDialog() override;
-
-    // Tries to set the feature enabled, if its dialog is mandatory, shows the
-    // dailog for the first time feature is enabled.
-    void SetEnabled(bool enabled) override;
-    // If the dialog have not been accepted, we show it. When it is accepted, we
-    // call SetEnabled() and invoke |completion_callback|.
-    void SetEnabledWithDialog(bool enabled,
-                              base::OnceClosure completion_callback);
-    void SetDialogAccepted();
-    bool WasDialogAccepted() const;
-
-   private:
-    Dialog dialog_;
-  };
-
   AccessibilityControllerImpl();
   ~AccessibilityControllerImpl() override;
 
@@ -158,13 +61,6 @@ class ASH_EXPORT AccessibilityControllerImpl : public AccessibilityController,
   void AddObserver(AccessibilityObserver* observer);
   void RemoveObserver(AccessibilityObserver* observer);
 
-  Feature& GetFeature(FeatureType feature) const;
-
-  // Getters for the corresponding features.
-  Feature& autoclick() const;
-  Feature& caret_highlight() const;
-  Feature& cursor_highlight() const;
-
   // The following functions read and write to their associated preference.
   // These values are then used to determine whether the accelerator
   // confirmation dialog for the respective preference has been accepted before.
@@ -180,7 +76,7 @@ class ASH_EXPORT AccessibilityControllerImpl : public AccessibilityController,
   bool HasDisplayRotationAcceleratorDialogBeenAccepted() const;
 
   void SetAutoclickEnabled(bool enabled);
-  bool autoclick_enabled() const { return autoclick().enabled(); }
+  bool autoclick_enabled() const { return autoclick_enabled_; }
   bool IsAutoclickSettingVisibleInTray();
   bool IsEnterpriseIconVisibleForAutoclick();
 
@@ -196,12 +92,12 @@ class ASH_EXPORT AccessibilityControllerImpl : public AccessibilityController,
   void UpdateAutoclickMenuBoundsIfNeeded();
 
   void SetCaretHighlightEnabled(bool enabled);
-  bool caret_highlight_enabled() const { return caret_highlight().enabled(); }
+  bool caret_highlight_enabled() const { return caret_highlight_enabled_; }
   bool IsCaretHighlightSettingVisibleInTray();
   bool IsEnterpriseIconVisibleForCaretHighlight();
 
   void SetCursorHighlightEnabled(bool enabled);
-  bool cursor_highlight_enabled() const { return cursor_highlight().enabled(); }
+  bool cursor_highlight_enabled() const { return cursor_highlight_enabled_; }
   bool IsCursorHighlightSettingVisibleInTray();
   bool IsEnterpriseIconVisibleForCursorHighlight();
 
@@ -372,12 +268,6 @@ class ASH_EXPORT AccessibilityControllerImpl : public AccessibilityController,
   SwitchAccessEventHandler* GetSwitchAccessEventHandlerForTest();
 
  private:
-  // Populate |features_| with the feature of the correct type.
-  void CreateAccessibilityFeatures();
-
-  // Propagates the state of |feature| according to |feature->enabled()|.
-  void OnFeatureChanged(FeatureType feature);
-
   // TabletModeObserver:
   void OnTabletModeStarted() override;
   void OnTabletModeEnded() override;
@@ -386,15 +276,15 @@ class ASH_EXPORT AccessibilityControllerImpl : public AccessibilityController,
   // initial settings.
   void ObservePrefs(PrefService* prefs);
 
-  // Updates the actual feature status based on the prefs value.
-  void UpdateFeatureFromPref(FeatureType feature);
-
+  void UpdateAutoclickFromPref();
   void UpdateAutoclickDelayFromPref();
   void UpdateAutoclickEventTypeFromPref();
   void UpdateAutoclickRevertToLeftClickFromPref();
   void UpdateAutoclickStabilizePositionFromPref();
   void UpdateAutoclickMovementThresholdFromPref();
   void UpdateAutoclickMenuPositionFromPref();
+  void UpdateCaretHighlightFromPref();
+  void UpdateCursorHighlightFromPref();
   void UpdateDictationFromPref();
   void UpdateFocusHighlightFromPref();
   void UpdateHighContrastFromPref();
@@ -416,12 +306,19 @@ class ASH_EXPORT AccessibilityControllerImpl : public AccessibilityController,
   void MaybeCreateSelectToSpeakEventHandler();
   void MaybeCreateSwitchAccessEventHandler();
 
+  // The pref service of the currently active user or the signin profile before
+  // user logs in. Can be null in ash_unittests.
+  PrefService* active_user_prefs_ = nullptr;
+
+  std::unique_ptr<PrefChangeRegistrar> pref_change_registrar_;
+
   // Client interface in chrome browser.
   AccessibilityControllerClient* client_ = nullptr;
 
-  std::unique_ptr<Feature> features_[kFeatureCount];
-
+  bool autoclick_enabled_ = false;
   base::TimeDelta autoclick_delay_;
+  bool caret_highlight_enabled_ = false;
+  bool cursor_highlight_enabled_ = false;
   bool dictation_enabled_ = false;
   bool focus_highlight_enabled_ = false;
   bool high_contrast_enabled_ = false;
@@ -458,15 +355,6 @@ class ASH_EXPORT AccessibilityControllerImpl : public AccessibilityController,
 
   base::ObserverList<AccessibilityObserver> observers_;
 
-  // The pref service of the currently active user or the signin profile before
-  // user logs in. Can be null in ash_unittests.
-  PrefService* active_user_prefs_ = nullptr;
-  // This has to be the first one to be destroyed so we don't get updates about
-  // any prefs during destruction.
-  std::unique_ptr<PrefChangeRegistrar> pref_change_registrar_;
-
-  base::WeakPtrFactory<AccessibilityControllerImpl> weak_ptr_factory_{this};
-
   DISALLOW_COPY_AND_ASSIGN(AccessibilityControllerImpl);
 };