summaryrefslogtreecommitdiffstats
path: root/ui/views/animation
diff options
context:
space:
mode:
authordanakj@chromium.org <danakj@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-10-23 16:51:47 +0000
committerdanakj@chromium.org <danakj@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-10-23 16:51:47 +0000
commitd40305001bfd61bef7e118c8580895f68d36b163 (patch)
tree05a4a731233c5cb3deb56306019231e22d474deb /ui/views/animation
parentea75f30985b5b24627bed4c613a1ef403235aa02 (diff)
downloadchromium_src-d40305001bfd61bef7e118c8580895f68d36b163.zip
chromium_src-d40305001bfd61bef7e118c8580895f68d36b163.tar.gz
chromium_src-d40305001bfd61bef7e118c8580895f68d36b163.tar.bz2
Make gfx::Rect class operations consistently mutate the class they are called on.
Currently some methods mutate the class, and some return a new value, requiring API users to know what kind of method they are calling each time, and making inconsistent code. For example: gfx::Rect rect; rect.Inset(1, 1, 1, 1); rect = rect.Intersect(other_rect); rect.Offset(1, 1); Instead of: gfx::Rect rect; rect.Inset(1, 1, 1, 1); rect.Intersect(other_rect); rect.Offset(1, 1); We could go either way - making the class immutable or all methods return a new instance - but I believe it is better to instead make all methods mutate the class. This allows for shorter lines of code by avoiding having to repeat the object's name twice in order to change it. This patch changes gfx::Rect classes and all the callsites that uses these methods. It should make no change in behaviour, so no new tests added. R=sky BUG=147395 Review URL: https://codereview.chromium.org/11110004 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@163579 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'ui/views/animation')
-rw-r--r--ui/views/animation/bounds_animator.cc8
-rw-r--r--ui/views/animation/bounds_animator_unittest.cc5
2 files changed, 6 insertions, 7 deletions
diff --git a/ui/views/animation/bounds_animator.cc b/ui/views/animation/bounds_animator.cc
index 2c3fd05..037b9d6 100644
--- a/ui/views/animation/bounds_animator.cc
+++ b/ui/views/animation/bounds_animator.cc
@@ -235,14 +235,12 @@ void BoundsAnimator::AnimationProgressed(const Animation* animation) {
gfx::Rect new_bounds =
animation->CurrentValueBetween(data.start_bounds, data.target_bounds);
if (new_bounds != view->bounds()) {
- gfx::Rect total_bounds = new_bounds.Union(view->bounds());
+ gfx::Rect total_bounds = view->bounds();
+ total_bounds.Union(new_bounds);
// Build up the region to repaint in repaint_bounds_. We'll do the repaint
// when all animations complete (in AnimationContainerProgressed).
- if (repaint_bounds_.IsEmpty())
- repaint_bounds_ = total_bounds;
- else
- repaint_bounds_ = repaint_bounds_.Union(total_bounds);
+ repaint_bounds_.Union(total_bounds);
view->SetBoundsRect(new_bounds);
}
diff --git a/ui/views/animation/bounds_animator_unittest.cc b/ui/views/animation/bounds_animator_unittest.cc
index 93b13b5..2dbfc48 100644
--- a/ui/views/animation/bounds_animator_unittest.cc
+++ b/ui/views/animation/bounds_animator_unittest.cc
@@ -76,7 +76,7 @@ class TestView : public View {
if (dirty_rect_.IsEmpty())
dirty_rect_ = r;
else
- dirty_rect_ = dirty_rect_.Union(r);
+ dirty_rect_.Union(r);
}
const gfx::Rect& dirty_rect() const { return dirty_rect_; }
@@ -129,7 +129,8 @@ TEST_F(BoundsAnimatorTest, AnimateViewTo) {
// The parent should have been told to repaint as the animation progressed.
// The resulting rect is the union of the original and target bounds.
- EXPECT_EQ(target_bounds.Union(initial_bounds), parent()->dirty_rect());
+ target_bounds.Union(initial_bounds);
+ EXPECT_EQ(target_bounds, parent()->dirty_rect());
}
// Make sure an AnimationDelegate is deleted when canceled.