Fixing the hiding error bubble animation bug on second opening of create user dialog.

chrome/browser/resources/options/manage_profile_overlay.js

Index: chrome/browser/resources/options/manage_profile_overlay.js
diff --git a/chrome/browser/resources/options/manage_profile_overlay.js b/chrome/browser/resources/options/manage_profile_overlay.js
index 4e9ae665c28f6d89104fb7b4fb7d78c76e73bbbe..c881350c595f3f11e8f0556d4b81198485e92e0d 100644
--- a/chrome/browser/resources/options/manage_profile_overlay.js
+++ b/chrome/browser/resources/options/manage_profile_overlay.js
@@ -371,6 +371,11 @@ cr.define('options', function() {
      * @private
      */
     receiveExistingSupervisedUsers_: function(supervisedUsers) {
+      // Because the callback is called after the supervised user is created

[Marc Treib, 2016/01/07 14:42:48]

Hm, this kinda makes it sound like that's the only situation in which it is
called. Maybe something like:
During the creation of a SU, this gets called with a list including the
just-created SU. Ignore, to prevent the "already exists" bubble from showing up.

[atanasova, 2016/01/07 15:03:10]

Done.

+      // and before the overlay is closed, the error bubble is shown. That
+      // causes the hiding animation to be shown when you reopen the dialog.
+      if (CreateProfileOverlay.getCreateInProgress())
+        return;
       $('import-existing-supervised-user-link').hidden =
           supervisedUsers.length === 0;
       if (!$('create-profile-supervised').checked)
@@ -406,8 +411,10 @@ cr.define('options', function() {
                 HTMLEscape(elide(newName, /* maxLength */ 50)));
         this.showErrorBubble_(errorHtml, 'create', true);
 
-        $('supervised-user-import-existing').onclick =
-            this.getImportHandler_(supervisedUsers[i], nameIsUnique);
+        if ($('supervised-user-import-existing')) {

[Marc Treib, 2016/01/07 14:42:48]

Why is this necessary?

[atanasova, 2016/01/07 15:03:10]

If we display the error without the import option, this element does not exist
and it causes a null error.

[Marc Treib, 2016/01/07 16:11:23]

Ah, so it's a pre-existing problem? Okay then, carry on! :)

+          $('supervised-user-import-existing').onclick =
+              this.getImportHandler_(supervisedUsers[i], nameIsUnique);
+        }
         $('create-profile-ok').disabled = true;
         return;
       }
@@ -693,6 +700,15 @@ cr.define('options', function() {
     },
 
     /**
+     * Get createInProgress_. This is needed for the create user overlay.
+     * We do not want to show the errorBubble if a profile is being created.

[Marc Treib, 2016/01/07 14:42:48]

Please describe what the method does, not what it's used for (that is bound to
become outdated!)
Returns whether a profile creation is currently in progress. ?

[atanasova, 2016/01/07 15:03:10]

Done.

+     * @private
+     */
+    getCreateInProgress_: function() {
+      return this.createInProgress_;
+    },
+
+    /**
      * Updates the UI when a profile create step begins or ends.
      * Note that hideErrorBubble_() also enables the "OK" button, so it
      * must be called before this function if both are used.
@@ -759,7 +775,6 @@ cr.define('options', function() {
      * @private
      */
     onSuccess_: function(profileInfo) {
-      this.updateCreateInProgress_(false);

[Marc Treib, 2016/01/07 14:42:48]

Why remove this?

[atanasova, 2016/01/07 15:03:10]

In the description I tried to explain it, but I noticed a weird behavior. In
theory based on this method the overlay should have been closed before we show
the confirm overlay. However it is not closed and I can see it in the html and
the callback is still called. What happens is that we set the update in progress
to false, but the callback keeps getting called which causes the error to
appear. This variable is only used in two places - in the callback and when
checking whether the checkbox for creating SU is disabled. Since when opening
the dialog again this value will be reset, there should not be a problem with
not setting it at this point.

A thing I could not figure out is why the overlay is not actually closed.

[Marc Treib, 2016/01/07 16:11:23]

Hm. I guess the callback being called does not necessarily mean that the overlay
is still open? Is it actually still visible?

updateCreateInProgress_ does more than just set the variable. Are you sure
nothing of that is relevant here?
Also, I dislike that onError (above) still calls it but onSuccess now doesn't.
Can the call in onError be removed as well?

[atanasova, 2016/01/07 16:22:44]

It is needed in the onError method, since then we will not close the overlay and
need to be able to do changes to the name/checkbox/buttons.

The rest of the things that are being set in the method are all part of the
overlay (correct me if I am wrong on this point). So during the onSuccess part
we either open a createConfirmOverlay which does not care about those things or
simply open a new window with that user (non SU case). I _think_ both those
cases will not care about the update (since it simply makes all the things
enabled again).

       PageManager.closeOverlay();
       if (profileInfo.isSupervised) {
         options.SupervisedUserListData.resetPromise();
@@ -867,6 +882,7 @@ cr.define('options', function() {
     'onError',
     'onSuccess',
     'onWarning',
+    'getCreateInProgress',
     'updateCreateInProgress',
     'updateSignedInStatus',
     'updateSupervisedUsersAllowed',