WebUI NTP: move scroll border code into utils

Moving the scroll border code out of the customize dialog makes it
easier to test. If there is a flake, it will also be easier to diagnose.

This CL enables the ntp-customize-dialog unit tests which was previously
disabled due to flakiness (due to the scroll borders tests).

Bug: 1056688
Change-Id: Ic74f4b79c33c578f9a9d759d80ed569c93f12550
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2082901
Commit-Queue: Esmael Elmoslimany <aee@chromium.org>
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Auto-Submit: Esmael Elmoslimany <aee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746221}

chrome/browser/resources/new_tab_page/BUILD.gn

@@ -55,6 +55,7 @@ js_library("customize_dialog") {
     ":customize_backgrounds",
     ":customize_shortcuts",
     ":customize_themes",
+    ":utils",
     "//third_party/polymer/v3_0/components-chromium/polymer:polymer_bundled",
     "//ui/webui/resources/cr_elements/cr_button:cr_button.m",
     "//ui/webui/resources/cr_elements/cr_dialog:cr_dialog.m",

chrome/browser/resources/new_tab_page/customize_dialog.js

@@ -13,6 +13,7 @@ import './customize_themes.js';
 import {html, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 
 import {BrowserProxy} from './browser_proxy.js';
+import {createScrollBorders} from './utils.js';
 
 /**
  * Dialog that lets the user customize the NTP such as the background color or
@@ -61,33 +62,10 @@ class CustomizeDialogElement extends PolymerElement {
   /** @override */
   ready() {
     super.ready();
-
-    ['menu', 'pages'].forEach(id => {
-      const container = this.$[id];
-      const topProbe = document.createElement('div');
-      container.prepend(topProbe);
-      const bottomProbe = document.createElement('div');
-      container.append(bottomProbe);
-      const topBorder = document.createElement('div');
-      topBorder.toggleAttribute('scroll-border', true);
-      container.parentNode.insertBefore(topBorder, container);
-      const bottomBorder = document.createElement('div');
-      bottomBorder.toggleAttribute('scroll-border', true);
-      container.parentNode.insertBefore(bottomBorder, container.nextSibling);
-      const observer = new IntersectionObserver(entries => {
-        entries.forEach(({target, intersectionRatio}) => {
-          const show = intersectionRatio === 0;
-          if (target === topProbe) {
-            topBorder.toggleAttribute('show', show);
-          } else if (target === bottomProbe) {
-            bottomBorder.toggleAttribute('show', show);
-          }
-        });
-      }, {root: container});
-      observer.observe(topProbe);
-      observer.observe(bottomProbe);
-      this.intersectionObservers_.push(observer);
-    });
+    this.intersectionObservers_ = [
+      this.$.menu,
+      this.$.pages,
+    ].map(createScrollBorders);
   }
 
   /** @private */

chrome/browser/resources/new_tab_page/utils.js

@@ -34,3 +34,40 @@ export function hexColorToSkColor(hexColor) {
   const b = parseInt(hexColor.substring(5, 7), 16);
   return {value: 0xff000000 + (r << 16) + (g << 8) + b};
 }
+
+/**
+ * Given a |container| that has scrollable content, <div>'s before and after the
+ * |container| are created with an attribute "scroll-border". These <div>'s are
+ * updated to have an attribute "show" when there is more content in the
+ * direction of the "scroll-border". Styling is left to the caller.
+ *
+ * Returns an |IntersectionObserver| so the caller can disconnect the observer
+ * when needed.
+ * @param {!HTMLElement} container
+ * @return {!IntersectionObserver}
+ */
+export function createScrollBorders(container) {
+  const topProbe = document.createElement('div');
+  container.prepend(topProbe);
+  const bottomProbe = document.createElement('div');
+  container.append(bottomProbe);
+  const topBorder = document.createElement('div');
+  topBorder.toggleAttribute('scroll-border', true);
+  container.parentNode.insertBefore(topBorder, container);
+  const bottomBorder = document.createElement('div');
+  bottomBorder.toggleAttribute('scroll-border', true);
+  container.parentNode.insertBefore(bottomBorder, container.nextSibling);
+  const observer = new IntersectionObserver(entries => {
+    entries.forEach(({target, intersectionRatio}) => {
+      const show = intersectionRatio === 0;
+      if (target === topProbe) {
+        topBorder.toggleAttribute('show', show);
+      } else if (target === bottomProbe) {
+        bottomBorder.toggleAttribute('show', show);
+      }
+    });
+  }, {root: container});
+  observer.observe(topProbe);
+  observer.observe(bottomProbe);
+  return observer;
+}

chrome/test/data/webui/new_tab_page/customize_dialog_test.js

@@ -53,46 +53,4 @@ suite('NewTabPageCustomizeDialogTest', () => {
     assertEquals(shownPages.length, 1);
     assertEquals(shownPages[0].getAttribute('page-name'), 'themes');
   });
-
-  suite('scroll borders', () => {
-    /**
-     * @param {!HTMLElement} container
-     * @private
-     */
-    async function testScrollBorders(container) {
-      const assertHidden = el => {
-        assertTrue(el.matches('[scroll-border]:not([show])'));
-      };
-      const assertShown = el => {
-        assertTrue(el.matches('[scroll-border][show]'));
-      };
-      const {firstElementChild: top, lastElementChild: bottom} = container;
-      const scrollableElement = top.nextSibling;
-      const dialogBody =
-          customizeDialog.shadowRoot.querySelector('div[slot=body]');
-      const heightWithBorders = `${scrollableElement.scrollHeight + 2}px`;
-      dialogBody.style.height = heightWithBorders;
-      assertHidden(top);
-      assertHidden(bottom);
-      dialogBody.style.height = '50px';
-      await waitAfterNextRender();
-      assertHidden(top);
-      assertShown(bottom);
-      scrollableElement.scrollTop = 1;
-      await waitAfterNextRender();
-      assertShown(top);
-      assertShown(bottom);
-      scrollableElement.scrollTop = scrollableElement.scrollHeight;
-      await waitAfterNextRender();
-      assertShown(top);
-      assertHidden(bottom);
-      dialogBody.style.height = heightWithBorders;
-      await waitAfterNextRender();
-      assertHidden(top);
-      assertHidden(bottom);
-    }
-
-    test('menu', () => testScrollBorders(customizeDialog.$.menuContainer));
-    test('pages', () => testScrollBorders(customizeDialog.$.pagesContainer));
-  });
 });

chrome/test/data/webui/new_tab_page/new_tab_page_browsertest.js

@@ -59,8 +59,7 @@ var NewTabPageCustomizeDialogTest = class extends NewTabPageBrowserTest {
   }
 };
 
-// Test is flaky: crbug.com/1056688.
-TEST_F('NewTabPageCustomizeDialogTest', 'DISABLED_All', function() {
+TEST_F('NewTabPageCustomizeDialogTest', 'All', function() {
   mocha.run();
 });
 

chrome/test/data/webui/new_tab_page/utils_test.js

@@ -5,7 +5,8 @@
 // So that mojo is defined.
 import 'chrome://resources/mojo/mojo/public/js/mojo_bindings_lite.js';
 
-import {hexColorToSkColor, skColorToRgb} from 'chrome://new-tab-page/utils.js';
+import {createScrollBorders, hexColorToSkColor, skColorToRgb} from 'chrome://new-tab-page/utils.js';
+import {waitAfterNextRender} from 'chrome://test/test_util.m.js';
 
 suite('NewTabPageUtilsTest', () => {
   test('Can convert simple SkColors to rgb strings', () => {
@@ -46,3 +47,70 @@ suite('NewTabPageUtilsTest', () => {
     assertDeepEquals(hexColorToSkColor('ffffff'), {value: 0});
   });
 });
+
+suite('scroll borders', () => {
+  /** @type {!HTMLElement} */
+  let container;
+  /** @type {!HTMLElement} */
+  let content;
+  /** @type {!HTMLElement} */
+  let top;
+  /** @type {!HTMLElement} */
+  let bottom;
+  /** @type {!IntersectionObserver} */
+  let observer;
+
+  /** @param {!HTMLElement} el */
+  function assertHidden(el) {
+    assertTrue(el.matches('[scroll-border]:not([show])'));
+  }
+
+  /** @param {!HTMLElement} el */
+  function assertShown(el) {
+    assertTrue(el.matches('[scroll-border][show]'));
+  }
+
+  setup(async () => {
+    document.body.innerHTML =
+        '<div id="container"><div id="content"></div></div>';
+    container = document.querySelector('#container');
+    container.style.height = '100px';
+    container.style.overflow = 'auto';
+    content = document.querySelector('#content');
+    content.style.height = '200px';
+    observer = createScrollBorders(container);
+    top = document.body.firstElementChild;
+    bottom = document.body.lastElementChild;
+    await waitAfterNextRender();
+  });
+
+  teardown(() => {
+    observer.disconnect();
+  });
+
+  test('bottom border show when more content available below', () => {
+    assertHidden(top);
+    assertShown(bottom);
+  });
+
+  test('borders shown when content available above and below', async () => {
+    container.scrollTop = 10;
+    await waitAfterNextRender();
+    assertShown(top);
+    assertShown(bottom);
+  });
+
+  test('bottom border hidden when no content available below', async () => {
+    container.scrollTop = 200;
+    await waitAfterNextRender();
+    assertShown(top);
+    assertHidden(bottom);
+  });
+
+  test('borders hidden when all content is shown', async () => {
+    content.style.height = '100px';
+    await waitAfterNextRender();
+    assertHidden(top);
+    assertHidden(bottom);
+  });
+});