Clean up of tile sizing code for the Apps Grid View.

ui/app_list/views/apps_grid_view.cc

Index: ui/app_list/views/apps_grid_view.cc
diff --git a/ui/app_list/views/apps_grid_view.cc b/ui/app_list/views/apps_grid_view.cc
index ec8f29a0a2ca2bcd7e08df7bb004704ed7fefee9..20eea62341d4079e28e903bbeaf9ba2331a5128e 100644
--- a/ui/app_list/views/apps_grid_view.cc
+++ b/ui/app_list/views/apps_grid_view.cc
@@ -878,15 +878,12 @@ bool AppsGridView::IsAnimatingView(views::View* view) {
 
 gfx::Size AppsGridView::GetPreferredSize() const {
   const gfx::Insets insets(GetInsets());
-  const gfx::Size tile_size = gfx::Size(kPreferredTileWidth,
-                                        kPreferredTileHeight);
   int page_switcher_height = kBottomPadding;
   if (page_switcher_view_)
     page_switcher_height = page_switcher_view_->GetPreferredSize().height();
-  return gfx::Size(
-      tile_size.width() * cols_ + insets.width(),
-      tile_size.height() * rows_per_page_ +
-          page_switcher_height + insets.height());
+  gfx::Size size = GetTileGridSize();
+  size.Enlarge(insets.width(), insets.height() + page_switcher_height);
+  return size;
 }
 
 bool AppsGridView::GetDropFormats(
@@ -1166,16 +1163,12 @@ void AppsGridView::CalculateIdealBounds() {
   if (rect.IsEmpty())
     return;
 
-  gfx::Size tile_size(kPreferredTileWidth, kPreferredTileHeight);
-
-  gfx::Rect grid_rect(gfx::Size(tile_size.width() * cols_,
-                                tile_size.height() * rows_per_page_));
-  grid_rect.Intersect(rect);
+  gfx::Size grid_size = GetTileGridSize();

[Matt Giuca, 2014/09/10 05:35:06]

This is actually fixing a subtle bug: calling grid_rect.Intersect(rect) is bogus
because it intersects rects in two different spaces. As a result, page_width and
page_height will be too small.

I think this should be fixed first in a separate CL before this refactor.

[calamity, 2014/09/11 06:53:34]

Amended CL description as discussed.

[Matt Giuca, 2014/09/12 01:04:46]

Acknowledged.

 
   // Page size including padding pixels. A tile.x + page_width means the same
   // tile slot in the next page; similarly for tile.y + page_height.
-  const int page_width = grid_rect.width() + kPagePadding;
-  const int page_height = grid_rect.height() + kPagePadding;
+  const int page_width = grid_size.width() + kPagePadding;
+  const int page_height = grid_size.height() + kPagePadding;
 
   // If there is a transition, calculates offset for current and target page.
   const int current_page = pagination_model_.selected_page();
@@ -1241,10 +1234,8 @@ void AppsGridView::CalculateIdealBounds() {
 
     const int row = view_index.slot / cols_;
     const int col = view_index.slot % cols_;
-    gfx::Rect tile_slot(
-        gfx::Point(grid_rect.x() + col * tile_size.width() + x_offset,
-                   grid_rect.y() + row * tile_size.height() + y_offset),
-        tile_size);
+    gfx::Rect tile_slot = GetExpectedTileBounds(row, col);
+    tile_slot.Offset(x_offset, y_offset);
     if (i < view_model_.view_size()) {
       view_model_.set_ideal_bounds(i, tile_slot);
     } else {
@@ -2188,7 +2179,7 @@ gfx::Rect AppsGridView::GetTileBoundsForPoint(const gfx::Point& point,
   int y = point.y();
   int col = (x - bounds.x()) / kPreferredTileWidth;
   int row = (y - bounds.y()) / kPreferredTileHeight;
-  gfx::Rect tile_rect = GetTileBounds(row, col);
+  gfx::Rect tile_rect = GetExpectedTileBounds(row, col);
 
   // Check if |point| is outside a valid item's tile.
   Index index(pagination_model_.selected_page(), row * cols_ + col);
@@ -2196,16 +2187,19 @@ gfx::Rect AppsGridView::GetTileBoundsForPoint(const gfx::Point& point,
   return tile_rect;
 }
 
-gfx::Rect AppsGridView::GetTileBounds(int row, int col) const {
+gfx::Size AppsGridView::GetTileGridSize() const {
+  gfx::Rect bounds;
+  bounds.Union(GetExpectedTileBounds(0, 0));

[Matt Giuca, 2014/09/10 05:35:06]

No need to do this first union. Just:
gfx::Rect bounds(GetExpectedTileBounds(0, 0));

[calamity, 2014/09/11 06:53:35]

Done.

+  bounds.Union(GetExpectedTileBounds(rows_per_page_ - 1, cols_ - 1));
+  return bounds.size();
+}
+
+gfx::Rect AppsGridView::GetExpectedTileBounds(int row, int col) const {
   gfx::Rect bounds(GetContentsBounds());
   gfx::Size tile_size(kPreferredTileWidth, kPreferredTileHeight);
-  gfx::Rect grid_rect(gfx::Size(tile_size.width() * cols_,
-                                tile_size.height() * rows_per_page_));
-  grid_rect.Intersect(bounds);
-  gfx::Rect tile_rect(
-      gfx::Point(grid_rect.x() + col * tile_size.width(),
-                 grid_rect.y() + row * tile_size.height()),
-      tile_size);
+  gfx::Rect tile_rect(gfx::Point(bounds.x() + col * tile_size.width(),

[Matt Giuca, 2014/09/10 05:35:06]

Just return this directly.

[calamity, 2014/09/11 06:53:35]

Done.

+                                 bounds.y() + row * tile_size.height()),
+                      tile_size);
   return tile_rect;
 }
 
@@ -2222,7 +2216,7 @@ views::View* AppsGridView::GetViewAtSlotOnCurrentPage(int slot) {
   // Calculate the original bound of the tile at |index|.
   int row = slot / cols_;
   int col = slot % cols_;
-  gfx::Rect tile_rect = GetTileBounds(row, col);
+  gfx::Rect tile_rect = GetExpectedTileBounds(row, col);
 
   for (int i = 0; i < view_model_.view_size(); ++i) {
     views::View* view = view_model_.view_at(i);