Enable tooltips for aura.

Enable tooltips for aura.

BUG=97249
TEST=none


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=107630

[Ben Goodger (Google), 2011-10-25 22:24:44]

http://codereview.chromium.org/8371024/diff/11001/ui/aura/window.h
File ui/aura/window.h (right):

http://codereview.chromium.org/8371024/diff/11001/ui/aura/window.h#newcode71
ui/aura/window.h:71: const string16& tooltip() const { return tooltip_; }
Seems like at the shell level we are always querying the TTM for the current
text... so we don't need to carry this anywhere...

[varunjain, 2011-10-25 22:35:05]

http://codereview.chromium.org/8371024/diff/11001/ui/aura/window.h
File ui/aura/window.h (right):

http://codereview.chromium.org/8371024/diff/11001/ui/aura/window.h#newcode71
ui/aura/window.h:71: const string16& tooltip() const { return tooltip_; }
On 2011/10/25 22:24:45, Ben Goodger (Google) wrote:
> Seems like at the shell level we are always querying the TTM for the current
> text... so we don't need to carry this anywhere...

I am not sure I completely understand. We never query the TTM.. in fact TTM is
the one that queries the window or view currently under the mouse cursor for the
current tooltip.

[Ben Goodger (Google), 2011-10-25 22:41:21]

OK I looked at your code a bit more.

I don't think we should explicitly add the tooltip stuff to aura::Window...
since the tooltip implementation lives in the shell and in views. Instead, we
should use ViewProps. Oshima was going to move this onto aura::Window, so you'd
be able to create an arbitrary named property (from views, or the shell, or
content module) and set it on the window, then read it back in the TTM.

[varunjain, 2011-10-26 04:44:48]

On 2011/10/25 22:41:21, Ben Goodger (Google) wrote:
> OK I looked at your code a bit more.
> 
> I don't think we should explicitly add the tooltip stuff to aura::Window...
> since the tooltip implementation lives in the shell and in views. Instead, we
> should use ViewProps. Oshima was going to move this onto aura::Window, so
you'd
> be able to create an arbitrary named property (from views, or the shell, or
> content module) and set it on the window, then read it back in the TTM.

Done. (this CL depends on oshima's change now)

[Ben Goodger (Google), 2011-10-26 19:11:22]

http://codereview.chromium.org/8371024/diff/16008/ui/aura/aura_constants.h
File ui/aura/aura_constants.h (right):

http://codereview.chromium.org/8371024/diff/16008/ui/aura/aura_constants.h#ne...
ui/aura/aura_constants.h:13: AURA_EXPORT const char* kTooltipTextKey =
"TooltipTextKey";
this file should live in aura_shell, not aura

[varunjain, 2011-10-27 04:01:29]

On 2011/10/26 19:11:22, Ben Goodger (Google) wrote:
> http://codereview.chromium.org/8371024/diff/16008/ui/aura/aura_constants.h
> File ui/aura/aura_constants.h (right):
> 
>
http://codereview.chromium.org/8371024/diff/16008/ui/aura/aura_constants.h#ne...
> ui/aura/aura_constants.h:13: AURA_EXPORT const char* kTooltipTextKey =
> "TooltipTextKey";
> this file should live in aura_shell, not aura

Done.

[Ben Goodger (Google), 2011-10-27 15:39:11]

Cool. LGTM.

http://codereview.chromium.org/8371024/diff/22003/ui/aura_shell/aura_constants.h
File ui/aura_shell/aura_constants.h (right):

http://codereview.chromium.org/8371024/diff/22003/ui/aura_shell/aura_constant...
ui/aura_shell/aura_constants.h:5: #ifndef UI_AURA_SHELL_AURA_CONSTANTS_H_
aura_shell_constants.h, etc for the include guards.

[varunjain, 2011-10-27 17:18:27]

http://codereview.chromium.org/8371024/diff/22003/ui/aura_shell/aura_constants.h
File ui/aura_shell/aura_constants.h (right):

http://codereview.chromium.org/8371024/diff/22003/ui/aura_shell/aura_constant...
ui/aura_shell/aura_constants.h:5: #ifndef UI_AURA_SHELL_AURA_CONSTANTS_H_
On 2011/10/27 15:39:11, Ben Goodger (Google) wrote:
> aura_shell_constants.h, etc for the include guards.

Done.

[commit-bot: I haz the power, 2011-10-27 17:18:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/varunjain@chromium.org/8371024/12012

[commit-bot: I haz the power, 2011-10-27 18:07:48]

Try job failure for 8371024-12012 (retry) on linux_rel for step "check_deps".
It's a second try, previously, steps "check_deps, browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2011-10-27 18:42:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/varunjain@chromium.org/8371024/12013

[commit-bot: I haz the power, 2011-10-27 21:39:37]

Change committed as 107630

[oshima, 2011-10-28 08:52:56]

drive by and q

http://codereview.chromium.org/8371024/diff/12013/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

http://codereview.chromium.org/8371024/diff/12013/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_view_aura.cc:13: #include
"ui/aura/aura_constants.h"
i saw ben's comment that this should be aura_shell/aura_shell_constants.h

has this changed? (maybe because views/content shouldn't depend on shell?)

I too need a place to put property name that is shared by views and shell and
want to know if I can use this.

http://codereview.chromium.org/8371024/diff/12013/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_view_aura.cc:185:
window_->SetProperty(aura::kTooltipTextKey, tooltip);
this will leak memory. you need to delete old property if present.

[varunjain, 2011-10-28 17:15:34]

http://codereview.chromium.org/8371024/diff/12013/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

http://codereview.chromium.org/8371024/diff/12013/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_view_aura.cc:13: #include
"ui/aura/aura_constants.h"
On 2011/10/28 08:52:56, oshima wrote:
> i saw ben's comment that this should be aura_shell/aura_shell_constants.h
> 
> has this changed? (maybe because views/content shouldn't depend on shell?)
> 
> I too need a place to put property name that is shared by views and shell and
> want to know if I can use this.

yes. content is not allowed to depend on shell.. so I moved this back to aura
after discussing it with Ben.

http://codereview.chromium.org/8371024/diff/12013/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_view_aura.cc:185:
window_->SetProperty(aura::kTooltipTextKey, tooltip);
On 2011/10/28 08:52:56, oshima wrote:
> this will leak memory. you need to delete old property if present.
>  

I will fix this.