views: Fix progress bar tooltip text function.

ui/views/controls/progress_bar.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/views/controls/progress_bar.h"
 
 #include <algorithm>
 #include <string>
 
 #include "base/logging.h"
@@ skipping 150 matching lines @@
     current_value_ = value;
     SchedulePaint();
   }
 }
 
 void ProgressBar::SetTooltipText(const string16& tooltip_text) {
   tooltip_text_ = 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) {
   state->role = ui::AccessibilityTypes::ROLE_PROGRESSBAR;
   state->state = ui::AccessibilityTypes::STATE_READONLY;
 }
 
 gfx::Size ProgressBar::GetPreferredSize() {
   return gfx::Size(100, 16);
 }
@@ skipping 109 matching lines @@
   if (progress_width > 1) {
     FillRoundRect(canvas, 0, 0, progress_width, height(), kCornerRadius,
                   kBarColorStart, kBarColorEnd, false);
   }
   StrokeRoundRect(canvas, 0, 0, width(), height(), kCornerRadius,
                   kBorderColor, kBorderWidth);
 #endif
 }
 
 }  // namespace views