DevTools: respect minimum width/height when resizing.

DevTools: respect minimum width/height when resizing.

BUG=196566
TEST=views_unittests.SingleSplitViewTest

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=190209

[dgozman, 2013-03-15 15:13:24]

Take a look, please.

[dgozman, 2013-03-19 09:56:31]

ping

[pfeldman, 2013-03-19 10:42:06]

devtools lgtm

https://codereview.chromium.org/12871013/diff/7/chrome/browser/devtools/devto...
File chrome/browser/devtools/devtools_window.cc (right):

https://codereview.chromium.org/12871013/diff/7/chrome/browser/devtools/devto...
chrome/browser/devtools/devtools_window.cc:93: const int kMinDevToolsHeight =
23;
I'd leave this 50.

https://codereview.chromium.org/12871013/diff/7/chrome/browser/ui/views/frame...
File chrome/browser/ui/views/frame/browser_view.cc (right):

https://codereview.chromium.org/12871013/diff/7/chrome/browser/ui/views/frame...
chrome/browser/ui/views/frame/browser_view.cc:2233: gfx::Size
minDevtoolsSize(devtools_window_->GetMinimumWidth(),
min_devtools_size

[dgozman, 2013-03-19 11:34:35]

sky, please review changes in all UI paths

[sky, 2013-03-19 14:40:50]

+avi mac
+erg gtk

https://codereview.chromium.org/12871013/diff/7/chrome/browser/ui/views/frame...
File chrome/browser/ui/views/frame/browser_view.cc (right):

https://codereview.chromium.org/12871013/diff/7/chrome/browser/ui/views/frame...
chrome/browser/ui/views/frame/browser_view.cc:2235:
devtools_container_->SetPreferredSize(minDevtoolsSize);
This changes the preferred size, are you sure you want that?

[Avi (use Gerrit), 2013-03-19 14:43:23]

I'm all nitpicky about style and stuff but if fixed it lgtm

https://codereview.chromium.org/12871013/diff/7/chrome/browser/ui/cocoa/dev_t...
File chrome/browser/ui/cocoa/dev_tools_controller.mm (right):

https://codereview.chromium.org/12871013/diff/7/chrome/browser/ui/cocoa/dev_t...
chrome/browser/ui/cocoa/dev_tools_controller.mm:232: -
(CGFloat)splitView:(NSSplitView *)splitView
no space before *

https://codereview.chromium.org/12871013/diff/7/chrome/browser/ui/cocoa/dev_t...
chrome/browser/ui/cocoa/dev_tools_controller.mm:234:
ofSubviewAt:(NSInteger)dividerIndex {
If you need to indent the "constrain" line, OK, but align "ofsubviewat" to the
first line.

[Elliot Glaysher, 2013-03-19 16:48:44]

gtk lgtm

[dgozman, 2013-03-21 09:55:25]

sky, please take another look. I've also changed SingleSplitView.

https://codereview.chromium.org/12871013/diff/7/chrome/browser/devtools/devto...
File chrome/browser/devtools/devtools_window.cc (right):

https://codereview.chromium.org/12871013/diff/7/chrome/browser/devtools/devto...
chrome/browser/devtools/devtools_window.cc:93: const int kMinDevToolsHeight =
23;
On 2013/03/19 10:42:06, pfeldman wrote:
> I'd leave this 50.

Done.

https://codereview.chromium.org/12871013/diff/7/chrome/browser/ui/cocoa/dev_t...
File chrome/browser/ui/cocoa/dev_tools_controller.mm (right):

https://codereview.chromium.org/12871013/diff/7/chrome/browser/ui/cocoa/dev_t...
chrome/browser/ui/cocoa/dev_tools_controller.mm:232: -
(CGFloat)splitView:(NSSplitView *)splitView
On 2013/03/19 14:43:23, Avi wrote:
> no space before *

Done.

https://codereview.chromium.org/12871013/diff/7/chrome/browser/ui/cocoa/dev_t...
chrome/browser/ui/cocoa/dev_tools_controller.mm:232: -
(CGFloat)splitView:(NSSplitView *)splitView
On 2013/03/19 14:43:23, Avi wrote:
> no space before *

Done.

https://codereview.chromium.org/12871013/diff/7/chrome/browser/ui/cocoa/dev_t...
chrome/browser/ui/cocoa/dev_tools_controller.mm:234:
ofSubviewAt:(NSInteger)dividerIndex {
On 2013/03/19 14:43:23, Avi wrote:
> If you need to indent the "constrain" line, OK, but align "ofsubviewat" to the
> first line.

I did as in method above. Also, style guide suggests to align all but first
parameters.

https://codereview.chromium.org/12871013/diff/7/chrome/browser/ui/views/frame...
File chrome/browser/ui/views/frame/browser_view.cc (right):

https://codereview.chromium.org/12871013/diff/7/chrome/browser/ui/views/frame...
chrome/browser/ui/views/frame/browser_view.cc:2233: gfx::Size
minDevtoolsSize(devtools_window_->GetMinimumWidth(),
On 2013/03/19 10:42:06, pfeldman wrote:
> min_devtools_size

Done.

https://codereview.chromium.org/12871013/diff/7/chrome/browser/ui/views/frame...
chrome/browser/ui/views/frame/browser_view.cc:2235:
devtools_container_->SetPreferredSize(minDevtoolsSize);
On 2013/03/19 14:40:50, sky wrote:
> This changes the preferred size, are you sure you want that?

I did not found another way to set minimum size for WebView, and subclassing
looks unnecessary.

[sky, 2013-03-21 15:36:30]

The changes LGTM, but please add test coverage of SingleSplitView.

[dgozman, 2013-03-22 14:25:02]

Thanks for review. Modified test to check for minimum sizes.

[commit-bot: I haz the power, 2013-03-22 14:25:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dgozman@chromium.org/12871013/35002

[commit-bot: I haz the power, 2013-03-22 19:00:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dgozman@chromium.org/12871013/35002

[commit-bot: I haz the power, 2013-03-23 15:42:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dgozman@chromium.org/12871013/35002

[commit-bot: I haz the power, 2013-03-24 05:26:44]

Change committed as 190209