Fix bookmark bubble/star view highlighting for Harmony.

Fix bookmark bubble/star view highlighting for Harmony.

Also remove BrowserView::GetToolbarView() because it was redundant
with BrowserView::toolbar().

BUG=651138
Committed: https://crrev.com/65ebe03b5aad3a435abe4f1659f4cf82a0baafa9
Cr-Commit-Position: refs/heads/master@{#421926}

[Evan Stade, 2016-09-28 22:29:28]

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

[commit-bot: I haz the power, 2016-09-28 22:29:51]

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

[Evan Stade, 2016-09-28 22:30:58]

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

[Evan Stade, 2016-09-28 22:30:59]

https://codereview.chromium.org/2374253002/diff/1/chrome/browser/ui/views/too...
File chrome/browser/ui/views/toolbar/toolbar_view.cc (right):

https://codereview.chromium.org/2374253002/diff/1/chrome/browser/ui/views/too...
chrome/browser/ui/views/toolbar/toolbar_view.cc:311: void
ToolbarView::OnBubbleCreatedForAnchor(views::View* anchor_view,
I honestly don't see what the purpose of this function is. We will probably wind
up removing it after applying similar changes for the two views that are left.

[commit-bot: I haz the power, 2016-09-28 22:42:02]

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

[commit-bot: I haz the power, 2016-09-28 22:42:03]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[sky, 2016-09-29 03:17:40]

https://codereview.chromium.org/2374253002/diff/1/chrome/browser/ui/views/fra...
File chrome/browser/ui/views/frame/browser_view.cc (right):

https://codereview.chromium.org/2374253002/diff/1/chrome/browser/ui/views/fra...
chrome/browser/ui/views/frame/browser_view.cc:1247: views::View* anchor_view =
toolbar_->GetBookmarkBubbleAnchor();
I find this change confusing because it's assuming a particular implementation
in Toolbarview. That is, it's assuming star view is the one that wants to be the
animation observer and that anchor_view may be something else. How about
forwarding the ShowBookmarkBubble call to ToolbarView as it's really the one
that knows how everything is wired. I think that would mean Toolbarview doesn't
need to expose GetBookmarkBubbleAnchor(), which seems like a good thing too.

[Evan Stade, 2016-09-29 17:12:31]

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

[commit-bot: I haz the power, 2016-09-29 17:12:48]

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

[Evan Stade, 2016-09-29 17:19:58]

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

[Evan Stade, 2016-09-29 17:20:01]

https://codereview.chromium.org/2374253002/diff/1/chrome/browser/ui/views/fra...
File chrome/browser/ui/views/frame/browser_view.cc (right):

https://codereview.chromium.org/2374253002/diff/1/chrome/browser/ui/views/fra...
chrome/browser/ui/views/frame/browser_view.cc:1247: views::View* anchor_view =
toolbar_->GetBookmarkBubbleAnchor();
On 2016/09/29 03:17:40, sky wrote:
> I find this change confusing because it's assuming a particular implementation
> in Toolbarview. That is, it's assuming star view is the one that wants to be
the
> animation observer and that anchor_view may be something else. How about
> forwarding the ShowBookmarkBubble call to ToolbarView as it's really the one
> that knows how everything is wired. I think that would mean Toolbarview
doesn't
> need to expose GetBookmarkBubbleAnchor(), which seems like a good thing too.

sure, sounds reasonable. done.

[commit-bot: I haz the power, 2016-09-29 17:20:20]

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-09-29 18:04:48]

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

[commit-bot: I haz the power, 2016-09-29 18:04:49]

Dry run: This issue passed the CQ dry run.

[sky, 2016-09-29 18:24:19]

Thanks! LGTM

[Evan Stade, 2016-09-29 20:35:27]

The CQ bit was checked by estade@chromium.org

[commit-bot: I haz the power, 2016-09-29 20:35:46]

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-09-29 20:41:59]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-09-29 20:44:11]

Description was changed from

==========
Fix bookmark bubble/star view highlighting for Harmony.

Also remove BrowserView::GetToolbarView() because it was redundant
with BrowserView::toolbar().

BUG=651138
==========

to

==========
Fix bookmark bubble/star view highlighting for Harmony.

Also remove BrowserView::GetToolbarView() because it was redundant
with BrowserView::toolbar().

BUG=651138

Committed: https://crrev.com/65ebe03b5aad3a435abe4f1659f4cf82a0baafa9
Cr-Commit-Position: refs/heads/master@{#421926}
==========

[commit-bot: I haz the power, 2016-09-29 20:44:12]

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