Change undocumented semantics of chrome.identity.onSignInChangedEvent() |
||
Issue descriptionAt this time, the semantics of chrome.identity.onSignInChangedEvent() wrt secondary accounts are the following: - When a secondary account's refresh token is available, the event fires only if the primary account is present. - When a secondary account's refresh token is revoked, the event fires only if the primary account is present. - When the primary account signs in, events are fired for all secondary accounts with refresh tokens. - When the primary account signs out, events are fired for all secondary accounts with refresh tokens. In a post-AccountConsistency world, it's not clear whether this coupling between the primary and secondary accounts still makes sense, or whether we should change the semantics so that events firing for secondary accounts is decoupled from the state of the primary account.
,
Mar 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/798a107597af39aba4fd365d920d58015516c9b4 commit 798a107597af39aba4fd365d920d58015516c9b4 Author: Colin Blundell <blundell@chromium.org> Date: Thu Mar 22 15:43:47 2018 Change undefined behavior of chrome.identity.onSigninChanged event The chrome.identity.onSigninChanged extension API event is defined as follows: "Fired when signin state changes for an account on the user's profile." (https://developer.chrome.com/apps/identity#event-onSignInChanged) The current browser-side implementation uses gaia::AccountTracker, which ignores events for secondary accounts if there is no primary account (i.e., syncing account) present. Thus, this browser-side implementation also fires events for secondary accounts only if there is a primary account present. However: (1) This behavior is not defined in the documented semantics above (2) In practice, this case has historically never been encountered by end users, as it has been impossible in desktop Chrome to have a secondary account without a primary account present. This CL changes this undocumented behavior, so that events for secondary events will fire regardless of whether a primary account is present. Post-project DICE, it will be possible for this case to be encountered by end users. In that world, it will actually be more sensible for events to fire for secondary accounts regardless of whether or not the user has designated a syncing account. The CL implements this change by porting IdentityAPI away from using gaia::AccountTracker to observing ProfileOAuth2TokenService and AccountTrackerService directly. By doing so, IdentityAPI observes (and fires events for) signin change events for secondary accounts regardless of whether a primary account is present. This change has a side benefit in removing usage of the AccountTracker class, which is deprecated with an eye toward complete removal (see details in crbug.com/729590). It will be followed up by a conversion of this code to use the Identity Service client library. To test, install a Chrome extension with the identity permissions in its manifest. Go to chrome://extensions, enable developer mode, and inspect the background page of the above app. At the JS console that that brings up, execute: chrome.identity.onSignInChanged.addListener((account, signed_in) => {console.log(account.id + " " + signed_in);} ) Sign out of the browser. Verify that you receive a callback at the console with a value of false. Sign back in. Verify that you receive another callback at the console for the same account ID with a value of true. Bug: 729589 , 729542 , 769700 Change-Id: I99dd48d34b380067ac34a2207effd9d3279191fd Reviewed-on: https://chromium-review.googlesource.com/596368 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: Mihai Sardarescu <msarda@chromium.org> Cr-Commit-Position: refs/heads/master@{#545081} [modify] https://crrev.com/798a107597af39aba4fd365d920d58015516c9b4/chrome/browser/extensions/api/identity/identity_api.cc [modify] https://crrev.com/798a107597af39aba4fd365d920d58015516c9b4/chrome/browser/extensions/api/identity/identity_api.h [modify] https://crrev.com/798a107597af39aba4fd365d920d58015516c9b4/chrome/browser/extensions/api/identity/identity_apitest.cc
,
Mar 23 2018
|
||
►
Sign in to add a comment |
||
Comment 1 by blundell@chromium.org
, Mar 19 2018Status: Started (was: Available)
Summary: Change undocumented semantics of chrome.identity.onSignInChangedEvent() (was: Re-evaluate semantics of chrome.identity.onSignInChangedEvent() in post-AccountConsistency world)