Make avatar menu bubble as singleton and destroy all the menu items and menu model when bubble cl...

Gtk:AvatarMenu:Destroy AvatarMenuBubble object properly when bubble closes.

BUG=101243
TEST=As described in the bug.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=107156

[SanjoyPal, 2011-10-22 05:11:22]

Previously, we were not properly destoying AvatarMenuBubble object, so its
containing objects AvatarMenuModel was not getting destroyed.
AvatarMenuModel keeps a browser object pointer which was becoming stale after
deleting corresponding browser window and when stale AvatarMenuModel observer
was getting called ,it was crashing in AvatarMenuModel::RebuildMenu.

Please review the fix. Thanks.

[sail, 2011-10-24 16:38:46]

Hi Sanjoy. Thanks for taking a look at this.

Could you investigate why the bubble was not being destroyed. On Windows and Mac
we destroy the bubble when it closes.

I think fixing this would be better than making it a singleton.

Thanks!

[SanjoyPal, 2011-10-25 08:06:45]

Hi Sailesh,

I investigated the issue, please follow my comments below

On 2011/10/24 16:38:46, sail wrote:
> Hi Sanjoy. Thanks for taking a look at this.
> 
> Could you investigate why the bubble was not being destroyed. On Windows and
Mac
> we destroy the bubble when it closes.

No, we do not destroy the AvatarMenuBubble object when bubble closes and its
destructor is not getting called. We just destroy Gtk window when bubble closes.
We need to connect to the "destroy" event of the window and destroy the object
properly. That is what I am doing in my patch. (referrence: BookmarkBubbleGtk)

> 
> I think fixing this would be better than making it a singleton.

Yes, making bubble a singleton is NOT an absolute necessity to fix this issue,
but i thought as this bubble is an app modal then it can have only one instance
at a time.(again referrence: BookmarkBubbleGtk)

As per your suggestion i removed the code which makes it a singleton in Patch
set 2. 

Please review the patch.

> 
> Thanks!

Thanks.

[sail, 2011-10-25 16:47:57]

LGTM!
Please verify that the bug is fixed before committing.
Also, please merge to the M16 branch (912). Let me know if you'd like me to do
it.

Thanks!

[commit-bot: I haz the power, 2011-10-25 16:52:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ncj674@motorola.com/8370021/8001

[Evan Stade, 2011-10-25 16:53:43]

ideally, you would let a patch bake for at least several canaries before merging
to the branch. And I believe non-committers have no way of merging.

[sail, 2011-10-25 16:59:40]

On 2011/10/25 16:53:43, Evan Stade wrote:
> ideally, you would let a patch bake for at least several canaries before
merging
> to the branch. And I believe non-committers have no way of merging.

Sounds good. I'll keep the bug open and merge in a few days.

[commit-bot: I haz the power, 2011-10-25 18:08:51]

Change committed as 107156