WebUI: FocusRowBehavior, remove the first control logic from

|removeObservers_()|

When an row is updated, the row is reprocessed by calling |addItems_()|
which removes all the observers, repopulates the row with elements that
have the focus-row-control attribute and updates the first control if
needed.

The code that was removing observers removed the keydown event handler
from the first control. The code that updates the first control only
will register a keydown event handler if the first control has changed.
In the scenario that the row has been updated and the first control did
not change, the first control event handler will be removed and never
added back.

Bug: 927272
Change-Id: Iaf498889a4a78b33413751bcd9f74731ce2a7e01
Reviewed-on: https://chromium-review.googlesource.com/c/1460061Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Commit-Queue: Esmael El-Moslimany <aee@chromium.org>
Auto-Submit: Esmael El-Moslimany <aee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#630614}

chrome/test/data/webui/cr_focus_row_behavior_test.js

@@ -44,6 +44,11 @@ suite('cr-focus-row-behavior-test', function() {
     Polymer({
       is: 'focus-row-element',
       behaviors: [cr.ui.FocusRowBehavior],
+      focusCallCount: 0,
+
+      focus: function() {
+        this.focusCallCount++;
+      },
     });
   });
 
@@ -120,4 +125,26 @@ suite('cr-focus-row-behavior-test', function() {
       assertEquals('fake button three', button.textContent.trim());
     });
   });
+
+  test('when shift+tab pressed on first control, focus on container', () => {
+    const first = testElement.$.control;
+    const second = testElement.$.controlTwo;
+    MockInteractions.pressAndReleaseKeyOn(first, '', 'shift', 'Tab');
+    assertEquals(1, testElement.focusCallCount);
+    MockInteractions.pressAndReleaseKeyOn(second, '', 'shift', 'Tab');
+    assertEquals(1, testElement.focusCallCount);
+
+    // Simulate updating a row with same first control.
+    testElement.fire('dom-change');
+    MockInteractions.pressAndReleaseKeyOn(first, '', 'shift', 'Tab');
+    assertEquals(2, testElement.focusCallCount);
+    MockInteractions.pressAndReleaseKeyOn(second, '', 'shift', 'Tab');
+    assertEquals(2, testElement.focusCallCount);
+
+    // Simulate updating row with different first control.
+    first.remove();
+    testElement.fire('dom-change');
+    MockInteractions.pressAndReleaseKeyOn(second, '', 'shift', 'Tab');
+    assertEquals(3, testElement.focusCallCount);
+  });
 });

ui/webui/resources/js/cr/ui/focus_row_behavior.js

@@ -147,6 +147,9 @@ cr.define('cr.ui', function() {
       this.unlisten(this, 'mousedown', 'onMouseDown_');
       this.unlisten(this, 'blur', 'onBlur_');
       this.removeObservers_();
+      if (this.firstControl_) {
+        this.unlisten(this.firstControl_, 'keydown', 'onFirstControlKeydown_');
+      }
       if (this.row_) {
         this.row_.destroy();
       }
@@ -172,9 +175,6 @@ cr.define('cr.ui', function() {
 
     /** @private */
     removeObservers_: function() {
-      if (this.firstControl_) {
-        this.unlisten(this.firstControl_, 'keydown', 'onFirstControlKeydown_');
-      }
       if (this.controlObservers_.length > 0) {
         this.controlObservers_.forEach(observer => {
           observer.disconnect();