Added a CallbackLayerAnimationObserver.

ui/compositor/layer_animation_observer.h

Index: ui/compositor/layer_animation_observer.h
diff --git a/ui/compositor/layer_animation_observer.h b/ui/compositor/layer_animation_observer.h
index 26bfdfaf0affe45c1578e535741cafe3e9f945e2..986631672654dd8a83b515ba4a835e9dda6c4cb6 100644
--- a/ui/compositor/layer_animation_observer.h
+++ b/ui/compositor/layer_animation_observer.h
@@ -15,6 +15,10 @@
 
 namespace ui {
 
+namespace test {
+class LayerAnimationObserverTestApi;
+}  // namespace test
+
 class LayerAnimationSequence;
 class ScopedLayerAnimationSettings;
 class ImplicitAnimationObserver;
@@ -46,9 +50,18 @@ class COMPOSITOR_EXPORT LayerAnimationObserver  {
 
   // If the animator is destroyed during an animation, the animations are
   // aborted. The resulting NotifyAborted notifications will NOT be sent to
-  // this observer if this function returns false. NOTE: IF YOU override THIS
-  // FUNCTION TO RETURN TRUE, YOU MUST REMEMBER TO REMOVE YOURSELF AS AN
-  // OBSERVER WHEN YOU ARE DESTROYED.
+  // this observer if this function returns false. An observer who wants to
+  // receive the NotifyAborted notifications during destruction can override
+  // this function to return true.
+  //
+  // *** IMPORTANT ***: If you override this function to return true AND you are
+  // using the pattern described below, then make sure you explicity remove

[danakj, 2015/10/12 21:33:56]

I would say "... and you are the owner of the observer ..." rather than
referring to later text.

[bruthig, 2015/10/13 19:41:42]

I've tried to re-word this. PTAL.

+  // yourself as an observer before the animator is destroyed!
+  //
+  // This opt-in pattern is used because it is a common pattern for a class to
+  // be the observer of an animation that it itself owns. In such a case the
+  // animator will be destroyed as the owning class is destructed and we don't
+  // want the NotifyAbort being invoked on a partially destroyed observer.
   virtual bool RequiresNotificationWhenAnimatorDestroyed() const;
 
   // Called when |this| is added to |sequence|'s observer list.
@@ -66,6 +79,7 @@ class COMPOSITOR_EXPORT LayerAnimationObserver  {
 
  private:
   friend class LayerAnimationSequence;
+  friend class test::LayerAnimationObserverTestApi;

[danakj, 2015/10/12 21:33:56]

What's the advantage to this instead of just making a LayerAnimationSequence in
the unittest?

[bruthig, 2015/10/13 19:41:42]

There are a few advantages to testing the CallbackLayerAnimationObserver in
isolation which we can discuss offline if you are interested but I will outline
the 2 most compelling ones here.

(1) The test author doesn't need to know the implementation details of the
LayerAnimationSequence just so they can manipulate the SUT as desired.  This is
also very helpful to the test reader because they won't have to sift through
relevant and irrelevant details where irrelevant details are the setup and
configuration of the test collaborator, aka LayerAnimationSequence.  A more
concrete example is to think about the code required to setup and manipulate a
LayerAnimationSequence just to get
LayerAnimationObserver::OnLayerAnimationStarted() called.  The test would have
to instantiate and manage a LayerAnimationSequence.  It would have to configure
the LayerAnimationSequence to have a non-empty |elements_| vector which would
require creating a LayerAnimationElement. And the test would have to poke the
LayerAnimationSequence::Start() method so that the
LayerAnimationObserver::OnLayerAnimationStarted() method is called.  That's a
lot of work and indirection that will more easily confuse test authors and
readers.

(2) By using the real LayerAnimationSequence in a test for the
LayerAnimationObserver you are making the tests more coupled and thus brittle.
i.e. if the implementation of the LayerAnimationSequence were to change, but not
it's interaction contract with the observer, then you may have to update/modify
the LayerAnimationObserver tests just so they would still work.  If the scope of
the test is high, and the details/code required to manipulate the SUT or
collaborators is high then the test becomes brittle and there starts to be a
high overhead to test maintenance when changing the collaborators.

 
   // Called when |this| is added to |sequence|'s observer list.
   void AttachedToSequence(LayerAnimationSequence* sequence);