Reduce minimum size of Panels to allow 1-text-line tight autosize.

Reduce minimum size of Panels to allow 1-text-line tight autosize.

BUG=102708
TEST=PanelBrowserTest.SizeClamping

This patch does not attempt to calculate the actual size of titlebar and text line on a particular machine, just changes defaults to more accommodating.

Instead of 100x100 we now have 100x20. Width of 100 seems to be a good arbitrary value that allows reasonable rendering of titlebar with close button, favicon and few characters of title text. Height of 20 allows auto-sizing to tightly wrap around small content (like a text line)


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

[Dmitry Titov, 2011-11-22 00:08:29]

more try runs:
http://build.chromium.org/p/tryserver.chromium/builders/linux/builds/4051
http://build.chromium.org/p/tryserver.chromium/builders/win/builds/4340

[jennb, 2011-11-22 00:25:26]

Hmmm...maybe we should leave min values in .h and move default ones there too so
test can be a friend and access them.

http://codereview.chromium.org/8621002/diff/2001/chrome/browser/ui/panels/pan...
File chrome/browser/ui/panels/panel_browsertest.cc (right):

http://codereview.chromium.org/8621002/diff/2001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_browsertest.cc:1514: EXPECT_EQ(100,
panel->GetBounds().width());
Should fetched these numbers from panel_manager. That way the test will continue
to pass even if you decide to change to, say, 101x21.

http://codereview.chromium.org/8621002/diff/2001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_browsertest.cc:1523: EXPECT_EQ(240,
panel->GetBounds().width());
Ditto.

[Dmitry Titov, 2011-11-22 02:12:33]

Done all, and added one more test that verifies scenario I actually do this
change for :-)

[Dmitry Titov, 2011-11-23 04:26:43]

Merged, updated tests. PTAL.

[jennb, 2011-11-23 04:38:42]

lgtm

http://codereview.chromium.org/8621002/diff/9001/chrome/browser/ui/panels/pan...
File chrome/browser/ui/panels/panel_browsertest.cc (right):

http://codereview.chromium.org/8621002/diff/9001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_browsertest.cc:1501: CreatePanelParams
params("Panel", gfx::Rect(0, 0, 0, 0), SHOW_AS_ACTIVE);
gfx::Rect() is the same as passing all zeroes.

http://codereview.chromium.org/8621002/diff/9001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_browsertest.cc:1520: CreatePanelParams
params2("Panel2", gfx::Rect(0, 0, 0, 200), SHOW_AS_ACTIVE);
nit: use a var for the 200 literal.

http://codereview.chromium.org/8621002/diff/9001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_browsertest.cc:1530: CreatePanelParams
params("Panel", gfx::Rect(0, 0, 0, 0), SHOW_AS_ACTIVE);
gfx::Rect()

http://codereview.chromium.org/8621002/diff/9001/chrome/browser/ui/panels/pan...
File chrome/browser/ui/panels/panel_manager.cc (right):

http://codereview.chromium.org/8621002/diff/9001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_manager.cc:53: const int
PanelManager::kPanelMinWidth;
Are these needed?

[Dmitry Titov, 2011-11-23 18:47:49]

http://codereview.chromium.org/8621002/diff/9001/chrome/browser/ui/panels/pan...
File chrome/browser/ui/panels/panel_browsertest.cc (right):

http://codereview.chromium.org/8621002/diff/9001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_browsertest.cc:1501: CreatePanelParams
params("Panel", gfx::Rect(0, 0, 0, 0), SHOW_AS_ACTIVE);
On 2011/11/23 04:38:42, jennb wrote:
> gfx::Rect() is the same as passing all zeroes.

Done.

http://codereview.chromium.org/8621002/diff/9001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_browsertest.cc:1520: CreatePanelParams
params2("Panel2", gfx::Rect(0, 0, 0, 200), SHOW_AS_ACTIVE);
On 2011/11/23 04:38:42, jennb wrote:
> nit: use a var for the 200 literal.

Done.

http://codereview.chromium.org/8621002/diff/9001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_browsertest.cc:1530: CreatePanelParams
params("Panel", gfx::Rect(0, 0, 0, 0), SHOW_AS_ACTIVE);
On 2011/11/23 04:38:42, jennb wrote:
> gfx::Rect()

Done.

http://codereview.chromium.org/8621002/diff/9001/chrome/browser/ui/panels/pan...
File chrome/browser/ui/panels/panel_manager.cc (right):

http://codereview.chromium.org/8621002/diff/9001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_manager.cc:53: const int
PanelManager::kPanelMinWidth;
On 2011/11/23 04:38:42, jennb wrote:
> Are these needed?

Yes! Without these, linker often complains that it doesn't find the variable to
link with. It doesn't always inline, with rules being somewhat random-looking.
Sometimes it works fine (when inlining happens). In this case,
interactive_ui_tests can not be linked with library containing PanelManager w/o
these.

[Dmitry Titov, 2011-11-23 19:39:16]

Grrr 
Win failed compile with that usage of static int... Converting it to old-style.

[Dmitry Titov, 2011-11-23 20:38:16]

On 2011/11/23 19:39:16, Dmitry Titov wrote:
> Grrr 
> Win failed compile with that usage of static int... Converting it to
old-style.

The reason Win fails the compile:
https://groups.google.com/a/chromium.org/group/chromium-dev/browse_frm/thread...

[Dmitry Titov, 2011-11-24 00:38:04]

Jenn, could you take a look at pateches 6 and 7?

[jennb, 2011-11-24 01:04:56]

lgtm

http://codereview.chromium.org/8621002/diff/17002/chrome/test/data/extensions...
File chrome/test/data/extensions/api_test/window_update/show_state/test.html
(right):

http://codereview.chromium.org/8621002/diff/17002/chrome/test/data/extensions...
chrome/test/data/extensions/api_test/window_update/show_state/test.html:61: //
'height' parameter prevents 'panel' windows form attempt to compute size
asynchronously.
s/form/from

http://codereview.chromium.org/8621002/diff/17002/chrome/test/data/extensions...
chrome/test/data/extensions/api_test/window_update/show_state/test.html:62:
chrome.windows.create({'url': 'hello.html', 'type': windowType, 'height':100 },
might as well provide width too. it's pretty unusual to only provide one
dimension.

[commit-bot: I haz the power, 2011-11-24 01:13:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dimich@chromium.org/8621002/20005

[commit-bot: I haz the power, 2011-11-24 02:27:02]

Change committed as 111480