Change the ViewStorage API back to the way it was (void for StoreView) since returning an int is just a test optimization.

Expose a view_count() accessor on ViewStorage for unit tests to discern the number of views stored in it.

Review URL: http://codereview.chromium.org/113091

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@15534 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 33 insertions, 15 deletions

diff --git a/chrome/views/focus/view_storage.cc b/chrome/views/focus/view_storage.cc
index 7299809..d8196b2 100644
--- a/chrome/views/focus/view_storage.cc
+++ b/chrome/views/focus/view_storage.cc
@@ -52,7 +52,7 @@ int ViewStorage::CreateStorageID() {
   return view_storage_next_id_++;
 }
 
-int ViewStorage::StoreView(int storage_id, View* view) {
+void ViewStorage::StoreView(int storage_id, View* view) {
   DCHECK(view);
   std::map<int, ViewLocationInfo*>::iterator iter =
       id_to_view_location_.find(storage_id);
@@ -90,7 +90,6 @@ int ViewStorage::StoreView(int storage_id, View* view) {
     ids = id_iter->second;
   }
   ids->push_back(storage_id);
-  return storage_id;
 }
 
 View* ViewStorage::RetrieveView(int storage_id) {
diff --git a/chrome/views/focus/view_storage.h b/chrome/views/focus/view_storage.h
index 8ba9c9a..09bb674 100644
--- a/chrome/views/focus/view_storage.h
+++ b/chrome/views/focus/view_storage.h
@@ -34,9 +34,8 @@ class ViewStorage {
   // Returns a unique storage id that can be used to store/retrieve views.
   int CreateStorageID();
 
-  // Associates |view| with the specified |storage_id|. Returns the
-  // |storage_id|.
-  int StoreView(int storage_id, View* view);
+  // Associates |view| with the specified |storage_id|.
+  void StoreView(int storage_id, View* view);
 
   // Returns the view associated with |storage_id| if any, NULL otherwise.
   View* RetrieveView(int storage_id);
@@ -47,6 +46,10 @@ class ViewStorage {
   // Notifies the ViewStorage that a view was removed from its parent somewhere.
   void ViewRemoved(View* parent, View* removed);
 
+#ifdef UNIT_TEST
+  size_t view_count() const { return view_to_ids_.size(); }
+#endif
+
  private:
   friend struct DefaultSingletonTraits<ViewStorage>;
 
diff --git a/chrome/views/view_unittest.cc b/chrome/views/view_unittest.cc
index 5b4f4ee..b8cac1f 100644
--- a/chrome/views/view_unittest.cc
+++ b/chrome/views/view_unittest.cc
@@ -438,50 +438,65 @@ TEST_F(ViewTest, RemoveNotification) {
   views::RootView* root_view = window->GetRootView();
 
   View* v1 = new View;
-  int s1 = vs->StoreView(vs->CreateStorageID(), v1);
+  int s1 = vs->CreateStorageID();
+  vs->StoreView(s1, v1);
   root_view->AddChildView(v1);
   View* v11 = new View;
-  int s11 = vs->StoreView(vs->CreateStorageID(), v11);
+  int s11 = vs->CreateStorageID();
+  vs->StoreView(s11, v11);
   v1->AddChildView(v11);
   View* v111 = new View;
-  int s111 = vs->StoreView(vs->CreateStorageID(), v111);
+  int s111 = vs->CreateStorageID();
+  vs->StoreView(s111, v111);
   v11->AddChildView(v111);
   View* v112 = new View;
-  int s112 = vs->StoreView(vs->CreateStorageID(), v112);
+  int s112 = vs->CreateStorageID();
+  vs->StoreView(s112, v112);
   v11->AddChildView(v112);
   View* v113 = new View;
-  int s113 = vs->StoreView(vs->CreateStorageID(), v113);
+  int s113 = vs->CreateStorageID();
+  vs->StoreView(s113, v113);
   v11->AddChildView(v113);
   View* v1131 = new View;
-  int s1131 = vs->StoreView(vs->CreateStorageID(), v1131);
+  int s1131 = vs->CreateStorageID();
+  vs->StoreView(s1131, v1131);
   v113->AddChildView(v1131);
   View* v12 = new View;
-  int s12 = vs->StoreView(vs->CreateStorageID(), v12);
+  int s12 = vs->CreateStorageID();
+  vs->StoreView(s12, v12);
   v1->AddChildView(v12);
 
   View* v2 = new View;
-  int s2 = vs->StoreView(vs->CreateStorageID(), v2);
+  int s2 = vs->CreateStorageID();
+  vs->StoreView(s2, v2);
   root_view->AddChildView(v2);
   View* v21 = new View;
-  int s21 = vs->StoreView(vs->CreateStorageID(), v21);
+  int s21 = vs->CreateStorageID();
+  vs->StoreView(s21, v21);
   v2->AddChildView(v21);
   View* v211 = new View;
-  int s211 = vs->StoreView(vs->CreateStorageID(), v211);
+  int s211 = vs->CreateStorageID();
+  vs->StoreView(s211, v211);
   v21->AddChildView(v211);
 
+  size_t stored_views = vs->view_count();
+
   // Try removing a leaf view.
   v21->RemoveChildView(v211);
+  EXPECT_EQ(stored_views - 1, vs->view_count());
   EXPECT_EQ(NULL, vs->RetrieveView(s211));
   delete v211;  // We won't use this one anymore.
 
   // Now try removing a view with a hierarchy of depth 1.
   v11->RemoveChildView(v113);
+  EXPECT_EQ(stored_views - 3, vs->view_count());
   EXPECT_EQ(NULL, vs->RetrieveView(s113));
   EXPECT_EQ(NULL, vs->RetrieveView(s1131));
   delete v113;  // We won't use this one anymore.
 
   // Now remove even more.
   root_view->RemoveChildView(v1);
+  EXPECT_EQ(stored_views - 8, vs->view_count());
   EXPECT_EQ(NULL, vs->RetrieveView(s1));
   EXPECT_EQ(NULL, vs->RetrieveView(s11));
   EXPECT_EQ(NULL, vs->RetrieveView(s12));
@@ -495,6 +510,7 @@ TEST_F(ViewTest, RemoveNotification) {
   // Now delete the root view (deleting the window will trigger a delete of the
   // RootView) and make sure we are notified that the views were removed.
   delete window;
+  EXPECT_EQ(stored_views - 10, vs->view_count());
   EXPECT_EQ(NULL, vs->RetrieveView(s1));
   EXPECT_EQ(NULL, vs->RetrieveView(s12));
   EXPECT_EQ(NULL, vs->RetrieveView(s11));