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 | |
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')
-rw-r--r-- | ash/desktop_background/desktop_background_view.cc | 3 | ||||
-rw-r--r-- | ash/display/multi_display_manager.cc | 3 | ||||
-rw-r--r-- | ash/drag_drop/drag_drop_controller_unittest.cc | 2 | ||||
-rw-r--r-- | ash/launcher/launcher_view.cc | 2 | ||||
-rw-r--r-- | ash/launcher/launcher_view_unittest.cc | 10 | ||||
-rw-r--r-- | ash/system/drive/tray_drive.cc | 9 | ||||
-rw-r--r-- | ash/system/tray/tray_views.cc | 7 | ||||
-rw-r--r-- | ash/system/user/tray_user.cc | 6 | ||||
-rw-r--r-- | ash/tooltips/tooltip_controller.cc | 3 | ||||
-rw-r--r-- | ash/tooltips/tooltip_controller_unittest.cc | 2 | ||||
-rw-r--r-- | ash/wm/base_layout_manager.cc | 4 | ||||
-rw-r--r-- | ash/wm/partial_screenshot_view.cc | 4 | ||||
-rw-r--r-- | ash/wm/shelf_layout_manager.cc | 6 | ||||
-rw-r--r-- | ash/wm/system_modal_container_layout_manager.cc | 4 | ||||
-rw-r--r-- | ash/wm/window_util.cc | 3 | ||||
-rw-r--r-- | ash/wm/workspace/workspace_layout_manager2.cc | 4 | ||||
-rw-r--r-- | ash/wm/workspace/workspace_window_resizer.cc | 4 |
17 files changed, 48 insertions, 28 deletions
diff --git a/ash/desktop_background/desktop_background_view.cc b/ash/desktop_background/desktop_background_view.cc index aefe985..3b15075 100644 --- a/ash/desktop_background/desktop_background_view.cc +++ b/ash/desktop_background/desktop_background_view.cc @@ -134,7 +134,8 @@ void DesktopBackgroundView::OnPaint(gfx::Canvas* canvas) { RoundPositive(static_cast<double>(height()) / horizontal_ratio)); } - gfx::Rect wallpaper_cropped_rect = wallpaper_rect.Center(cropped_size); + gfx::Rect wallpaper_cropped_rect = wallpaper_rect; + wallpaper_cropped_rect.ClampToCenteredSize(cropped_size); canvas->DrawImageInt(wallpaper, wallpaper_cropped_rect.x(), wallpaper_cropped_rect.y(), wallpaper_cropped_rect.width(), wallpaper_cropped_rect.height(), diff --git a/ash/display/multi_display_manager.cc b/ash/display/multi_display_manager.cc index a1f26ba..7386115 100644 --- a/ash/display/multi_display_manager.cc +++ b/ash/display/multi_display_manager.cc @@ -346,7 +346,8 @@ const gfx::Display& MultiDisplayManager::GetDisplayMatching( for (std::vector<gfx::Display>::const_iterator iter = displays_.begin(); iter != displays_.end(); ++iter) { const gfx::Display& display = *iter; - gfx::Rect intersect = display.bounds().Intersect(rect); + gfx::Rect intersect = display.bounds(); + intersect.Intersect(rect); int area = intersect.width() * intersect.height(); if (area > max) { max = area; diff --git a/ash/drag_drop/drag_drop_controller_unittest.cc b/ash/drag_drop/drag_drop_controller_unittest.cc index b7c94e8..bbc3975 100644 --- a/ash/drag_drop/drag_drop_controller_unittest.cc +++ b/ash/drag_drop/drag_drop_controller_unittest.cc @@ -207,7 +207,7 @@ void AddViewToWidgetAndResize(views::Widget* widget, views::View* view) { contents_view->AddChildView(view); view->SetBounds(contents_view->width(), 0, 100, 100); gfx::Rect contents_view_bounds = contents_view->bounds(); - contents_view_bounds = contents_view_bounds.Union(view->bounds()); + contents_view_bounds.Union(view->bounds()); contents_view->SetBoundsRect(contents_view_bounds); widget->SetBounds(contents_view_bounds); } 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))); diff --git a/ash/system/drive/tray_drive.cc b/ash/system/drive/tray_drive.cc index f37ad80..489427e 100644 --- a/ash/system/drive/tray_drive.cc +++ b/ash/system/drive/tray_drive.cc @@ -220,7 +220,8 @@ class DriveDetailedView : public TrayDetailsView, kBottomPadding - status_img_->GetPreferredSize().height())/2), status_img_->GetPreferredSize()); - status_img_->SetBoundsRect(bounds_status.Intersect(child_area)); + bounds_status.Intersect(child_area); + status_img_->SetBoundsRect(bounds_status); pos_x += status_img_->bounds().width() + kHorizontalPadding; gfx::Rect bounds_label(pos_x, @@ -230,7 +231,8 @@ class DriveDetailedView : public TrayDetailsView, status_img_->GetPreferredSize().width() - cancel_button_->GetPreferredSize().width(), label_container_->GetPreferredSize().height()); - label_container_->SetBoundsRect(bounds_label.Intersect(child_area)); + bounds_label.Intersect(child_area); + label_container_->SetBoundsRect(bounds_label); pos_x += label_container_->bounds().width() + kHorizontalPadding; gfx::Rect bounds_button( @@ -239,7 +241,8 @@ class DriveDetailedView : public TrayDetailsView, kBottomPadding - cancel_button_->GetPreferredSize().height())/2), cancel_button_->GetPreferredSize()); - cancel_button_->SetBoundsRect(bounds_button.Intersect(child_area)); + bounds_button.Intersect(child_area); + cancel_button_->SetBoundsRect(bounds_button); } // views::ButtonListener overrides. diff --git a/ash/system/tray/tray_views.cc b/ash/system/tray/tray_views.cc index ddc2b8c..3b25b38 100644 --- a/ash/system/tray/tray_views.cc +++ b/ash/system/tray/tray_views.cc @@ -622,9 +622,10 @@ void SpecialPopupRow::Layout() { gfx::Rect bounds(button_container_->GetPreferredSize()); bounds.set_height(content_bounds.height()); - bounds = content_bounds.Center(bounds.size()); - bounds.set_x(content_bounds.width() - bounds.width()); - button_container_->SetBoundsRect(bounds); + gfx::Rect container_bounds = content_bounds; + container_bounds.ClampToCenteredSize(bounds.size()); + container_bounds.set_x(content_bounds.width() - bounds.width()); + button_container_->SetBoundsRect(container_bounds); bounds = content_->bounds(); bounds.set_width(button_container_->x()); diff --git a/ash/system/user/tray_user.cc b/ash/system/user/tray_user.cc index 7b90d87..262ae6f 100644 --- a/ash/system/user/tray_user.cc +++ b/ash/system/user/tray_user.cc @@ -76,7 +76,8 @@ class RoundedImageView : public views::View { virtual void OnPaint(gfx::Canvas* canvas) OVERRIDE { View::OnPaint(canvas); gfx::Rect image_bounds(GetPreferredSize()); - image_bounds = gfx::Rect(size()).Center(image_bounds.size()); + image_bounds = gfx::Rect(size()); + image_bounds.ClampToCenteredSize(image_bounds.size()); image_bounds.Inset(GetInsets()); const SkScalar kRadius = SkIntToScalar(corner_radius_); SkPath path; @@ -220,7 +221,8 @@ class UserView : public views::View, container_->SetBoundsRect(gfx::Rect(size())); if (signout_ && user_info_) { gfx::Rect signout_bounds(signout_->GetPreferredSize()); - signout_bounds = bounds().Center(signout_bounds.size()); + signout_bounds = bounds(); + signout_bounds.ClampToCenteredSize(signout_bounds.size()); signout_bounds.set_x(width() - signout_bounds.width() - kTrayPopupPaddingHorizontal); signout_->SetBoundsRect(signout_bounds); diff --git a/ash/tooltips/tooltip_controller.cc b/ash/tooltips/tooltip_controller.cc index 3bb99bc..45c4785 100644 --- a/ash/tooltips/tooltip_controller.cc +++ b/ash/tooltips/tooltip_controller.cc @@ -179,7 +179,8 @@ class TooltipController::Tooltip : public views::WidgetObserver { if (tooltip_rect.bottom() > display_bounds.bottom()) tooltip_rect.set_y(mouse_pos.y() - tooltip_height); - GetWidget()->SetBounds(tooltip_rect.AdjustToFit(display_bounds)); + tooltip_rect.AdjustToFit(display_bounds); + GetWidget()->SetBounds(tooltip_rect); } views::Widget* GetWidget() { diff --git a/ash/tooltips/tooltip_controller_unittest.cc b/ash/tooltips/tooltip_controller_unittest.cc index d48db06..9efbd75 100644 --- a/ash/tooltips/tooltip_controller_unittest.cc +++ b/ash/tooltips/tooltip_controller_unittest.cc @@ -72,7 +72,7 @@ void AddViewToWidgetAndResize(views::Widget* widget, views::View* view) { contents_view->AddChildView(view); view->SetBounds(contents_view->width(), 0, 100, 100); gfx::Rect contents_view_bounds = contents_view->bounds(); - contents_view_bounds = contents_view_bounds.Union(view->bounds()); + contents_view_bounds.Union(view->bounds()); contents_view->SetBoundsRect(contents_view_bounds); widget->SetBounds(gfx::Rect(widget->GetWindowBoundsInScreen().origin(), contents_view_bounds.size())); diff --git a/ash/wm/base_layout_manager.cc b/ash/wm/base_layout_manager.cc index 1bf6434a..78019cd 100644 --- a/ash/wm/base_layout_manager.cc +++ b/ash/wm/base_layout_manager.cc @@ -224,7 +224,9 @@ void BaseLayoutManager::AdjustWindowSizesForScreenChange() { gfx::Rect display_rect = ScreenAsh::GetDisplayWorkAreaBoundsInParent(window); // Put as much of the window as possible within the display area. - window->SetBounds(window->bounds().AdjustToFit(display_rect)); + gfx::Rect bounds = window->bounds(); + bounds.AdjustToFit(display_rect); + window->SetBounds(bounds); } } } diff --git a/ash/wm/partial_screenshot_view.cc b/ash/wm/partial_screenshot_view.cc index 2c3e09d..dd0bd1da 100644 --- a/ash/wm/partial_screenshot_view.cc +++ b/ash/wm/partial_screenshot_view.cc @@ -121,8 +121,10 @@ void PartialScreenshotView::OnMouseReleased(const ui::MouseEvent& event) { is_dragging_ = false; if (screenshot_delegate_) { aura::RootWindow *root_window = Shell::GetPrimaryRootWindow(); + gfx::Rect root_window_screenshot_rect = root_window->bounds(); + root_window_screenshot_rect.Intersect(GetScreenshotRect()); screenshot_delegate_->HandleTakePartialScreenshot( - root_window, root_window->bounds().Intersect(GetScreenshotRect())); + root_window, root_window_screenshot_rect); } } diff --git a/ash/wm/shelf_layout_manager.cc b/ash/wm/shelf_layout_manager.cc index 8aff1ae..307c3ba 100644 --- a/ash/wm/shelf_layout_manager.cc +++ b/ash/wm/shelf_layout_manager.cc @@ -746,7 +746,8 @@ void ShelfLayoutManager::UpdateTargetBoundsForGesture( target_bounds->launcher_bounds_in_root.height() + move - translate); // The statusbar should be in the center. - gfx::Rect status_y = target_bounds->launcher_bounds_in_root.Center( + gfx::Rect status_y = target_bounds->launcher_bounds_in_root; + status_y.ClampToCenteredSize( target_bounds->status_bounds_in_root.size()); target_bounds->status_bounds_in_root.set_y(status_y.y()); } @@ -774,7 +775,8 @@ void ShelfLayoutManager::UpdateTargetBoundsForGesture( } // The statusbar should be in the center. - gfx::Rect status_x = target_bounds->launcher_bounds_in_root.Center( + gfx::Rect status_x = target_bounds->launcher_bounds_in_root; + status_x.ClampToCenteredSize( target_bounds->status_bounds_in_root.size()); target_bounds->status_bounds_in_root.set_x(status_x.x()); } diff --git a/ash/wm/system_modal_container_layout_manager.cc b/ash/wm/system_modal_container_layout_manager.cc index 2bfc73a..22dadc8 100644 --- a/ash/wm/system_modal_container_layout_manager.cc +++ b/ash/wm/system_modal_container_layout_manager.cc @@ -79,7 +79,9 @@ void SystemModalContainerLayoutManager::OnWindowResized() { if (!modal_windows_.empty()) { aura::Window::Windows::iterator it = modal_windows_.begin(); for (it = modal_windows_.begin(); it != modal_windows_.end(); ++it) { - (*it)->SetBounds((*it)->bounds().AdjustToFit(container_->bounds())); + gfx::Rect bounds = (*it)->bounds(); + bounds.AdjustToFit(container_->bounds()); + (*it)->SetBounds(bounds); } } } diff --git a/ash/wm/window_util.cc b/ash/wm/window_util.cc index 8b9dcb0..55513af 100644 --- a/ash/wm/window_util.cc +++ b/ash/wm/window_util.cc @@ -110,7 +110,8 @@ void ToggleMaximizedWindow(aura::Window* window) { void CenterWindow(aura::Window* window) { const gfx::Display display = Shell::GetScreen()->GetDisplayNearestWindow(window); - gfx::Rect center = display.work_area().Center(window->bounds().size()); + gfx::Rect center = display.work_area(); + center.ClampToCenteredSize(window->bounds().size()); window->SetBounds(center); } diff --git a/ash/wm/workspace/workspace_layout_manager2.cc b/ash/wm/workspace/workspace_layout_manager2.cc index 6731557..29e4c6c 100644 --- a/ash/wm/workspace/workspace_layout_manager2.cc +++ b/ash/wm/workspace/workspace_layout_manager2.cc @@ -289,7 +289,9 @@ void WorkspaceLayoutManager2::AdjustWindowSizeForScreenChange( if (reason == ADJUST_WINDOW_SCREEN_SIZE_CHANGED) { // The work area may be smaller than the full screen. Put as much of the // window as possible within the display area. - window->SetBounds(window->bounds().AdjustToFit(work_area_)); + gfx::Rect bounds = window->bounds(); + bounds.AdjustToFit(work_area_); + window->SetBounds(bounds); } else if (reason == ADJUST_WINDOW_DISPLAY_INSETS_CHANGED) { // If the window is completely outside the display work area, then move it // enough to be visible again. diff --git a/ash/wm/workspace/workspace_window_resizer.cc b/ash/wm/workspace/workspace_window_resizer.cc index e16ba23..d3cc9b7 100644 --- a/ash/wm/workspace/workspace_window_resizer.cc +++ b/ash/wm/workspace/workspace_window_resizer.cc @@ -688,8 +688,8 @@ void WorkspaceWindowResizer::UpdateDragPhantomWindow(const gfx::Rect& bounds, const gfx::Rect root_bounds_in_screen(another_root->GetBoundsInScreen()); const gfx::Rect bounds_in_screen = ScreenAsh::ConvertRectToScreen(window()->parent(), bounds); - const gfx::Rect bounds_in_another_root = - root_bounds_in_screen.Intersect(bounds_in_screen); + gfx::Rect bounds_in_another_root = root_bounds_in_screen; + bounds_in_another_root.Intersect(bounds_in_screen); const float fraction_in_another_window = (bounds_in_another_root.width() * bounds_in_another_root.height()) / |