diff options
| author | danakj@chromium.org <danakj@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-10-23 16:51:47 +0000 |
|---|---|---|
| committer | danakj@chromium.org <danakj@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-10-23 16:51:47 +0000 |
| commit | d40305001bfd61bef7e118c8580895f68d36b163 (patch) | |
| tree | 05a4a731233c5cb3deb56306019231e22d474deb /ash/launcher | |
| parent | ea75f30985b5b24627bed4c613a1ef403235aa02 (diff) | |
| download | chromium_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 'ash/launcher')
| -rw-r--r-- | ash/launcher/launcher_view.cc | 2 | ||||
| -rw-r--r-- | ash/launcher/launcher_view_unittest.cc | 10 |
2 files changed, 6 insertions, 6 deletions
diff --git a/ash/launcher/launcher_view.cc b/ash/launcher/launcher_view.cc index a3c2fe7..c5044ad 100644 --- a/ash/launcher/launcher_view.cc +++ b/ash/launcher/launcher_view.cc @@ -705,7 +705,7 @@ bool LauncherView::ShouldHideTooltip(const gfx::Point& cursor_location) { continue; gfx::Rect child_bounds = child->GetMirroredBounds(); - active_bounds = active_bounds.Union(child_bounds); + active_bounds.Union(child_bounds); } return !active_bounds.Contains(cursor_location); diff --git a/ash/launcher/launcher_view_unittest.cc b/ash/launcher/launcher_view_unittest.cc index fdb062f..3e1e806 100644 --- a/ash/launcher/launcher_view_unittest.cc +++ b/ash/launcher/launcher_view_unittest.cc @@ -645,8 +645,9 @@ TEST_F(LauncherViewTest, ShouldHideTooltipTest) { gfx::Rect app_button_rect = GetButtonByID(app_button_id)->GetMirroredBounds(); gfx::Rect tab_button_rect = GetButtonByID(tab_button_id)->GetMirroredBounds(); ASSERT_FALSE(app_button_rect.Intersects(tab_button_rect)); - EXPECT_FALSE(launcher_view_->ShouldHideTooltip( - app_button_rect.Union(tab_button_rect).CenterPoint())); + gfx::Rect union_rect = app_button_rect; + union_rect.Union(tab_button_rect); + EXPECT_FALSE(launcher_view_->ShouldHideTooltip(union_rect.CenterPoint())); // The tooltip should hide if it's outside of all buttons. gfx::Rect all_area; @@ -655,10 +656,9 @@ TEST_F(LauncherViewTest, ShouldHideTooltipTest) { if (!button) continue; - all_area = all_area.Union(button->GetMirroredBounds()); + all_area.Union(button->GetMirroredBounds()); } - all_area = all_area.Union( - launcher_view_->GetAppListButtonView()->GetMirroredBounds()); + all_area.Union(launcher_view_->GetAppListButtonView()->GetMirroredBounds()); EXPECT_FALSE(launcher_view_->ShouldHideTooltip(all_area.origin())); EXPECT_FALSE(launcher_view_->ShouldHideTooltip( gfx::Point(all_area.right() - 1, all_area.bottom() - 1))); |