Place newly created app windows on screen.

Place newly created app windows on screen.

Application windows with IDs are restored to their
previous location, however if the screen bounds have
changed they previously might be created offscreen.

This change saves the previous screen bounds, and
if the current screen is not contained that that
area, it moves the window, resizing if needed, to
fit in the new screen.

This patch was developed by Chaobin Zhang <zhchbin@gmail.com>;
and review started on https://codereview.chromium.org/17378003/
but has been tested and edited by scheib@chromium.org
to remove the "if defined(OS_WIN)".


BUG=145752, 225734
TEST=Run https://github.com/GoogleChrome/chrome-app-samples/tree/master/window-state sample, launch windows with ID, move to a second monitor or far right bottom, close window, resize or disconnect other monitor, relaunch that window with ID: verify newly created window is visible.

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

[scheib, 2013-06-22 00:10:13]

ben, owners review please.

This patch was developed by Chaobin Zhang <zhchbin@gmail.com>
and I lgtm'ed on https://codereview.chromium.org/17378003/
but has been tested and edited by scheib@chromium.org to remove the "if
defined(OS_WIN)". 

Chaobin Zhang, I've done this to land the change as soon as possible -- you have
still been given author's credit for the change. If this change is indeed not
landed by Sunday please update your initial change in
https://codereview.chromium.org/17378003/ and we can land from there after it
get's an lgtm there.

[benwells, 2013-06-22 00:56:02]

lgtm.

The description is cut off, please fill it in.

https://codereview.chromium.org/17564005/diff/1/chrome/browser/extensions/pla...
File chrome/browser/extensions/platform_app_browsertest_util.cc (right):

https://codereview.chromium.org/17564005/diff/1/chrome/browser/extensions/pla...
chrome/browser/extensions/platform_app_browsertest_util.cc:173:
window->AdjustBoundsToBeVisibleOnScreen(cached_bounds,
nit: why do you need this function in the utility class? Can't you just call
window->Adjust....(...) in the call sites instead?

If you feel it is much nicer like this, consider making it static.

https://codereview.chromium.org/17564005/diff/1/chrome/browser/ui/extensions/...
File chrome/browser/ui/extensions/shell_window.cc (right):

https://codereview.chromium.org/17564005/diff/1/chrome/browser/ui/extensions/...
chrome/browser/ui/extensions/shell_window.cc:169: // case of the screen
resolution changed before show on windows platform.
nit: in case the screen ... (delete of).

[zhchbin, 2013-06-22 07:16:16]

On 2013/06/22 00:10:13, scheib wrote:
> ben, owners review please.
> 
> This patch was developed by Chaobin Zhang <mailto:zhchbin@gmail.com>
> and I lgtm'ed on https://codereview.chromium.org/17378003/
> but has been tested and edited by mailto:scheib@chromium.org to remove the "if
> defined(OS_WIN)". 
> 
> Chaobin Zhang, I've done this to land the change as soon as possible -- you
have
> still been given author's credit for the change. If this change is indeed not
> landed by Sunday please update your initial change in
> https://codereview.chromium.org/17378003/ and we can land from there after it
> get's an lgtm there.

Thanks for the help. I also find that we are suffering from differences of
timezone.

[scheib, 2013-06-23 04:43:37]

Thanks Ben.

https://codereview.chromium.org/17564005/diff/1/chrome/browser/extensions/pla...
File chrome/browser/extensions/platform_app_browsertest_util.cc (right):

https://codereview.chromium.org/17564005/diff/1/chrome/browser/extensions/pla...
chrome/browser/extensions/platform_app_browsertest_util.cc:173:
window->AdjustBoundsToBeVisibleOnScreen(cached_bounds,
On 2013/06/22 00:56:02, benwells wrote:
> nit: why do you need this function in the utility class? Can't you just call
> window->Adjust....(...) in the call sites instead?
> 
> If you feel it is much nicer like this, consider making it static. 

Done.

https://codereview.chromium.org/17564005/diff/1/chrome/browser/ui/extensions/...
File chrome/browser/ui/extensions/shell_window.cc (right):

https://codereview.chromium.org/17564005/diff/1/chrome/browser/ui/extensions/...
chrome/browser/ui/extensions/shell_window.cc:169: // case of the screen
resolution changed before show on windows platform.
On 2013/06/22 00:56:02, benwells wrote:
> nit: in case the screen ... (delete of).

Done.

[commit-bot: I haz the power, 2013-06-23 04:43:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scheib@chromium.org/17564005/24001

[commit-bot: I haz the power, 2013-06-23 05:16:37]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[scheib, 2013-06-23 05:38:06]

https://codereview.chromium.org/17564005/diff/1/chrome/browser/extensions/pla...
File chrome/browser/extensions/platform_app_browsertest_util.cc (right):

https://codereview.chromium.org/17564005/diff/1/chrome/browser/extensions/pla...
chrome/browser/extensions/platform_app_browsertest_util.cc:173:
window->AdjustBoundsToBeVisibleOnScreen(cached_bounds,
On 2013/06/23 04:43:37, scheib wrote:
> On 2013/06/22 00:56:02, benwells wrote:
> > nit: why do you need this function in the utility class? Can't you just call
> > window->Adjust....(...) in the call sites instead?
> > 
> > If you feel it is much nicer like this, consider making it static. 
> 
> Done.

Ah, needed to access private function, using PlatformAppBrowserTest's friend
status. I'm leaving it as is, but fixing the name.

[commit-bot: I haz the power, 2013-06-23 05:38:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scheib@chromium.org/17564005/33001

[commit-bot: I haz the power, 2013-06-23 05:42:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scheib@chromium.org/17564005/8004

[commit-bot: I haz the power, 2013-06-23 14:20:59]

Change committed as 208128