Added views::ViewModelT<T>, a type-safe template version of ViewModel.

A common use of views::ViewModel is to call view_at() then static_cast
down to the correct type, which is unsafe. ViewModel clients are now
encouraged to use ViewModelT<T> for compile-time type-checked access to
the views inside. ViewModel still exists for clients that don't care
about the view type, or with more complex downcasting logic.

Updated clients, where possible, to use ViewModelT:
TabStrip: ViewModelT<Tab>
AppsGridView: ViewModelT<AppListItemView>, ViewModelT<PulsingBlockView>.

BUG=418461

Review URL: https://codereview.chromium.org/598013003

Cr-Commit-Position: refs/heads/master@{#297979}

14 files changed, 114 insertions, 88 deletions

diff --git a/ash/shelf/shelf_view.h b/ash/shelf/shelf_view.h
index e4344e6..39b3857 100644
--- a/ash/shelf/shelf_view.h
+++ b/ash/shelf/shelf_view.h
@@ -18,6 +18,7 @@
 #include "ui/views/controls/button/button.h"
 #include "ui/views/focus/focus_manager.h"
 #include "ui/views/view.h"
+#include "ui/views/view_model.h"
 
 namespace ui {
 class MenuModel;
@@ -26,7 +27,6 @@ class MenuModel;
 namespace views {
 class BoundsAnimator;
 class MenuRunner;
-class ViewModel;
 }
 
 namespace ash {
diff --git a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc
index 9960694..d80bd02 100644
--- a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc
+++ b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc
@@ -56,6 +56,7 @@
 #include "extensions/common/switches.h"
 #include "extensions/test/extension_test_message_listener.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/app_list/views/app_list_item_view.h"
 #include "ui/app_list/views/apps_grid_view.h"
 #include "ui/aura/client/aura_constants.h"
 #include "ui/aura/window.h"
@@ -1633,7 +1634,8 @@ IN_PROC_BROWSER_TEST_F(ShelfAppBrowserTest, DISABLED_DragAndDrop) {
   ASSERT_TRUE(grid_view->has_drag_and_drop_host_for_test());
 
   // There should be 2 items in our application list.
-  const views::ViewModel* vm_grid = grid_view->view_model_for_test();
+  const views::ViewModelT<app_list::AppListItemView>* vm_grid =
+      grid_view->view_model_for_test();
   EXPECT_EQ(2, vm_grid->view_size());
 
   // Test #1: Drag an app list which does not exist yet item into the
@@ -1783,7 +1785,8 @@ IN_PROC_BROWSER_TEST_F(ShelfAppBrowserTestWithMultiMonitor,
   ASSERT_TRUE(grid_view->has_drag_and_drop_host_for_test());
 
   // There should be 2 items in our application list.
-  const views::ViewModel* vm_grid = grid_view->view_model_for_test();
+  const views::ViewModelT<app_list::AppListItemView>* vm_grid =
+      grid_view->view_model_for_test();
   EXPECT_EQ(2, vm_grid->view_size());
 
   // Drag an app list item which does not exist yet in the shelf.
@@ -1957,7 +1960,8 @@ IN_PROC_BROWSER_TEST_F(ShelfAppBrowserTest, ClickItem) {
       ash::test::AppListControllerTestApi(ash::Shell::GetInstance()).
           GetRootGridView();
   ASSERT_TRUE(grid_view);
-  const views::ViewModel* vm_grid = grid_view->view_model_for_test();
+  const views::ViewModelT<app_list::AppListItemView>* vm_grid =
+      grid_view->view_model_for_test();
   EXPECT_EQ(2, vm_grid->view_size());
   gfx::Rect bounds_grid_1 = vm_grid->view_at(1)->GetBoundsInScreen();
   // Test now that a click does create a new application tab.
diff --git a/chrome/browser/ui/views/tabs/stacked_tab_strip_layout.cc b/chrome/browser/ui/views/tabs/stacked_tab_strip_layout.cc
index 88a83ce..263acb5 100644
--- a/chrome/browser/ui/views/tabs/stacked_tab_strip_layout.cc
+++ b/chrome/browser/ui/views/tabs/stacked_tab_strip_layout.cc
@@ -13,7 +13,7 @@ StackedTabStripLayout::StackedTabStripLayout(const gfx::Size& size,
                                              int padding,
                                              int stacked_padding,
                                              int max_stacked_count,
-                                             views::ViewModel* view_model)
+                                             views::ViewModelBase* view_model)
     : size_(size),
       padding_(padding),
       stacked_padding_(stacked_padding),
diff --git a/chrome/browser/ui/views/tabs/stacked_tab_strip_layout.h b/chrome/browser/ui/views/tabs/stacked_tab_strip_layout.h
index f9624d5..e5e49ee 100644
--- a/chrome/browser/ui/views/tabs/stacked_tab_strip_layout.h
+++ b/chrome/browser/ui/views/tabs/stacked_tab_strip_layout.h
@@ -11,9 +11,7 @@
 #include "ui/gfx/size.h"
 #include "ui/views/view_model.h"
 
-namespace views {
-class ViewModel;
-}
+class Tab;
 
 // StackedTabStripLayout is used by TabStrip in touch
 // mode. StackedTabStripLayout is responsible for managing the bounds of the
@@ -36,7 +34,7 @@ class StackedTabStripLayout {
                         int padding,
                         int stacked_padding,
                         int max_stacked_count,
-                        views::ViewModel* view_model);
+                        views::ViewModelBase* view_model);
   ~StackedTabStripLayout();
 
   // Sets the x-coordinate the normal tabs start at as well as the mini-tab
@@ -217,7 +215,9 @@ class StackedTabStripLayout {
   const int max_stacked_count_;
 
   // Where bounds are placed. This is owned by TabStrip.
-  views::ViewModel* view_model_;
+  // (Note: This is a ViewModelBase, not a ViewModelT<Tab>, because the tests do
+  // not populate the model with Tab views.)
+  views::ViewModelBase* view_model_;
 
   // x-coordinate normal tabs start at.
   int x_;
diff --git a/chrome/browser/ui/views/tabs/tab_strip.h b/chrome/browser/ui/views/tabs/tab_strip.h
index 34cc810..47f06dc 100644
--- a/chrome/browser/ui/views/tabs/tab_strip.h
+++ b/chrome/browser/ui/views/tabs/tab_strip.h
@@ -149,9 +149,7 @@ class TabStrip : public views::View,
   }
 
   // Returns the Tab at |index|.
-  Tab* tab_at(int index) const {
-    return static_cast<Tab*>(tabs_.view_at(index));
-  }
+  Tab* tab_at(int index) const { return tabs_.view_at(index); }
 
   // Returns the index of the specified tab in the model coordinate system, or
   // -1 if tab is closing or not valid.
@@ -596,7 +594,7 @@ class TabStrip : public views::View,
   // |tabs_closing_map_|. When the animation completes the tab is removed from
   // |tabs_closing_map_|. The painting code ensures both sets of tabs are
   // painted, and the event handling code ensures only tabs in |tabs_| are used.
-  views::ViewModel tabs_;
+  views::ViewModelT<Tab> tabs_;
   TabsClosingMap tabs_closing_map_;
 
   scoped_ptr<TabStripController> controller_;
diff --git a/ui/app_list/views/app_list_folder_view.h b/ui/app_list/views/app_list_folder_view.h
index 2548538..2692578 100644
--- a/ui/app_list/views/app_list_folder_view.h
+++ b/ui/app_list/views/app_list_folder_view.h
@@ -15,10 +15,7 @@
 #include "ui/compositor/layer_animation_observer.h"
 #include "ui/views/controls/button/button.h"
 #include "ui/views/view.h"
-
-namespace views {
-class ViewModel;
-}
+#include "ui/views/view_model.h"
 
 namespace app_list {
 
diff --git a/ui/app_list/views/app_list_main_view_unittest.cc b/ui/app_list/views/app_list_main_view_unittest.cc
index 9de87fd..f92e041 100644
--- a/ui/app_list/views/app_list_main_view_unittest.cc
+++ b/ui/app_list/views/app_list_main_view_unittest.cc
@@ -98,7 +98,8 @@ class AppListMainViewTest : public views::ViewsTestBase {
   // |point| is in |grid_view|'s coordinates.
   AppListItemView* GetItemViewAtPointInGrid(AppsGridView* grid_view,
                                             const gfx::Point& point) {
-    const views::ViewModel* view_model = grid_view->view_model_for_test();
+    const views::ViewModelT<AppListItemView>* view_model =
+        grid_view->view_model_for_test();
     for (int i = 0; i < view_model->view_size(); ++i) {
       views::View* view = view_model->view_at(i);
       if (view->bounds().Contains(point)) {
@@ -161,11 +162,11 @@ class AppListMainViewTest : public views::ViewsTestBase {
 
   AppsGridView* FolderGridView() { return FolderView()->items_grid_view(); }
 
-  const views::ViewModel* RootViewModel() {
+  const views::ViewModelT<AppListItemView>* RootViewModel() {
     return RootGridView()->view_model_for_test();
   }
 
-  const views::ViewModel* FolderViewModel() {
+  const views::ViewModelT<AppListItemView>* FolderViewModel() {
     return FolderGridView()->view_model_for_test();
   }
 
@@ -270,8 +271,9 @@ TEST_F(AppListMainViewTest, DragLastItemFromFolderAndDropAtLastSlot) {
 
   // Folder icon view should be gone and there is only one item view.
   EXPECT_EQ(1, RootViewModel()->view_size());
-  EXPECT_EQ(AppListItemView::kViewClassName,
-            RootViewModel()->view_at(0)->GetClassName());
+  EXPECT_EQ(
+      AppListItemView::kViewClassName,
+      static_cast<views::View*>(RootViewModel()->view_at(0))->GetClassName());
 
   // The item view should be in slot 1 instead of slot 2 where it is dropped.
   AppsGridViewTestApi root_grid_view_test_api(RootGridView());
diff --git a/ui/app_list/views/apps_grid_view.cc b/ui/app_list/views/apps_grid_view.cc
index 4971720..7c4a06f 100644
--- a/ui/app_list/views/apps_grid_view.cc
+++ b/ui/app_list/views/apps_grid_view.cc
@@ -762,7 +762,7 @@ void AppsGridView::StopPageFlipTimer() {
 }
 
 AppListItemView* AppsGridView::GetItemViewAt(int index) const {
-  return static_cast<AppListItemView*>(view_model_.view_at(index));
+  return view_model_.view_at(index);
 }
 
 void AppsGridView::SetTopItemViewsVisible(bool visible) {
@@ -1057,7 +1057,7 @@ void AppsGridView::UpdatePulsingBlockViews() {
     return;
 
   while (pulsing_blocks_model_.view_size() > desired) {
-    PulsingBlockView* view = GetPulsingBlockViewAt(0);
+    PulsingBlockView* view = pulsing_blocks_model_.view_at(0);
     pulsing_blocks_model_.Remove(0);
     delete view;
   }
@@ -1069,10 +1069,6 @@ void AppsGridView::UpdatePulsingBlockViews() {
   }
 }
 
-PulsingBlockView* AppsGridView::GetPulsingBlockViewAt(int index) const {
-  return static_cast<PulsingBlockView*>(pulsing_blocks_model_.view_at(index));
-}
-
 AppListItemView* AppsGridView::CreateViewForItemAtIndex(size_t index) {
   // The drag_view_ might be pending for deletion, therefore view_model_
   // may have one more item than item_list_.
diff --git a/ui/app_list/views/apps_grid_view.h b/ui/app_list/views/apps_grid_view.h
index 878ebd3..b6781f1 100644
--- a/ui/app_list/views/apps_grid_view.h
+++ b/ui/app_list/views/apps_grid_view.h
@@ -196,7 +196,9 @@ class APP_LIST_EXPORT AppsGridView : public views::View,
   void OnFolderItemRemoved();
 
   // Return the view model for test purposes.
-  const views::ViewModel* view_model_for_test() const { return &view_model_; }
+  const views::ViewModelT<AppListItemView>* view_model_for_test() const {
+    return &view_model_;
+  }
 
   // For test: Return if the drag and drop handler was set.
   bool has_drag_and_drop_host_for_test() { return NULL != drag_and_drop_host_; }
@@ -259,10 +261,6 @@ class APP_LIST_EXPORT AppsGridView : public views::View,
   // number of apps.
   void UpdatePulsingBlockViews();
 
-  // Returns the pulsing block view of the item at |index| in the pulsing block
-  // model.
-  PulsingBlockView* GetPulsingBlockViewAt(int index) const;
-
   AppListItemView* CreateViewForItemAtIndex(size_t index);
 
   // Convert between the model index and the visual index. The model index
@@ -482,11 +480,11 @@ class APP_LIST_EXPORT AppsGridView : public views::View,
   int cols_;
   int rows_per_page_;
 
-  // List of AppListItemViews. There is a view per item in |model_|.
-  views::ViewModel view_model_;
+  // List of app item views. There is a view per item in |model_|.
+  views::ViewModelT<AppListItemView> view_model_;
 
-  // List of PulsingBlockViews.
-  views::ViewModel pulsing_blocks_model_;
+  // List of pulsing block views.
+  views::ViewModelT<PulsingBlockView> pulsing_blocks_model_;
 
   AppListItemView* selected_view_;
 
diff --git a/ui/app_list/views/contents_view.h b/ui/app_list/views/contents_view.h
index dbb74cf..092f08d 100644
--- a/ui/app_list/views/contents_view.h
+++ b/ui/app_list/views/contents_view.h
@@ -14,15 +14,12 @@
 #include "ui/app_list/pagination_model.h"
 #include "ui/app_list/pagination_model_observer.h"
 #include "ui/views/view.h"
+#include "ui/views/view_model.h"
 
 namespace gfx {
 class Rect;
 }
 
-namespace views {
-class ViewModel;
-}
-
 namespace app_list {
 
 class AppsGridView;
diff --git a/ui/views/view_model.cc b/ui/views/view_model.cc
index a0a7549..14f60e0 100644
--- a/ui/views/view_model.cc
+++ b/ui/views/view_model.cc
@@ -9,22 +9,11 @@
 
 namespace views {
 
-ViewModel::ViewModel() {
-}
-
-ViewModel::~ViewModel() {
+ViewModelBase::~ViewModelBase() {
   // view are owned by their parent, no need to delete them.
 }
 
-void ViewModel::Add(View* view, int index) {
-  DCHECK_LE(index, static_cast<int>(entries_.size()));
-  DCHECK_GE(index, 0);
-  Entry entry;
-  entry.view = view;
-  entries_.insert(entries_.begin() + index, entry);
-}
-
-void ViewModel::Remove(int index) {
+void ViewModelBase::Remove(int index) {
   if (index == -1)
     return;
 
@@ -32,7 +21,7 @@ void ViewModel::Remove(int index) {
   entries_.erase(entries_.begin() + index);
 }
 
-void ViewModel::Move(int index, int target_index) {
+void ViewModelBase::Move(int index, int target_index) {
   DCHECK_LT(index, static_cast<int>(entries_.size()));
   DCHECK_GE(index, 0);
   DCHECK_LT(target_index, static_cast<int>(entries_.size()));
@@ -45,7 +34,7 @@ void ViewModel::Move(int index, int target_index) {
   entries_.insert(entries_.begin() + target_index, entry);
 }
 
-void ViewModel::MoveViewOnly(int index, int target_index) {
+void ViewModelBase::MoveViewOnly(int index, int target_index) {
   if (index == target_index)
     return;
   if (target_index < index) {
@@ -61,14 +50,14 @@ void ViewModel::MoveViewOnly(int index, int target_index) {
   }
 }
 
-void ViewModel::Clear() {
+void ViewModelBase::Clear() {
   Entries entries;
   entries.swap(entries_);
   for (size_t i = 0; i < entries.size(); ++i)
     delete entries[i].view;
 }
 
-int ViewModel::GetIndexOfView(const View* view) const {
+int ViewModelBase::GetIndexOfView(const View* view) const {
   for (size_t i = 0; i < entries_.size(); ++i) {
     if (entries_[i].view == view)
       return static_cast<int>(i);
@@ -76,4 +65,15 @@ int ViewModel::GetIndexOfView(const View* view) const {
   return -1;
 }
 
+ViewModelBase::ViewModelBase() {
+}
+
+void ViewModelBase::AddUnsafe(View* view, int index) {
+  DCHECK_LE(index, static_cast<int>(entries_.size()));
+  DCHECK_GE(index, 0);
+  Entry entry;
+  entry.view = view;
+  entries_.insert(entries_.begin() + index, entry);
+}
+
 }  // namespace views
diff --git a/ui/views/view_model.h b/ui/views/view_model.h
index e8c84a9..8fff26b 100644
--- a/ui/views/view_model.h
+++ b/ui/views/view_model.h
@@ -15,20 +15,16 @@
 namespace views {
 
 class View;
-
-// ViewModel is used to track an 'interesting' set of a views. Often times
-// during animations views are removed after a delay, which makes for tricky
-// coordinate conversion as you have to account for the possibility of the
-// indices from the model not lining up with those you expect. This class lets
-// you define the 'interesting' views and operate on those views.
-class VIEWS_EXPORT ViewModel {
+class ViewModelUtils;
+
+// Internal implementation of the templated ViewModelT class. Provides
+// non-templated "unsafe" methods for ViewModelT to wrap around. Any methods
+// that allow insertion of or access to a View* should be protected, and have a
+// public method in the ViewModelT subclass that provides type-safe access to
+// the correct View subclass.
+class VIEWS_EXPORT ViewModelBase {
  public:
-  ViewModel();
-  ~ViewModel();
-
-  // Adds |view| to this model. This does not add |view| to a view hierarchy,
-  // only to this model.
-  void Add(View* view, int index);
+  ~ViewModelBase();
 
   // Removes the view at the specified index. This does not actually remove the
   // view from the view hierarchy.
@@ -50,12 +46,6 @@ class VIEWS_EXPORT ViewModel {
   // Removes and deletes all the views.
   void Clear();
 
-  // Returns the view at the specified index.
-  View* view_at(int index) const {
-    check_index(index);
-    return entries_[index].view;
-  }
-
   void set_ideal_bounds(int index, const gfx::Rect& bounds) {
     check_index(index);
     entries_[index].ideal_bounds = bounds;
@@ -70,7 +60,25 @@ class VIEWS_EXPORT ViewModel {
   // model.
   int GetIndexOfView(const View* view) const;
 
+ protected:
+  ViewModelBase();
+
+  // Returns the view at the specified index. Note: Most users should use
+  // view_at() in the subclass, to get a view of the correct type. (Do not call
+  // ViewAtBase then static_cast to the desired type.)
+  View* ViewAtBase(int index) const {
+    check_index(index);
+    return entries_[index].view;
+  }
+
+  // Adds |view| to this model. This does not add |view| to a view hierarchy,
+  // only to this model.
+  void AddUnsafe(View* view, int index);
+
  private:
+  // For access to ViewAtBase().
+  friend class ViewModelUtils;
+
   struct Entry {
     Entry() : view(NULL) {}
 
@@ -90,9 +98,35 @@ class VIEWS_EXPORT ViewModel {
 
   Entries entries_;
 
-  DISALLOW_COPY_AND_ASSIGN(ViewModel);
+  DISALLOW_COPY_AND_ASSIGN(ViewModelBase);
+};
+
+// ViewModelT is used to track an 'interesting' set of a views. Often times
+// during animations views are removed after a delay, which makes for tricky
+// coordinate conversion as you have to account for the possibility of the
+// indices from the model not lining up with those you expect. This class lets
+// you define the 'interesting' views and operate on those views.
+template <class T>
+class ViewModelT : public ViewModelBase {
+ public:
+  ViewModelT<T>() {}
+
+  // Adds |view| to this model. This does not add |view| to a view hierarchy,
+  // only to this model.
+  void Add(T* view, int index) { AddUnsafe(view, index); }
+
+  // Returns the view at the specified index.
+  T* view_at(int index) const { return static_cast<T*>(ViewAtBase(index)); }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ViewModelT<T>);
 };
 
+// ViewModel is a collection of views with no specfic type. If all views have
+// the same type, the use of ViewModelT is preferred so that the views can be
+// retrieved without potentially unsafe downcasts.
+typedef ViewModelT<View> ViewModel;
+
 }  // namespace views
 
 #endif  // UI_VIEWS_VIEW_MODEL_H_
diff --git a/ui/views/view_model_utils.cc b/ui/views/view_model_utils.cc
index 056e75b..4bb3ac4 100644
--- a/ui/views/view_model_utils.cc
+++ b/ui/views/view_model_utils.cc
@@ -23,15 +23,15 @@ int primary_axis_coordinate(ViewModelUtils::Alignment alignment,
 }  // namespace
 
 // static
-void ViewModelUtils::SetViewBoundsToIdealBounds(const ViewModel& model) {
+void ViewModelUtils::SetViewBoundsToIdealBounds(const ViewModelBase& model) {
   for (int i = 0; i < model.view_size(); ++i)
-    model.view_at(i)->SetBoundsRect(model.ideal_bounds(i));
+    model.ViewAtBase(i)->SetBoundsRect(model.ideal_bounds(i));
 }
 
 // static
-bool ViewModelUtils::IsAtIdealBounds(const ViewModel& model) {
+bool ViewModelUtils::IsAtIdealBounds(const ViewModelBase& model) {
   for (int i = 0; i < model.view_size(); ++i) {
-    View* view = model.view_at(i);
+    View* view = model.ViewAtBase(i);
     if (view->bounds() != model.ideal_bounds(i))
       return false;
   }
@@ -39,7 +39,7 @@ bool ViewModelUtils::IsAtIdealBounds(const ViewModel& model) {
 }
 
 // static
-int ViewModelUtils::DetermineMoveIndex(const ViewModel& model,
+int ViewModelUtils::DetermineMoveIndex(const ViewModelBase& model,
                                        View* view,
                                        Alignment alignment,
                                        int x,
diff --git a/ui/views/view_model_utils.h b/ui/views/view_model_utils.h
index 2d7eb89..a01fecb 100644
--- a/ui/views/view_model_utils.h
+++ b/ui/views/view_model_utils.h
@@ -11,7 +11,7 @@
 namespace views {
 
 class View;
-class ViewModel;
+class ViewModelBase;
 
 class VIEWS_EXPORT ViewModelUtils {
  public:
@@ -21,13 +21,13 @@ class VIEWS_EXPORT ViewModelUtils {
   };
 
   // Sets the bounds of each view to its ideal bounds.
-  static void SetViewBoundsToIdealBounds(const ViewModel& model);
+  static void SetViewBoundsToIdealBounds(const ViewModelBase& model);
 
   // Returns true if the Views in |model| are at their ideal bounds.
-  static bool IsAtIdealBounds(const ViewModel& model);
+  static bool IsAtIdealBounds(const ViewModelBase& model);
 
   // Returns the index to move |view| to based on a coordinate of |x| and |y|.
-  static int DetermineMoveIndex(const ViewModel& model,
+  static int DetermineMoveIndex(const ViewModelBase& model,
                                 View* view,
                                 Alignment alignment,
                                 int x,