Replace own callback handling with Promises.

chrome/browser/ui/webui/options/manage_profile_browsertest.js

Index: chrome/browser/ui/webui/options/manage_profile_browsertest.js
diff --git a/chrome/browser/ui/webui/options/manage_profile_browsertest.js b/chrome/browser/ui/webui/options/manage_profile_browsertest.js
index a592709c88f55b984971102cbe078960cbbe54e8..380e8e12deeb739ee0601d818307b01f9f0bd416 100644
--- a/chrome/browser/ui/webui/options/manage_profile_browsertest.js
+++ b/chrome/browser/ui/webui/options/manage_profile_browsertest.js
@@ -151,13 +151,13 @@ TEST_F('ManageProfileUITest', 'CreateExistingManagedUser', function() {
   this.setProfileManaged_(false, 'create');
 
   // Initialize the list of existing managed users.
-  var managedUserListData = options.ManagedUserListData.getInstance();
-  managedUserListData.managedUsers_ = [
+  var managedUsers = [
     {
       id: 'managedUser1',
       name: 'Rosalie',
       iconURL: 'chrome://path/to/icon/image',
       onCurrentDevice: false,
+      nameConflict: false,
       needAvatar: false
     },
     {
@@ -165,6 +165,7 @@ TEST_F('ManageProfileUITest', 'CreateExistingManagedUser', function() {
       name: 'Fritz',
       iconURL: 'chrome://path/to/icon/image',
       onCurrentDevice: false,
+      nameConflict: false,
       needAvatar: true
     },
     {
@@ -172,8 +173,13 @@ TEST_F('ManageProfileUITest', 'CreateExistingManagedUser', function() {
       name: 'Test',
       iconURL: 'chrome://path/to/icon/image',
       onCurrentDevice: true,
+      nameConflict: true,
       needAvatar: false
     }];
+  var promise = new Promise(function(resolve, reject) {

[Bernhard Bauer, 2014/01/13 14:33:50]

This is available as Promise.resolve(...).

[Adrian Kuegel, 2014/01/13 15:43:37]

Cool. Thanks for pointing that out :)

+    resolve(managedUsers);
+  });
+  options.ManagedUserListData.getInstance().promise_ = promise;
   // Also add the name 'Test' to |profileNames_| to simulate that the profile
   // exists on the device.
   ManageProfileOverlay.getInstance().profileNames_.Test = true;
@@ -189,24 +195,32 @@ TEST_F('ManageProfileUITest', 'CreateExistingManagedUser', function() {
   // A profile which already has an avatar.
   nameField.value = 'Rosalie';
   ManageProfileOverlay.getInstance().onNameChanged_('create');
-  assertTrue($('create-profile-ok').disabled);
-  assertFalse($('supervised-user-import') == null);
-  // A profile which doesn't have an avatar yet.
-  nameField.value = 'Fritz';
-  ManageProfileOverlay.getInstance().onNameChanged_('create');
-  assertTrue($('create-profile-ok').disabled);
-  assertFalse($('supervised-user-import') == null);
-  // A profile which already exists on the device.
-  nameField.value = 'Test';
-  ManageProfileOverlay.getInstance().onNameChanged_('create');
-  assertTrue($('create-profile-ok').disabled);
-  assertTrue($('supervised-user-import') == null);
-
-  // A profile which does not exist yet.
-  nameField.value = 'NewProfileName';
-  ManageProfileOverlay.getInstance().onNameChanged_('create');
-  assertFalse($('create-profile-ok').disabled);
-  assertTrue($('supervised-user-import') == null);
+  // Need to wait until the promise resolves.
+  promise.then(function() {
+    assertTrue($('create-profile-ok').disabled);
+    assertFalse($('supervised-user-import') == null);
+  }).then(function() {

[Bernhard Bauer, 2014/01/13 14:33:50]

This will be called immediately afterwards, so you might as well remove it. The
reason for that is that `then(f)` itself returns a promise that will be
fulfilled with the return value of |f|, or if that return value itself is a
promise, when that promise is fulfilled. Savvy? :-) Basically, what it means is
that you can chain multiple asynchronous calls by returning a promise in your
callback function. If you don't return a promise (as in this case, where you
implicitly return undef), the returned promise will be immediately fulfilled, so
the next callback will run immediately (where "immediately" means "on the next
turn of the event loop").

I suspect what you want to do here is repeatedly change the name, wait for the
user list to get updated, then verify properties? In that case, you can simply
do the same thing as above: Set the name, get the promise out of the
ManagedProfileOverlay instance, and wait on that promise.

Also, it's very likely that all of this code doesn't run at all, because the
test ends before that (remember that the callbacks only run on the next turn of
the event loop!). What you should do here is mark the test as asynchronous (by
setting isAsync to true on the test fixture prototype), then calling testDone()
at the end when you're finished. Sorry this is getting more complicated, but
this way you can actually test the whole thing including the communication with
the browser.

[Adrian Kuegel, 2014/01/13 15:43:37]

Thank you very much for the detailed explanation. Makes perfect sense now to me.
I changed the test to async, and use a few nested then calls which wait for the
promise to resolve after each name change.

+    // A profile which doesn't have an avatar yet.
+    nameField.value = 'Fritz';
+    ManageProfileOverlay.getInstance().onNameChanged_('create');
+  }).then(function() {
+    assertTrue($('create-profile-ok').disabled);
+    assertFalse($('supervised-user-import') == null);
+  }).then(function() {
+    // A profile which already exists on the device.
+    nameField.value = 'Test';
+    ManageProfileOverlay.getInstance().onNameChanged_('create');
+  }).then(function() {
+    assertTrue($('create-profile-ok').disabled);
+    assertTrue($('supervised-user-import') == null);
+  }).then(function() {
+    // A profile which does not exist yet.
+    nameField.value = 'NewProfileName';
+    ManageProfileOverlay.getInstance().onNameChanged_('create');
+  }).then(function() {
+    assertFalse($('create-profile-ok').disabled);
+    assertTrue($('supervised-user-import') == null);
+  });
 });
 
 // Managed users should not be able to edit their profile names, and the initial
@@ -325,17 +339,23 @@ TEST_F('ManageProfileUITest', 'CreateConfirmationText', function () {
               // independent of whether they were escaped in the setter.
               'It\'s "<HTML> injection" & more!');
 
-  // Test elision. MAX_LENGTH = 50, minus 3 for the ellipsis.
-  var name47Characters = '01234567890123456789012345678901234567890123456';
-  var name60Characters = name47Characters + '0123456789012';
-  checkDialog(name60Characters, name47Characters + '...');
+  // Test elision. MAX_LENGTH = 50, minus 1 for the ellipsis.
+  var name49Characters = '0123456789012345678901234567890123456789012345678';
+  var name50Characters = name49Characters + '9';
+  var name51Characters = name50Characters + '0';
+  var name60Characters = name51Characters + '123456789';
+  checkDialog(name49Characters, name49Characters);
+  checkDialog(name50Characters, name50Characters);
+  checkDialog(name51Characters, name49Characters + '\u2026');
+  checkDialog(name60Characters, name49Characters + '\u2026');
 
   // Test both elision and HTML escaping. The allowed string length is the
   // visible length, not the length including the entity names.
-  name47Characters = name47Characters.replace('0', '&').replace('1', '>');
+  name49Characters = name49Characters.replace('0', '&').replace('1', '>');
   name60Characters = name60Characters.replace('0', '&').replace('1', '>');
-  var escaped = name47Characters.replace('&', '&').replace('>', '>');
-  checkDialog(name60Characters, name47Characters + '...', escaped + '...');
+  var escaped = name49Characters.replace('&', '&').replace('>', '>');
+  checkDialog(
+      name60Characters, name49Characters + '\u2026', escaped + '\u2026');
 });
 
 // An additional warning should be shown when deleting a managed user.
@@ -469,9 +489,8 @@ TEST_F('ManageProfileUITest', 'ManagedDelete', function() {
 
   this.setProfileManaged_(false, 'manage');
   var messages = clickAndListen();
-  assertEquals(2, messages.length);
+  assertEquals(1, messages.length);
   assertEquals('deleteProfile', messages[0]);
-  assertEquals('requestManagedUserImportUpdate', messages[1]);
   assertEquals('settings', OptionsPage.getTopmostVisiblePage().name);
 
   ManageProfileOverlay.showDeleteDialog(this.testProfileInfo_(false));