Add a 'hidden' option to chrome.app.window.create()

Add a 'hidden' option to chrome.app.window.create()

This also adds AppWindow.show()/hide(), to manipulate the visibility of the
window once it has been created.

R=asargent@chromium.org,ben@chromium.org
BUG=131742
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=164016

[jeremya, 2012-10-16 02:55:16]

Hi folks,

This is a first smash at implementing this, and doesn't yet compile on mac or
linux. I'd also like to bind the ShowInactive() function from BaseWindow, so
apps can show a window without bringing it to the foreground. Mailing it out now
because this is more or less what the final form will be, so it's ready for
review on Win and CrOS :)

Thanks!

[benwells, 2012-10-16 05:54:54]

https://codereview.chromium.org/11147037/diff/3001/chrome/browser/ui/views/fr...
File chrome/browser/ui/views/frame/browser_view.cc (right):

https://codereview.chromium.org/11147037/diff/3001/chrome/browser/ui/views/fr...
chrome/browser/ui/views/frame/browser_view.cc:615: frame_->Hide();
Is this implementation used by anything? If not, is it needed (unused code tends
to decay)?

https://codereview.chromium.org/11147037/diff/3001/chrome/common/extensions/a...
File chrome/common/extensions/api/app_window.idl (right):

https://codereview.chromium.org/11147037/diff/3001/chrome/common/extensions/a...
chrome/common/extensions/api/app_window.idl:56: boolean? hidden;
Hidden windows will keep their browser keep alive, preventing the process from
being destroyed. Is there any indication to the user that there is a window?
We've been aiming for no indication of app indication -> no browser keep alive,
this change may break that.

[jeremya, 2012-10-16 06:31:32]

https://codereview.chromium.org/11147037/diff/3001/chrome/browser/ui/views/fr...
File chrome/browser/ui/views/frame/browser_view.cc (right):

https://codereview.chromium.org/11147037/diff/3001/chrome/browser/ui/views/fr...
chrome/browser/ui/views/frame/browser_view.cc:615: frame_->Hide();
On 2012/10/16 05:54:55, benwells wrote:
> Is this implementation used by anything? If not, is it needed (unused code
tends
> to decay)?

Good point. Replaced with a comment.

https://codereview.chromium.org/11147037/diff/3001/chrome/common/extensions/a...
File chrome/common/extensions/api/app_window.idl (right):

https://codereview.chromium.org/11147037/diff/3001/chrome/common/extensions/a...
chrome/common/extensions/api/app_window.idl:56: boolean? hidden;
On 2012/10/16 05:54:55, benwells wrote:
> Hidden windows will keep their browser keep alive, preventing the process from
> being destroyed. Is there any indication to the user that there is a window?
> We've been aiming for no indication of app indication -> no browser keep
alive,
> this change may break that.

That's true. Perhaps the keepalive should check visibility of windows?

[Ben Goodger (Google), 2012-10-16 18:16:23]

qq. if you're adding a method like 'show' to window, are you going to need to
consider a show state param at some point, and what values will it need to
support? is it possible to add one later in a backward-compatible way?

e.g. on windows/ash we have various show states for restored, maximized,
fullscreen etc.

windows also allows you to specify whether the window is activated as it is
shown, which is convenient since it saves you from having to call activate()
after showing it.

[jeremya, 2012-10-16 23:04:18]

On 2012/10/16 18:16:23, Ben Goodger (Google) wrote:
> qq. if you're adding a method like 'show' to window, are you going to need to
> consider a show state param at some point, and what values will it need to
> support? is it possible to add one later in a backward-compatible way?
> 
> e.g. on windows/ash we have various show states for restored, maximized,
> fullscreen etc.
> 
> windows also allows you to specify whether the window is activated as it is
> shown, which is convenient since it saves you from having to call activate()
> after showing it.

We're planning to add state accessors to the app window API for this
information: http://crbug.com/134068

This show() function also activates (see shell_window_views.cc:440), but we
should also add a showInactive() function to show without activating.

[jeremya, 2012-10-17 02:45:22]

Regarding keeping the app alive, I've filed crbug.com/156259. tldr: hidden
windows shouldn't keep the app alive.

[Ben Goodger (Google), 2012-10-17 16:28:19]

views bits lgtm

[asargent_no_longer_on_chrome, 2012-10-17 16:45:59]

lgtm

[commit-bot: I haz the power, 2012-10-23 00:08:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jeremya@chromium.org/11147037/19001

[commit-bot: I haz the power, 2012-10-23 00:25:51]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
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.

[commit-bot: I haz the power, 2012-10-23 04:23:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jeremya@chromium.org/11147037/28002

[commit-bot: I haz the power, 2012-10-24 23:18:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jeremya@chromium.org/11147037/21013

[commit-bot: I haz the power, 2012-10-25 00:44:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jeremya@chromium.org/11147037/34002

[commit-bot: I haz the power, 2012-10-25 04:59:18]

Change committed as 164016