App windows on ChromeOS cannot be sized to their maximum height

App windows on ChromeOS cannot be sized to their maximum height

CustomFrameViewAsh::GetMaximumSize() should add the height of the
title bar, if a maximum size constraint is specified.

BUG=349229
TEST=ash_unittests (CustomFrameViewAshTest.*)

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

[tmdiep, 2014-03-06 04:29:53]

pkotwicz: This is hopefully not risky. Please review this patch. Thanks in
advance.

[pkotwicz, 2014-03-07 01:45:14]

https://codereview.chromium.org/183793011/diff/1/ash/wm/custom_frame_view_ash.cc
File ash/wm/custom_frame_view_ash.cc (right):

https://codereview.chromium.org/183793011/diff/1/ash/wm/custom_frame_view_ash...
ash/wm/custom_frame_view_ash.cc:485: : NonClientTopBorderHeight() +
max_client_size.height());
Optional Nit: This might be easier to read

int width = 0;
int height = 0;
if (max_client_size.width() > 0) {
  width = std::max(header_view_->GetMinimumWidth(), max_client_size.width());
}
if (max_client_size.height() > 0)
  height = NonClientTopBorderHeight() + max_client_size.height();

return gfx::Size(width, height);

Otherwise, LGTM

[tmdiep, 2014-03-07 02:08:34]

Thanks for the review. I've refactored the function as per your suggestion.

https://codereview.chromium.org/183793011/diff/1/ash/wm/custom_frame_view_ash.cc
File ash/wm/custom_frame_view_ash.cc (right):

https://codereview.chromium.org/183793011/diff/1/ash/wm/custom_frame_view_ash...
ash/wm/custom_frame_view_ash.cc:485: : NonClientTopBorderHeight() +
max_client_size.height());
On 2014/03/07 01:45:14, pkotwicz wrote:
> Optional Nit: This might be easier to read
> 
> int width = 0;
> int height = 0;
> if (max_client_size.width() > 0) {
>   width = std::max(header_view_->GetMinimumWidth(), max_client_size.width());
> }
> if (max_client_size.height() > 0)
>   height = NonClientTopBorderHeight() + max_client_size.height();
> 
> return gfx::Size(width, height);
> 
> Otherwise, LGTM

Done.

[tmdiep, 2014-03-07 04:31:36]

The CQ bit was checked by tmdiep@chromium.org

[commit-bot: I haz the power, 2014-03-07 04:34:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tmdiep@chromium.org/183793011/20001

[commit-bot: I haz the power, 2014-03-07 04:43:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-07 04:43:17]

Try jobs failed on following builders: mac_chromium_rel

[tmdiep, 2014-03-07 11:41:47]

The CQ bit was checked by tmdiep@chromium.org

[commit-bot: I haz the power, 2014-03-07 11:42:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tmdiep@chromium.org/183793011/20001

[commit-bot: I haz the power, 2014-03-07 15:31:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-07 15:31:25]

Retried try job too often on win_rel for step(s) content_browsertests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[tmdiep, 2014-03-07 21:52:10]

The CQ bit was checked by tmdiep@chromium.org

[commit-bot: I haz the power, 2014-03-07 21:54:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tmdiep@chromium.org/183793011/20001

[commit-bot: I haz the power, 2014-03-08 11:03:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tmdiep@chromium.org/183793011/20001

[commit-bot: I haz the power, 2014-03-08 14:11:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-08 14:11:23]

Retried try job too often on win_rel for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[tmdiep, 2014-03-09 22:26:46]

The CQ bit was checked by tmdiep@chromium.org

[commit-bot: I haz the power, 2014-03-09 22:26:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tmdiep@chromium.org/183793011/20001

[commit-bot: I haz the power, 2014-03-10 06:29:42]

Change committed as 255884