New issue
Advanced search Search tips

Issue 913850 link

Starred by 1 user
Status: Fixed
Owner:
svil...@igalia.com
Closed: Dec 14
Cc:
msarda@chromium.org
svil...@igalia.com
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 796544



Sign in to add a comment

Eliminate *::DiagnosticObserver::NotifySigninValueChanged() and its usage

Project Member Reported by blundell@chromium.org, Dec 11 
See the discussion on https://chromium-review.googlesource.com/c/chromium/src/+/1361066: The semantics of this observer method are strange and unclear, and rather than fixing it up, we can just eliminate it as it is used only in legacy code.

This bug tracks eliminating the usage, followed by eliminating the observer methods and their implementations from IdentityManager and SigninManager.

This will be a nice cleanup of the IdentityManager API surface, as we won't need this strange observer method in it.

Comment 1 by blundell@chromium.org, Dec 11

Labels: -Pri-2 Proj-Servicification-VendorBug Pri-1
Owner: ----
Status: Available (was: Assigned)

Comment 2 by svil...@igalia.com, Dec 11

Owner: svil...@igalia.com
I think I'm taking this as I'm familiar with the discussion
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 14 

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

commit af91372fe82b802c7a777bff834c58db2fec8af5
Author: Sergio Villar Senin <svillar@igalia.com>
Date: Fri Dec 14 14:32:42 2018

Remove DiagnosticObserver::NotifySigninValueChanged() and its usage

The semantics of this observer method are strange and unclear, and
rather than fixing it up, we can just eliminate it as it is used only
in legacy code.

By removing this we can cleanup the API surface of SigninManager and
IdentityManager.

Bug:  913850 
Change-Id: I7e7ebc455eaab208d57b91f7398055f62a915af5
Reviewed-on: https://chromium-review.googlesource.com/c/1373453
Commit-Queue: Sergio Villar <svillar@igalia.com>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616665}
[modify] https://crrev.com/af91372fe82b802c7a777bff834c58db2fec8af5/components/signin/core/browser/about_signin_internals.cc
[modify] https://crrev.com/af91372fe82b802c7a777bff834c58db2fec8af5/components/signin/core/browser/about_signin_internals.h
[modify] https://crrev.com/af91372fe82b802c7a777bff834c58db2fec8af5/components/signin/core/browser/signin_internals_util.cc
[modify] https://crrev.com/af91372fe82b802c7a777bff834c58db2fec8af5/components/signin/core/browser/signin_internals_util.h
[modify] https://crrev.com/af91372fe82b802c7a777bff834c58db2fec8af5/components/signin/core/browser/signin_manager.cc
[modify] https://crrev.com/af91372fe82b802c7a777bff834c58db2fec8af5/components/signin/core/browser/signin_manager_base.cc
[modify] https://crrev.com/af91372fe82b802c7a777bff834c58db2fec8af5/components/signin/core/browser/signin_manager_base.h
[modify] https://crrev.com/af91372fe82b802c7a777bff834c58db2fec8af5/services/identity/public/cpp/identity_manager.cc
[modify] https://crrev.com/af91372fe82b802c7a777bff834c58db2fec8af5/services/identity/public/cpp/identity_manager.h

Comment 4 by svil...@igalia.com, Dec 14

Status: Fixed (was: Available)
►

Sign in to add a comment