Provide a default role for "bubble" windows.

Provide a default role for "bubble" windows.

This provides a default WM_ROLE property for "bubble" windows in a similar
manner to how "popup" windows do.

BUG=427430
Committed: https://crrev.com/f5b2b5137608438fe47fc4204740d8910f657adb
Cr-Commit-Position: refs/heads/master@{#302589}

[mithro-old, 2014-10-31 05:06:09]

mithro@mithis.com changed reviewers:
+ erg@google.com, sadrul@chromium.org

[mithro-old, 2014-10-31 05:06:10]

Hello,

I also have a patch which makes the message center provide correct roles for
it's windows - but it seems like it might be a good idea to provide default
values here for bubbles like we do for popup windows.

Is there any reason we don't either provide default values or else require these
values be set? They are pretty vital to things like tiling window managers under
Linux.

Tim 'mithro' Ansell

[Elliot Glaysher, 2014-10-31 19:42:14]

erg@chromium.org changed reviewers:
+ erg@chromium.org

[Elliot Glaysher, 2014-10-31 19:42:15]

This patch lgtm++, but I'm not an X11-pert, so I'll defer the final say to
sadrul@.

> Is there any reason we don't either provide default values
> or else require these values be set?

I don't know. Playing with xprop locally, it looks like our main browser window
gets "browser". I'm not sure if we should be setting a role at all times; emacs
windows, for example, don't appear to have WM_WINDOW_ROLE set.

https://codereview.chromium.org/693113002/diff/1/ui/views/widget/desktop_aura...
File ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc (right):

https://codereview.chromium.org/693113002/diff/1/ui/views/widget/desktop_aura...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1196: wm_role_name
= std::string(kX11WindowRolePopup);
It should be safe here to just say `wm_role_name = "popup"`.

[sadrul, 2014-11-03 17:14:03]

Nitted lgtm

https://codereview.chromium.org/693113002/diff/1/ui/views/widget/desktop_aura...
File ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc (right):

https://codereview.chromium.org/693113002/diff/1/ui/views/widget/desktop_aura...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1189: std::string
wm_role_name = params.wm_role_name;
Can you use a const char* here instead of making copies/creating std::strings?

https://codereview.chromium.org/693113002/diff/1/ui/views/widget/desktop_aura...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1208: if
(!wm_role_name.empty()) {
No {} for single line blocks

[mithro-old, 2014-11-04 05:48:44]

https://codereview.chromium.org/693113002/diff/1/ui/views/widget/desktop_aura...
File ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc (right):

https://codereview.chromium.org/693113002/diff/1/ui/views/widget/desktop_aura...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1189: std::string
wm_role_name = params.wm_role_name;
On 2014/11/03 17:14:03, sadrul wrote:
> Can you use a const char* here instead of making copies/creating std::strings?

Done.

https://codereview.chromium.org/693113002/diff/1/ui/views/widget/desktop_aura...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1196: wm_role_name
= std::string(kX11WindowRolePopup);
On 2014/10/31 19:42:15, Elliot Glaysher wrote:
> It should be safe here to just say `wm_role_name = "popup"`.

Obsolete by sadrul's comments.

https://codereview.chromium.org/693113002/diff/1/ui/views/widget/desktop_aura...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1208: if
(!wm_role_name.empty()) {
On 2014/11/03 17:14:03, sadrul wrote:
> No {} for single line blocks

Done.

[mithro-old, 2014-11-04 07:33:28]

The CQ bit was checked by mithro@mithis.com

[commit-bot: I haz the power, 2014-11-04 07:35:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/693113002/20001

[commit-bot: I haz the power, 2014-11-04 08:19:27]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2014-11-04 08:20:07]

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