[iOS] Add WeakPtr support to FullscreenMediator.

The completion block added to FullscreenAnimator updates the
FullscreenModel and cleans up state in the FullscreenMediator.
FullscreenMediator calls StopAnimating() in its Disconnect() function,
which is guaranteed to be executed before deallocation.  This calls
into FullscreenAnimator's |-stopAnimation:| with a |withoutFinishing|
value of YES, which according to the UIViewAnimating protocol, should
immediately stop the animation.  However, UIViewPropertyAnimator's
implementation does not guarantee that the completion blocks are
called synchronously, so there is a possibility that the completion is
executed after the mediator's destruction.

The previous implementation of AnimateWithStyle() used a pointer to
the FullscreenModel to attempt to prevent crashes here.  However, this
is incorrect, as the raw pointer value is used. When the mediator is
disconnected, it resets the |model_| ivar to nullptr, so when the
pointer value is checked in the completion block, it was a use-after-
free error.  It only worked because the ivar was reset, but this
behavior was non-deterministic, as the memory can be reallocated
before the completion block is executed.

Instead, this CL updates FullscreenMediator to support weak pointers,
and uses a weak pointer from within the completion and animation blocks
to prevent use-after-free issues.  Additionally, this CL cleans up
FullscreenMediator::AnimateWithStyle() to early return before creating
the animator if there is no fullscreen progress update, preventing the
animation observer callbacks from being executed unnecessarily.

Bug: 1071777
Change-Id: I75aa1a54899d99befcdba140a2fefd846d785bc3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2155071
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Auto-Submit: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760576}

ios/chrome/browser/ui/fullscreen/fullscreen_animator.h

@@ -13,6 +13,9 @@ enum class FullscreenAnimatorStyle : short {
   EXIT_FULLSCREEN
 };
 
+// Returns the final fullscreen progress for an animation with |style|.
+CGFloat GetFinalFullscreenProgressForAnimation(FullscreenAnimatorStyle style);
+
 // Helper object for animating changes to fullscreen progress.  Subclasses of
 // this object are provided to FullscreenControllerObservers to coordinate
 // animations across several different ojects.

ios/chrome/browser/ui/fullscreen/fullscreen_animator.mm

@@ -16,6 +16,10 @@
 #error "This file requires ARC support."
 #endif
 
+CGFloat GetFinalFullscreenProgressForAnimation(FullscreenAnimatorStyle style) {
+  return style == FullscreenAnimatorStyle::ENTER_FULLSCREEN ? 0.0 : 1.0;
+}
+
 @interface FullscreenAnimator () {
   // The bezier backing the timing curve.
   std::unique_ptr<gfx::CubicBezier> _bezier;
@@ -44,8 +48,7 @@
     DCHECK_LE(startProgress, 1.0);
     _style = style;
     _startProgress = startProgress;
-    _finalProgress =
-        _style == FullscreenAnimatorStyle::ENTER_FULLSCREEN ? 0.0 : 1.0;
+    _finalProgress = GetFinalFullscreenProgressForAnimation(_style);
     _bezier = std::make_unique<gfx::CubicBezier>(
         timingParams.controlPoint1.x, timingParams.controlPoint1.y,
         timingParams.controlPoint2.x, timingParams.controlPoint2.y);

ios/chrome/browser/ui/fullscreen/fullscreen_mediator.h

@@ -9,6 +9,7 @@
 #include <memory>
 
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #import "ios/chrome/browser/ui/fullscreen/fullscreen_animator.h"
 #import "ios/chrome/browser/ui/fullscreen/fullscreen_model_observer.h"
@@ -92,6 +93,8 @@ class FullscreenMediator : public FullscreenModelObserver {
   // changes.
   base::ObserverList<FullscreenControllerObserver>::Unchecked observers_;
 
+  base::WeakPtrFactory<FullscreenMediator> weak_factory_{this};
+
   DISALLOW_COPY_AND_ASSIGN(FullscreenMediator);
 };
 

ios/chrome/browser/ui/fullscreen/fullscreen_mediator.mm

@@ -160,23 +160,29 @@ void FullscreenMediator::AnimateWithStyle(FullscreenAnimatorStyle style) {
   StopAnimating(true);
   DCHECK(!animator_);
 
+  // Early return if there is no progress change.
+  CGFloat start_progress = model_->progress();
+  CGFloat final_progress = GetFinalFullscreenProgressForAnimation(style);
+  if (AreCGFloatsEqual(start_progress, final_progress))
+    return;
+
   // Create the animator and set up its completion block.
-  animator_ =
-      [[FullscreenAnimator alloc] initWithStartProgress:model_->progress()
+  animator_ = [[FullscreenAnimator alloc] initWithStartProgress:start_progress
                                                           style:style];
-  __weak FullscreenAnimator* weakAnimator = animator_;
-  FullscreenModel** modelPtr = &model_;
+  base::WeakPtr<FullscreenMediator> weak_mediator = weak_factory_.GetWeakPtr();
   [animator_ addAnimations:^{
     // Updates the WebView frame during the animation to have it animated.
-    [resizer_ forceToUpdateToProgress:animator_.finalProgress];
+    FullscreenMediator* mediator = weak_mediator.get();
+    if (mediator)
+      [mediator->resizer_ forceToUpdateToProgress:final_progress];
   }];
   [animator_ addCompletion:^(UIViewAnimatingPosition finalPosition) {
     DCHECK_EQ(finalPosition, UIViewAnimatingPositionEnd);
-    if (!weakAnimator || !*modelPtr)
+    FullscreenMediator* mediator = weak_mediator.get();
+    if (!mediator)
       return;
-    model_->AnimationEndedWithProgress(
-        [weakAnimator progressForAnimatingPosition:finalPosition]);
-    animator_ = nil;
+    mediator->model_->AnimationEndedWithProgress(final_progress);
+    mediator->animator_ = nil;
   }];
 
   // Notify observers that the animation will occur.
@@ -184,10 +190,8 @@ void FullscreenMediator::AnimateWithStyle(FullscreenAnimatorStyle style) {
     observer.FullscreenWillAnimate(controller_, animator_);
   }
 
-  // Only start the animator if animations have been added and it has a non-zero
-  // progress change.
-  if (animator_.hasAnimations &&
-      !AreCGFloatsEqual(animator_.startProgress, animator_.finalProgress)) {
+  // Only start the animator if animations have been added.
+  if (animator_.hasAnimations) {
     [animator_ startAnimation];
   } else {
     animator_ = nil;