Use Closure instead of ScopedRunnableFactory in StatusAreaView.

Use Closure instead of ScopedRunnableFactory in StatusAreaView.

BUG=chromium-os:22024
TEST=tests pass.
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=107081

[achuithb, 2011-10-25 00:50:27]

Please review. Thanks!

[satorux1, 2011-10-25 01:08:16]

LGTM with one nit.

http://codereview.chromium.org/8332016/diff/1004/chrome/browser/chromeos/stat...
File chrome/browser/chromeos/status/status_area_view.cc (right):

http://codereview.chromium.org/8332016/diff/1004/chrome/browser/chromeos/stat...
chrome/browser/chromeos/status/status_area_view.cc:141: const
ReturnFocusCallback& return_focus_cb) {
nit: indentation for parameters should be vertically aligned. You can move "bool
reverse" back to the next line.

http://codereview.chromium.org/8332016/diff/1004/chrome/browser/chromeos/stat...
File chrome/browser/chromeos/status/status_area_view.h (right):

http://codereview.chromium.org/8332016/diff/1004/chrome/browser/chromeos/stat...
chrome/browser/chromeos/status/status_area_view.h:28: public
base::SupportsWeakPtr<StatusAreaView> {
Just FYI. You can use WeakPtrFactory instead, but it's more like a matter of
taste. I found WeakPtrFactory to be  slightly more preferable, as it's
composition rather than inheritance.

[achuithb, 2011-10-25 08:49:09]

Thanks for the review, Satoru.

http://codereview.chromium.org/8332016/diff/1004/chrome/browser/chromeos/stat...
File chrome/browser/chromeos/status/status_area_view.cc (right):

http://codereview.chromium.org/8332016/diff/1004/chrome/browser/chromeos/stat...
chrome/browser/chromeos/status/status_area_view.cc:141: const
ReturnFocusCallback& return_focus_cb) {
On 2011/10/25 01:08:16, satorux1 wrote:
> nit: indentation for parameters should be vertically aligned. You can move
"bool
> reverse" back to the next line.

Done.

http://codereview.chromium.org/8332016/diff/1004/chrome/browser/chromeos/stat...
File chrome/browser/chromeos/status/status_area_view.h (right):

http://codereview.chromium.org/8332016/diff/1004/chrome/browser/chromeos/stat...
chrome/browser/chromeos/status/status_area_view.h:28: public
base::SupportsWeakPtr<StatusAreaView> {
If you look at my first patch, I used WeakPtrFactory, then decided to go with
SupportsWeakPtr because I think SupportsWeakPtr is more readable and familiar as
a coding pattern. WeakPtrFactory is a bit more esoteric (I had to read the
code/comments to figure out what it does). Anyway, I'd like to stick with
SupportsWeakptr here if you don't mind.

[satorux1, 2011-10-25 16:17:29]

LGTM

http://codereview.chromium.org/8332016/diff/1004/chrome/browser/chromeos/stat...
File chrome/browser/chromeos/status/status_area_view.h (right):

http://codereview.chromium.org/8332016/diff/1004/chrome/browser/chromeos/stat...
chrome/browser/chromeos/status/status_area_view.h:28: public
base::SupportsWeakPtr<StatusAreaView> {
On 2011/10/25 08:49:09, achuith.bhandarkar wrote:
> If you look at my first patch, I used WeakPtrFactory, then decided to go with
> SupportsWeakPtr because I think SupportsWeakPtr is more readable and familiar
as
> a coding pattern. WeakPtrFactory is a bit more esoteric (I had to read the
> code/comments to figure out what it does). Anyway, I'd like to stick with
> SupportsWeakptr here if you don't mind.

Please go ahead. :)