Enable app popups to share the same workspace as a maximized window of the same app

Enable app popups to share the same workspace as a maximized window of the same app.

BUG=176577
Test=Manual, see bug

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

[pkotwicz, 2013-03-01 22:29:47]

Sky@, can you please take a look?

If you like my approach, I will add unittests for this.
I imagine that we will changing the logic to do something more consistent
soonish.

[sky, 2013-03-01 23:45:18]

What effect are you trying for here?

[pkotwicz, 2013-03-02 00:21:37]

The idea is:
i) If my fullscreen app opens a popup window, the popup window will have the
same workspace as the full screen app window.
ii) If I click outside of the popup window to bring the maximized window to the
front, the popup window goes to the desktop workspace where it will stay till
the end of time.
I believe that (ii) is different from how windows which persist across all
workspaces work.

I am checking if the popup window belongs to same app as the maximized workspace
to prevent overlaying any window above the maximized app window which kuscher@
does not want. (From Chrome's perspective all non platform app windows are of
type Browser::TYPE_POPUP)

From reading the bug discussion, I believe the goal to right now fix the
scenario described by the bug and worry about finding a good solution for how
things should work later

[sky, 2013-03-04 00:43:32]

You just described persists across all workspaces. The reason you're
seeing the behavior mentioned in the bug is because the popup is an
app and we hit this code:

  if (browser_view->browser()->type() != Browser::TYPE_POPUP ||
      browser_view->browser()->is_app()) {
    ash::SetPersistsAcrossAllWorkspaces(
        GetNativeWindow(),
        ash::WINDOW_PERSISTS_ACROSS_ALL_WORKSPACES_VALUE_NO);
  }

Is there a way to know the app popup is different so that you don't
trigger setting persists to false?

  -Scott

On Fri, Mar 1, 2013 at 4:21 PM,  <pkotwicz@chromium.org> wrote:
> The idea is:
> i) If my fullscreen app opens a popup window, the popup window will have the
> same workspace as the full screen app window.
> ii) If I click outside of the popup window to bring the maximized window to
> the
> front, the popup window goes to the desktop workspace where it will stay
> till
> the end of time.
> I believe that (ii) is different from how windows which persist across all
> workspaces work.
>
> I am checking if the popup window belongs to same app as the maximized
> workspace
> to prevent overlaying any window above the maximized app window which
> kuscher@
> does not want. (From Chrome's perspective all non platform app windows are
> of
> type Browser::TYPE_POPUP)
>
> From reading the bug discussion, I believe the goal to right now fix the
> scenario described by the bug and worry about finding a good solution for
> how
> things should work later
>
> https://codereview.chromium.org/12386025/

[pkotwicz, 2013-03-05 21:24:08]

Good point. Scott, can you please take another look?

[sky, 2013-03-05 23:36:27]

This assumes all secondary windows shouldn't persist. Do you know representative
that is of apps?

[pkotwicz, 2013-03-05 23:56:08]

Scott, can you post a comment to this effect on the bug?
I am not sure how representative this is of apps.

In general, I believe that it is a bug for an app to open a non modal secondary
window.
I think that a non modal secondary window is a bug because it is really painful
to go and find the non modal secondary window again if you happen to punt it to
the desktop workspace.

[sky, 2013-03-06 00:57:43]

Done, since ping this review when you need me to look again.

  -Scott

On Tue, Mar 5, 2013 at 3:56 PM,  <pkotwicz@chromium.org> wrote:
> Scott, can you post a comment to this effect on the bug?
> I am not sure how representative this is of apps.
>
> In general, I believe that it is a bug for an app to open a non modal
> secondary
> window.
> I think that a non modal secondary window is a bug because it is really
> painful
> to go and find the non modal secondary window again if you happen to punt it
> to
> the desktop workspace.
>
> https://codereview.chromium.org/12386025/

[pkotwicz, 2013-03-06 17:10:10]

Scott, can you please take another look?

[sky, 2013-03-06 18:01:53]

I don't know enough about APP_TYPE_HOST to know if that is doing what you want.
Maybe Steven knows?

Steven, Peter is looking for some way to distinguish secondary windows created
by an app. Is APP_TYPE_HOST such an identifier?

[stevenjb, 2013-03-06 18:20:41]

On 2013/03/06 18:01:53, sky wrote:
> I don't know enough about APP_TYPE_HOST to know if that is doing what you
want.
> Maybe Steven knows?
> 
> Steven, Peter is looking for some way to distinguish secondary windows created
> by an app. Is APP_TYPE_HOST such an identifier?

Yes, this should do what you want it to. We should however also remove the TODO
in the comment above AppType in browser.h since we're using it in ash to affect
the window frame (and now the desktop behavior).

[sky, 2013-03-06 21:41:15]

LGTM

[commit-bot: I haz the power, 2013-03-06 23:29:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/12386025/13002

[commit-bot: I haz the power, 2013-03-07 01:46:34]

Change committed as 186584