[ntp][modules] Fix promo and module coordination

* Show modules even if there is no promo. Modules wait until the promo
  has sent a loaded event. Before, this event was only sent if there was
  a promo. This CL changes it so that the event is sent even if there is
  no promo.

* Only log NewTabPage.Modules.ShownTime after modules are allowed to be
  visible. This could be after they are added to the DOM in case the
  promo takes longer to load.

Bug: 1134269
Change-Id: I5975524ba3c783de66dda2b98d347442f6b73a96
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2443693
Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
Reviewed-by: Moe Ahmadi <mahmadi@chromium.org>
Auto-Submit: Tibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813285}

chrome/browser/resources/new_tab_page/app.html

@@ -110,7 +110,7 @@
 
   :host(:not([promo-and-modules-loaded_])) ntp-middle-slot-promo,
   :host(:not([promo-and-modules-loaded_])) ntp-module-wrapper {
-    visibility: hidden;
+    display: none;
   }
 
   #customizeButtonSpacer {
@@ -290,7 +290,7 @@
           on-ntp-middle-slot-promo-loaded="onMiddleSlotPromoLoaded_" hidden>
       </ntp-middle-slot-promo>
       <template is="dom-repeat" items="[[moduleDescriptors_]]" id="modules"
-          on-dom-change="onModulesRendered_">
+          on-dom-change="onModulesLoaded_">
         <ntp-module-wrapper descriptor="[[item]]"
             on-dismiss-module="onDismissModule_">
         </ntp-module-wrapper>

chrome/browser/resources/new_tab_page/app.js

@@ -216,6 +216,7 @@ class AppElement extends PolymerElement {
         computed: `computePromoAndModulesLoaded_(middleSlotPromoLoaded_,
             modulesLoaded_)`,
         reflectToAttribute: true,
+        observer: 'onPromoAndModulesLoadedChange_',
       },
 
       /**
@@ -507,7 +508,6 @@ class AppElement extends PolymerElement {
     }
     this.moduleDescriptors_ =
         await ModuleRegistry.getInstance().initializeModules();
-    this.modulesLoaded_ = true;
   }
 
   /** @private */
@@ -592,6 +592,11 @@ class AppElement extends PolymerElement {
     this.updateBackgroundImagePath_();
   }
 
+  /** @private */
+  onPromoAndModulesLoadedChange_() {
+    this.pageHandler_.onModulesRendered(BrowserProxy.getInstance().now());
+  }
+
   /**
    * Set the #backgroundImage |path| only when different and non-empty. Reset
    * the customize dialog background selection if the dialog is closed.
@@ -808,8 +813,8 @@ class AppElement extends PolymerElement {
   }
 
   /** @private */
-  onModulesRendered_() {
-    this.pageHandler_.onModulesRendered(BrowserProxy.getInstance().now());
+  onModulesLoaded_() {
+    this.modulesLoaded_ = true;
   }
 
   /**

chrome/browser/resources/new_tab_page/middle_slot_promo.js

@@ -50,41 +50,40 @@ class MiddleSlotPromoElement extends PolymerElement {
   connectedCallback() {
     super.connectedCallback();
     this.pageHandler_.getPromo().then(({promo}) => {
-      if (!promo) {
-        return;
-      }
-      promo.middleSlotParts.forEach(({image, link, text}) => {
-        let el;
-        if (image) {
-          el = new ImgElement();
-          el.autoSrc = image.imageUrl.url;
-          if (image.target) {
-            const anchor = this.createAnchor_(image.target);
-            if (anchor) {
-              anchor.appendChild(el);
-              el = anchor;
+      if (promo) {
+        promo.middleSlotParts.forEach(({image, link, text}) => {
+          let el;
+          if (image) {
+            el = new ImgElement();
+            el.autoSrc = image.imageUrl.url;
+            if (image.target) {
+              const anchor = this.createAnchor_(image.target);
+              if (anchor) {
+                anchor.appendChild(el);
+                el = anchor;
+              }
             }
+            el.classList.add('image');
+          } else if (link) {
+            el = this.createAnchor_(link.url);
+          } else if (text) {
+            el = document.createElement('span');
           }
-          el.classList.add('image');
-        } else if (link) {
-          el = this.createAnchor_(link.url);
-        } else if (text) {
-          el = document.createElement('span');
-        }
-        const linkOrText = link || text;
-        if (el && linkOrText) {
-          el.innerText = linkOrText.text;
-          if (linkOrText.color) {
-            el.style.color = linkOrText.color;
+          const linkOrText = link || text;
+          if (el && linkOrText) {
+            el.innerText = linkOrText.text;
+            if (linkOrText.color) {
+              el.style.color = linkOrText.color;
+            }
           }
-        }
-        if (el) {
-          this.$.container.appendChild(el);
-        }
-      });
-      this.pageHandler_.onPromoRendered(
-          BrowserProxy.getInstance().now(), promo.logUrl || null);
-      this.hidden = false;
+          if (el) {
+            this.$.container.appendChild(el);
+          }
+        });
+        this.pageHandler_.onPromoRendered(
+            BrowserProxy.getInstance().now(), promo.logUrl || null);
+        this.hidden = false;
+      }
       this.dispatchEvent(new Event(
           'ntp-middle-slot-promo-loaded', {bubbles: true, composed: true}));
     });

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

@@ -8,18 +8,18 @@ import {TestBrowserProxy} from 'chrome://test/test_browser_proxy.m.js';
 import {eventToPromise, flushTasks} from 'chrome://test/test_util.m.js';
 
 suite('NewTabPageMiddleSlotPromoTest', () => {
-  /** @type {!MiddleSlotPromoElement} */
-  let middleSlotPromo;
-
   /**
    * @implements {BrowserProxy}
    * @extends {TestBrowserProxy}
    */
   let testProxy;
 
-  setup(async () => {
+  setup(() => {
     PolymerTest.clearBody();
     testProxy = createTestProxy();
+  });
+
+  async function createMiddleSlotPromo() {
     testProxy.handler.setResultFor('getPromo', Promise.resolve({
       promo: {
         middleSlotParts: [
@@ -57,12 +57,14 @@ suite('NewTabPageMiddleSlotPromoTest', () => {
     BrowserProxy.instance_ = testProxy;
     const loaded =
         eventToPromise('ntp-middle-slot-promo-loaded', document.body);
-    middleSlotPromo = document.createElement('ntp-middle-slot-promo');
+    const middleSlotPromo = document.createElement('ntp-middle-slot-promo');
     document.body.appendChild(middleSlotPromo);
     await loaded;
-  });
+    return middleSlotPromo;
+  }
 
-  test('render', () => {
+  test('render', async () => {
+    const middleSlotPromo = await createMiddleSlotPromo();
     const parts = middleSlotPromo.$.container.children;
     assertEquals(6, parts.length);
     const [image, imageWithLink, imageWithCommand, text, link, command] = parts;
@@ -88,6 +90,7 @@ suite('NewTabPageMiddleSlotPromoTest', () => {
   });
 
   test('clicking on command', async () => {
+    const middleSlotPromo = await createMiddleSlotPromo();
     const testProxy = PromoBrowserCommandProxy.getInstance();
     testProxy.handler = TestBrowserProxy.fromClass(
         promoBrowserCommand.mojom.CommandHandlerRemote);
@@ -115,4 +118,13 @@ suite('NewTabPageMiddleSlotPromoTest', () => {
           expectedClickInfo);
     }));
   });
+
+  test('sends loaded event if no promo', async () => {
+    testProxy.handler.setResultFor('getPromo', Promise.resolve({promo: null}));
+    const loaded =
+        eventToPromise('ntp-middle-slot-promo-loaded', document.body);
+    const middleSlotPromo = document.createElement('ntp-middle-slot-promo');
+    document.body.appendChild(middleSlotPromo);
+    await loaded;
+  });
 });