New issue
Advanced search Search tips

Issue 685654 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 683124



Sign in to add a comment

Move identity extension API function impls to their own files

Project Member Reported by blundell@chromium.org, Jan 26 2017

Issue description

As a precursor to incrementally converting the extension API function implementations to use the Identity Service, we will move these function impls to their own files (rather than being lumped together in identity_api.cc). This will make the analysis and changes easier to perform (and should make them easier to review as well). It is also a code readability win in any case IMO.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 27 2017

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

commit 1915164756cbde01d3c694435dfa30a7742fa7d3
Author: blundell <blundell@chromium.org>
Date: Fri Jan 27 08:50:56 2017

Move impl of identity.LaunchWebAuthFlow() extension API to its own file

As a precursor to a refactoring of the identity extension API implementation
to use the Identity Service, I am moving the implementations of the six
identity extension API functions to their own files, rather than all
being lumped together in identity_api.*. This will make it easier to
analyze each of these function implementations independently and
incrementally convert them to use the Identity Service.

BUG= 685654 

Review-Url: https://codereview.chromium.org/2654023005
Cr-Commit-Position: refs/heads/master@{#446627}

[modify] https://crrev.com/1915164756cbde01d3c694435dfa30a7742fa7d3/chrome/browser/extensions/BUILD.gn
[modify] https://crrev.com/1915164756cbde01d3c694435dfa30a7742fa7d3/chrome/browser/extensions/api/identity/identity_api.cc
[modify] https://crrev.com/1915164756cbde01d3c694435dfa30a7742fa7d3/chrome/browser/extensions/api/identity/identity_api.h
[modify] https://crrev.com/1915164756cbde01d3c694435dfa30a7742fa7d3/chrome/browser/extensions/api/identity/identity_apitest.cc
[add] https://crrev.com/1915164756cbde01d3c694435dfa30a7742fa7d3/chrome/browser/extensions/api/identity/identity_launch_web_auth_flow_function.cc
[add] https://crrev.com/1915164756cbde01d3c694435dfa30a7742fa7d3/chrome/browser/extensions/api/identity/identity_launch_web_auth_flow_function.h

Project Member

Comment 2 by bugdroid1@chromium.org, Jan 27 2017

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

commit fb7f1c578c62b98cc6a6e3c1c20c90408cb42213
Author: blundell <blundell@chromium.org>
Date: Fri Jan 27 15:45:00 2017

Move impl of identity.RemoveCachedAuthToken() extension API to its own file

As a precursor to a refactoring of the identity extension API implementation
to use the Identity Service, I am moving the implementations of the six
identity extension API functions to their own files, rather than all
being lumped together in identity_api.*. This will make it easier to
analyze each of these function implementations independently and
incrementally convert them to use the Identity Service.

BUG= 685654 

Review-Url: https://codereview.chromium.org/2657223002
Cr-Commit-Position: refs/heads/master@{#446673}

[modify] https://crrev.com/fb7f1c578c62b98cc6a6e3c1c20c90408cb42213/chrome/browser/extensions/BUILD.gn
[modify] https://crrev.com/fb7f1c578c62b98cc6a6e3c1c20c90408cb42213/chrome/browser/extensions/api/identity/identity_api.cc
[modify] https://crrev.com/fb7f1c578c62b98cc6a6e3c1c20c90408cb42213/chrome/browser/extensions/api/identity/identity_api.h
[modify] https://crrev.com/fb7f1c578c62b98cc6a6e3c1c20c90408cb42213/chrome/browser/extensions/api/identity/identity_apitest.cc
[add] https://crrev.com/fb7f1c578c62b98cc6a6e3c1c20c90408cb42213/chrome/browser/extensions/api/identity/identity_remove_cached_auth_token_function.cc
[add] https://crrev.com/fb7f1c578c62b98cc6a6e3c1c20c90408cb42213/chrome/browser/extensions/api/identity/identity_remove_cached_auth_token_function.h

Project Member

Comment 3 by bugdroid1@chromium.org, Feb 1 2017

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

commit 06fb5480e59a9ad8f10a733c7e45bec4f4c7bec9
Author: blundell <blundell@chromium.org>
Date: Wed Feb 01 17:27:27 2017

Move impl of identity.GetProfileUserInfo() extension API to its own file

As a precursor to a refactoring of the identity extension API implementation
to use the Identity Service, I am moving the implementations of the six
identity extension API functions to their own files, rather than all
being lumped together in identity_api.*. This will make it easier to
analyze each of these function implementations independently and
incrementally convert them to use the Identity Service.

Of note: This CL makes a minor code change to eliminate usage of the
GetPrimaryAccountId() helper function defined in identity_api.cc.
The new code is functionally equivalent to the old code: both are
effectively AccountTrackerService::GetAccountInfo(
    SigninManager::GetAuthenticatedAccountId()), wherein the
AccountTrackerService and SigninManager instances are the ones
corresponding to the Profile in question.

BUG= 685654 

Review-Url: https://codereview.chromium.org/2663883003
Cr-Commit-Position: refs/heads/master@{#447548}

[modify] https://crrev.com/06fb5480e59a9ad8f10a733c7e45bec4f4c7bec9/chrome/browser/extensions/BUILD.gn
[modify] https://crrev.com/06fb5480e59a9ad8f10a733c7e45bec4f4c7bec9/chrome/browser/extensions/api/identity/identity_api.cc
[modify] https://crrev.com/06fb5480e59a9ad8f10a733c7e45bec4f4c7bec9/chrome/browser/extensions/api/identity/identity_api.h
[modify] https://crrev.com/06fb5480e59a9ad8f10a733c7e45bec4f4c7bec9/chrome/browser/extensions/api/identity/identity_apitest.cc
[add] https://crrev.com/06fb5480e59a9ad8f10a733c7e45bec4f4c7bec9/chrome/browser/extensions/api/identity/identity_get_profile_user_info_function.cc
[add] https://crrev.com/06fb5480e59a9ad8f10a733c7e45bec4f4c7bec9/chrome/browser/extensions/api/identity/identity_get_profile_user_info_function.h

Project Member

Comment 4 by bugdroid1@chromium.org, Feb 3 2017

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

commit 202f1212a29d67d81515dfa55f31b0ec8501efb3
Author: blundell <blundell@chromium.org>
Date: Fri Feb 03 09:30:23 2017

Remove IdentityGetAuthTokenFunction interface dep on IdentityAPI

In preparation for IdentityGetAuthTokenFunction being moved to its own
file, we must break the cyclic interface dependency between IdentityAPI
and IdentityGetAuthTokenFunction (identity_api.h will need to include
identity_get_auth_token_function.h for the extension registration
mechanism to see the latter). This CL removes IdentityAPI::ShutdownObserver
and replaces it with IdentityAPI directly calling
IdentityGetAuthTokenFunction::Shutdown().

BUG= 685654 

Review-Url: https://codereview.chromium.org/2673443002
Cr-Commit-Position: refs/heads/master@{#447970}

[modify] https://crrev.com/202f1212a29d67d81515dfa55f31b0ec8501efb3/chrome/browser/extensions/api/identity/identity_api.cc
[modify] https://crrev.com/202f1212a29d67d81515dfa55f31b0ec8501efb3/chrome/browser/extensions/api/identity/identity_api.h
[modify] https://crrev.com/202f1212a29d67d81515dfa55f31b0ec8501efb3/chrome/browser/extensions/api/identity/identity_apitest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Feb 3 2017

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

commit e1b2ceb34172034512c26b014f8c2e4900187171
Author: blundell <blundell@chromium.org>
Date: Fri Feb 03 13:03:15 2017

Move impl of identity.GetAuthToken() extension API to its own file

As a precursor to a refactoring of the identity extension API implementation
to use the Identity Service, I am moving the implementations of the six
identity extension API functions to their own files, rather than all
being lumped together in identity_api.*. This will make it easier to
analyze each of these function implementations independently and
incrementally convert them to use the Identity Service.

BUG= 685654 

Review-Url: https://codereview.chromium.org/2668203004
Cr-Commit-Position: refs/heads/master@{#447981}

[modify] https://crrev.com/e1b2ceb34172034512c26b014f8c2e4900187171/chrome/browser/extensions/BUILD.gn
[modify] https://crrev.com/e1b2ceb34172034512c26b014f8c2e4900187171/chrome/browser/extensions/api/identity/identity_api.cc
[modify] https://crrev.com/e1b2ceb34172034512c26b014f8c2e4900187171/chrome/browser/extensions/api/identity/identity_api.h
[modify] https://crrev.com/e1b2ceb34172034512c26b014f8c2e4900187171/chrome/browser/extensions/api/identity/identity_apitest.cc
[add] https://crrev.com/e1b2ceb34172034512c26b014f8c2e4900187171/chrome/browser/extensions/api/identity/identity_get_auth_token_function.cc
[add] https://crrev.com/e1b2ceb34172034512c26b014f8c2e4900187171/chrome/browser/extensions/api/identity/identity_get_auth_token_function.h

Project Member

Comment 6 by bugdroid1@chromium.org, Feb 3 2017

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

commit e1b2a78c415b404e5dc975c34d5e0b4dee94ba1f
Author: blundell <blundell@chromium.org>
Date: Fri Feb 03 15:00:05 2017

Move impl of identity.GetAccounts() extension API to its own file

As a precursor to a refactoring of the identity extension API implementation
to use the Identity Service, I am moving the implementations of the six
identity extension API functions to their own files, rather than all
being lumped together in identity_api.*. This will make it easier to
analyze each of these function implementations independently and
incrementally convert them to use the Identity Service.

BUG= 685654 

Review-Url: https://codereview.chromium.org/2671653004
Cr-Commit-Position: refs/heads/master@{#447998}

[modify] https://crrev.com/e1b2a78c415b404e5dc975c34d5e0b4dee94ba1f/chrome/browser/extensions/BUILD.gn
[modify] https://crrev.com/e1b2a78c415b404e5dc975c34d5e0b4dee94ba1f/chrome/browser/extensions/api/identity/identity_api.cc
[modify] https://crrev.com/e1b2a78c415b404e5dc975c34d5e0b4dee94ba1f/chrome/browser/extensions/api/identity/identity_api.h
[modify] https://crrev.com/e1b2a78c415b404e5dc975c34d5e0b4dee94ba1f/chrome/browser/extensions/api/identity/identity_apitest.cc
[add] https://crrev.com/e1b2a78c415b404e5dc975c34d5e0b4dee94ba1f/chrome/browser/extensions/api/identity/identity_get_accounts_function.cc
[add] https://crrev.com/e1b2a78c415b404e5dc975c34d5e0b4dee94ba1f/chrome/browser/extensions/api/identity/identity_get_accounts_function.h

Status: Fixed (was: Started)

Comment 8 by st...@chromium.org, Feb 8 2017

Cc: rdevlin....@chromium.org michae...@chromium.org
Adding some folks who should know about these changes.

Comment 9 by r...@chromium.org, Mar 3 2017

Cc: r...@chromium.org

Sign in to add a comment