[Win] Update HWND style when SizeConstraintsChanged.

[Win] Update HWND style when SizeConstraintsChanged.

Previously, caption buttons were set at creation time.
With this CL, minimize and maximize buttons can be added and
removed throughout the lifetime of a window.

BUG=412241
Committed: https://crrev.com/5150e6e0fdab64a3fe08d63646121958048b78b1
Cr-Commit-Position: refs/heads/master@{#296617}

[jackhou1, 2014-09-18 08:42:06]

jackhou@chromium.org changed reviewers:
+ sky@chromium.org

[jackhou1, 2014-09-18 08:42:07]

sky, could you please review this?

Also, would you know if there's an overarching reason why we don't show the
minimize button when the window can't maximized? I'm aware of
http://crbug.com/341010, but we have the same behavior on CrOS. Is it just for
consistency?

[sky, 2014-09-18 14:27:10]

https://codereview.chromium.org/581953002/diff/20001/ui/views/widget/widget_h...
File ui/views/widget/widget_hwnd_utils.cc (right):

https://codereview.chromium.org/581953002/diff/20001/ui/views/widget/widget_h...
ui/views/widget/widget_hwnd_utils.cc:89: *style &= ~(WS_MINIMIZEBOX |
WS_MAXIMIZEBOX);
Why remove the minimize button if the window can't maximize?

[jackhou1, 2014-09-18 23:14:17]

The CQ bit was checked by jackhou@chromium.org

[jackhou1, 2014-09-18 23:14:18]

The CQ bit was unchecked by jackhou@chromium.org

[jackhou1, 2014-09-18 23:28:35]

https://codereview.chromium.org/581953002/diff/20001/ui/views/widget/widget_h...
File ui/views/widget/widget_hwnd_utils.cc (right):

https://codereview.chromium.org/581953002/diff/20001/ui/views/widget/widget_h...
ui/views/widget/widget_hwnd_utils.cc:89: *style &= ~(WS_MINIMIZEBOX |
WS_MAXIMIZEBOX);
On 2014/09/18 14:27:09, sky wrote:
> Why remove the minimize button if the window can't maximize?

Hah, I was hoping you would know. There's this bug, http://crbug.com/341010,
referenced in HWNDMessageHandler::OnInitMenu. Essentially we want to disable the
Minimize in the right click menu to prevent certain dialog boxes from being
minimizable.

But it seems that throughout Windows and CrOS, we always hide the minimize
button when a window is not maximizable. Any idea who might know more about
this?

This CL maintains the existing semantics. If I can't find out why we do this,
I'll put together another CL that changes it.

[sky, 2014-09-19 02:48:55]

sky@chromium.org changed reviewers:
+ ananta@chromium.org

[sky, 2014-09-19 02:49:13]

+ananta
I'm sure we could disable the maximize menu item if worse comes to worse.

[ananta, 2014-09-20 01:16:22]

On 2014/09/19 02:49:13, sky wrote:
> +ananta
> I'm sure we could disable the maximize menu item if worse comes to worse.

The minimize button should not be dependent on the maximize button. Will look
into why our code is broken the way it is.

[jackhou1, 2014-09-20 02:48:53]

On 2014/09/20 01:16:22, ananta wrote:
> On 2014/09/19 02:49:13, sky wrote:
> > +ananta
> > I'm sure we could disable the maximize menu item if worse comes to worse.
> 
> The minimize button should not be dependent on the maximize button. Will look
> into why our code is broken the way it is.

It sounds like we could fix these cases by adding CanMinimize to views::Widget.
See https://codereview.chromium.org/553333007/ and http://crbug.com/328241. I'll
put together another CL to do that.

ananta, sky, do you have any more comments for this CL?

[jackhou1, 2014-09-24 11:40:34]

sky, ananta, PTAL

Both minimize and maximize buttons now update based on
WidgetDelegate::Can[Min|Max]imize.

[sky, 2014-09-24 15:33:00]

LGTM

https://codereview.chromium.org/581953002/diff/60001/ui/views/win/hwnd_messag...
File ui/views/win/hwnd_message_handler.cc (right):

https://codereview.chromium.org/581953002/diff/60001/ui/views/win/hwnd_messag...
ui/views/win/hwnd_message_handler.cc:878: style &= ~WS_THICKFRAME |
WS_MAXIMIZEBOX;
Can you put () around WS_THICKFRAME | WS_MAXIMIZEBOX, eg ~(WS_THICKFRAME |
WS_MAXIMIZEBOX) ?

[ananta, 2014-09-24 18:13:06]

On 2014/09/20 01:16:22, ananta wrote:
> On 2014/09/19 02:49:13, sky wrote:
> > +ananta
> > I'm sure we could disable the maximize menu item if worse comes to worse.
> 
> The minimize button should not be dependent on the maximize button. Will look
> into why our code is broken the way it is.

The use case mentioned in http://crbug.com/341010 brings up a modal window which
causes the main 
browser window to be disabled. I don't think modal dialogs should allow
themselves to be minimized.
The browser window should also not get minimized in this case.

[jackhou1, 2014-09-24 23:30:27]

On 2014/09/24 18:13:06, ananta wrote:
> On 2014/09/20 01:16:22, ananta wrote:
> > On 2014/09/19 02:49:13, sky wrote:
> > > +ananta
> > > I'm sure we could disable the maximize menu item if worse comes to worse.
> > 
> > The minimize button should not be dependent on the maximize button. Will
look
> > into why our code is broken the way it is.
> 
> The use case mentioned in http://crbug.com/341010 brings up a modal window
which
> causes the main 
> browser window to be disabled. I don't think modal dialogs should allow
> themselves to be minimized.
> The browser window should also not get minimized in this case.

I've checked the case in http://crbug.com/341010 and neither the modal window
nor the browser window are minimizable.

[jackhou1, 2014-09-24 23:30:35]

https://codereview.chromium.org/581953002/diff/60001/ui/views/win/hwnd_messag...
File ui/views/win/hwnd_message_handler.cc (right):

https://codereview.chromium.org/581953002/diff/60001/ui/views/win/hwnd_messag...
ui/views/win/hwnd_message_handler.cc:878: style &= ~WS_THICKFRAME |
WS_MAXIMIZEBOX;
On 2014/09/24 15:33:00, sky wrote:
> Can you put () around WS_THICKFRAME | WS_MAXIMIZEBOX, eg ~(WS_THICKFRAME |
> WS_MAXIMIZEBOX) ?

Oh, good catch.

[ananta, 2014-09-24 23:57:48]

lgtm

[jackhou1, 2014-09-25 00:09:55]

The CQ bit was checked by jackhou@chromium.org

[commit-bot: I haz the power, 2014-09-25 00:12:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/581953002/80001

[commit-bot: I haz the power, 2014-09-25 01:48:22]

Committed patchset #5 (id:80001) as 9e5983211a9decaa12b46284aa6701f821ee8681

[commit-bot: I haz the power, 2014-09-25 01:49:01]

Patchset 5 (id:??) landed as
https://crrev.com/5150e6e0fdab64a3fe08d63646121958048b78b1
Cr-Commit-Position: refs/heads/master@{#296617}