Hide the focus highlight on tab change

Bug: 1122961
Change-Id: I7e442291cb2b830227ee6f1e0aa1b3896ac8eb5e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2466012Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Commit-Queue: Alice Boxhall <aboxhall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818321}

chrome/browser/ui/views/accessibility/accessibility_focus_highlight.cc

@@ -89,12 +89,12 @@ AccessibilityFocusHighlight::AccessibilityFocusHighlight(
   profile_pref_registrar_.Init(browser_view_->browser()->profile()->GetPrefs());
   profile_pref_registrar_.Add(
       prefs::kAccessibilityFocusHighlightEnabled,
-      base::BindRepeating(
-          &AccessibilityFocusHighlight::AddOrRemoveFocusObserver,
-          base::Unretained(this)));
+      base::BindRepeating(&AccessibilityFocusHighlight::AddOrRemoveObservers,
+                          base::Unretained(this)));
 
-  // Initialise focus observer based on current preferences.
-  AddOrRemoveFocusObserver();
+  // Initialise focus and tab strip model observers based on current
+  // preferences.
+  AddOrRemoveObservers();
 
   // One-time initialization of statics the first time an instance is created.
   if (fade_in_time_.is_zero()) {
@@ -215,8 +215,10 @@ void AccessibilityFocusHighlight::RemoveLayer() {
   }
 }
 
-void AccessibilityFocusHighlight::AddOrRemoveFocusObserver() {
-  PrefService* prefs = browser_view_->browser()->profile()->GetPrefs();
+void AccessibilityFocusHighlight::AddOrRemoveObservers() {
+  Browser* browser = browser_view_->browser();
+  PrefService* prefs = browser->profile()->GetPrefs();
+  TabStripModel* tab_strip_model = browser->tab_strip_model();
 
   if (prefs->GetBoolean(prefs::kAccessibilityFocusHighlightEnabled)) {
     // Listen for focus changes. Automatically deregisters when destroyed,
@@ -224,15 +226,18 @@ void AccessibilityFocusHighlight::AddOrRemoveFocusObserver() {
     notification_registrar_.Add(this,
                                 content::NOTIFICATION_FOCUS_CHANGED_IN_PAGE,
                                 content::NotificationService::AllSources());
-    return;
-  }
 
-  if (notification_registrar_.IsRegistered(
+    tab_strip_model->AddObserver(this);
+    return;
+  } else {
+    if (notification_registrar_.IsRegistered(
+            this, content::NOTIFICATION_FOCUS_CHANGED_IN_PAGE,
+            content::NotificationService::AllSources())) {
+      notification_registrar_.Remove(
           this, content::NOTIFICATION_FOCUS_CHANGED_IN_PAGE,
-          content::NotificationService::AllSources())) {
-    notification_registrar_.Remove(this,
-                                   content::NOTIFICATION_FOCUS_CHANGED_IN_PAGE,
-                                   content::NotificationService::AllSources());
+          content::NotificationService::AllSources());
+    }
+    tab_strip_model->RemoveObserver(this);
   }
 }
 
@@ -390,3 +395,10 @@ void AccessibilityFocusHighlight::OnCompositingShuttingDown(
     compositor_ = nullptr;
   }
 }
+
+void AccessibilityFocusHighlight::OnTabStripModelChanged(
+    TabStripModel*,
+    const TabStripModelChange&,
+    const TabStripSelectionChange&) {
+  RemoveLayer();
+}

chrome/browser/ui/views/accessibility/accessibility_focus_highlight.h

@@ -10,6 +10,7 @@
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/time/time.h"
+#include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
 #include "components/prefs/pref_change_registrar.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
@@ -29,7 +30,8 @@ class Layer;
 // highlight the focused UI element for accessibility.
 class AccessibilityFocusHighlight : public ui::LayerDelegate,
                                     public ui::CompositorAnimationObserver,
-                                    public content::NotificationObserver {
+                                    public content::NotificationObserver,
+                                    public TabStripModelObserver {
  public:
   explicit AccessibilityFocusHighlight(BrowserView* browser_view);
   ~AccessibilityFocusHighlight() override;
@@ -56,8 +58,8 @@ class AccessibilityFocusHighlight : public ui::LayerDelegate,
   // Get rid of the layer and stop animation.
   void RemoveLayer();
 
-  // Handle preference changes by adding or removing the layer as necessary.
-  void AddOrRemoveFocusObserver();
+  // Handle preference changes by adding or removing observers as necessary.
+  void AddOrRemoveObservers();
 
   // content::NotificationObserver overrides:
   void Observe(int type,
@@ -73,6 +75,11 @@ class AccessibilityFocusHighlight : public ui::LayerDelegate,
   void OnAnimationStep(base::TimeTicks timestamp) override;
   void OnCompositingShuttingDown(ui::Compositor* compositor) override;
 
+  // TabStripModelObserver
+  void OnTabStripModelChanged(TabStripModel*,
+                              const TabStripModelChange&,
+                              const TabStripSelectionChange&) override;
+
   // Compute the highlight color based on theme colors and defaults.
   SkColor GetHighlightColor();