[Win] Fix detection of normal windows in HWNDMessageHandler::OnSizeConstraintsChanged.

[Win] Fix detection of normal windows in HWNDMessageHandler::OnSizeConstraintsChanged.

OnSizeConstraintsChanged should be skipped if the window is a popup or child.

BUG=439038
Committed: https://crrev.com/05866938f778954fe867ce07a5b803bb600a4faf
Cr-Commit-Position: refs/heads/master@{#308016}

[jackhou1, 2014-12-11 04:17:09]

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

[jackhou1, 2014-12-11 04:17:09]

sky, could you take a look?

[sky, 2014-12-11 15:28:13]

LGTM

[jackhou1, 2014-12-11 22:58:33]

The CQ bit was checked by jackhou@chromium.org

[commit-bot: I haz the power, 2014-12-11 23:10:54]

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

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

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2014-12-12 01:13:39]

Patchset 1 (id:??) landed as
https://crrev.com/05866938f778954fe867ce07a5b803bb600a4faf
Cr-Commit-Position: refs/heads/master@{#308016}

[brucedawson, 2015-01-07 21:57:50]

I'm concerned about this change so I summarized the history of this function:

ver 202 - 5150e6e 2014-09-25 jackhou - SizeConstraintsChanged() added
ver 203 - 6561311 2014-10-01 jackhou - WS_OVERLAPPED check added, caused
function to always return
ver 208 - 26b57c2 2014-11-05 brucedawson - WS_OVERLAPPED check changed to run
function if WS_POPUP or WS_CHILD are set (originally intended?)
ver 209 - 2e4803c 2014-11-12 brucedawson - don't set WS_THICKFRAME if
WS_COMPOSITED is set
ver 216 - 0586693 2014-12-12 jackhou - reversed sense of WS_OVERLAPPED check,
now runs function only if WS_POPUP and WS_CHILD are *not* set

So, over the course of this functions lifetime it's behavior has been:
- always executing, presumably a bug, quickly fixed
- never executing, presumably a bug
- executing when WS_POPUP or WS_CHILD are set - this was my attempt to get the
behavior to match the apparent intent of the code, without being clear on
whether that apparent intent was correct
- changing its behavior when executed to fix the edit-bookmark dialog
- executing when WS_POPUP or WS_CHILD are not set

Are we sure that this is correct now? The name of the function seems to indicate
when it is called, but not what it is supposed to do, so I am left uncertain.

Change 2e4803c is presumably unnecessary now, but harmless enough.

[sky, 2015-01-07 23:22:45]

Do you have some reason to believe it isn't correct now?

On Wed, Jan 7, 2015 at 1:57 PM,  <brucedawson@chromium.org> wrote:
> I'm concerned about this change so I summarized the history of this
> function:
>
> ver 202 - 5150e6e 2014-09-25 jackhou - SizeConstraintsChanged() added
> ver 203 - 6561311 2014-10-01 jackhou - WS_OVERLAPPED check added, caused
> function to always return
> ver 208 - 26b57c2 2014-11-05 brucedawson - WS_OVERLAPPED check changed to
> run
> function if WS_POPUP or WS_CHILD are set (originally intended?)
> ver 209 - 2e4803c 2014-11-12 brucedawson - don't set WS_THICKFRAME if
> WS_COMPOSITED is set
> ver 216 - 0586693 2014-12-12 jackhou - reversed sense of WS_OVERLAPPED
> check,
> now runs function only if WS_POPUP and WS_CHILD are *not* set
>
> So, over the course of this functions lifetime it's behavior has been:
> - always executing, presumably a bug, quickly fixed
> - never executing, presumably a bug
> - executing when WS_POPUP or WS_CHILD are set - this was my attempt to get
> the
> behavior to match the apparent intent of the code, without being clear on
> whether that apparent intent was correct
> - changing its behavior when executed to fix the edit-bookmark dialog
> - executing when WS_POPUP or WS_CHILD are not set
>
> Are we sure that this is correct now? The name of the function seems to
> indicate
> when it is called, but not what it is supposed to do, so I am left
> uncertain.
>
> Change 2e4803c is presumably unnecessary now, but harmless enough.
>
> https://codereview.chromium.org/792303002/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.