views: Fix progress bar tooltip text function.

ui/views/controls/progress_bar.cc

Index: ui/views/controls/progress_bar.cc
diff --git a/ui/views/controls/progress_bar.cc b/ui/views/controls/progress_bar.cc
index 0b93a1fc3f9552a1393efd040bef4353305f1eac..4b1c74a1909d168ae78977a4072534b43f29b4af 100644
--- a/ui/views/controls/progress_bar.cc
+++ b/ui/views/controls/progress_bar.cc
@@ -168,11 +168,11 @@ void ProgressBar::SetTooltipText(const string16& tooltip_text) {
 }
 
 bool ProgressBar::GetTooltipText(const gfx::Point& p, string16* tooltip) const {
-  DCHECK(tooltip);

[Peter Kasting, 2012/10/15 19:03:10]

Why remove this?

-  if (tooltip == NULL)
+  if (tooltip_text_.empty())

[Peter Kasting, 2012/10/15 19:03:10]

This is a behavior change: the old code still assigned to |tooltip| in this
case.  Are we guaranteed the new behavior is safe and correct?  Note that the
old behavior is actually simpler to implement.

[tfarina, 2012/10/15 19:15:49]

The impact would be minimal, ProgressBar control is used only by
ash/system/drive/tray_drive.cc and progress_bar_example.cc
It seems more correct to me and in pair with the implementation of button.cc and
label.cc
I'm fine to back to it if you want, and just remove the handle of DCHECK
failure.

[Peter Kasting, 2012/10/15 19:19:54]

As long as the callers work fine with the behavioral change here I'm OK with it,
it's good to preserve consistency with button.cc and label.cc.

However I wonder if the better fix is to also change those (and any other
similar implementations of GetTooltipText()) to just always set the passed-in
string.

     return false;
-  tooltip->assign(tooltip_text_);
-  return !tooltip_text_.empty();
+
+  *tooltip = tooltip_text_;
+  return true;
 }
 
 void ProgressBar::GetAccessibleState(ui::AccessibleViewState* state) {