Update the bookmark tooltip text only when necessary

Update the bookmark tooltip text only when necessary

BUG=612682
TEST=manual. see bug for repro step.

Committed: https://crrev.com/e1b6ed732fb14986aa4d1db97de7208356b861f7
Cr-Commit-Position: refs/heads/master@{#432874}

[oshima, 2016-11-16 13:52:09]

The CQ bit was checked by oshima@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-16 13:52:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[oshima, 2016-11-16 13:52:26]

Description was changed from

==========
Update the bookmark tooltip text only when necessary

BUG=612682
TEST=
==========

to

==========
Update the bookmark tooltip text only when necessary

BUG=612682
TEST=manual. see bug for repro step.
==========

[commit-bot: I haz the power, 2016-11-16 14:42:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-16 14:42:59]

Dry run: This issue passed the CQ dry run.

[oshima, 2016-11-16 16:22:20]

The CQ bit was checked by oshima@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-16 16:23:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-16 17:27:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-16 17:27:33]

Dry run: This issue passed the CQ dry run.

[oshima, 2016-11-16 22:09:15]

Patchset #2 (id:20001) has been deleted

[oshima, 2016-11-17 00:12:21]

oshima@chromium.org changed reviewers:
+ sky@chromium.org

[sky, 2016-11-17 01:08:28]

LGTM with the following.

https://codereview.chromium.org/2507743002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc (right):

https://codereview.chromium.org/2507743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc:270: int max_width =
tooltip_manager->GetMaxWidth(location);
Please add a comment as to why this is being done.

https://codereview.chromium.org/2507743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc:271: if (!tooltip_.size()
|| max_width != max_width_) {
!size() -> empty()

https://codereview.chromium.org/2507743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc:288: mutable int
max_width_ = 0;
max_width_ by itself isn't clear, especially with no comment. How about
max_tooltip_width_, and add a comment.

https://codereview.chromium.org/2507743002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/bookmarks/bookmark_bar_view.h (right):

https://codereview.chromium.org/2507743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/bookmarks/bookmark_bar_view.h:149: // text is clipped to
fit the |max_width|.
'the max_width' -> 'max_width'

[oshima, 2016-11-17 14:02:12]

Patchset #3 (id:60001) has been deleted

[oshima, 2016-11-17 14:02:33]

https://codereview.chromium.org/2507743002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc (right):

https://codereview.chromium.org/2507743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc:270: int max_width =
tooltip_manager->GetMaxWidth(location);
On 2016/11/17 01:08:28, sky wrote:
> Please add a comment as to why this is being done.

Done.

https://codereview.chromium.org/2507743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc:271: if (!tooltip_.size()
|| max_width != max_width_) {
On 2016/11/17 01:08:28, sky wrote:
> !size() -> empty()

Done.

https://codereview.chromium.org/2507743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc:288: mutable int
max_width_ = 0;
On 2016/11/17 01:08:28, sky wrote:
> max_width_ by itself isn't clear, especially with no comment. How about
> max_tooltip_width_, and add a comment.

Done.

https://codereview.chromium.org/2507743002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/bookmarks/bookmark_bar_view.h (right):

https://codereview.chromium.org/2507743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/bookmarks/bookmark_bar_view.h:149: // text is clipped to
fit the |max_width|.
On 2016/11/17 01:08:28, sky wrote:
> 'the max_width' -> 'max_width'

Done.

[oshima, 2016-11-17 14:02:38]

The CQ bit was checked by oshima@chromium.org

[oshima, 2016-11-17 14:02:38]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2507743002/#ps80001
(title: "Update the bookmark tooltip text only when necessary")

[commit-bot: I haz the power, 2016-11-17 14:03:01]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-17 14:53:46]

Description was changed from

==========
Update the bookmark tooltip text only when necessary

BUG=612682
TEST=manual. see bug for repro step.
==========

to

==========
Update the bookmark tooltip text only when necessary

BUG=612682
TEST=manual. see bug for repro step.
==========

[commit-bot: I haz the power, 2016-11-17 14:53:48]

Committed patchset #3 (id:80001)

[commit-bot: I haz the power, 2016-11-17 14:55:57]

Description was changed from

==========
Update the bookmark tooltip text only when necessary

BUG=612682
TEST=manual. see bug for repro step.
==========

to

==========
Update the bookmark tooltip text only when necessary

BUG=612682
TEST=manual. see bug for repro step.

Committed: https://crrev.com/e1b6ed732fb14986aa4d1db97de7208356b861f7
Cr-Commit-Position: refs/heads/master@{#432874}
==========

[commit-bot: I haz the power, 2016-11-17 14:55:58]

Patchset 3 (id:??) landed as
https://crrev.com/e1b6ed732fb14986aa4d1db97de7208356b861f7
Cr-Commit-Position: refs/heads/master@{#432874}