diff options
author | pkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-02-22 21:44:21 +0000 |
---|---|---|
committer | pkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-02-22 21:44:21 +0000 |
commit | 6f0790bc152361e47d8d525442446fa4a1ba5743 (patch) | |
tree | 2e0506d637fda8d892013ad7a858d34755779cae /ui/views/focus | |
parent | e3cc52018a52135221bf68d330ed81f61eb8982b (diff) | |
download | chromium_src-6f0790bc152361e47d8d525442446fa4a1ba5743.zip chromium_src-6f0790bc152361e47d8d525442446fa4a1ba5743.tar.gz chromium_src-6f0790bc152361e47d8d525442446fa4a1ba5743.tar.bz2 |
View API/implementation cleanup:
* Don't include the container type ("vector") in the typedef for "a bunch of children". Users generally should not know or care what the container is, so this makes reading easier as the code is not constantly pointing out to you, "hey! I'm a vector!" Added bonus: less verbose, allows condensing a lot of loop declarations onto one line.
* Consistently put getters before setters.
* Remove 4-arg form of SetBounds() and make people use Rects (we should move the codebase towards Points, Sizes, and Rects wherever possible).
* Use "origin" instead of "position" to be consistent with Rect's terminology.
* Minor naming changes, e.g. GetViewById() -> GetViewByID().
* Remove const qualifier on member functions that are not logically const. This also got rid of all the const_cast<>()s.
* Better comments.
* Use const ref args for Views whenever the provided View is not being modified, to make that obvious to the caller.
* Turn some accessors into pairs of (non-const, const) accessors. In these cases make the non-const version call the const version. (GetWidget() does this in the header because the const version is virtual; this way people who override the const version can see why they don't need to override the non-const version).
* Make RemoveChildView() take a bool for consistency with RemoveAllChildViews() (also eliminates the need to return a View*).
* Add STL-style iterators and rename a few accessors to match STL terminology ("size" instead of "count").
* Turn IsFocusable() into a cheap inline getter.
* Greatly simplify private tree ops ("NotifyHierarchyChangedXXX()") by realizing that they were always being called with |parent| == |this|.
* Declare iterators inside loops, not above them.
* Standardize iterator naming to |i|. The existing code wasn't always consistent, and while there's nothing wrong with |it|, using that would have made almost every loop declaration into two lines instead of one.
* Simpler code, sometimes by using STL algorithms.
* Unindent via early-returns.
* Use CHECK_NE() and similar where possible.
* Fix memory corruption in RemoveAllChildViews() due to using an iterator after modifying its container.
BUG=none
TEST=none
Review URL: http://codereview.chromium.org/6541030
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@75642 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'ui/views/focus')
-rw-r--r-- | ui/views/focus/focus_manager.cc | 2 | ||||
-rw-r--r-- | ui/views/focus/focus_search.cc | 21 |
2 files changed, 11 insertions, 12 deletions
diff --git a/ui/views/focus/focus_manager.cc b/ui/views/focus/focus_manager.cc index 20baabe..fa2b8f2 100644 --- a/ui/views/focus/focus_manager.cc +++ b/ui/views/focus/focus_manager.cc @@ -143,7 +143,7 @@ bool FocusManager::OnKeyEvent(const KeyEvent& event) { key_code == ui::VKEY_LEFT || key_code == ui::VKEY_RIGHT)) { bool next = (key_code == ui::VKEY_RIGHT || key_code == ui::VKEY_DOWN); std::vector<View*> views; - focused_view_->parent()->GetViewsWithGroup(focused_view_->group(), &views); + focused_view_->parent()->GetViewsInGroup(focused_view_->group(), &views); std::vector<View*>::const_iterator iter = std::find(views.begin(), views.end(), focused_view_); diff --git a/ui/views/focus/focus_search.cc b/ui/views/focus/focus_search.cc index c8093fd0..083fc7c 100644 --- a/ui/views/focus/focus_search.cc +++ b/ui/views/focus/focus_search.cc @@ -24,7 +24,7 @@ View* FocusSearch::FindNextFocusableView(View* starting_view, *focus_traversable = NULL; *focus_traversable_view = NULL; - if (root_->child_count() == 0) { + if (root_->children_empty()) { NOTREACHED(); // Nothing to focus on here. return NULL; @@ -39,14 +39,14 @@ View* FocusSearch::FindNextFocusableView(View* starting_view, // Default to the first/last child starting_view = reverse ? - root_->GetChildViewAt(root_->child_count() - 1) : - root_->GetChildViewAt(0); + root_->child_at(root_->children_size() - 1) : + root_->child_at(0); // If there was no starting view, then the one we select is a potential // focus candidate. check_starting_view = true; } else { // The starting view should be a direct or indirect child of the root. - DCHECK(root_->Contains(starting_view)); + DCHECK(root_->Contains(*starting_view)); } View* v = NULL; @@ -70,7 +70,7 @@ View* FocusSearch::FindNextFocusableView(View* starting_view, } // Don't set the focus to something outside of this view hierarchy. - if (v && v != root_ && !root_->Contains(v)) + if (v && v != root_ && !root_->Contains(*v)) v = NULL; // If |cycle_| is true, prefer to keep cycling rather than returning NULL. @@ -121,7 +121,7 @@ View* FocusSearch::FindSelectedViewForGroup(View* view) const { } View* FocusSearch::GetParent(View* v) const { - return root_->Contains(v) ? v->parent() : NULL; + return root_->Contains(*v) ? v->parent() : NULL; } // Strategy for finding the next focusable view: @@ -158,8 +158,8 @@ View* FocusSearch::FindNextFocusableViewImpl( // First let's try the left child. if (can_go_down) { - if (starting_view->child_count() > 0) { - View* v = FindNextFocusableViewImpl(starting_view->GetChildViewAt(0), + if (!starting_view->children_empty()) { + View* v = FindNextFocusableViewImpl(starting_view->child_at(0), true, false, true, skip_group_id, focus_traversable, focus_traversable_view); @@ -223,9 +223,8 @@ View* FocusSearch::FindPreviousFocusableViewImpl( return NULL; } - if (starting_view->child_count() > 0) { - View* view = - starting_view->GetChildViewAt(starting_view->child_count() - 1); + if (!starting_view->children_empty()) { + View* view = starting_view->child_at(starting_view->children_size() - 1); View* v = FindPreviousFocusableViewImpl(view, true, false, true, skip_group_id, focus_traversable, |