Make tooltips toplevel windows on Linux

Make tooltips have their own toplevel window on Linux. This allows tooltips for bookmarks in a menu not to be clipped.

BUG=353533
TEST=Manual, see bug

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

[pkotwicz, 2014-05-16 02:22:53]

Elliot, can you please take a look?

This CL depends on https://codereview.chromium.org/282133005/

https://codereview.chromium.org/290573003/diff/40001/chrome/browser/ui/libgtk...
File chrome/browser/ui/libgtk2ui/native_theme_gtk2.cc (right):

https://codereview.chromium.org/290573003/diff/40001/chrome/browser/ui/libgtk...
chrome/browser/ui/libgtk2ui/native_theme_gtk2.cc:389:
gtk_widget_set_name(fake_tooltip_, "gtk-tooltip");
I found this by luck. I have not found any official documentation which suggests
doing this.

"gtk-tooltip" is mentioned in _gtk_rc_init()
gtk_tooltip_init() does call gtk_widget_set_name() with "gtk-tooltip"

This returns the correct colors when the tooltip colors are changed via
gnome-color-chooser

https://codereview.chromium.org/290573003/diff/40001/ui/views/corewm/tooltip_...
File ui/views/corewm/tooltip_aura.cc (right):

https://codereview.chromium.org/290573003/diff/40001/ui/views/corewm/tooltip_...
ui/views/corewm/tooltip_aura.cc:203: if (!widget_) {
Is it worth adding a comment as to why we want to avoid recreating the tooltip?

[Elliot Glaysher, 2014-05-16 19:39:41]

+oshima for question about Ash + multiple root windows * the change in
tooltip_aura.cc

https://codereview.chromium.org/290573003/diff/40001/chrome/browser/ui/libgtk...
File chrome/browser/ui/libgtk2ui/native_theme_gtk2.cc (right):

https://codereview.chromium.org/290573003/diff/40001/chrome/browser/ui/libgtk...
chrome/browser/ui/libgtk2ui/native_theme_gtk2.cc:389:
gtk_widget_set_name(fake_tooltip_, "gtk-tooltip");
On 2014/05/16 02:22:53, pkotwicz wrote:
> I found this by luck. I have not found any official documentation which
suggests
> doing this.
> 
> "gtk-tooltip" is mentioned in _gtk_rc_init()
> gtk_tooltip_init() does call gtk_widget_set_name() with "gtk-tooltip"
> 
> This returns the correct colors when the tooltip colors are changed via
> gnome-color-chooser

A lot of this stuff isn't documented and has to be inferred from the  gtk source
code, sadly.

https://codereview.chromium.org/290573003/diff/40001/ui/views/corewm/tooltip_...
File ui/views/corewm/tooltip_aura.cc (right):

https://codereview.chromium.org/290573003/diff/40001/ui/views/corewm/tooltip_...
ui/views/corewm/tooltip_aura.cc:203: if (!widget_) {
On 2014/05/16 02:22:53, pkotwicz wrote:
> Is it worth adding a comment as to why we want to avoid recreating the
tooltip?

I'm not sure if this is safe; IIRC, chromeos can have multiple root windows, and
I thought this class was shared between linux and chromeos.

[oshima, 2014-05-16 19:49:48]

https://codereview.chromium.org/290573003/diff/40001/ui/views/corewm/tooltip_...
File ui/views/corewm/tooltip_aura.cc (right):

https://codereview.chromium.org/290573003/diff/40001/ui/views/corewm/tooltip_...
ui/views/corewm/tooltip_aura.cc:203: if (!widget_) {
On 2014/05/16 19:39:42, Elliot Glaysher wrote:
> On 2014/05/16 02:22:53, pkotwicz wrote:
> > Is it worth adding a comment as to why we want to avoid recreating the
> tooltip?
> 
> I'm not sure if this is safe; IIRC, chromeos can have multiple root windows,
and
> I thought this class was shared between linux and chromeos.

yes, erg@ is correct.

https://code.google.com/p/chromium/codesearch#chromium/src/ash/shell.cc&l=961

[pkotwicz, 2014-05-16 20:02:16]

https://codereview.chromium.org/290573003/diff/40001/ui/views/corewm/tooltip_...
File ui/views/corewm/tooltip_aura.cc (right):

https://codereview.chromium.org/290573003/diff/40001/ui/views/corewm/tooltip_...
ui/views/corewm/tooltip_aura.cc:203: if (!widget_) {
My understanding is that views::Widget::SetBounds() will reparent the tooltip to
the new root window if necessary on CrOS
erg@, oshima@, am I missing something?

[Elliot Glaysher, 2014-05-16 20:07:19]

lgtm

https://codereview.chromium.org/290573003/diff/40001/ui/views/corewm/tooltip_...
File ui/views/corewm/tooltip_aura.cc (right):

https://codereview.chromium.org/290573003/diff/40001/ui/views/corewm/tooltip_...
ui/views/corewm/tooltip_aura.cc:203: if (!widget_) {
On 2014/05/16 20:02:16, pkotwicz wrote:
> My understanding is that views::Widget::SetBounds() will reparent the tooltip
to
> the new root window if necessary on CrOS
> erg@, oshima@, am I missing something?

Huh, it looks like you're right. ash::ScreenPositionController::SetBounds() has
code to move windows from one RootWindow to another.

[oshima, 2014-05-16 20:11:12]

https://codereview.chromium.org/290573003/diff/40001/ui/views/corewm/tooltip_...
File ui/views/corewm/tooltip_aura.cc (right):

https://codereview.chromium.org/290573003/diff/40001/ui/views/corewm/tooltip_...
ui/views/corewm/tooltip_aura.cc:203: if (!widget_) {
On 2014/05/16 20:02:16, pkotwicz wrote:
> My understanding is that views::Widget::SetBounds() will reparent the tooltip
to
> the new root window if necessary on CrOS
> erg@, oshima@, am I missing something?

It does, but doe it based on the best matching display for given bounds, which
may not be what you want.

This is probably what you want:

widget->GetNativeView()->SetSetBoundsInScreen(bounds_in_scren,
    gfx::Screen::GetScreenFor(window)->GetDisplayNearestWindow(window));

[oshima, 2014-05-17 01:19:57]

On 2014/05/16 20:11:12, oshima wrote:
>
https://codereview.chromium.org/290573003/diff/40001/ui/views/corewm/tooltip_...
> File ui/views/corewm/tooltip_aura.cc (right):
> 
>
https://codereview.chromium.org/290573003/diff/40001/ui/views/corewm/tooltip_...
> ui/views/corewm/tooltip_aura.cc:203: if (!widget_) {
> On 2014/05/16 20:02:16, pkotwicz wrote:
> > My understanding is that views::Widget::SetBounds() will reparent the
tooltip
> to
> > the new root window if necessary on CrOS
> > erg@, oshima@, am I missing something?
> 
> It does, but doe it based on the best matching display for given bounds, which
> may not be what you want.
> 
> This is probably what you want:
> 
> widget->GetNativeView()->SetSetBoundsInScreen(bounds_in_scren,
>     gfx::Screen::GetScreenFor(window)->GetDisplayNearestWindow(window));

I looked into the code and I believe the tooltipbounds is adjusted to fit the
display bounds, so this is not necessary.
lgtm.

[pkotwicz, 2014-05-17 02:10:28]

Scott, PTAL

[sky, 2014-05-19 15:57:32]

LGTM

[pkotwicz, 2014-05-27 17:13:24]

The CQ bit was checked by pkotwicz@chromium.org

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

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

[commit-bot: I haz the power, 2014-05-27 20:12:27]

Change committed as 273014