Revert "tray: Do not close accessibilty detailed menu when item selected."

This reverts commit ab3c1bb4.

Reason for revert: <INSERT REASONING HERE>

Original change's description:
> tray: Do not close accessibilty detailed menu when item selected.
> 
> It is desired to remain open so users can try out the features without
> having to tray->accessibilty->details each time. Also modified
> hover_highlight_view to support showing/removing checkmarks dynamically,
> for this purpose (other details views just recreate the whole view).
> 
> Bug: 747473
> Test: browser_tests TrayAccessibility.DetailMenuRemainsOpen
> Change-Id: I1160c29baac2afc8dc8a25dee3a87415ccef1adf
> Reviewed-on: https://chromium-review.googlesource.com/692680
> Commit-Queue: Sammie Quon <sammiequon@chromium.org>
> Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#506755}

TBR=stevenjb@chromium.org,sammiequon@chromium.org

Change-Id: I139e7255e7162e47b95715d5c7b006844ec15c37
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 747473
Reviewed-on: https://chromium-review.googlesource.com/703934Reviewed-by: Sammie Quon <sammiequon@chromium.org>
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506887}

ash/system/tray/hover_highlight_view.cc

@@ -53,6 +53,7 @@ void HoverHighlightView::AddRightView(views::View* view,
   right_view_ = view;
   right_view_->SetEnabled(enabled());
   tri_view_->AddView(TriView::Container::END, right_view_);
+  tri_view_->SetContainerVisible(TriView::Container::END, true);
 }
 
 void HoverHighlightView::SetRightViewVisible(bool visible) {
@@ -60,7 +61,6 @@ void HoverHighlightView::SetRightViewVisible(bool visible) {
   if (!right_view_)
     return;
 
-  tri_view_->SetContainerVisible(TriView::Container::END, visible);
   right_view_->SetVisible(visible);
   Layout();
 }

ash/system/tray_accessibility.cc

@@ -152,62 +152,6 @@ AccessibilityDetailedView::AccessibilityDetailedView(SystemTrayItem* owner)
   Layout();
 }
 
-void AccessibilityDetailedView::OnAccessibilityStatusChanged() {
-  AccessibilityDelegate* delegate = Shell::Get()->accessibility_delegate();
-  AccessibilityController* controller =
-      Shell::Get()->accessibility_controller();
-
-  spoken_feedback_enabled_ = delegate->IsSpokenFeedbackEnabled();
-  TrayPopupUtils::UpdateCheckMarkVisibility(spoken_feedback_view_,
-                                            spoken_feedback_enabled_);
-
-  high_contrast_enabled_ = controller->IsHighContrastEnabled();
-  TrayPopupUtils::UpdateCheckMarkVisibility(high_contrast_view_,
-                                            high_contrast_enabled_);
-
-  screen_magnifier_enabled_ = delegate->IsMagnifierEnabled();
-  TrayPopupUtils::UpdateCheckMarkVisibility(screen_magnifier_view_,
-                                            screen_magnifier_enabled_);
-
-  autoclick_enabled_ = delegate->IsAutoclickEnabled();
-  TrayPopupUtils::UpdateCheckMarkVisibility(autoclick_view_,
-                                            autoclick_enabled_);
-
-  virtual_keyboard_enabled_ = delegate->IsVirtualKeyboardEnabled();
-  TrayPopupUtils::UpdateCheckMarkVisibility(virtual_keyboard_view_,
-                                            virtual_keyboard_enabled_);
-
-  large_cursor_enabled_ = controller->IsLargeCursorEnabled();
-  TrayPopupUtils::UpdateCheckMarkVisibility(large_cursor_view_,
-                                            large_cursor_enabled_);
-
-  mono_audio_enabled_ = delegate->IsMonoAudioEnabled();
-  TrayPopupUtils::UpdateCheckMarkVisibility(mono_audio_view_,
-                                            mono_audio_enabled_);
-
-  caret_highlight_enabled_ = delegate->IsCaretHighlightEnabled();
-  TrayPopupUtils::UpdateCheckMarkVisibility(caret_highlight_view_,
-                                            caret_highlight_enabled_);
-
-  highlight_mouse_cursor_enabled_ = delegate->IsCursorHighlightEnabled();
-  TrayPopupUtils::UpdateCheckMarkVisibility(highlight_mouse_cursor_view_,
-                                            highlight_mouse_cursor_enabled_);
-
-  if (highlight_keyboard_focus_view_) {
-    highlight_keyboard_focus_enabled_ = delegate->IsFocusHighlightEnabled();
-    TrayPopupUtils::UpdateCheckMarkVisibility(
-        highlight_keyboard_focus_view_, highlight_keyboard_focus_enabled_);
-  }
-
-  sticky_keys_enabled_ = delegate->IsStickyKeysEnabled();
-  TrayPopupUtils::UpdateCheckMarkVisibility(sticky_keys_view_,
-                                            sticky_keys_enabled_);
-
-  tap_dragging_enabled_ = delegate->IsTapDraggingEnabled();
-  TrayPopupUtils::UpdateCheckMarkVisibility(tap_dragging_view_,
-                                            tap_dragging_enabled_);
-}
-
 void AccessibilityDetailedView::AppendAccessibilityList() {
   CreateScrollableList();
 
@@ -507,7 +451,7 @@ void TrayAccessibility::OnAccessibilityStatusChanged(
     return;
 
   if (detailed_menu_)
-    detailed_menu_->OnAccessibilityStatusChanged();
+    detailed_menu_->GetWidget()->Close();
 
   message_center::MessageCenter* message_center =
       message_center::MessageCenter::Get();

ash/system/tray_accessibility.h

@@ -27,7 +27,6 @@ class View;
 }
 
 namespace ash {
-class HoverHighlightView;
 class SystemTrayItem;
 
 namespace tray {
@@ -38,8 +37,6 @@ class AccessibilityDetailedView : public TrayDetailsView {
   explicit AccessibilityDetailedView(SystemTrayItem* owner);
   ~AccessibilityDetailedView() override {}
 
-  void OnAccessibilityStatusChanged();
-
  private:
   friend class chromeos::TrayAccessibilityTest;
 
@@ -58,20 +55,20 @@ class AccessibilityDetailedView : public TrayDetailsView {
   // Add the accessibility feature list.
   void AppendAccessibilityList();
 
-  HoverHighlightView* spoken_feedback_view_ = nullptr;
-  HoverHighlightView* high_contrast_view_ = nullptr;
-  HoverHighlightView* screen_magnifier_view_ = nullptr;
-  HoverHighlightView* large_cursor_view_ = nullptr;
-  HoverHighlightView* autoclick_view_ = nullptr;
-  HoverHighlightView* virtual_keyboard_view_ = nullptr;
-  HoverHighlightView* mono_audio_view_ = nullptr;
-  HoverHighlightView* caret_highlight_view_ = nullptr;
-  HoverHighlightView* highlight_mouse_cursor_view_ = nullptr;
-  HoverHighlightView* highlight_keyboard_focus_view_ = nullptr;
-  HoverHighlightView* sticky_keys_view_ = nullptr;
-  HoverHighlightView* tap_dragging_view_ = nullptr;
+  views::View* spoken_feedback_view_ = nullptr;
+  views::View* high_contrast_view_ = nullptr;
+  views::View* screen_magnifier_view_ = nullptr;
+  views::View* large_cursor_view_ = nullptr;
   views::Button* help_view_ = nullptr;
   views::Button* settings_view_ = nullptr;
+  views::View* autoclick_view_ = nullptr;
+  views::View* virtual_keyboard_view_ = nullptr;
+  views::View* mono_audio_view_ = nullptr;
+  views::View* caret_highlight_view_ = nullptr;
+  views::View* highlight_mouse_cursor_view_ = nullptr;
+  views::View* highlight_keyboard_focus_view_ = nullptr;
+  views::View* sticky_keys_view_ = nullptr;
+  views::View* tap_dragging_view_ = nullptr;
 
   // These exist for tests. The canonical state is stored in prefs.
   bool spoken_feedback_enabled_ = false;

chrome/browser/chromeos/system/tray_accessibility_browsertest.cc

@@ -9,7 +9,6 @@
 #include "ash/public/cpp/ash_pref_names.h"
 #include "ash/shell.h"
 #include "ash/shell_test_api.h"
-#include "ash/system/tray/hover_highlight_view.h"
 #include "ash/system/tray/system_tray.h"
 #include "ash/system/tray/system_tray_test_api.h"
 #include "ash/system/tray_accessibility.h"
@@ -171,10 +170,6 @@ class TrayAccessibilityTest
     return tray()->detailed_menu_ != nullptr;
   }
 
-  ash::tray::AccessibilityDetailedView* GetDetailedMenu() {
-    return tray()->detailed_menu_;
-  }
-
   void CloseDetailMenu() {
     ASSERT_TRUE(tray()->detailed_menu_);
     tray()->OnDetailedViewDestroyed();
@@ -182,81 +177,76 @@ class TrayAccessibilityTest
   }
 
   void ClickSpokenFeedbackOnDetailMenu() {
-    ash::HoverHighlightView* view =
-        tray()->detailed_menu_->spoken_feedback_view_;
-    ASSERT_TRUE(view);
-    tray()->detailed_menu_->OnViewClicked(view);
+    views::View* button = tray()->detailed_menu_->spoken_feedback_view_;
+    ASSERT_TRUE(button);
+    tray()->detailed_menu_->OnViewClicked(button);
   }
 
   void ClickHighContrastOnDetailMenu() {
-    ash::HoverHighlightView* view = tray()->detailed_menu_->high_contrast_view_;
-    ASSERT_TRUE(view);
-    tray()->detailed_menu_->OnViewClicked(view);
+    views::View* button = tray()->detailed_menu_->high_contrast_view_;
+    ASSERT_TRUE(button);
+    tray()->detailed_menu_->OnViewClicked(button);
   }
 
   void ClickScreenMagnifierOnDetailMenu() {
-    ash::HoverHighlightView* view =
-        tray()->detailed_menu_->screen_magnifier_view_;
-    ASSERT_TRUE(view);
-    tray()->detailed_menu_->OnViewClicked(view);
+    views::View* button = tray()->detailed_menu_->screen_magnifier_view_;
+    ASSERT_TRUE(button);
+    tray()->detailed_menu_->OnViewClicked(button);
   }
 
   void ClickAutoclickOnDetailMenu() {
-    ash::HoverHighlightView* view = tray()->detailed_menu_->autoclick_view_;
-    ASSERT_TRUE(view);
-    tray()->detailed_menu_->OnViewClicked(view);
+    views::View* button = tray()->detailed_menu_->autoclick_view_;
+    ASSERT_TRUE(button);
+    tray()->detailed_menu_->OnViewClicked(button);
   }
 
   void ClickVirtualKeyboardOnDetailMenu() {
-    ash::HoverHighlightView* view =
-        tray()->detailed_menu_->virtual_keyboard_view_;
-    ASSERT_TRUE(view);
-    tray()->detailed_menu_->OnViewClicked(view);
+    views::View* button = tray()->detailed_menu_->virtual_keyboard_view_;
+    ASSERT_TRUE(button);
+    tray()->detailed_menu_->OnViewClicked(button);
   }
 
   void ClickLargeMouseCursorOnDetailMenu() {
-    ash::HoverHighlightView* view = tray()->detailed_menu_->large_cursor_view_;
-    ASSERT_TRUE(view);
-    tray()->detailed_menu_->OnViewClicked(view);
+    views::View* button = tray()->detailed_menu_->large_cursor_view_;
+    ASSERT_TRUE(button);
+    tray()->detailed_menu_->OnViewClicked(button);
   }
 
   void ClickMonoAudioOnDetailMenu() {
-    ash::HoverHighlightView* view = tray()->detailed_menu_->mono_audio_view_;
-    ASSERT_TRUE(view);
-    tray()->detailed_menu_->OnViewClicked(view);
+    views::View* button = tray()->detailed_menu_->mono_audio_view_;
+    ASSERT_TRUE(button);
+    tray()->detailed_menu_->OnViewClicked(button);
   }
 
   void ClickCaretHighlightOnDetailMenu() {
-    ash::HoverHighlightView* view =
-        tray()->detailed_menu_->caret_highlight_view_;
-    ASSERT_TRUE(view);
-    tray()->detailed_menu_->OnViewClicked(view);
+    views::View* button = tray()->detailed_menu_->caret_highlight_view_;
+    ASSERT_TRUE(button);
+    tray()->detailed_menu_->OnViewClicked(button);
   }
 
   void ClickHighlightMouseCursorOnDetailMenu() {
-    ash::HoverHighlightView* view =
-        tray()->detailed_menu_->highlight_mouse_cursor_view_;
-    ASSERT_TRUE(view);
-    tray()->detailed_menu_->OnViewClicked(view);
+    views::View* button = tray()->detailed_menu_->highlight_mouse_cursor_view_;
+    ASSERT_TRUE(button);
+    tray()->detailed_menu_->OnViewClicked(button);
   }
 
-  void ClickHighlightKeyboardFocusOnDetailMenu() {
-    ash::HoverHighlightView* view =
+  void ClickHighlishtKeyboardFocusOnDetailMenu() {
+    views::View* button =
         tray()->detailed_menu_->highlight_keyboard_focus_view_;
-    ASSERT_TRUE(view);
-    tray()->detailed_menu_->OnViewClicked(view);
+    ASSERT_TRUE(button);
+    tray()->detailed_menu_->OnViewClicked(button);
   }
 
   void ClickStickyKeysOnDetailMenu() {
-    ash::HoverHighlightView* view = tray()->detailed_menu_->sticky_keys_view_;
-    ASSERT_TRUE(view);
-    tray()->detailed_menu_->OnViewClicked(view);
+    views::View* button = tray()->detailed_menu_->sticky_keys_view_;
+    ASSERT_TRUE(button);
+    tray()->detailed_menu_->OnViewClicked(button);
   }
 
   void ClickTapDraggingOnDetailMenu() {
-    ash::HoverHighlightView* view = tray()->detailed_menu_->tap_dragging_view_;
-    ASSERT_TRUE(view);
-    tray()->detailed_menu_->OnViewClicked(view);
+    views::View* button = tray()->detailed_menu_->tap_dragging_view_;
+    ASSERT_TRUE(button);
+    tray()->detailed_menu_->OnViewClicked(button);
   }
 
   bool IsSpokenFeedbackEnabledOnDetailMenu() const {
@@ -1114,11 +1104,11 @@ IN_PROC_BROWSER_TEST_P(TrayAccessibilityTest, ClickDetailMenu) {
   EXPECT_FALSE(AccessibilityManager::Get()->IsFocusHighlightEnabled());
 
   EXPECT_TRUE(CreateDetailedMenu());
-  ClickHighlightKeyboardFocusOnDetailMenu();
+  ClickHighlishtKeyboardFocusOnDetailMenu();
   EXPECT_TRUE(AccessibilityManager::Get()->IsFocusHighlightEnabled());
 
   EXPECT_TRUE(CreateDetailedMenu());
-  ClickHighlightKeyboardFocusOnDetailMenu();
+  ClickHighlishtKeyboardFocusOnDetailMenu();
   EXPECT_FALSE(AccessibilityManager::Get()->IsFocusHighlightEnabled());
 
   // Confirms that the check item toggles sticky keys.
@@ -1718,29 +1708,6 @@ IN_PROC_BROWSER_TEST_P(TrayAccessibilityTest, CheckMenuVisibilityOnDetailMenu) {
   CloseDetailMenu();
 }
 
-// Verify that the accessiblity system detailed menu remains open when an item
-// is selected or deselected.
-IN_PROC_BROWSER_TEST_P(TrayAccessibilityTest, DetailMenuRemainsOpen) {
-  EXPECT_TRUE(CreateDetailedMenu());
-  ASSERT_TRUE(IsAutoclickMenuShownOnDetailMenu());
-
-  ClickAutoclickOnDetailMenu();
-  EXPECT_TRUE(IsAutoclickEnabledOnDetailMenu());
-  {
-    base::RunLoop run_loop;
-    run_loop.RunUntilIdle();
-  }
-  EXPECT_TRUE(GetDetailedMenu());
-
-  ClickAutoclickOnDetailMenu();
-  EXPECT_FALSE(IsAutoclickEnabledOnDetailMenu());
-  {
-    base::RunLoop run_loop;
-    run_loop.RunUntilIdle();
-  }
-  EXPECT_TRUE(GetDetailedMenu());
-}
-
 INSTANTIATE_TEST_CASE_P(TrayAccessibilityTestInstance,
                         TrayAccessibilityTest,
                         testing::Values(PREF_SERVICE,