WebUI NTP: fix OneGoogleBar with dynamic bar height

This CL removes the assumption that overlays are visible elements
that extend past 64px (the fixed bar height) because it does not account
for the push-down promo which can increase the height of the bar below
the 64px threshold.

This CL also removes the code that attempts to reduce the number of
overlays that are tracked by trying to find a common parent overlay that
houses child overlay elements.

Bug: b/157846704, b/157136561
Change-Id: Ibb872a4ee1c4d7f146676b124b578fe0ba80a358
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2226634
Commit-Queue: Esmael Elmoslimany <aee@chromium.org>
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#774719}

chrome/browser/resources/new_tab_page/app.html

@@ -312,8 +312,6 @@
 <div id="oneGoogleBarOverlayBackdrop"></div>
 <svg>
   <defs>
-    <clipPath id="oneGoogleBarClipPath">
-      <rect x="0" y="0" width="100vw" height="56"></rect>
-    </clipPath>
+    <clipPath id="oneGoogleBarClipPath"></clipPath>
   </defs>
 </svg>

chrome/browser/resources/new_tab_page/app.js

@@ -618,10 +618,9 @@ class AppElement extends PolymerElement {
       BrowserProxy.getInstance().handler.onOneGoogleBarRendered(
           BrowserProxy.getInstance().now());
     } else if (data.messageType === 'overlaysUpdated') {
-      this.$.oneGoogleBarClipPath.querySelectorAll('rect:not(:first-child)')
-          .forEach(el => {
-            el.remove();
-          });
+      this.$.oneGoogleBarClipPath.querySelectorAll('rect').forEach(el => {
+        el.remove();
+      });
       const overlayRects = /** @type {!Array<!DOMRect>} */ (data.data);
       overlayRects.forEach(({x, y, width, height}) => {
         const rectElement =

chrome/browser/resources/new_tab_page/untrusted/one_google_bar.js

@@ -2,29 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-const oneGoogleBarHeightInPixels = 64;
-let modalOverlays = false;
-let darkThemeEnabled = false;
-let shouldUndoDarkTheme = false;
-
-/**
- * @param {boolean} enabled
- * @return {!Promise}
- */
-async function enableDarkTheme(enabled) {
-  if (!window.gbar) {
-    return;
-  }
-  darkThemeEnabled = enabled;
-  const ogb = await window.gbar.a.bf();
-  ogb.pc.call(ogb, enabled ? 1 : 0);
-}
-
 /**
  * The following |messageType|'s are sent to the parent frame:
  *  - loaded: sent on initial load.
  *  - overlaysUpdated: sent when an overlay is updated. The overlay bounding
- *       rects are included in the |data|.
+ *        rects are included in the |data|.
  *  - activate/deactivate: When an overlay is open, 'activate' is sent to the
  *        to ntp-app so it can layer the OneGoogleBar over the NTP content. When
  *        no overlays are open, 'deactivate' is sent to ntp-app so the NTP
@@ -42,114 +24,158 @@ function postMessage(messageType, data) {
       'chrome://new-tab-page');
 }
 
-const overlays = new Set();
+// Object that exposes:
+//  - |getEnabled()|: returns whether dark theme is enabled.
+//  - |setEnabled(value)|: updates whether dark theme is enabled using the
+//        OneGoogleBar API.
+const darkTheme = (() => {
+  let enabled = false;
+
+  /** @return {boolean} */
+  const getEnabled = () => enabled;
 
-function sendOverlayUpdate() {
-  // Remove overlays detached from DOM or elements in a parent overlay.
-  Array.from(overlays).forEach(overlay => {
-    if (!overlay.parentElement) {
-      overlays.delete(overlay);
+  /**
+   * @param {boolean} value
+   * @return {!Promise}
+   */
+  const setEnabled = async value => {
+    if (!window.gbar) {
       return;
     }
-    let parent = overlay.parentElement;
-    while (parent) {
-      if (overlays.has(parent)) {
+    enabled = value;
+    const ogb = await window.gbar.a.bf();
+    ogb.pc.call(ogb, enabled ? 1 : 0);
+  };
+
+  return {getEnabled, setEnabled};
+})();
+
+// Object that exposes:
+//  - |track()|: sets up MutationObserver to track element visibility changes.
+//  - |update(potentialNewOverlays)|: determines visibility of tracked elements
+//        and sends an update to the top frame about element visibility.
+const overlayUpdater = (() => {
+  const modalOverlays = document.documentElement.hasAttribute('modal-overlays');
+  let shouldUndoDarkTheme = false;
+
+  /** @type {!Set<!Element>} */
+  const overlays = new Set();
+
+  /** @param {!Array<!Element>} potentialNewOverlays */
+  const update = (potentialNewOverlays) => {
+    Array.from(potentialNewOverlays).forEach(overlay => {
+      if (overlay.getBoundingClientRect().width > 0) {
+        overlays.add(overlay);
+      }
+    });
+    // Remove overlays detached from DOM.
+    Array.from(overlays).forEach(overlay => {
+      if (!overlay.parentElement) {
         overlays.delete(overlay);
         return;
       }
-      parent = parent.parentElement;
-    }
-  });
-  // Check if an overlay and its parents are visible.
-  const overlayRects =
-      Array.from(overlays)
-          .filter(overlay => {
-            if (window.getComputedStyle(overlay).visibility === 'hidden') {
-              return false;
-            }
-            let current = overlay;
-            while (current) {
-              if (window.getComputedStyle(current).display === 'none') {
+    });
+    const barHeight = document.body.querySelector('#gb').offsetHeight;
+    // Check if an overlay and its parents are visible.
+    const overlayRects =
+        Array.from(overlays)
+            .filter(overlay => {
+              if (window.getComputedStyle(overlay).visibility === 'hidden') {
                 return false;
               }
-              current = current.parentElement;
-            }
-            return true;
-          })
-          .map(el => el.getBoundingClientRect());
-  if (!modalOverlays) {
-    postMessage('overlaysUpdated', overlayRects);
-    return;
-  }
-  const overlayShown = overlayRects.length > 0;
-  postMessage(overlayShown ? 'activate' : 'deactivate');
-  // If the overlays are modal, a dark backdrop is displayed below the
-  // OneGoogleBar iframe. The dark theme for the OneGoogleBar is then enabled
-  // for better visibility.
-  if (overlayShown) {
-    if (!darkThemeEnabled) {
-      shouldUndoDarkTheme = true;
-      enableDarkTheme(true);
-    }
-  } else if (shouldUndoDarkTheme) {
-    shouldUndoDarkTheme = false;
-    enableDarkTheme(false);
-  }
-}
-
-function trackOverlayState() {
-  const observer = new MutationObserver(mutations => {
-    // After loaded, there could exist overlays that are shown, but not mutated.
-    // Add all elements that could be an overlay. The children of the actual
-    // overlay element are removed before sending any overlay update message.
-    if (overlays.size === 0) {
-      Array.from(document.body.querySelectorAll('*')).forEach(el => {
-        if (el.offsetTop + el.offsetHeight > oneGoogleBarHeightInPixels) {
-          overlays.add(el);
-        }
-      });
+              let current = overlay;
+              while (current) {
+                if (window.getComputedStyle(current).display === 'none') {
+                  return false;
+                }
+                current = current.parentElement;
+              }
+              return true;
+            })
+            .map(el => el.getBoundingClientRect())
+            .filter(rect => !modalOverlays || rect.bottom > barHeight);
+    if (!modalOverlays) {
+      postMessage('overlaysUpdated', overlayRects);
+      return;
     }
-    // Add any mutated element that is an overlay to |overlays|.
-    mutations.forEach(({target}) => {
-      if (target.id === 'gb' || target.tagName === 'BODY' ||
-          overlays.has(target)) {
-        return;
-      }
-      // When overlays are modal, the tooltips should not be treated like an
-      // overlay.
-      if (modalOverlays && target.parentElement &&
-          target.parentElement.tagName === 'BODY') {
-        return;
+    const overlayShown = overlayRects.length > 0;
+    postMessage(overlayShown ? 'activate' : 'deactivate');
+    // If the overlays are modal, a dark backdrop is displayed below the
+    // OneGoogleBar iframe. The dark theme for the OneGoogleBar is then enabled
+    // for better visibility.
+    if (overlayShown) {
+      if (!darkTheme.getEnabled()) {
+        shouldUndoDarkTheme = true;
+        darkTheme.setEnabled(true);
       }
-      if (target.offsetTop + target.offsetHeight > oneGoogleBarHeightInPixels) {
-        overlays.add(target);
-      }
-      // Update links that are loaded dynamically to ensure target is "_blank"
-      // or "_top".
-      // TODO(crbug.com/1039913): remove after OneGoogleBar links are updated.
-      if (target.parentElement) {
-        target.parentElement.querySelectorAll('a').forEach(el => {
-          if (el.target !== '_blank' && el.target !== '_top') {
-            el.target = '_top';
-          }
+    } else if (shouldUndoDarkTheme) {
+      shouldUndoDarkTheme = false;
+      darkTheme.setEnabled(false);
+    }
+  };
+
+  const track = () => {
+    const observer = new MutationObserver(mutations => {
+      const potentialNewOverlays = [];
+      // After loaded, there could exist overlays that are shown, but not
+      // mutated. Add all elements that could be an overlay. The children of the
+      // actual overlay element are removed before sending any overlay update
+      // message.
+      if (!modalOverlays && overlays.size === 0) {
+        Array.from(document.body.querySelectorAll('*')).forEach(el => {
+          potentialNewOverlays.push(el);
         });
       }
+      // Add any mutated element that is an overlay to |overlays|.
+      mutations.forEach(({target}) => {
+        if (target.id === 'gb' || target.tagName === 'BODY' ||
+            overlays.has(target)) {
+          return;
+        }
+        // When overlays are modal, the tooltips should not be treated like an
+        // overlay.
+        if (modalOverlays && target.parentElement &&
+            target.parentElement.tagName === 'BODY') {
+          return;
+        }
+        potentialNewOverlays.push(target);
+        // Update links that are loaded dynamically to ensure target is "_blank"
+        // or "_top".
+        // TODO(crbug.com/1039913): remove after OneGoogleBar links are updated.
+        if (target.parentElement) {
+          target.parentElement.querySelectorAll('a').forEach(el => {
+            if (el.target !== '_blank' && el.target !== '_top') {
+              el.target = '_top';
+            }
+          });
+        }
+      });
+      update(potentialNewOverlays);
     });
-    sendOverlayUpdate();
-  });
-  observer.observe(
-      document, {attributes: true, childList: true, subtree: true});
-}
+    observer.observe(document, {
+      attributes: true,
+      childList: true,
+      subtree: true,
+    });
+    if (!modalOverlays) {
+      update([document.body.querySelector('#gb')]);
+    }
+  };
+
+  return {track, update};
+})();
 
 window.addEventListener('message', ({data}) => {
   if (data.type === 'enableDarkTheme') {
-    enableDarkTheme(data.enabled);
+    darkTheme.setEnabled(data.enabled);
   }
 });
 
 // Need to send overlay updates on resize because overlay bounding rects are
 // absolutely positioned.
-window.addEventListener('resize', sendOverlayUpdate);
+window.addEventListener('resize', () => {
+  overlayUpdater.update([]);
+});
 
 // When the account overlay is shown, it does not close on blur. It does close
 // when clicking the body.
@@ -167,5 +193,5 @@ document.addEventListener('DOMContentLoaded', () => {
   });
   modalOverlays = document.documentElement.hasAttribute('modal-overlays');
   postMessage('loaded');
-  trackOverlayState();
+  overlayUpdater.track();
 });