New issue
Advanced search Search tips

Issue 919519 link

Starred by 2 users
Status: Fixed
Owner:
blundell@chromium.org
Closed: Jan 9
Cc:
ma...@igalia.com
bsazonov@chromium.org
msarda@chromium.org
sdefresne@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 907901



Sign in to add a comment

Support AccountsMutator only on !Android && !iOS

Project Member Reported by blundell@chromium.org, Jan 7 
To summarize offline discussion that occurred between Boris, Mihai, Sylvain and me: 

* The overall API shape is good for platforms where refresh token mutation is supported but simply isn't applicable for platforms where it's not (Android/iOS).
* Hence, AccountsMutator will not be supported on Android/iOS (analogous to PrimaryAccountMutator).
* Cross-platform tests that need to pre-populate IdentityManager with a given state of accounts should use IdentityTestEnvironment/identity_test_utils.* as they are doing today.
* If we have any legacy usages of APIs that we are looking to avoid supporting on Android/iOS in the long-term, we will port them via Legacy*() methods on IdentityManager and/or a LegacyAccountsMutator class, with bugs for eliminating these.
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 9 

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

commit 0ec006c5b6890fad572b4540960d2e0a6a77952b
Author: Colin Blundell <blundell@chromium.org>
Date: Wed Jan 09 12:13:51 2019

Do not support AccountsMutator on mobile

This CL changes AccountsMutator to be supported only when not on
Android and iOS.

This comprises three changes:

- Have IdentityManager take in AccountsMutator rather than build it
  internally
- Change AccountsMutator into a pure interface with an
  AccountsMutatorImpl implementation
- Have AccountsMutatorImpl be built/passed in only when not on Android
  or iOS

The structure of AccountsMutator now parallels the structure of
PrimaryAccountMutator, which is built/used only when not on ChromeOS.

See the bug for the motivations for this change.

TBR=msarda@chromium.org

Bug:  919519 
Change-Id: I1255410b03818ce0dfddf902d23c979b9e2c0023
Reviewed-on: https://chromium-review.googlesource.com/c/1400581
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621118}
[modify] https://crrev.com/0ec006c5b6890fad572b4540960d2e0a6a77952b/chrome/browser/signin/identity_manager_factory.cc
[modify] https://crrev.com/0ec006c5b6890fad572b4540960d2e0a6a77952b/ios/chrome/browser/signin/identity_manager_factory.cc
[modify] https://crrev.com/0ec006c5b6890fad572b4540960d2e0a6a77952b/ios/web_view/internal/signin/web_view_identity_manager_factory.mm
[modify] https://crrev.com/0ec006c5b6890fad572b4540960d2e0a6a77952b/services/identity/BUILD.gn
[modify] https://crrev.com/0ec006c5b6890fad572b4540960d2e0a6a77952b/services/identity/public/cpp/BUILD.gn
[delete] https://crrev.com/63f8d538f64e348815218755ff634663221ea238/services/identity/public/cpp/accounts_mutator.cc
[modify] https://crrev.com/0ec006c5b6890fad572b4540960d2e0a6a77952b/services/identity/public/cpp/accounts_mutator.h
[add] https://crrev.com/0ec006c5b6890fad572b4540960d2e0a6a77952b/services/identity/public/cpp/accounts_mutator_impl.cc
[add] https://crrev.com/0ec006c5b6890fad572b4540960d2e0a6a77952b/services/identity/public/cpp/accounts_mutator_impl.h
[rename] https://crrev.com/0ec006c5b6890fad572b4540960d2e0a6a77952b/services/identity/public/cpp/accounts_mutator_impl_unittest.cc
[modify] https://crrev.com/0ec006c5b6890fad572b4540960d2e0a6a77952b/services/identity/public/cpp/identity_manager.cc
[modify] https://crrev.com/0ec006c5b6890fad572b4540960d2e0a6a77952b/services/identity/public/cpp/identity_manager.h
[modify] https://crrev.com/0ec006c5b6890fad572b4540960d2e0a6a77952b/services/identity/public/cpp/identity_manager_unittest.cc
[modify] https://crrev.com/0ec006c5b6890fad572b4540960d2e0a6a77952b/services/identity/public/cpp/identity_test_environment.cc

Comment 2 by blundell@chromium.org, Jan 9

Status: Fixed (was: Started)
►

Sign in to add a comment