Add switch to provide app window size on startup

Add switch to provide app window size on startup

Current use of this switch would be for new user session on Chrome OS where we're launching Getting Started Guide via --app=[URL] and want to initialize with the specific size.
Otherwise app window is initialized with the default size and then resized by GSG itself. (see issue).

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

[Nikita (slow), 2012-09-19 17:01:37]

mihaip@ - please review chrome/browser/ui/extensions/*
sky@ - please review whole CL

[sky, 2012-09-19 17:20:14]

http://codereview.chromium.org/10949023/diff/1/chrome/browser/chromeos/login/...
File chrome/browser/chromeos/login/existing_user_controller.cc (right):

http://codereview.chromium.org/10949023/diff/1/chrome/browser/chromeos/login/...
chrome/browser/chromeos/login/existing_user_controller.cc:78: const char
kGSGAppWindowSize[] = "820,550";
Shouldn't this be specified externally so we can tune per device?

http://codereview.chromium.org/10949023/diff/1/chrome/browser/ui/startup/star...
File chrome/browser/ui/startup/startup_browser_creator_impl.cc (right):

http://codereview.chromium.org/10949023/diff/1/chrome/browser/ui/startup/star...
chrome/browser/ui/startup/startup_browser_creator_impl.cc:221: size_t num1_size
= str.find_first_of(',');
Seems like this would be a lot simpler if you used SplitString.

http://codereview.chromium.org/10949023/diff/1/chrome/browser/ui/startup/star...
chrome/browser/ui/startup/startup_browser_creator_impl.cc:539: if
(command_line_.HasSwitch(switches::kAppWindowSize)) {
Move this into its own function.

http://codereview.chromium.org/10949023/diff/1/chrome/browser/ui/startup/star...
chrome/browser/ui/startup/startup_browser_creator_impl.cc:544:
override_bounds.set_size(gfx::Size(width, height));
Make sure the size fits in the work area too.

http://codereview.chromium.org/10949023/diff/1/chrome/browser/ui/startup/star...
chrome/browser/ui/startup/startup_browser_creator_impl.cc:553:
extra_height_work_area = ash::kLauncherPreferredSize;
The work area shouldn't include kLauncherPreferredSize, how come you need to do
this?

http://codereview.chromium.org/10949023/diff/1/chrome/common/chrome_switches.cc
File chrome/common/chrome_switches.cc (right):

http://codereview.chromium.org/10949023/diff/1/chrome/common/chrome_switches....
chrome/common/chrome_switches.cc:65: // Specify the initial "application" mode
window size: --app-window-size=w,h
Specifies the initial size for application windows launched with --app.

[Nikita (slow), 2012-09-19 17:46:41]

http://codereview.chromium.org/10949023/diff/1/chrome/browser/chromeos/login/...
File chrome/browser/chromeos/login/existing_user_controller.cc (right):

http://codereview.chromium.org/10949023/diff/1/chrome/browser/chromeos/login/...
chrome/browser/chromeos/login/existing_user_controller.cc:78: const char
kGSGAppWindowSize[] = "820,550";
On 2012/09/19 17:20:14, sky wrote:
> Shouldn't this be specified externally so we can tune per device?

Min said that we do want to start with same size across all platforms. If there
would be need to override this size per device basis, I would add this support.
No need for that for now.
If

http://codereview.chromium.org/10949023/diff/1/chrome/browser/ui/startup/star...
File chrome/browser/ui/startup/startup_browser_creator_impl.cc (right):

http://codereview.chromium.org/10949023/diff/1/chrome/browser/ui/startup/star...
chrome/browser/ui/startup/startup_browser_creator_impl.cc:553:
extra_height_work_area = ash::kLauncherPreferredSize;
On 2012/09/19 17:20:14, sky wrote:
> The work area shouldn't include kLauncherPreferredSize, how come you need to
do
> this?

Probably you're right but from what I've seen on linux dev machine it does
include launcher. That could be one of 2:

1. We're grabbing work_area() before launcher has been added into layout
2. chromeos Chrome default window size height is larger then  laptop formfactor
so it does include launcher into work area.

I'll check on device (lumpy).

[sky, 2012-09-19 20:54:24]

On Wed, Sep 19, 2012 at 10:46 AM,  <nkostylev@chromium.org> wrote:
>
>
http://codereview.chromium.org/10949023/diff/1/chrome/browser/chromeos/login/...
> File chrome/browser/chromeos/login/existing_user_controller.cc (right):
>
>
http://codereview.chromium.org/10949023/diff/1/chrome/browser/chromeos/login/...
> chrome/browser/chromeos/login/existing_user_controller.cc:78: const char
> kGSGAppWindowSize[] = "820,550";
> On 2012/09/19 17:20:14, sky wrote:
>>
>> Shouldn't this be specified externally so we can tune per device?
>
>
> Min said that we do want to start with same size across all platforms.
> If there would be need to override this size per device basis, I would
> add this support. No need for that for now.
> If
>
>
>
http://codereview.chromium.org/10949023/diff/1/chrome/browser/ui/startup/star...
> File chrome/browser/ui/startup/startup_browser_creator_impl.cc (right):
>
>
http://codereview.chromium.org/10949023/diff/1/chrome/browser/ui/startup/star...
> chrome/browser/ui/startup/startup_browser_creator_impl.cc:553:
> extra_height_work_area = ash::kLauncherPreferredSize;
> On 2012/09/19 17:20:14, sky wrote:
>>
>> The work area shouldn't include kLauncherPreferredSize, how come you
>
> need to do
>>
>> this?
>
>
> Probably you're right but from what I've seen on linux dev machine it
> does include launcher. That could be one of 2:
>
> 1. We're grabbing work_area() before launcher has been added into layout

That is entirely possible, but I think we should make the code (in
ash) deal correctly with this so that you don't have to special case
here. Additionally the launcher may be aligned to the side or even
hidden, which you don't want to duplicate either.

> 2. chromeos Chrome default window size height is larger then  laptop
> formfactor so it does include launcher into work area.

It should work correctly if you try forced to a smaller display too though.

  -Scott

[Nikita (slow), 2012-09-20 15:01:19]

On 2012/09/19 20:54:24, sky wrote:
> >
> > Probably you're right but from what I've seen on linux dev machine it
> > does include launcher. That could be one of 2:
> >
> > 1. We're grabbing work_area() before launcher has been added into layout
> 
> That is entirely possible, but I think we should make the code (in
> ash) deal correctly with this so that you don't have to special case
> here. Additionally the launcher may be aligned to the side or even
> hidden, which you don't want to duplicate either.

I've just checked on linux desktop, at this point working area is returned as
1280x1024 i.e. entire screen, doesn't take launcher into account.

> 
> > 2. chromeos Chrome default window size height is larger then  laptop
> > formfactor so it does include launcher into work area.
> 
> It should work correctly if you try forced to a smaller display too though.
> 
>   -Scott

[Nikita (slow), 2012-09-20 15:01:27]

http://codereview.chromium.org/10949023/diff/1/chrome/browser/ui/startup/star...
File chrome/browser/ui/startup/startup_browser_creator_impl.cc (right):

http://codereview.chromium.org/10949023/diff/1/chrome/browser/ui/startup/star...
chrome/browser/ui/startup/startup_browser_creator_impl.cc:221: size_t num1_size
= str.find_first_of(',');
On 2012/09/19 17:20:14, sky wrote:
> Seems like this would be a lot simpler if you used SplitString.

Done.

http://codereview.chromium.org/10949023/diff/1/chrome/browser/ui/startup/star...
chrome/browser/ui/startup/startup_browser_creator_impl.cc:539: if
(command_line_.HasSwitch(switches::kAppWindowSize)) {
On 2012/09/19 17:20:14, sky wrote:
> Move this into its own function.

Done.

http://codereview.chromium.org/10949023/diff/1/chrome/browser/ui/startup/star...
chrome/browser/ui/startup/startup_browser_creator_impl.cc:544:
override_bounds.set_size(gfx::Size(width, height));
On 2012/09/19 17:20:14, sky wrote:
> Make sure the size fits in the work area too.

Not sure if this is really needs to be done here as window will be sized to
working area size even if I pass something like 3000x3000

http://codereview.chromium.org/10949023/diff/1/chrome/browser/ui/startup/star...
chrome/browser/ui/startup/startup_browser_creator_impl.cc:544:
override_bounds.set_size(gfx::Size(width, height));
On 2012/09/19 17:20:14, sky wrote:
> Make sure the size fits in the work area too.

Done.

[Nikita (slow), 2012-09-20 16:07:35]

http://codereview.chromium.org/10949023/diff/1/chrome/common/chrome_switches.cc
File chrome/common/chrome_switches.cc (right):

http://codereview.chromium.org/10949023/diff/1/chrome/common/chrome_switches....
chrome/common/chrome_switches.cc:65: // Specify the initial "application" mode
window size: --app-window-size=w,h
On 2012/09/19 17:20:14, sky wrote:
> Specifies the initial size for application windows launched with --app.

Done.

[Nikita (slow), 2012-09-20 16:55:00]

http://codereview.chromium.org/10949023/diff/1/chrome/browser/ui/startup/star...
File chrome/browser/ui/startup/startup_browser_creator_impl.cc (right):

http://codereview.chromium.org/10949023/diff/1/chrome/browser/ui/startup/star...
chrome/browser/ui/startup/startup_browser_creator_impl.cc:553:
extra_height_work_area = ash::kLauncherPreferredSize;
On 2012/09/19 17:20:14, sky wrote:
> The work area shouldn't include kLauncherPreferredSize, how come you need to
do
> this?

Removed for now.
Work area should not include launcher but it does now.

[sky, 2012-09-20 16:59:17]

LGTM

[Nikita (slow), 2012-09-20 17:37:58]

+Mike for owners review

[Nikita (slow), 2012-09-20 17:37:58]

+Mike for owners review

[miket_OOO, 2012-09-20 17:40:04]

On 2012/09/20 17:37:58, Nikita Kostylev wrote:
> +Mike for owners review

OWNERS LGTM. Thanks!