Add overflow indicator count on Windows.

Add overflow indicator count on Windows.

BUG=none
TEST=new tests


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

[jennb, 2011-12-19 22:24:50]

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel.cc
File chrome/browser/ui/panels/panel.cc (right):

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel....
chrome/browser/ui/panels/panel.cc:196:
manager()->OnPanelAttentionStateChanged(this);
Instead of everything going through the panel manager, you could use a new
NOTIFICATION and have the overflow strip observe it for all sources.

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel_...
File chrome/browser/ui/panels/panel_overflow_browsertest.cc (right):

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:23: const int
kMaxVisibleOnShrunkForTesting = 3;
"OnShrunk" and "OnExpanded" aren't as self-explanatory. Suggest keep the
original name and then have kMaxVisibleOverflowOfOverflowForTesting. We've been
calling the feature overflow-of-overflow. Maybe you can come up with something
better.

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:217: EXPECT_EQ(0,
panel_overflow_strip->overflow_indicator()->GetCount());
Do you have test coverage to check that if this count is 0, no "element" is
included in the UI of the overflow strip?

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:732:
IN_PROC_BROWSER_TEST_F(PanelOverflowBrowserTest, MAYBE_OverflowIndicatorCount) {
Need case for moving a big overflow panel into normal panel strip so that it
bumps >1 panel into overflow. Test does not cover any panels moving between
strips that affects the indicator count.

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:757: MakePanelName(5 +
i),
Should replace this 5 with size of panels vector. That way this doesn't break if
line 744 changes. You can still do +i for the name, but you'd have to loop from
i = 0 while i < 5.

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:774: EXPECT_EQ(1,
overflow_indicator->GetCount());
Check that one of the overflow-on-overflow panels is now visible.

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:776: // Shrunk the
overflow area by moving mouse over it.
Stop hovering the mouse over the overflow area.

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:780: EXPECT_EQ(4,
overflow_indicator->GetCount());
Verify overflow-of-overflow panels no longer visible.

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:792:
panel_manager->enable_auto_sizing(true);
This surprised me in a draw attention test. I actually thought it was a
cut-and-paste error. Maybe you can bump a panel to overflow using other means,
like moving a large overflow panel into the panel strip?

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:862: // Stop drawing
attention for anther overflow-on-overflow panel by activating
anther

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel_...
File chrome/browser/ui/panels/panel_overflow_indicator.h (right):

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_overflow_indicator.h:13: class
PanelOverflowIndicator {
Class comment please.

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel_...
File chrome/browser/ui/panels/panel_overflow_indicator_view.h (right):

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_overflow_indicator_view.h:40: int count_;
count_ and is_drawing_attention_ should be the same across all platforms. Should
the base class have these in the basic implementation and call a virtual method
for the subclass to add platform-specific logic?

[jianli, 2011-12-20 01:20:41]

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel.cc
File chrome/browser/ui/panels/panel.cc (right):

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel....
chrome/browser/ui/panels/panel.cc:196:
manager()->OnPanelAttentionStateChanged(this);
On 2011/12/19 22:24:50, jennb wrote:
> Instead of everything going through the panel manager, you could use a new
> NOTIFICATION and have the overflow strip observe it for all sources.

Per discussion, we leave it as it is. We will refactor it when we have better
understanding on the road.

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel_...
File chrome/browser/ui/panels/panel_overflow_browsertest.cc (right):

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:23: const int
kMaxVisibleOnShrunkForTesting = 3;
On 2011/12/19 22:24:50, jennb wrote:
> "OnShrunk" and "OnExpanded" aren't as self-explanatory. Suggest keep the
> original name and then have kMaxVisibleOverflowOfOverflowForTesting. We've
been
> calling the feature overflow-of-overflow. Maybe you can come up with something
> better. 

Done.

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:217: EXPECT_EQ(0,
panel_overflow_strip->overflow_indicator()->GetCount());
On 2011/12/19 22:24:50, jennb wrote:
> Do you have test coverage to check that if this count is 0, no "element" is
> included in the UI of the overflow strip?

Done.

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:732:
IN_PROC_BROWSER_TEST_F(PanelOverflowBrowserTest, MAYBE_OverflowIndicatorCount) {
On 2011/12/19 22:24:50, jennb wrote:
> Need case for moving a big overflow panel into normal panel strip so that it
> bumps >1 panel into overflow. Test does not cover any panels moving between
> strips that affects the indicator count.

Done.

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:757: MakePanelName(5 +
i),
On 2011/12/19 22:24:50, jennb wrote:
> Should replace this 5 with size of panels vector. That way this doesn't break
if
> line 744 changes. You can still do +i for the name, but you'd have to loop
from
> i = 0 while i < 5.

Done.

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:774: EXPECT_EQ(1,
overflow_indicator->GetCount());
On 2011/12/19 22:24:50, jennb wrote:
> Check that one of the overflow-on-overflow panels is now visible.

Done.

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:776: // Shrunk the
overflow area by moving mouse over it.
On 2011/12/19 22:24:50, jennb wrote:
> Stop hovering the mouse over the overflow area.

Done.

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:780: EXPECT_EQ(4,
overflow_indicator->GetCount());
On 2011/12/19 22:24:50, jennb wrote:
> Verify overflow-of-overflow panels no longer visible.

Done.

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:792:
panel_manager->enable_auto_sizing(true);
On 2011/12/19 22:24:50, jennb wrote:
> This surprised me in a draw attention test. I actually thought it was a
> cut-and-paste error. Maybe you can bump a panel to overflow using other means,
> like moving a large overflow panel into the panel strip?

Done.

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:862: // Stop drawing
attention for anther overflow-on-overflow panel by activating
On 2011/12/19 22:24:50, jennb wrote:
> anther

Done.

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel_...
File chrome/browser/ui/panels/panel_overflow_indicator.h (right):

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_overflow_indicator.h:13: class
PanelOverflowIndicator {
On 2011/12/19 22:24:50, jennb wrote:
> Class comment please.

Done.

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel_...
File chrome/browser/ui/panels/panel_overflow_indicator_view.h (right):

http://codereview.chromium.org/8953040/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_overflow_indicator_view.h:40: int count_;
On 2011/12/19 22:24:50, jennb wrote:
> count_ and is_drawing_attention_ should be the same across all platforms.
Should
> the base class have these in the basic implementation and call a virtual
method
> for the subclass to add platform-specific logic?

Per discussion, we leave as it is now. Will deal with this if we need to share
the common code for other platforms.

[jennb, 2011-12-20 02:08:22]

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
File chrome/browser/ui/panels/panel_overflow_browsertest.cc (right):

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:23: const int
kMaxVisiblePanelsForTesting = 3;
Prefer to have overflow in the name of these 2 as there are definitely more
than3 panels visible.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:271:
EXPECT_FALSE(panel_overflow_strip->overflow_indicator() &&
Don't need the GetCount() check. Just verify that there should not be an
indicator.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:285: EXPECT_EQ(0,
panel_overflow_strip->overflow_indicator()->GetCount());
Should still be no indicator as you don't need it yet.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:825: EXPECT_EQ(0,
overflow_indicator->GetCount());
Expect no indicator.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:831: int
num_existing_panels = static_cast<int>(panels.size());
panel_manager->num_panels() ? then you won't have to cast.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:867: // Shrunk the
overflow area by stopping hovering the mouse over the overflow
Shrink

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:893: // oevrflow area and
also get the top visivle overflow panel bumped to the
visivle

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:911:
IN_PROC_BROWSER_TEST_F(PanelOverflowBrowserTest, MAYBE_DrawOverflowAttention) {
Very thorough coverage of many scenarios!

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:932: // Draw attention
for an overflow panel.
a visible overflow panel

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
File chrome/browser/ui/panels/panel_overflow_strip.cc (right):

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_strip.cc:21: // Maximium number of
visible overflow panels when the overflow area is shrunk.
Still using shrunk?

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_strip.cc:24: // Height to display-area
ratio that maximium number of visible overflow panels
Lumpy sentence. Phrasing it like the comment you wrote in the Update method for
this will improve it.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_strip.cc:40: registrar_.Add(this,
content::NOTIFICATION_APP_TERMINATING,
Comment here about why we register for this.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_strip.cc:113:
overflow_indicator_.reset();
Seems odd to reset the indicator only to recreate it again in line 115. Maybe
the indicator should not exist if overflow panels do not exceed
max_visible_overflow_panels_?

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_strip.cc:170:
UpdateMaxVisibkePanelsOnHover();
Shouldn't need to recompute this every time a panel is added/removed from
overflow. The max should be unchanged as long as display area does not change.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_strip.cc:194: is_drawing_attention =
true;
break out of loop as soon as you find one drawing attention

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_strip.cc:307:
UpdateOverflowIndicator(current_display_width);
Count won't be changing during animation. Maybe UpdateOverflowIndicator is doing
too much in one function? Sometimes, you will only need to update the count and
not the bounds and sometimes, just the bounds. Perhaps separate the logic?

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_strip.cc:316: // The overflow indicator
needs to be disposed when the last browser is
Don't need this if you destroy the indicator when panels are removed.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
File chrome/browser/ui/panels/panel_overflow_strip.h (right):

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_strip.h:49: void
OnPanelAttentionStateChanged(Panel* panel);
You already have to make this class an observer in this change so it's not that
much more overhead to observe the draw attention changed notification.
Reconsider?

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_strip.h:65: PanelOverflowIndicator*
overflow_indicator() const {
Comment. Does this return NULL sometimes?

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_strip.h:91: void
UpdateMaxVisibkePanelsOnHover();
typo

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_strip.h:96: void
UpdateOverflowIndicator(int width);
Comment. I was expecting the param to be 'count'. Not sure what 'width' would be
from just looking at the .h.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_strip.h:134: // hovers over the overflow
area and cause it to be expanded.
s/cause/causes

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_strip.h:137: // User to show "+N"
indicator when number of overflow panels exceed
s/User/Used

[jianli, 2011-12-20 22:08:10]

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
File chrome/browser/ui/panels/panel_overflow_browsertest.cc (right):

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:23: const int
kMaxVisiblePanelsForTesting = 3;
On 2011/12/20 02:08:23, jennb wrote:
> Prefer to have overflow in the name of these 2 as there are definitely more
> than3 panels visible.

Done.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:271:
EXPECT_FALSE(panel_overflow_strip->overflow_indicator() &&
On 2011/12/20 02:08:23, jennb wrote:
> Don't need the GetCount() check. Just verify that there should not be an
> indicator.

Done.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:285: EXPECT_EQ(0,
panel_overflow_strip->overflow_indicator()->GetCount());
On 2011/12/20 02:08:23, jennb wrote:
> Should still be no indicator as you don't need it yet.

Done.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:825: EXPECT_EQ(0,
overflow_indicator->GetCount());
On 2011/12/20 02:08:23, jennb wrote:
> Expect no indicator.

Done.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:831: int
num_existing_panels = static_cast<int>(panels.size());
On 2011/12/20 02:08:23, jennb wrote:
> panel_manager->num_panels() ? then you won't have to cast.

Done.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:867: // Shrunk the
overflow area by stopping hovering the mouse over the overflow
On 2011/12/20 02:08:23, jennb wrote:
> Shrink

Done.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:893: // oevrflow area and
also get the top visivle overflow panel bumped to the
On 2011/12/20 02:08:23, jennb wrote:
> visivle

Done.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_browsertest.cc:932: // Draw attention
for an overflow panel.
On 2011/12/20 02:08:23, jennb wrote:
> a visible overflow panel

Done.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
File chrome/browser/ui/panels/panel_overflow_strip.cc (right):

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_strip.cc:21: // Maximium number of
visible overflow panels when the overflow area is shrunk.
On 2011/12/20 02:08:23, jennb wrote:
> Still using shrunk?

Done.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_strip.cc:24: // Height to display-area
ratio that maximium number of visible overflow panels
On 2011/12/20 02:08:23, jennb wrote:
> Lumpy sentence. Phrasing it like the comment you wrote in the Update method
for
> this will improve it.

Done.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_strip.cc:40: registrar_.Add(this,
content::NOTIFICATION_APP_TERMINATING,
On 2011/12/20 02:08:23, jennb wrote:
> Comment here about why we register for this.

Not needed since we do not need observer now.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_strip.cc:113:
overflow_indicator_.reset();
On 2011/12/20 02:08:23, jennb wrote:
> Seems odd to reset the indicator only to recreate it again in line 115. Maybe
> the indicator should not exist if overflow panels do not exceed
> max_visible_overflow_panels_?

Changed to create the overflow indicator on a as-needed basis.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_strip.cc:170:
UpdateMaxVisibkePanelsOnHover();
On 2011/12/20 02:08:23, jennb wrote:
> Shouldn't need to recompute this every time a panel is added/removed from
> overflow. The max should be unchanged as long as display area does not change.

Updated. When display area is changed, we might not be able to compute this max
value because no overflow panel is added yet and the max value is computed based
on the overflow panel's height.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_strip.cc:194: is_drawing_attention =
true;
On 2011/12/20 02:08:23, jennb wrote:
> break out of loop as soon as you find one drawing attention

Done.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_strip.cc:307:
UpdateOverflowIndicator(current_display_width);
On 2011/12/20 02:08:23, jennb wrote:
> Count won't be changing during animation. Maybe UpdateOverflowIndicator is
doing
> too much in one function? Sometimes, you will only need to update the count
and
> not the bounds and sometimes, just the bounds. Perhaps separate the logic?

Separated the logic as suggested.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_strip.cc:316: // The overflow indicator
needs to be disposed when the last browser is
On 2011/12/20 02:08:23, jennb wrote:
> Don't need this if you destroy the indicator when panels are removed.

Removed.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
File chrome/browser/ui/panels/panel_overflow_strip.h (right):

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_strip.h:49: void
OnPanelAttentionStateChanged(Panel* panel);
On 2011/12/20 02:08:23, jennb wrote:
> You already have to make this class an observer in this change so it's not
that
> much more overhead to observe the draw attention changed notification.
> Reconsider?

As discussed, we do not need to use the observer any more and I rather
delegating the logic to PanelOverflowStrip in current way since it is simpler.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_strip.h:65: PanelOverflowIndicator*
overflow_indicator() const {
On 2011/12/20 02:08:23, jennb wrote:
> Comment. Does this return NULL sometimes?

Done.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_strip.h:91: void
UpdateMaxVisibkePanelsOnHover();
On 2011/12/20 02:08:23, jennb wrote:
> typo

Done.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_strip.h:96: void
UpdateOverflowIndicator(int width);
On 2011/12/20 02:08:23, jennb wrote:
> Comment. I was expecting the param to be 'count'. Not sure what 'width' would
be
> from just looking at the .h.

'width' parameter is not needed any more.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_strip.h:134: // hovers over the overflow
area and cause it to be expanded.
On 2011/12/20 02:08:23, jennb wrote:
> s/cause/causes

Done.

http://codereview.chromium.org/8953040/diff/6001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_overflow_strip.h:137: // User to show "+N"
indicator when number of overflow panels exceed
On 2011/12/20 02:08:23, jennb wrote:
> s/User/Used

Done.

[Dmitry Titov, 2011-12-20 23:11:37]

lgtm on behalf of Jenn, lets wait for try bots as well.

[commit-bot: I haz the power, 2011-12-21 08:18:34]

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

[commit-bot: I haz the power, 2011-12-21 10:23:03]

Change committed as 115297