cc: Remove ScopedPtrVector and cc::remove_if.

cc/animation/layer_animation_controller.cc

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/animation/layer_animation_controller.h"
 
 #include <algorithm>
 #include <vector>
 
 #include "cc/animation/animation.h"
 #include "cc/animation/animation_delegate.h"
 #include "cc/animation/animation_registrar.h"
 #include "cc/animation/keyframed_animation_curve.h"
 #include "cc/animation/layer_animation_value_observer.h"
 #include "cc/animation/layer_animation_value_provider.h"
 #include "cc/animation/scroll_offset_animation_curve.h"
-#include "cc/base/scoped_ptr_algorithm.h"
 #include "cc/output/filter_operations.h"
 #include "ui/gfx/geometry/box_f.h"
 #include "ui/gfx/transform.h"
 
 namespace cc {
 
 LayerAnimationController::LayerAnimationController(int id)
     : registrar_(0),
       id_(id),
       is_active_(false),
@@ skipping 18 matching lines @@
                                               base::TimeDelta time_offset) {
   for (size_t i = 0; i < animations_.size(); ++i) {
     if (animations_[i]->id() == animation_id) {
       animations_[i]->SetRunState(Animation::PAUSED,
                                   time_offset + animations_[i]->start_time() +
                                       animations_[i]->time_offset());
     }
   }
 }
 
-struct HasAnimationId {
-  explicit HasAnimationId(int id) : id_(id) {}
-  bool operator()(Animation* animation) const {
-    return animation->id() == id_;
-  }
-
- private:
-  int id_;
-};
-
 void LayerAnimationController::UpdatePotentiallyAnimatingTransform() {
   bool was_potentially_animating_transform_for_active_observers =
       potentially_animating_transform_for_active_observers_;
   bool was_potentially_animating_transform_for_pending_observers =
       potentially_animating_transform_for_pending_observers_;
 
   potentially_animating_transform_for_active_observers_ = false;
   potentially_animating_transform_for_pending_observers_ = false;
 
-  for (Animation* animation : animations_) {
+  for (const scoped_ptr<Animation>& animation : animations_) {

[danakj, 2015/11/17 01:12:16]

I'd like this to be an auto& and I wrote as much to chromium-dev. We can see
what people say I guess.

[vmpstr, 2015/11/17 23:26:23]

Done.

     if (!animation->is_finished() &&
         animation->target_property() == Animation::TRANSFORM) {
       potentially_animating_transform_for_active_observers_ |=
           animation->affects_active_observers();
       potentially_animating_transform_for_pending_observers_ |=
           animation->affects_pending_observers();
     }
   }
 
   bool changed_for_active_observers =
       was_potentially_animating_transform_for_active_observers !=
       potentially_animating_transform_for_active_observers_;
   bool changed_for_pending_observers =
       was_potentially_animating_transform_for_pending_observers !=
       potentially_animating_transform_for_pending_observers_;
 
   if (!changed_for_active_observers && !changed_for_pending_observers)
     return;
 
   NotifyObserversTransformIsPotentiallyAnimatingChanged(
       changed_for_active_observers, changed_for_pending_observers);
 }
 
 void LayerAnimationController::RemoveAnimation(int animation_id) {
   bool removed_transform_animation = false;
-  auto animations_to_remove =
-      animations_.remove_if(HasAnimationId(animation_id));
+  auto animations_to_remove = std::stable_partition(

[danakj, 2015/11/17 01:12:16]

Can you leave a comment saying you used this instead of remove_if cuz we want to
use them before removing

[vmpstr, 2015/11/17 23:26:23]

Done.

+      animations_.begin(), animations_.end(),
+      [animation_id](const scoped_ptr<Animation>& animation) {
+        return animation->id() != animation_id;
+      });
   for (auto it = animations_to_remove; it != animations_.end(); ++it) {
     if ((*it)->target_property() == Animation::SCROLL_OFFSET) {
       scroll_offset_animation_was_interrupted_ = true;
     } else if ((*it)->target_property() == Animation::TRANSFORM &&
                !(*it)->is_finished()) {
       removed_transform_animation = true;
     }
   }
 
   animations_.erase(animations_to_remove, animations_.end());
   UpdateActivation(NORMAL_ACTIVATION);
   if (removed_transform_animation)
     UpdatePotentiallyAnimatingTransform();
 }
 
-struct HasAnimationIdAndProperty {
-  HasAnimationIdAndProperty(int id, Animation::TargetProperty target_property)
-      : id_(id), target_property_(target_property) {}
-  bool operator()(Animation* animation) const {
-    return animation->id() == id_ &&
-        animation->target_property() == target_property_;
-  }
-
- private:
-  int id_;
-  Animation::TargetProperty target_property_;
-};
-
 void LayerAnimationController::RemoveAnimation(
     int animation_id,
     Animation::TargetProperty target_property) {
   bool removed_transform_animation = false;
-  auto animations_to_remove = animations_.remove_if(
-      HasAnimationIdAndProperty(animation_id, target_property));
+  auto animations_to_remove = std::stable_partition(

[danakj, 2015/11/17 01:12:16]

ditto comment

[vmpstr, 2015/11/17 23:26:23]

Done.

+      animations_.begin(), animations_.end(),
+      [animation_id, target_property](const scoped_ptr<Animation>& animation) {

[danakj, 2015/11/17 01:12:16]

in general i find it easier to read these types of things if you store the
lambda in a var (has_animation_id_and_property?). WDYT? this one isn't atrocious
but a general feeling.

[vmpstr, 2015/11/17 23:26:23]

Yeah I think that's very reasonable. Let me know of any other place you want
this changed.

+        return animation->id() != animation_id ||
+               animation->target_property() != target_property;
+      });
   if (animations_to_remove == animations_.end())
     return;
 
   if (target_property == Animation::SCROLL_OFFSET)
     scroll_offset_animation_was_interrupted_ = true;
   else if (target_property == Animation::TRANSFORM &&
            !(*animations_to_remove)->is_finished())

[vmpstr, 2015/11/17 23:26:23]

This _looks_ like there can only be one of such animations. If that's the case,
this should be a find_if/erase. However, I don't know this code that much...
+ajuma for a second opinion.

     removed_transform_animation = true;
 
   animations_.erase(animations_to_remove, animations_.end());
   UpdateActivation(NORMAL_ACTIVATION);
   if (removed_transform_animation)
     UpdatePotentiallyAnimatingTransform();
 }
 
 void LayerAnimationController::AbortAnimations(
     Animation::TargetProperty target_property) {
@@ skipping 41 matching lines @@
   last_tick_time_ = monotonic_time;
 }
 
 void LayerAnimationController::AccumulatePropertyUpdates(
     base::TimeTicks monotonic_time,
     AnimationEventsVector* events) {
   if (!events)
     return;
 
   for (size_t i = 0; i < animations_.size(); ++i) {
-    Animation* animation = animations_[i];
+    Animation* animation = animations_[i].get();
     if (!animation->is_impl_only())
       continue;
 
     if (!animation->InEffect(monotonic_time))
       continue;
 
     base::TimeDelta trimmed =
         animation->TrimTimeToCurrentIteration(monotonic_time);
     switch (animation->target_property()) {
       case Animation::OPACITY: {
@@ skipping 65 matching lines @@
   if (needs_to_start_animations_ && start_ready_animations) {
     StartAnimations(last_tick_time_);
     PromoteStartedAnimations(last_tick_time_, events);
   }
 
   AccumulatePropertyUpdates(last_tick_time_, events);
 
   UpdateActivation(NORMAL_ACTIVATION);
 }
 
-struct AffectsNoObservers {
-  bool operator()(Animation* animation) const {
-    return !animation->affects_active_observers() &&
-           !animation->affects_pending_observers();
-  }
-};
-
 void LayerAnimationController::ActivateAnimations() {
   bool changed_transform_animation = false;
   for (size_t i = 0; i < animations_.size(); ++i) {
     if (animations_[i]->affects_active_observers() !=
             animations_[i]->affects_pending_observers() &&
         animations_[i]->target_property() == Animation::TRANSFORM)
       changed_transform_animation = true;
     animations_[i]->set_affects_active_observers(
         animations_[i]->affects_pending_observers());
   }
-  animations_.erase(cc::remove_if(&animations_,
-                                  animations_.begin(),
-                                  animations_.end(),
-                                  AffectsNoObservers()),
-                    animations_.end());
+  animations_.erase(
+      std::remove_if(animations_.begin(), animations_.end(),
+                     [](const scoped_ptr<Animation>& animation) {
+                       return !animation->affects_active_observers() &&

[danakj, 2015/11/17 01:12:16]

kinda like having a lambda here too, similar to combining multiple bool
conditions into a single bool helper than explains them all

[vmpstr, 2015/11/17 23:26:23]

Done.

+                              !animation->affects_pending_observers();
+                     }),
+      animations_.end());
   scroll_offset_animation_was_interrupted_ = false;
   UpdateActivation(NORMAL_ACTIVATION);
   if (changed_transform_animation)
     UpdatePotentiallyAnimatingTransform();
 }
 
 void LayerAnimationController::AddAnimation(scoped_ptr<Animation> animation) {
   bool added_transform_animation =
       animation->target_property() == Animation::TRANSFORM;
   animations_.push_back(animation.Pass());
   needs_to_start_animations_ = true;
   UpdateActivation(NORMAL_ACTIVATION);
   if (added_transform_animation)
     UpdatePotentiallyAnimatingTransform();
 }
 
 Animation* LayerAnimationController::GetAnimation(
     Animation::TargetProperty target_property) const {
   for (size_t i = 0; i < animations_.size(); ++i) {
     size_t index = animations_.size() - i - 1;
     if (animations_[index]->target_property() == target_property)
-      return animations_[index];
+      return animations_[index].get();
   }
-  return 0;
+  return nullptr;
 }
 
 Animation* LayerAnimationController::GetAnimationById(int animation_id) const {
   for (size_t i = 0; i < animations_.size(); ++i)
     if (animations_[i]->id() == animation_id)
-      return animations_[i];
+      return animations_[i].get();
   return nullptr;
 }
 
 bool LayerAnimationController::HasActiveAnimation() const {
   for (size_t i = 0; i < animations_.size(); ++i) {
     if (!animations_[i]->is_finished())
       return true;
   }
   return false;
 }
@@ skipping 377 matching lines @@
 static bool IsCompleted(
     Animation* animation,
     const LayerAnimationController* main_thread_controller) {
   if (animation->is_impl_only()) {
     return (animation->run_state() == Animation::WAITING_FOR_DELETION);
   } else {
     return !main_thread_controller->GetAnimationById(animation->id());
   }
 }
 
-static bool AffectsActiveOnlyAndIsWaitingForDeletion(Animation* animation) {
-  return animation->run_state() == Animation::WAITING_FOR_DELETION &&
-         !animation->affects_pending_observers();
-}
-
 void LayerAnimationController::RemoveAnimationsCompletedOnMainThread(
     LayerAnimationController* controller_impl) const {
   bool removed_transform_animation = false;
   // Animations removed on the main thread should no longer affect pending
   // observers, and should stop affecting active observers after the next call
   // to ActivateAnimations. If already WAITING_FOR_DELETION, they can be removed
   // immediately.
-  ScopedPtrVector<Animation>& animations = controller_impl->animations_;
+  std::vector<scoped_ptr<Animation>>& animations = controller_impl->animations_;

[danakj, 2015/11/17 01:12:16]

that's a lot of templatefu. consider auto&? do we want to fix the type here?

[vmpstr, 2015/11/17 23:26:23]

Done.

   for (size_t i = 0; i < animations.size(); ++i) {
-    if (IsCompleted(animations[i], this)) {
+    if (IsCompleted(animations[i].get(), this)) {
       animations[i]->set_affects_pending_observers(false);
       if (animations[i]->target_property() == Animation::TRANSFORM)
         removed_transform_animation = true;
     }
   }
-  animations.erase(cc::remove_if(&animations,
-                                 animations.begin(),
-                                 animations.end(),
-                                 AffectsActiveOnlyAndIsWaitingForDeletion),
-                   animations.end());
+  animations.erase(std::remove_if(
+      animations.begin(), animations.end(),
+      [](const scoped_ptr<Animation>& animation) {
+        return animation->run_state() == Animation::WAITING_FOR_DELETION &&
+               !animation->affects_pending_observers();
+      }));

[ericrk, 2015/11/16 19:20:52]

missing animations.end()?

[vmpstr, 2015/11/17 23:26:23]

Done.

 
   if (removed_transform_animation)
     controller_impl->UpdatePotentiallyAnimatingTransform();
 }
 
 void LayerAnimationController::PushPropertiesToImplThread(
     LayerAnimationController* controller_impl) {
   for (size_t i = 0; i < animations_.size(); ++i) {
     Animation* current_impl =
         controller_impl->GetAnimationById(animations_[i]->id());
@@ skipping 28 matching lines @@
     } else if (animations_[i]->run_state() ==
                Animation::WAITING_FOR_TARGET_AVAILABILITY) {
       animations_waiting_for_target.push_back(i);
     }
   }
 
   for (size_t i = 0; i < animations_waiting_for_target.size(); ++i) {
       // Collect all properties for animations with the same group id (they
       // should all also be in the list of animations).
     size_t animation_index = animations_waiting_for_target[i];
-    Animation* animation_waiting_for_target = animations_[animation_index];
+    Animation* animation_waiting_for_target =
+        animations_[animation_index].get();
     // Check for the run state again even though the animation was waiting
     // for target because it might have changed the run state while handling
     // previous animation in this loop (if they belong to same group).
     if (animation_waiting_for_target->run_state() ==
         Animation::WAITING_FOR_TARGET_AVAILABILITY) {
       TargetProperties enqueued_properties;
       bool affects_active_observers =
           animation_waiting_for_target->affects_active_observers();
       bool affects_pending_observers =
           animation_waiting_for_target->affects_pending_observers();
@@ skipping 178 matching lines @@
           animations_[animation_index]->SetRunState(
               Animation::WAITING_FOR_DELETION, monotonic_time);
       }
       marked_animations_for_deletions = true;
     }
   }
   if (marked_animations_for_deletions)
     NotifyObserversAnimationWaitingForDeletion();
 }
 
-static bool IsWaitingForDeletion(Animation* animation) {
-  return animation->run_state() == Animation::WAITING_FOR_DELETION;
-}
-
 void LayerAnimationController::PurgeAnimationsMarkedForDeletion() {
-  animations_.erase(cc::remove_if(&animations_,
-                                  animations_.begin(),
-                                  animations_.end(),
-                                  IsWaitingForDeletion),
+  animations_.erase(std::remove_if(animations_.begin(), animations_.end(),
+                                   [](const scoped_ptr<Animation>& animation) {
+                                     return animation->run_state() ==
+                                            Animation::WAITING_FOR_DELETION;
+                                   }),
                     animations_.end());
 }
 
 void LayerAnimationController::TickAnimations(base::TimeTicks monotonic_time) {
   for (size_t i = 0; i < animations_.size(); ++i) {
     if (animations_[i]->run_state() == Animation::STARTING ||
         animations_[i]->run_state() == Animation::RUNNING ||
         animations_[i]->run_state() == Animation::PAUSED) {
       if (!animations_[i]->InEffect(monotonic_time))
         continue;
@@ skipping 186 matching lines @@
         &value_observers_);
     LayerAnimationValueObserver* obs;
     while ((obs = it.GetNext()) != nullptr)
       if (obs->IsActive())
         return true;
   }
   return false;
 }
 
 }  // namespace cc