Add a browser test for the chrome://site-engagement WebUI.

chrome/browser/resources/engagement/site_engagement.js

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 'use strict';
 
+// Allow a function to be provided by tests, which will be called when
+// the page has been populated with site engagement details.
+var pagePopulatedCallback = null;
+var isPagePopulated = false;
+
+function setPagePopulatedCallbackForTest(callback) {

[Bernhard Bauer, 2017/03/31 13:04:59]

This looks like you could use Promises
(https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects...

[calamity, 2017/04/03 04:38:52]

Indeed!

I think something like

var populatedResolve = null;
var populatedPromise = new Promise(function(resolve, reject) {
  populatedResolve = resolve;
});

function waitForPagePopulated() {
  return populatedPromise;
}

function notifyPagePopulated() {
  populatedResolve();
}

would work well here.

[Wez, 2017/04/05 20:49:37]

Done.

Note that the downside is that we're allocating a Promise in production that is
only actually used for testing, which does not seem desirable - although it's
only a single Promise, all these little bits of waste add up.

[Bernhard Bauer, 2017/04/06 15:20:47]

I personally don't think it's a big issue, but you could always create the
promise lazily.

[Wez, 2017/04/06 23:48:06]

The problem is then that the page might have completed loading data before the
test registers interest, so you end up replicating this old-style flag+callback
logic.  I'll stick with the always-created Promise.

+  if (isPagePopulated) {
+    callback();
+  } else {
+    pagePopulatedCallback = callback;
+  }
+}
+
+function notifyPagePopulated() {
+  isPagePopulated = true;
+  if (pagePopulatedCallback) {
+    pagePopulatedCallback();
+    pagePopulatedCallback = null;
+  }
+}
+
 define('main', [
     'chrome/browser/engagement/site_engagement.mojom',
     'content/public/renderer/frame_interfaces',
 ], function(siteEngagementMojom, frameInterfaces) {
   return function() {
     var uiHandler = new siteEngagementMojom.SiteEngagementUIHandlerPtr(
         frameInterfaces.getInterface(
             siteEngagementMojom.SiteEngagementUIHandler.name));
 
     var engagementTableBody = $('engagement-table-body');
@@ skipping 131 matching lines @@
      * Regenerates the engagement table from |info|.
      */
     function renderTable() {
       clearTable();
       sortInfo();
       // Round each score to 2 decimal places.
       info.forEach(function(info) {
         info.score = Number(Math.round(info.score * 100) / 100);
         engagementTableBody.appendChild(createRow(info));
       });
+
+      notifyPagePopulated();

[calamity, 2017/04/03 04:38:52]

I think this should move to line 191 so that the page is considered populated
exactly after the response from getSiteEngagementInfo() is received.

[Wez, 2017/04/05 20:49:37]

The browser test is actually verifying that there is a table element containing
the expected data, so that we catch the case where something is broken between
the response and the table being populated, so it needs to be signalled only
after the table has been updated.

     }
 
     /**
      * Retrieve site engagement info and render the engagement table.
      */
     function updateEngagementTable() {
       // Populate engagement table.
       uiHandler.getSiteEngagementInfo().then(function(response) {
         info = response.info;
         renderTable(info);
       });
     };
 
     updateEngagementTable();
     enableAutoupdate();
   };
 });