Enforce different min size for popups than tabbed browsers on MacOSX.

Enforce different min size for popups than tabbed browsers on MacOSX.

xib changes: Removed browser window minSize and minContentSize from xib file. Instead, set the min size programmatically in BrowserWindowController::awakeFromNib in order to customize the min size depending on the browser type.

BUG=83985
TEST=BrowserWindowControllerTest.TestSetBounds and TestSetBoundsPopup

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

[jennb, 2011-06-08 22:58:27]

+levin as I had to change his test.

[levin, 2011-06-08 23:04:24]

On 2011/06/08 22:58:27, jennb wrote:
> +levin as I had to change his test.

Test change ok.

[Nico, 2011-06-09 16:24:08]

http://codereview.chromium.org/7003063/diff/4001/chrome/browser/ui/cocoa/brow...
File chrome/browser/ui/cocoa/browser_window_cocoa.mm (right):

http://codereview.chromium.org/7003063/diff/4001/chrome/browser/ui/cocoa/brow...
chrome/browser/ui/cocoa/browser_window_cocoa.mm:79: gfx::Rect real_bounds =
[controller_ enforceMinWindowSize:bounds];
No abbreviations in method names.

http://codereview.chromium.org/7003063/diff/4001/chrome/browser/ui/cocoa/brow...
File chrome/browser/ui/cocoa/browser_window_controller.mm (right):

http://codereview.chromium.org/7003063/diff/4001/chrome/browser/ui/cocoa/brow...
chrome/browser/ui/cocoa/browser_window_controller.mm:470:
checked_bounds.set_height(minSize.height);
If you use setMinSize instead of setMinContentSize, is this still needed?

[jennb, 2011-06-09 16:44:54]

http://codereview.chromium.org/7003063/diff/4001/chrome/browser/ui/cocoa/brow...
File chrome/browser/ui/cocoa/browser_window_cocoa.mm (right):

http://codereview.chromium.org/7003063/diff/4001/chrome/browser/ui/cocoa/brow...
chrome/browser/ui/cocoa/browser_window_cocoa.mm:79: gfx::Rect real_bounds =
[controller_ enforceMinWindowSize:bounds];
On 2011/06/09 16:24:08, Nico wrote:
> No abbreviations in method names.

MinSize is used in many places. Prefer to keep the naming consistent.

http://codereview.chromium.org/7003063/diff/4001/chrome/browser/ui/cocoa/brow...
File chrome/browser/ui/cocoa/browser_window_controller.mm (right):

http://codereview.chromium.org/7003063/diff/4001/chrome/browser/ui/cocoa/brow...
chrome/browser/ui/cocoa/browser_window_controller.mm:470:
checked_bounds.set_height(minSize.height);
On 2011/06/09 16:24:08, Nico wrote:
> If you use setMinSize instead of setMinContentSize, is this still needed?

Yes, it's still needed. The .xib file used to set both, and yet SetBounds() was
able to bypass the min size (the cause of the bug I'm fixing).

I'll still change to use setMinSize. It makes it easier to see where the numbers
in the unittest comes from.

[Nico, 2011-06-09 16:54:35]

LGTM

When changing xib files,please add a comment like this to the CL description:
"xib changes: Added a frobolator view and connected its action to the -frobolate
method; moved the existing subtransbewler view 5 units to the right"

http://codereview.chromium.org/7003063/diff/3002/chrome/browser/ui/cocoa/brow...
File chrome/browser/ui/cocoa/browser_window_controller.h (right):

http://codereview.chromium.org/7003063/diff/3002/chrome/browser/ui/cocoa/brow...
chrome/browser/ui/cocoa/browser_window_controller.h:148: - (void)awakeFromNib;
awakeFromNib is usually not declared in .h files

http://codereview.chromium.org/7003063/diff/3002/chrome/browser/ui/cocoa/brow...
File chrome/browser/ui/cocoa/browser_window_controller.mm (right):

http://codereview.chromium.org/7003063/diff/3002/chrome/browser/ui/cocoa/brow...
chrome/browser/ui/cocoa/browser_window_controller.mm:464: gfx::Rect
checked_bounds = bounds;
since this is an objc method, this should be called checkedBounds

[jennb, 2011-06-09 17:54:51]

xib changes added to CL description.

http://codereview.chromium.org/7003063/diff/3002/chrome/browser/ui/cocoa/brow...
File chrome/browser/ui/cocoa/browser_window_controller.h (right):

http://codereview.chromium.org/7003063/diff/3002/chrome/browser/ui/cocoa/brow...
chrome/browser/ui/cocoa/browser_window_controller.h:148: - (void)awakeFromNib;
On 2011/06/09 16:54:35, Nico wrote:
> awakeFromNib is usually not declared in .h files

Removed. Good to know. Thx.

http://codereview.chromium.org/7003063/diff/3002/chrome/browser/ui/cocoa/brow...
File chrome/browser/ui/cocoa/browser_window_controller.mm (right):

http://codereview.chromium.org/7003063/diff/3002/chrome/browser/ui/cocoa/brow...
chrome/browser/ui/cocoa/browser_window_controller.mm:464: gfx::Rect
checked_bounds = bounds;
On 2011/06/09 16:54:35, Nico wrote:
> since this is an objc method, this should be called checkedBounds

Changed. Also fixed this style issue in unittest .mm file.

[Nico, 2011-06-09 17:56:18]

Thanks for the changes!

http://codereview.chromium.org/7003063/diff/3002/chrome/browser/ui/cocoa/brow...
File chrome/browser/ui/cocoa/browser_window_controller.mm (right):

http://codereview.chromium.org/7003063/diff/3002/chrome/browser/ui/cocoa/brow...
chrome/browser/ui/cocoa/browser_window_controller.mm:464: gfx::Rect
checked_bounds = bounds;
On 2011/06/09 17:54:51, jennb wrote:
> On 2011/06/09 16:54:35, Nico wrote:
> > since this is an objc method, this should be called checkedBounds
> 
> Changed. Also fixed this style issue in unittest .mm file.

In the mm file, the code is in a C++ class, so it should follow c++ naming there
:-P

It's not the filetype that determines it but if it's in an objc class or and c++
class.

Sorry that this is annoying :-/

[jennb, 2011-06-09 18:05:06]

On 2011/06/09 17:56:18, Nico wrote:
> Thanks for the changes!
> 
>
http://codereview.chromium.org/7003063/diff/3002/chrome/browser/ui/cocoa/brow...
> File chrome/browser/ui/cocoa/browser_window_controller.mm (right):
> 
>
http://codereview.chromium.org/7003063/diff/3002/chrome/browser/ui/cocoa/brow...
> chrome/browser/ui/cocoa/browser_window_controller.mm:464: gfx::Rect
> checked_bounds = bounds;
> On 2011/06/09 17:54:51, jennb wrote:
> > On 2011/06/09 16:54:35, Nico wrote:
> > > since this is an objc method, this should be called checkedBounds
> > 
> > Changed. Also fixed this style issue in unittest .mm file.
> 
> In the mm file, the code is in a C++ class, so it should follow c++ naming
there
> :-P
> 
> It's not the filetype that determines it but if it's in an objc class or and
c++
> class.
> 
> Sorry that this is annoying :-/

Ok, will change tests back before committing.