Fix 117074: Panels [WIN]: Crash data shows a stack trace consistently involving PanelManager

Fix 117074: Panels [WIN]: Crash data shows a stack trace consistently involving PanelManager

The fix is not to involve PanelManager when it is not needed on the balloon notification code.

BUG=117074
TEST=Manual test to verify PanelManager is not involved on start.


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

[jennb, 2012-03-08 04:34:18]

LGTM with nits

http://codereview.chromium.org/9621020/diff/2001/chrome/browser/notifications...
File chrome/browser/notifications/balloon_collection_impl.cc (right):

http://codereview.chromium.org/9621020/diff/2001/chrome/browser/notifications...
chrome/browser/notifications/balloon_collection_impl.cc:415: if
(!panel_offset_enabled_)
Does BalloonCollection always exist? No chance of a missed panel notification?

http://codereview.chromium.org/9621020/diff/2001/chrome/browser/notifications...
File chrome/browser/notifications/balloon_collection_impl.h (right):

http://codereview.chromium.org/9621020/diff/2001/chrome/browser/notifications...
chrome/browser/notifications/balloon_collection_impl.h:168: bool
panel_offset_enabled_;
panel_offset_enabled_ is rather vague. How about need_to_compute_panel_offset_ ?

[jennb, 2012-03-08 06:23:08]

On 2012/03/08 04:34:18, jennb wrote:
> LGTM with nits
> 
>
http://codereview.chromium.org/9621020/diff/2001/chrome/browser/notifications...
> File chrome/browser/notifications/balloon_collection_impl.cc (right):
> 
>
http://codereview.chromium.org/9621020/diff/2001/chrome/browser/notifications...
> chrome/browser/notifications/balloon_collection_impl.cc:415: if
> (!panel_offset_enabled_)
> Does BalloonCollection always exist? No chance of a missed panel notification?
> 
>
http://codereview.chromium.org/9621020/diff/2001/chrome/browser/notifications...
> File chrome/browser/notifications/balloon_collection_impl.h (right):
> 
>
http://codereview.chromium.org/9621020/diff/2001/chrome/browser/notifications...
> chrome/browser/notifications/balloon_collection_impl.h:168: bool
> panel_offset_enabled_;
> panel_offset_enabled_ is rather vague. How about need_to_compute_panel_offset_
?

Still LGTM.

Another thing to change would be to remove OnDisplayChanged() from
PanelManager's constructor. We already call it when the first panel is created,
so that's sufficient.

[jianli, 2012-03-08 18:26:45]

https://chromiumcodereview.appspot.com/9621020/diff/2001/chrome/browser/notif...
File chrome/browser/notifications/balloon_collection_impl.cc (right):

https://chromiumcodereview.appspot.com/9621020/diff/2001/chrome/browser/notif...
chrome/browser/notifications/balloon_collection_impl.cc:415: if
(!panel_offset_enabled_)
On 2012/03/08 04:34:18, jennb wrote:
> Does BalloonCollection always exist? No chance of a missed panel notification?

BalloonCollection is created when Chrome is started. When 1st panel is created,
we will receive a notification and turn on panel offset computation here.

https://chromiumcodereview.appspot.com/9621020/diff/2001/chrome/browser/notif...
File chrome/browser/notifications/balloon_collection_impl.h (right):

https://chromiumcodereview.appspot.com/9621020/diff/2001/chrome/browser/notif...
chrome/browser/notifications/balloon_collection_impl.h:168: bool
panel_offset_enabled_;
On 2012/03/08 04:34:18, jennb wrote:
> panel_offset_enabled_ is rather vague. How about need_to_compute_panel_offset_
?

Done.

[commit-bot: I haz the power, 2012-03-08 18:27:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jianli@chromium.org/9621020/4004

[commit-bot: I haz the power, 2012-03-08 20:57:35]

Change committed as 125679