fix(ngMessages): prevent race condition with ngAnimate

If `ngMessage` tried to add a message back in that was about to be removed
after an animation, the NgMessageController got confused and tried to detach
the newly added message, when the pending node was destroyed.

This change applies a unique `attachId` to the message object and its DOM
node when it is attached. This is then checked when a DOM node is being
destroyed to prevent unwanted calls to `detach`.

Closes #12856
Closes #12903

Author: petebacondarwin

src/ngMessages/messages.js

@@ -325,6 +325,9 @@ angular.module('ngMessages', [])
        controller: ['$element', '$scope', '$attrs', function($element, $scope, $attrs) {
          var ctrl = this;
          var latestKey = 0;
+         var nextAttachId = 0;
+
+         this.getAttachId = function getAttachId() { return nextAttachId++; };
 
          var messages = this.messages = {};
          var renderLater, cachedCollection;
@@ -636,11 +639,15 @@ function ngMessageDirectiveFactory(restrict) {
                 $animate.enter(elm, null, element);
                 currentElement = elm;
 
+                // Each time we attach this node to a message we get a new id that we can match
+                // when we are destroying the node later.
+                var $$attachId = currentElement.$$attachId = ngMessagesCtrl.getAttachId();
+
                 // in the event that the parent element is destroyed
                 // by any other structural directive then it's time
                 // to deregister the message from the controller
                 currentElement.on('$destroy', function() {
-                  if (currentElement) {
+                  if (currentElement && currentElement.$$attachId === $$attachId) {
                     ngMessagesCtrl.deregister(commentNode);
                     messageCtrl.detach();
                   }

test/ngMessages/messagesSpec.js

@@ -372,6 +372,50 @@ describe('ngMessages', function() {
     expect(trim(element.text())).toEqual("Enter something");
   }));
 
+  // issue #12856
+  it('should only detach the message object that is associated with the message node being removed',
+    inject(function($rootScope, $compile, $animate) {
+
+    // We are going to spy on the `leave` method to give us control over
+    // when the element is actually removed
+    spyOn($animate, 'leave');
+
+    // Create a basic ng-messages set up
+    element = $compile('<div ng-messages="col">' +
+                       '  <div ng-message="primary">Enter something</div>' +
+                       '</div>')($rootScope);
+
+    // Trigger the message to be displayed
+    $rootScope.col = { primary: true };
+    $rootScope.$digest();
+    expect(messageChildren(element).length).toEqual(1);
+    var oldMessageNode = messageChildren(element)[0];
+
+    // Remove the message
+    $rootScope.col = { primary: undefined };
+    $rootScope.$digest();
+
+    // Since we have spied on the `leave` method, the message node is still in the DOM
+    expect($animate.leave).toHaveBeenCalledOnce();
+    var nodeToRemove = $animate.leave.mostRecentCall.args[0][0];
+    expect(nodeToRemove).toBe(oldMessageNode);
+    $animate.leave.reset();
+
+    // Add the message back in
+    $rootScope.col = { primary: true };
+    $rootScope.$digest();
+
+    // Simulate the animation completing on the node
+    jqLite(nodeToRemove).remove();
+
+    // We should not get another call to `leave`
+    expect($animate.leave).not.toHaveBeenCalled();
+
+    // There should only be the new message node
+    expect(messageChildren(element).length).toEqual(1);
+    var newMessageNode = messageChildren(element)[0];
+    expect(newMessageNode).not.toBe(oldMessageNode);
+  }));
 
   it('should render animations when the active/inactive classes are added/removed', function() {
     module('ngAnimate');