Cleanup: Rename GetHeight to more appropriate GetPreferredInternalHeight.

Cleanup: Rename GetHeight to more appropriate GetPreferredInternalHeight.

Also, add impl method that lets us reuse logic in another place that
was doing the same calculation.

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

[Jói, 2012-05-17 21:18:16]

Hi Peter,

Looking at the code a bit to address your outstanding post-commit nit from the
last review you did for me (http://codereview.chromium.org/9985012/), I realized
that naming the method GetHeight() is a bit misleading, as this height is the
internal height after insets.

Therefore, renamed it GetPreferredInternalHeight() and made it call an impl
function that could also be used from the place you noted we were doing
essentially the same calculation.

I may be on a plane by the time you see this, so please if you are happy with
the patch as it stands, feel free to set the CQ bit for me.

Cheers,
Jói

[Peter Kasting, 2012-05-18 22:45:40]

LGTM but at least consider the second nit below.

http://codereview.chromium.org/10399074/diff/1/chrome/browser/ui/views/locati...
File chrome/browser/ui/views/location_bar/location_bar_view.cc (right):

http://codereview.chromium.org/10399074/diff/1/chrome/browser/ui/views/locati...
chrome/browser/ui/views/location_bar/location_bar_view.cc:1279: int
LocationBarView::GetPreferredInternalHeight() {
Nit: Functions must be defined in the same order they're declared.

http://codereview.chromium.org/10399074/diff/1/chrome/browser/ui/views/locati...
File chrome/browser/ui/views/location_bar/location_bar_view.h (right):

http://codereview.chromium.org/10399074/diff/1/chrome/browser/ui/views/locati...
chrome/browser/ui/views/location_bar/location_bar_view.h:316: int
GetPreferredInternalHeight();
Nit: I still think this is preferable:

  // Returns the height of the control without the top and bottom edges (i.e.
  // the height of the edit control inside).  If |use_preferred_size| is true
  // this will be the preferred height, otherwise it will be the current height.
  int GetInternalHeight(bool use_preferred_size);

This is not only simpler but seems clearer to me.

[Jói, 2012-05-21 13:34:37]

Modified the patch to match your suggestion.  Flipping the CQ bit now.

Cheers,
Jói

[commit-bot: I haz the power, 2012-05-21 13:34:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joi@chromium.org/10399074/6001

[commit-bot: I haz the power, 2012-05-21 21:24:09]

The commit queue went berserk retrying too often for a
seemingly flaky test. Builder is win_rel, revision is 138140, job name
was 10399074-6001 (retry) (retry) (retry) (retry).

[commit-bot: I haz the power, 2012-05-21 21:34:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joi@chromium.org/10399074/6001

[commit-bot: I haz the power, 2012-05-21 23:02:57]

Try job failure for 10399074-6001 (retry) on win_rel for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-05-22 09:59:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joi@chromium.org/10399074/6001

[commit-bot: I haz the power, 2012-05-22 12:03:55]

Failed to apply the patch.

[commit-bot: I haz the power, 2012-05-22 12:16:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joi@chromium.org/10399074/1007

[commit-bot: I haz the power, 2012-05-22 15:39:38]

Try job failure for 10399074-1007 (retry) on win_rel for step "sync_unit_tests".
It's a second try, previously, steps "base_unittests, sync_unit_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...