Only slide-down animation in message center too.

Only slide-down animation in message center too.

BUG=247796
TEST=manually
R=dewittj@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=207776

[Jun Mukai, 2013-06-18 20:17:15]

please take a look.
Note that this doesn't resize the bubble itself, so doesn't solve issue 248514.

I thought I could do that in a same CL, but I'm afraid that my work-in-progress
code is already too copmlicated to land for M29 (see crrev.com/17413005 FYI). So
let's do at least the slide-down animation for M29.

[dewittj, 2013-06-18 23:41:51]

lgtm with a few comments.

https://codereview.chromium.org/17382011/diff/1/ui/message_center/views/messa...
File ui/message_center/views/message_center_view.cc (right):

https://codereview.chromium.org/17382011/diff/1/ui/message_center/views/messa...
ui/message_center/views/message_center_view.cc:522: // insert before the
specified index.
wow, we were reordering things all this time?  Interesting.

https://codereview.chromium.org/17382011/diff/1/ui/message_center/views/messa...
ui/message_center/views/message_center_view.cc:643: int
RichMessageListView::GetActualIndex(int index) {
I wouldn't be surprised if IsValidChild starts to become a performance
bottleneck.

https://codereview.chromium.org/17382011/diff/1/ui/message_center/views/messa...
ui/message_center/views/message_center_view.cc:644: int i = child_count() - 1;
It seems like we want the "i" just before the nth from the last valid child. 
Could you add that as a comment?

[Jun Mukai, 2013-06-19 23:49:59]

https://codereview.chromium.org/17382011/diff/1/ui/message_center/views/messa...
File ui/message_center/views/message_center_view.cc (right):

https://codereview.chromium.org/17382011/diff/1/ui/message_center/views/messa...
ui/message_center/views/message_center_view.cc:643: int
RichMessageListView::GetActualIndex(int index) {
On 2013/06/18 23:41:51, dewittj wrote:
> I wouldn't be surprised if IsValidChild starts to become a performance
> bottleneck.

it could be. I think the current implementation will be okay since it just
checks visible() (a boolean member) and existence for
deleting_views_/deleted_when_done_ which are both std::map with usually a few
members.

https://codereview.chromium.org/17382011/diff/1/ui/message_center/views/messa...
ui/message_center/views/message_center_view.cc:644: int i = child_count() - 1;
On 2013/06/18 23:41:51, dewittj wrote:
> It seems like we want the "i" just before the nth from the last valid child. 
> Could you add that as a comment?

done, also renamed 'i' to 'actual_index'. 'i' won't be a very good naming even
in this length of code.

[commit-bot: I haz the power, 2013-06-20 20:39:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mukai@chromium.org/17382011/9001

[commit-bot: I haz the power, 2013-06-20 21:05:23]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_aura.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-06-20 21:26:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mukai@chromium.org/17382011/12002

[commit-bot: I haz the power, 2013-06-20 22:48:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mukai@chromium.org/17382011/14002

[commit-bot: I haz the power, 2013-06-21 00:27:03]

Retried try job too often on win_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-06-21 00:31:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mukai@chromium.org/17382011/14002

[commit-bot: I haz the power, 2013-06-21 09:52:46]

Change committed as 207776