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..45791528454b383a94c4d79a7d6b036b28ce17db 100644
--- a/chrome/browser/ui/webui/options/manage_profile_browsertest.js
+++ b/chrome/browser/ui/webui/options/manage_profile_browsertest.js
@@ -21,6 +21,8 @@ ManageProfileUITest.prototype = {
   /** @override */
   browsePreload: 'chrome://settings-frame/manageProfile',
 
+  isAsync: true,
+
   /**
    * No need to run these for every OptionsPage test, since they'll cover the
    * whole consolidated page each time.
@@ -96,6 +98,7 @@ TEST_F('ManageProfileUITest', 'NewProfileDefaultsFocus', function() {
   checkFocus('managedUserLearnMore',
              document.querySelector('#managed-user-learn-more-text a'),
              document.querySelector('#managed-user-learn-more-text a'));
+  testDone();

[Bernhard Bauer, 2014/01/13 17:18:47]

Alternatively, you could make a subclass for the async test. Up to you, really.

[Adrian Kuegel, 2014/01/15 14:15:35]

Ok, this seems to be the better solution, since we only need the async behavior
in one test. So I changed it.

 });
 
 // The default options should be reset each time the creation overlay is shown.
@@ -113,6 +116,7 @@ TEST_F('ManageProfileUITest', 'DefaultCreateOptions', function() {
   OptionsPage.showPageByName('createProfile');
   assertEquals(shortcutsAllowed, createShortcut.checked);
   assertFalse(createManaged.checked);
+  testDone();
 });
 
 // The checkbox label should change depending on whether the user is signed in.
@@ -138,6 +142,7 @@ TEST_F('ManageProfileUITest', 'CreateManagedUserText', function() {
   assertFalse(notSignedInText.hidden);
   assertFalse($('create-profile-managed').checked);
   assertTrue($('create-profile-managed').disabled);
+  testDone();
 });
 
 // The import link should show up if the user tries to create a profile with the
@@ -151,13 +156,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 +170,7 @@ TEST_F('ManageProfileUITest', 'CreateExistingManagedUser', function() {
       name: 'Fritz',
       iconURL: 'chrome://path/to/icon/image',
       onCurrentDevice: false,
+      nameConflict: false,
       needAvatar: true
     },
     {
@@ -172,8 +178,11 @@ TEST_F('ManageProfileUITest', 'CreateExistingManagedUser', function() {
       name: 'Test',
       iconURL: 'chrome://path/to/icon/image',
       onCurrentDevice: true,
+      nameConflict: true,
       needAvatar: false
     }];
+  var promise = Promise.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 +198,36 @@ 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);
+
+    // A profile which doesn't have an avatar yet.
+    nameField.value = 'Fritz';
+    ManageProfileOverlay.getInstance().onNameChanged_('create');
+    promise.then(function() {

[Bernhard Bauer, 2014/01/13 17:18:47]

You're waiting on the same promise, which is already fulfilled. If the test
currently succeeds, that tells me that you're relying on the fact that the
promise hasn't been reset in the mean time. I'm not sure if that's a good
assumption...

In any case, you can get rid of the nested calls (regardless of which promise
you end up waiting on) by returning the promise from the callback here.

[Adrian Kuegel, 2014/01/15 14:15:35]

You are right, in a normal execution it could have been reset in the mean time.
Good idea. Done.

+      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');
+      promise.then(function() {
+        assertTrue($('create-profile-ok').disabled);
+        assertTrue($('supervised-user-import') == null);
+
+        // A profile which does not exist yet.
+        nameField.value = 'NewProfileName';
+        ManageProfileOverlay.getInstance().onNameChanged_('create');
+        promise.then(function() {
+          assertFalse($('create-profile-ok').disabled);
+          assertTrue($('supervised-user-import') == null);
+          testDone();
+        });
+      });
+    });
+  });
 });
 
 // Managed users should not be able to edit their profile names, and the initial
@@ -225,6 +246,7 @@ TEST_F('ManageProfileUITest', 'EditManagedUserNameAllowed', function() {
   ManageProfileOverlay.showManageDialog();
   expectTrue(nameField.disabled);
   expectEquals($('manage-profile-ok'), document.activeElement);
+  testDone();
 });
 
 // Setting profile information should allow the confirmation to be shown.
@@ -236,6 +258,7 @@ TEST_F('ManageProfileUITest', 'ShowCreateConfirmation', function() {
   OptionsPage.showPageByName('managedUserCreateConfirm', false);
   assertEquals('managedUserCreateConfirm',
                OptionsPage.getTopmostVisiblePage().name);
+  testDone();
 });
 
 // Trying to show a confirmation dialog with no profile information should fall
@@ -245,6 +268,7 @@ TEST_F('ManageProfileUITest', 'NoEmptyConfirmation', function() {
   assertFalse(ManagedUserCreateConfirmOverlay.getInstance().canShowPage());
   OptionsPage.showPageByName('managedUserCreateConfirm', true);
   assertEquals('settings', OptionsPage.getTopmostVisiblePage().name);
+  testDone();
 });
 
 // A confirmation dialog should be shown after creating a new managed user.
@@ -260,6 +284,7 @@ TEST_F('ManageProfileUITest', 'ShowCreateConfirmationOnSuccess', function() {
   assertEquals('managedUserCreateConfirm',
                OptionsPage.getTopmostVisiblePage().name);
   expectEquals($('managed-user-created-switch'), document.activeElement);
+  testDone();
 });
 
 // An error should be shown if creating a new managed user fails.
@@ -272,6 +297,7 @@ TEST_F('ManageProfileUITest', 'NoCreateConfirmationOnError', function() {
   CreateProfileOverlay.onError('An Error Message!');
   assertEquals('createProfile', OptionsPage.getTopmostVisiblePage().name);
   assertFalse(errorBubble.hidden);
+  testDone();
 });
 
 // The name and email sould be inserted into the confirmation dialog.
@@ -325,17 +351,24 @@ 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');
+  testDone();
 });
 
 // An additional warning should be shown when deleting a managed user.
@@ -347,6 +380,7 @@ TEST_F('ManageProfileUITest', 'DeleteManagedUserWarning', function() {
 
   ManageProfileOverlay.showDeleteDialog(this.testProfileInfo_(false));
   assertTrue(addendum.hidden);
+  testDone();
 });
 
 // The policy prohibiting managed users should update the UI dynamically.
@@ -394,6 +428,7 @@ TEST_F('ManageProfileUITest', 'PolicyDynamicRefresh', function() {
   assertFalse(link.hidden, 're-allowed and signed in');
   assertEquals('none', window.getComputedStyle(indicator, null).display,
                're-allowed and signed in');
+  testDone();
 });
 
 // The managed user checkbox should correctly update its state during profile
@@ -424,6 +459,7 @@ TEST_F('ManageProfileUITest', 'CreateInProgress', function() {
 
   CreateProfileOverlay.updateCreateInProgress(false);
   assertFalse(checkbox.disabled, 'creation finished');
+  testDone();
 });
 
 // Managed users shouldn't be able to open the delete or create dialogs.
@@ -445,6 +481,7 @@ TEST_F('ManageProfileUITest', 'ManagedShowDeleteAndCreate', function() {
   assertEquals('settings', OptionsPage.getTopmostVisiblePage().name);
   ManageProfileOverlay.showDeleteDialog(this.testProfileInfo_(false));
   assertEquals('settings', OptionsPage.getTopmostVisiblePage().name);
+  testDone();
 });
 
 // Only non-managed users should be able to delete profiles.
@@ -469,9 +506,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));
@@ -479,6 +515,7 @@ TEST_F('ManageProfileUITest', 'ManagedDelete', function() {
   messages = clickAndListen();
   assertEquals(0, messages.length);
   assertEquals('settings', OptionsPage.getTopmostVisiblePage().name);
+  testDone();
 });
 
 GEN('#endif  // OS_CHROMEOS');