Reunite PanelStrip::AddPanel and its internals.

Reunite PanelStrip::AddPanel and its internals.

Was causing confusion with code within the file calling the helper functions directly.

Also put back accidentally deleted line and re-enabled NoOverlapping test.

BUG=None
TEST=Panel*Test.*

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

[jennb, 2011-12-02 01:27:26]

Here you go. This should resolve our confusion.

[dcheng, 2011-12-02 01:29:20]

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

http://codereview.chromium.org/8772029/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_strip.cc:143:
overflow_action_factory_.GetWeakPtr(),
It may be better to remove the WeakPtrFactory for now since we don't need
cancellation semantics.

[jianli, 2011-12-02 01:56:38]

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

http://codereview.chromium.org/8772029/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_strip.cc:87: DCHECK_EQ(Panel::EXPANDED,
panel->expansion_state());
This might not be true if |panel| is overflow panel.

http://codereview.chromium.org/8772029/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_strip.cc:146:
kMoveNewPanelToOverflowDelayMilliseconds);
Can you check remove_delays_for_testing_ here and use 0 for this case?

[jennb, 2011-12-02 02:20:37]

Also put back the accidentally deleted call to OnPanelRemoved().

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

http://codereview.chromium.org/8772029/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_strip.cc:87: DCHECK_EQ(Panel::EXPANDED,
panel->expansion_state());
On 2011/12/02 01:56:38, jianli wrote:
> This might not be true if |panel| is overflow panel.

Removed check for now as your patch will have logic for this.

http://codereview.chromium.org/8772029/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_strip.cc:143:
overflow_action_factory_.GetWeakPtr(),
On 2011/12/02 01:29:20, dcheng wrote:
> It may be better to remove the WeakPtrFactory for now since we don't need
> cancellation semantics.

Done.

http://codereview.chromium.org/8772029/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_strip.cc:146:
kMoveNewPanelToOverflowDelayMilliseconds);
On 2011/12/02 01:56:38, jianli wrote:
> Can you check remove_delays_for_testing_ here and use 0 for this case?

Done.

[jennb, 2011-12-02 18:23:55]

Also re-enable NoOverlapping test in this patch.

[jianli, 2011-12-02 18:26:10]

lgtm

[jennb, 2011-12-02 21:19:35]

Try job results, for the record:

http://build.chromium.org/p/tryserver.chromium/builders/win/builds/6350
http://build.chromium.org/p/tryserver.chromium/builders/mac/builds/5927
http://build.chromium.org/p/tryserver.chromium/builders/linux/builds/5993