Add AppWindow.setVisibleOnAllWorkspaces.

Add AppWindow.setVisibleOnAllWorkspaces.

For platforms that support multiple workspaces (currently Mac and Linux), this
allows app windows be visible on all workspaces simultaneously.

API proposal:
https://docs.google.com/document/d/1RC3CYwsrVxS_5hXg6nE3zA9y59G98z9Ezmmmq_Gzx9o/edit?usp=sharing

BUG=384644
Committed: https://chromium.googlesource.com/chromium/src/+/97adb18915b7909429b51d829e7c4aeb66c57d64

Committed: https://crrev.com/dcc8148538ebba44716bee6711db4797156b9a2d
Cr-Commit-Position: refs/heads/master@{#293832}

[benwells, 2014-08-14 22:48:30]

We need an API proposal for this.

[benwells, 2014-08-15 07:53:06]

https://codereview.chromium.org/469993003/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/app_window.idl (right):

https://codereview.chromium.org/469993003/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/app_window.idl:374: [nodoc] static void
setVisibleOnAllWorkspaces(boolean alwaysVisible);
Will there be matching create options?

Can we have a canSetVisi... function somewhere? I think it would be useful to
have this to allow apps to have options UIs to set this. That is they might have
an option to put their windows on all workspaces, but only show the option in
their UI if it is possible.

[jackhou1, 2014-08-18 06:13:06]

On 2014/08/15 07:53:06, benwells wrote:
>
https://codereview.chromium.org/469993003/diff/1/chrome/common/extensions/api...
> File chrome/common/extensions/api/app_window.idl (right):
> 
>
https://codereview.chromium.org/469993003/diff/1/chrome/common/extensions/api...
> chrome/common/extensions/api/app_window.idl:374: [nodoc] static void
> setVisibleOnAllWorkspaces(boolean alwaysVisible);
> Will there be matching create options?

There's not much difference to setting it immediately after creation (unlike
"hidden" for example). But it's easy to add for the sake of consistency if
there's desire to.

> Can we have a canSetVisi... function somewhere? I think it would be useful to
> have this to allow apps to have options UIs to set this. That is they might
have
> an option to put their windows on all workspaces, but only show the option in
> their UI if it is possible.

Done.

[benwells, 2014-08-19 00:50:22]

On 2014/08/18 06:13:06, jackhou1 wrote:
> On 2014/08/15 07:53:06, benwells wrote:
> >
>
https://codereview.chromium.org/469993003/diff/1/chrome/common/extensions/api...
> > File chrome/common/extensions/api/app_window.idl (right):
> > 
> >
>
https://codereview.chromium.org/469993003/diff/1/chrome/common/extensions/api...
> > chrome/common/extensions/api/app_window.idl:374: [nodoc] static void
> > setVisibleOnAllWorkspaces(boolean alwaysVisible);
> > Will there be matching create options?
> 
> There's not much difference to setting it immediately after creation (unlike
> "hidden" for example). But it's easy to add for the sake of consistency if
> there's desire to.

I think it would be good, for consistency and simplicity of apps.

> 
> > Can we have a canSetVisi... function somewhere? I think it would be useful
to
> > have this to allow apps to have options UIs to set this. That is they might
> have
> > an option to put their windows on all workspaces, but only show the option
in
> > their UI if it is possible.
> 
> Done.

[jackhou1, 2014-08-19 08:19:18]

On 2014/08/19 00:50:22, benwells wrote:
> On 2014/08/18 06:13:06, jackhou1 wrote:
> > On 2014/08/15 07:53:06, benwells wrote:
> > >
> >
>
https://codereview.chromium.org/469993003/diff/1/chrome/common/extensions/api...
> > > File chrome/common/extensions/api/app_window.idl (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/469993003/diff/1/chrome/common/extensions/api...
> > > chrome/common/extensions/api/app_window.idl:374: [nodoc] static void
> > > setVisibleOnAllWorkspaces(boolean alwaysVisible);
> > > Will there be matching create options?
> > 
> > There's not much difference to setting it immediately after creation (unlike
> > "hidden" for example). But it's easy to add for the sake of consistency if
> > there's desire to.
> 
> I think it would be good, for consistency and simplicity of apps.

Done.

> > 
> > > Can we have a canSetVisi... function somewhere? I think it would be useful
> to
> > > have this to allow apps to have options UIs to set this. That is they
might
> > have
> > > an option to put their windows on all workspaces, but only show the option
> in
> > > their UI if it is possible.
> > 
> > Done.

[benwells, 2014-08-20 05:02:08]

https://codereview.chromium.org/469993003/diff/60001/chrome/browser/apps/app_...
File chrome/browser/apps/app_window_browsertest.cc (right):

https://codereview.chromium.org/469993003/diff/60001/chrome/browser/apps/app_...
chrome/browser/apps/app_window_browsertest.cc:260: 
We should test that it is limited to dev channel.

https://codereview.chromium.org/469993003/diff/60001/chrome/browser/extension...
File
chrome/browser/extensions/api/app_current_window_internal/app_current_window_internal_api.cc
(right):

https://codereview.chromium.org/469993003/diff/60001/chrome/browser/extension...
chrome/browser/extensions/api/app_current_window_internal/app_current_window_internal_api.cc:407:
if (GetCurrentChannel() > chrome::VersionInfo::CHANNEL_DEV) {
Can we use the feature system to enforce this?

https://codereview.chromium.org/469993003/diff/60001/chrome/common/extensions...
File chrome/common/extensions/api/app_window.idl (right):

https://codereview.chromium.org/469993003/diff/60001/chrome/common/extensions...
chrome/common/extensions/api/app_window.idl:267: // If true, the window will be
visible on all workspaces.
You should mention that this is only available in dev channel.

https://codereview.chromium.org/469993003/diff/60001/chrome/common/extensions...
chrome/common/extensions/api/app_window.idl:377: [nodoc] static void
setVisibleOnAllWorkspaces(boolean alwaysVisible);
Can you remove the [nodoc] and mention that this is available in dev channel?

https://codereview.chromium.org/469993003/diff/60001/chrome/common/extensions...
chrome/common/extensions/api/app_window.idl:381: [nodoc] static boolean
canSetVisibleOnAllWorkspaces();
I just realised this is on the AppWindow. Can we just have it on
chrome.app.window? As it is the app needs a window before it can determine this.
Is there any reason to make it window specific?

https://codereview.chromium.org/469993003/diff/60001/chrome/test/data/extensi...
File chrome/test/data/extensions/platform_apps/window_api/test.js (right):

https://codereview.chromium.org/469993003/diff/60001/chrome/test/data/extensi...
chrome/test/data/extensions/platform_apps/window_api/test.js:1364:
'WaitForRoundTrip', callbackPass(function(reply) {}));
Does this actually test anything?

[jackhou1, 2014-08-27 05:10:45]

https://codereview.chromium.org/469993003/diff/60001/chrome/browser/apps/app_...
File chrome/browser/apps/app_window_browsertest.cc (right):

https://codereview.chromium.org/469993003/diff/60001/chrome/browser/apps/app_...
chrome/browser/apps/app_window_browsertest.cc:260: 
On 2014/08/20 05:02:08, benwells wrote:
> We should test that it is limited to dev channel.

Done.

https://codereview.chromium.org/469993003/diff/60001/chrome/browser/extension...
File
chrome/browser/extensions/api/app_current_window_internal/app_current_window_internal_api.cc
(right):

https://codereview.chromium.org/469993003/diff/60001/chrome/browser/extension...
chrome/browser/extensions/api/app_current_window_internal/app_current_window_internal_api.cc:407:
if (GetCurrentChannel() > chrome::VersionInfo::CHANNEL_DEV) {
On 2014/08/20 05:02:08, benwells wrote:
> Can we use the feature system to enforce this?

We can for chrome.app.window.canSetVisibleOnAllWorkspaces.

AppWindow.setVisibleOnAllWorkspaces is like setBadgeIcon; I don't think there's
an easy way to hide it, but we make it an error here.

https://codereview.chromium.org/469993003/diff/60001/chrome/common/extensions...
File chrome/common/extensions/api/app_window.idl (right):

https://codereview.chromium.org/469993003/diff/60001/chrome/common/extensions...
chrome/common/extensions/api/app_window.idl:267: // If true, the window will be
visible on all workspaces.
On 2014/08/20 05:02:08, benwells wrote:
> You should mention that this is only available in dev channel.

Done.

https://codereview.chromium.org/469993003/diff/60001/chrome/common/extensions...
chrome/common/extensions/api/app_window.idl:377: [nodoc] static void
setVisibleOnAllWorkspaces(boolean alwaysVisible);
On 2014/08/20 05:02:08, benwells wrote:
> Can you remove the [nodoc] and mention that this is available in dev channel?

Done.

https://codereview.chromium.org/469993003/diff/60001/chrome/common/extensions...
chrome/common/extensions/api/app_window.idl:381: [nodoc] static boolean
canSetVisibleOnAllWorkspaces();
On 2014/08/20 05:02:08, benwells wrote:
> I just realised this is on the AppWindow. Can we just have it on
> chrome.app.window? As it is the app needs a window before it can determine
this.
> Is there any reason to make it window specific?

Done.

https://codereview.chromium.org/469993003/diff/60001/chrome/test/data/extensi...
File chrome/test/data/extensions/platform_apps/window_api/test.js (right):

https://codereview.chromium.org/469993003/diff/60001/chrome/test/data/extensi...
chrome/test/data/extensions/platform_apps/window_api/test.js:1364:
'WaitForRoundTrip', callbackPass(function(reply) {}));
On 2014/08/20 05:02:08, benwells wrote:
> Does this actually test anything?

Nup, it's just a smoke test, like testBadging().

[benwells, 2014-08-31 23:28:59]

lgtm

[jackhou1, 2014-09-01 06:37:47]

jackhou@chromium.org changed reviewers:
+ isherman@chromium.org

[jackhou1, 2014-09-01 06:37:48]

isherman, could you please review for OWNERS:

extensions/browser/extension_function_histogram_value.h

[Ilya Sherman, 2014-09-02 20:36:14]

https://codereview.chromium.org/469993003/diff/140001/extensions/browser/exte...
File extensions/browser/extension_function_histogram_value.h (right):

https://codereview.chromium.org/469993003/diff/140001/extensions/browser/exte...
extensions/browser/extension_function_histogram_value.h:951:
APP_CURRENTWINDOWINTERNAL_SETVISIBLEONALLWORKSPACES,
Please run the script to update histograms.xml

[jackhou1, 2014-09-03 22:45:54]

https://codereview.chromium.org/469993003/diff/140001/extensions/browser/exte...
File extensions/browser/extension_function_histogram_value.h (right):

https://codereview.chromium.org/469993003/diff/140001/extensions/browser/exte...
extensions/browser/extension_function_histogram_value.h:951:
APP_CURRENTWINDOWINTERNAL_SETVISIBLEONALLWORKSPACES,
On 2014/09/02 20:36:14, Ilya Sherman wrote:
> Please run the script to update histograms.xml

Done.

[Ilya Sherman, 2014-09-03 22:47:07]

histograms lgtm, thanks

[jackhou1, 2014-09-05 09:26:25]

The CQ bit was checked by jackhou@chromium.org

[commit-bot: I haz the power, 2014-09-05 09:27:13]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jackhou@chromium.org/469993003/200001

[commit-bot: I haz the power, 2014-09-05 09:40:14]

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

[commit-bot: I haz the power, 2014-09-05 09:40:15]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[jackhou1, 2014-09-08 00:32:21]

The CQ bit was checked by jackhou@chromium.org

[commit-bot: I haz the power, 2014-09-08 00:33:07]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jackhou@chromium.org/469993003/220001

[commit-bot: I haz the power, 2014-09-08 00:54:05]

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

[commit-bot: I haz the power, 2014-09-08 00:54:08]

Try jobs failed on following builders:
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jackhou1, 2014-09-08 01:57:03]

The CQ bit was checked by jackhou@chromium.org

[commit-bot: I haz the power, 2014-09-08 01:57:37]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jackhou@chromium.org/469993003/240001

[commit-bot: I haz the power, 2014-09-08 03:57:51]

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

[commit-bot: I haz the power, 2014-09-08 03:57:52]

Exceeded time limit waiting for builds to trigger.

[jackhou1, 2014-09-08 04:12:36]

The CQ bit was checked by jackhou@chromium.org

[commit-bot: I haz the power, 2014-09-08 04:13:40]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jackhou@chromium.org/469993003/240001

[jackhou1, 2014-09-08 06:48:29]

The CQ bit was unchecked by jackhou@chromium.org

[commit-bot: I haz the power, 2014-09-08 06:48:57]

Committed patchset #13 (id:240001) as 97adb18915b7909429b51d829e7c4aeb66c57d64

[Nico, 2014-09-08 20:57:52]

Speculative, to see if it helps with

==12563== WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x7f834414fd2c in
views::DesktopWindowTreeHostX11::InitX11Window(views::Widget::InitParams const&)
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1164
    #1 0x7f834414dc6e in views::DesktopWindowTreeHostX11::Init(aura::Window*,
views::Widget::InitParams const&)
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:259
    #2 0x7f83441408e9 in
views::DesktopNativeWidgetAura::InitNativeWidget(views::Widget::InitParams
const&) ui/views/widget/desktop_aura/desktop_native_widget_aura.cc:420
    #3 0x7f834411722f in views::Widget::Init(views::Widget::InitParams const&)
ui/views/widget/widget.cc:358
    #4 0x7f834473bc8b in
ChromeNativeAppWindowViews::InitializeDefaultWindow(extensions::AppWindow::CreateParams
const&) chrome/browser/ui/views/apps/chrome_native_app_window_views.cc:231
    #5 0x7f834473f52d in
ChromeNativeAppWindowViews::InitializeWindow(extensions::AppWindow*,
extensions::AppWindow::CreateParams const&)
chrome/browser/ui/views/apps/chrome_native_app_window_views.cc:672
    #6 0x7f834c3d8bbe in
apps::NativeAppWindowViews::Init(extensions::AppWindow*,
extensions::AppWindow::CreateParams const&)
apps/ui/views/native_app_window_views.cc:46
    #7 0x7f834460443c in
ChromeAppsClient::CreateNativeAppWindowImpl(extensions::AppWindow*,
extensions::AppWindow::CreateParams const&)
chrome/browser/ui/views/apps/chrome_apps_client_views.cc:14
    #8 0x7f834be98433 in extensions::AppWindow::Init(GURL const&,
extensions::AppWindowContents*, extensions::AppWindow::CreateParams const&)
extensions/browser/app_window/app_window.cc:281
    #9 0x7f834bd6bbfb in extensions::AppWindowCreateFunction::RunAsync()
extensions/browser/api/app_window/app_window_api.cc:296
    #10 0x7f834bee4510 in AsyncExtensionFunction::Run()
extensions/browser/extension_function.cc:452
    #11 0x7f834bee9771 in
extensions::ExtensionFunctionDispatcher::DispatchWithCallbackInternal(ExtensionHostMsg_Request_Params
const&, content::RenderViewHost*, content::RenderFrameHost*, base::Callback<void
(ExtensionFunction::ResponseType, base::ListValue const&,
std::__1::basic_string<char, std::__1::char_traits<char>,
std::__1::allocator<char> > const&)> const&)
extensions/browser/extension_function_dispatcher.cc:392
    #12 0x7f834bee86b6 in
extensions::ExtensionFunctionDispatcher::Dispatch(ExtensionHostMsg_Request_Params
const&, content::RenderViewHost*)
extensions/browser/extension_function_dispatcher.cc:309
    #13 0x7f834bedac91 in OnRequest extensions/browser/extension_host.cc:342
    #14 0x7f8345add9d2 in
content::WebContentsImpl::OnMessageReceived(content::RenderViewHost*,
content::RenderFrameHost*, IPC::Message const&)
content/browser/web_contents/web_contents_impl.cc:526
    #15 0x7f83459847fe in
content::RenderViewHostImpl::OnMessageReceived(IPC::Message const&)
content/browser/renderer_host/render_view_host_impl.cc:894

Started happening here
http://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20MSan%20Tests...
(see stdio for symbols), and this looks like the only change in that build.

Line 1164 is

  if (params.visible_on_all_workspaces) {
    state_atom_list.push_back(atom_cache_.GetAtom("_NET_WM_STATE_STICKY"));
    ui::SetIntProperty(xwindow_, "_NET_WM_DESKTOP", "CARDINAL", kAllDesktops);
  }

[Nico, 2014-09-08 21:03:20]

Err, that was for the "Revert" message box. I'll try again.

[Nico, 2014-09-08 21:33:48]

The revert button gives me
"""
Unable to inverse 'A +' patches with 100% similarity indexes. Please revert
manually.
Sorry for the inconvenience.
"""

Filed a bug for that, will try to revert manually.

[jackhou1, 2014-09-09 00:28:17]

The CQ bit was checked by jackhou@chromium.org

[jackhou1, 2014-09-09 00:29:05]

On 2014/09/08 20:57:52, Nico (hiding) wrote:
> Speculative, to see if it helps with
> 
> ==12563== WARNING: MemorySanitizer: use-of-uninitialized-value
>     #0 0x7f834414fd2c in
> views::DesktopWindowTreeHostX11::InitX11Window(views::Widget::InitParams
const&)
> ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1164
>     #1 0x7f834414dc6e in views::DesktopWindowTreeHostX11::Init(aura::Window*,
> views::Widget::InitParams const&)
> ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:259
>     #2 0x7f83441408e9 in
> views::DesktopNativeWidgetAura::InitNativeWidget(views::Widget::InitParams
> const&) ui/views/widget/desktop_aura/desktop_native_widget_aura.cc:420
>     #3 0x7f834411722f in views::Widget::Init(views::Widget::InitParams const&)
> ui/views/widget/widget.cc:358
>     #4 0x7f834473bc8b in
>
ChromeNativeAppWindowViews::InitializeDefaultWindow(extensions::AppWindow::CreateParams
> const&) chrome/browser/ui/views/apps/chrome_native_app_window_views.cc:231
>     #5 0x7f834473f52d in
> ChromeNativeAppWindowViews::InitializeWindow(extensions::AppWindow*,
> extensions::AppWindow::CreateParams const&)
> chrome/browser/ui/views/apps/chrome_native_app_window_views.cc:672
>     #6 0x7f834c3d8bbe in
> apps::NativeAppWindowViews::Init(extensions::AppWindow*,
> extensions::AppWindow::CreateParams const&)
> apps/ui/views/native_app_window_views.cc:46
>     #7 0x7f834460443c in
> ChromeAppsClient::CreateNativeAppWindowImpl(extensions::AppWindow*,
> extensions::AppWindow::CreateParams const&)
> chrome/browser/ui/views/apps/chrome_apps_client_views.cc:14
>     #8 0x7f834be98433 in extensions::AppWindow::Init(GURL const&,
> extensions::AppWindowContents*, extensions::AppWindow::CreateParams const&)
> extensions/browser/app_window/app_window.cc:281
>     #9 0x7f834bd6bbfb in extensions::AppWindowCreateFunction::RunAsync()
> extensions/browser/api/app_window/app_window_api.cc:296
>     #10 0x7f834bee4510 in AsyncExtensionFunction::Run()
> extensions/browser/extension_function.cc:452
>     #11 0x7f834bee9771 in
>
extensions::ExtensionFunctionDispatcher::DispatchWithCallbackInternal(ExtensionHostMsg_Request_Params
> const&, content::RenderViewHost*, content::RenderFrameHost*,
base::Callback<void
> (ExtensionFunction::ResponseType, base::ListValue const&,
> std::__1::basic_string<char, std::__1::char_traits<char>,
> std::__1::allocator<char> > const&)> const&)
> extensions/browser/extension_function_dispatcher.cc:392
>     #12 0x7f834bee86b6 in
>
extensions::ExtensionFunctionDispatcher::Dispatch(ExtensionHostMsg_Request_Params
> const&, content::RenderViewHost*)
> extensions/browser/extension_function_dispatcher.cc:309
>     #13 0x7f834bedac91 in OnRequest extensions/browser/extension_host.cc:342
>     #14 0x7f8345add9d2 in
> content::WebContentsImpl::OnMessageReceived(content::RenderViewHost*,
> content::RenderFrameHost*, IPC::Message const&)
> content/browser/web_contents/web_contents_impl.cc:526
>     #15 0x7f83459847fe in
> content::RenderViewHostImpl::OnMessageReceived(IPC::Message const&)
> content/browser/renderer_host/render_view_host_impl.cc:894
> 
> Started happening here
>
http://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20MSan%20Tests...
> (see stdio for symbols), and this looks like the only change in that build.
> 
> Line 1164 is
> 
>   if (params.visible_on_all_workspaces) {
>     state_atom_list.push_back(atom_cache_.GetAtom("_NET_WM_STATE_STICKY"));
>     ui::SetIntProperty(xwindow_, "_NET_WM_DESKTOP", "CARDINAL", kAllDesktops);
>   }

Thanks for the info. Fixed in Patch Set 14.

[commit-bot: I haz the power, 2014-09-09 00:34:06]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jackhou@chromium.org/469993003/260001

[commit-bot: I haz the power, 2014-09-09 01:41:50]

Committed patchset #14 (id:260001) as 3f6f925d9580c7ab55a3d0316ea12d186ef2209d

[commit-bot: I haz the power, 2014-09-10 03:44:43]

Patchset 14 (id:??) landed as
https://crrev.com/c76ef7326840ea3eeb3fa96fb88f5269b5ec55a7
Cr-Commit-Position: refs/heads/master@{#293676}

[commit-bot: I haz the power, 2014-09-10 03:49:41]

Patchset 14 (id:??) landed as
https://crrev.com/dcc8148538ebba44716bee6711db4797156b9a2d
Cr-Commit-Position: refs/heads/master@{#293832}