New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 797897 link

Starred by 3 users

Issue metadata

Status: Started
Owner:
Last visit > 30 days ago
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 796544



Sign in to add a comment

Change toolbar code to talk to IdentityManager rather than SigninManager

Project Member Reported by blundell@chromium.org, Dec 28 2017

Issue description

This should be straightforward: media_router_contextual_menu.cc checks whether the user is signed in via SigninManager to determine whether to display a given command toggle.

app_menu_model.cc’s includes look like they’re unused and can be removed.
 
Components: Internals>Services>Identity
Blocking: 796544
Status: Available (was: Untriaged)
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 28 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c12550526c2bbd8c4da21d388312030a027fb567

commit c12550526c2bbd8c4da21d388312030a027fb567
Author: Jean-denis Muys <jdmuys@chromium.org>
Date: Thu Jun 28 08:48:39 2018

Remove unused signin includes in app_menu_model.cc

Converting code base away from direct useage of components/signin.
In this case the 4 include files were unused.

Also added include of signin_metrics.h, directly used in method
AddGlobalErrorMenuItems, but only included indirectly previously.

Bug: 797897
Change-Id: Ifa9af45e12da9651bab558d2c898f8ce6fa56299
Reviewed-on: https://chromium-review.googlesource.com/1057332
Reviewed-by: Trent Apted <tapted@chromium.org>
Commit-Queue: Jean-Denis Muys <jdmuys@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571049}
[modify] https://crrev.com/c12550526c2bbd8c4da21d388312030a027fb567/chrome/browser/ui/toolbar/app_menu_model.cc

Owner: jdmuys@chromium.org
Status: Started (was: Available)
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6769ee5104fc205c3e2128043b413ae5d9eba209

commit 6769ee5104fc205c3e2128043b413ae5d9eba209
Author: Jean-denis Muys <jdmuys@chromium.org>
Date: Thu Aug 09 10:40:31 2018

MediaRouterContextualMenu: Convert from using SigninManager to using IdentityManager

The unittest still requires knowledge of SigninManager, as the IdentityManager testing facilities
being used require passing in the SigninManager.
Affects media_router_contextual_menu_unittest.cc

Bug: 797897
Change-Id: I94d3fd68cff3143e461dc9ece3ae860d28d1f3ae
Reviewed-on: https://chromium-review.googlesource.com/1139061
Commit-Queue: Jean-Denis Muys <jdmuys@chromium.org>
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581854}
[modify] https://crrev.com/6769ee5104fc205c3e2128043b413ae5d9eba209/chrome/browser/ui/toolbar/media_router_contextual_menu.cc
[modify] https://crrev.com/6769ee5104fc205c3e2128043b413ae5d9eba209/chrome/browser/ui/toolbar/media_router_contextual_menu_unittest.cc

Sign in to add a comment