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

chrome/browser/resources/engagement/site_engagement.js

Index: chrome/browser/resources/engagement/site_engagement.js
diff --git a/chrome/browser/resources/engagement/site_engagement.js b/chrome/browser/resources/engagement/site_engagement.js
index f8c7ccccd9aa492e39e7f14c040a9ce41733ed15..e93cd1603ca6d08d42d03489ecafe3cafed455e1 100644
--- a/chrome/browser/resources/engagement/site_engagement.js
+++ b/chrome/browser/resources/engagement/site_engagement.js
@@ -4,6 +4,27 @@
 
 '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',
@@ -155,6 +176,8 @@ define('main', [
         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.

     }
 
     /**