commit f2d7c661cfb088abb0d2a98c988361e07c52cb88
parent b36b73270863f3b8ddaffdb2961ba1ca307591de
Author: Andrea Marchesini <amarchesini@mozilla.com>
Date: Tue, 7 Oct 2025 12:31:36 +0000
Bug 1992112 - IPProtectionService should not use UIState to check the sign-in flag, r=ip-protection-reviewers,rking,Gijs
Differential Revision: https://phabricator.services.mozilla.com/D267215
Diffstat:
9 files changed, 119 insertions(+), 126 deletions(-)
diff --git a/browser/components/ipprotection/IPPSignInWatcher.sys.mjs b/browser/components/ipprotection/IPPSignInWatcher.sys.mjs
@@ -0,0 +1,77 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+const lazy = {};
+
+ChromeUtils.defineESModuleGetters(lazy, {
+ IPProtectionService:
+ "resource:///modules/ipprotection/IPProtectionService.sys.mjs",
+ UIState: "resource://services-sync/UIState.sys.mjs",
+});
+
+ChromeUtils.defineLazyGetter(lazy, "fxAccounts", () => {
+ return ChromeUtils.importESModule(
+ "resource://gre/modules/FxAccounts.sys.mjs"
+ ).getFxAccountsSingleton();
+});
+
+/**
+ * This class monitors the Sign-In state and triggers the update of the service
+ * if needed.
+ */
+class IPPSignInWatcherSingleton {
+ #signedIn = false;
+
+ get isSignedIn() {
+ return this.#signedIn;
+ }
+
+ set isSignedIn(signedIn) {
+ this.#signedIn = signedIn;
+ }
+
+ /**
+ * Adds an observer for the FxA sign-in state.
+ */
+ async init() {
+ this.fxaObserver = {
+ QueryInterface: ChromeUtils.generateQI([
+ Ci.nsIObserver,
+ Ci.nsISupportsWeakReference,
+ ]),
+
+ observe() {
+ let { status } = lazy.UIState.get();
+ let signedIn = status == lazy.UIState.STATUS_SIGNED_IN;
+ if (signedIn !== IPPSignInWatcher.isSignedIn) {
+ IPPSignInWatcher.isSignedIn = signedIn;
+ lazy.IPProtectionService.updateState();
+ }
+ },
+ };
+
+ Services.obs.addObserver(this.fxaObserver, lazy.UIState.ON_UPDATE);
+
+ // Let's check the sign-in state only if we have seen the user signed in
+ // once at least.
+ if (Services.prefs.prefHasUserValue("services.sync.username")) {
+ const userData = await lazy.fxAccounts.getSignedInUser();
+ this.#signedIn = userData?.verified || false;
+ }
+ }
+
+ /**
+ * Removes the FxA sign-in state observer
+ */
+ uninit() {
+ if (this.fxaObserver) {
+ Services.obs.removeObserver(this.fxaObserver, lazy.UIState.ON_UPDATE);
+ this.fxaObserver = null;
+ }
+ }
+}
+
+const IPPSignInWatcher = new IPPSignInWatcherSingleton();
+
+export { IPPSignInWatcher };
diff --git a/browser/components/ipprotection/IPProtectionHelpers.sys.mjs b/browser/components/ipprotection/IPProtectionHelpers.sys.mjs
@@ -17,9 +17,10 @@ ChromeUtils.defineESModuleGetters(lazy, {
IPProtectionStates:
"resource:///modules/ipprotection/IPProtectionService.sys.mjs",
NimbusFeatures: "resource://nimbus/ExperimentAPI.sys.mjs",
- UIState: "resource://services-sync/UIState.sys.mjs",
});
+import { IPPSignInWatcher } from "resource:///modules/ipprotection/IPPSignInWatcher.sys.mjs";
+
const VPN_ADDON_ID = "vpn@mozilla.com";
/**
@@ -129,44 +130,6 @@ class VPNAddonHelper {
}
/**
- * This class monitors the Sign-In state and triggers the update of the service
- * if needed.
- */
-class SignInStateHelper {
- /**
- * Adds an observer for the FxA sign-in state.
- */
- init() {
- this.fxaObserver = {
- QueryInterface: ChromeUtils.generateQI([
- Ci.nsIObserver,
- Ci.nsISupportsWeakReference,
- ]),
-
- observe() {
- let { status } = lazy.UIState.get();
- let signedIn = status == lazy.UIState.STATUS_SIGNED_IN;
- if (signedIn !== lazy.IPProtectionService.signedIn) {
- lazy.IPProtectionService.updateState();
- }
- },
- };
-
- Services.obs.addObserver(this.fxaObserver, lazy.UIState.ON_UPDATE);
- }
-
- /**
- * Removes the FxA sign-in state observer
- */
- uninit() {
- if (this.fxaObserver) {
- Services.obs.removeObserver(this.fxaObserver, lazy.UIState.ON_UPDATE);
- this.fxaObserver = null;
- }
- }
-}
-
-/**
* This class monitors the eligibility flag from Nimbus
*/
class EligibilityHelper {
@@ -186,9 +149,9 @@ class EligibilityHelper {
const IPPHelpers = [
new AccountResetHelper(),
new EligibilityHelper(),
- new SignInStateHelper(),
new VPNAddonHelper(),
new UIHelper(),
+ IPPSignInWatcher,
];
export { IPPHelpers };
diff --git a/browser/components/ipprotection/IPProtectionPanel.sys.mjs b/browser/components/ipprotection/IPProtectionPanel.sys.mjs
@@ -12,6 +12,7 @@ ChromeUtils.defineESModuleGetters(lazy, {
IPProtectionStates:
"resource:///modules/ipprotection/IPProtectionService.sys.mjs",
IPProtection: "resource:///modules/ipprotection/IPProtection.sys.mjs",
+ IPPSignInWatcher: "resource:///modules/ipprotection/IPPSignInWatcher.sys.mjs",
});
import {
@@ -109,14 +110,11 @@ export class IPProtectionPanel {
constructor(window, variant = "") {
this.handleEvent = this.#handleEvent.bind(this);
- let {
- isSignedIn,
- activatedAt: protectionEnabledSince,
- hasUpgraded,
- } = lazy.IPProtectionService;
+ let { activatedAt: protectionEnabledSince, hasUpgraded } =
+ lazy.IPProtectionService;
this.state = {
- isSignedOut: !isSignedIn,
+ isSignedOut: !lazy.IPPSignInWatcher.isSignedIn,
isProtectionEnabled: !!protectionEnabledSince,
protectionEnabledSince,
location: {
@@ -359,7 +357,6 @@ export class IPProtectionPanel {
} else if (event.type == "IPProtectionService:StateChanged") {
let {
state,
- isSignedIn,
activatedAt: protectionEnabledSince,
hasUpgraded,
} = lazy.IPProtectionService;
@@ -368,7 +365,7 @@ export class IPProtectionPanel {
lazy.IPProtectionService.errors.includes(ERRORS.GENERIC);
this.setState({
- isSignedOut: !isSignedIn,
+ isSignedOut: !lazy.IPPSignInWatcher.isSignedIn,
isProtectionEnabled: !!protectionEnabledSince,
protectionEnabledSince,
hasUpgraded,
diff --git a/browser/components/ipprotection/IPProtectionService.sys.mjs b/browser/components/ipprotection/IPProtectionService.sys.mjs
@@ -10,7 +10,7 @@ ChromeUtils.defineESModuleGetters(lazy, {
GuardianClient: "resource:///modules/ipprotection/GuardianClient.sys.mjs",
IPPHelpers: "resource:///modules/ipprotection/IPProtectionHelpers.sys.mjs",
IPPProxyManager: "resource:///modules/ipprotection/IPPProxyManager.sys.mjs",
- UIState: "resource://services-sync/UIState.sys.mjs",
+ IPPSignInWatcher: "resource:///modules/ipprotection/IPPSignInWatcher.sys.mjs",
SpecialMessageActions:
"resource://messaging-system/lib/SpecialMessageActions.sys.mjs",
NimbusFeatures: "resource://nimbus/ExperimentAPI.sys.mjs",
@@ -76,7 +76,6 @@ class IPProtectionServiceSingleton extends EventTarget {
errors = [];
enrolling = null;
- signedIn = null;
guardian = null;
proxyManager = null;
@@ -143,7 +142,7 @@ class IPProtectionServiceSingleton extends EventTarget {
}
this.proxyManager = new lazy.IPPProxyManager(this.guardian);
- this.#helpers.forEach(helper => helper.init());
+ await Promise.allSettled(this.#helpers.map(helper => helper.init()));
await this.#updateState();
}
@@ -164,7 +163,6 @@ class IPProtectionServiceSingleton extends EventTarget {
this.#entitlement = null;
this.errors = [];
this.enrolling = null;
- this.signedIn = null;
this.#helpers.forEach(helper => helper.uninit());
@@ -268,7 +266,6 @@ class IPProtectionServiceSingleton extends EventTarget {
* Reset the statuses that are set based on a FxA account.
*/
resetAccount() {
- this.signedIn = null;
this.#entitlement = null;
if (this.proxyManager?.active) {
this.stop(false);
@@ -277,17 +274,6 @@ class IPProtectionServiceSingleton extends EventTarget {
}
/**
- * Checks if a user has signed in.
- *
- * @returns {boolean}
- */
- get isSignedIn() {
- let { status } = lazy.UIState.get();
- this.signedIn = status == lazy.UIState.STATUS_SIGNED_IN;
- return this.signedIn;
- }
-
- /**
* Checks if the user has enrolled with FxA to use the proxy.
*
* @param { boolean } onlyCached - if true only the cached clients will be checked.
@@ -436,9 +422,8 @@ class IPProtectionServiceSingleton extends EventTarget {
// For non authenticated users, we can check if they are eligible (the UI
// is shown and they have to login) or we don't know yet their current
// enroll state (no UI is shown).
- let signedIn = this.isSignedIn;
let eligible = this.isEligible;
- if (!signedIn) {
+ if (!lazy.IPPSignInWatcher.isSignedIn) {
return !eligible
? IPProtectionStates.UNAVAILABLE
: IPProtectionStates.UNAUTHENTICATED;
diff --git a/browser/components/ipprotection/moz.build b/browser/components/ipprotection/moz.build
@@ -21,6 +21,7 @@ EXTRA_JS_MODULES.ipprotection += [
"IPProtectionServerlist.sys.mjs",
"IPProtectionService.sys.mjs",
"IPProtectionUsage.sys.mjs",
+ "IPPSignInWatcher.sys.mjs",
]
BROWSER_CHROME_MANIFESTS += [
diff --git a/browser/components/ipprotection/tests/browser/head.js b/browser/components/ipprotection/tests/browser/head.js
@@ -13,6 +13,10 @@ const { IPProtectionService, IPProtectionStates } = ChromeUtils.importESModule(
"resource:///modules/ipprotection/IPProtectionService.sys.mjs"
);
+const { IPPSignInWatcher } = ChromeUtils.importESModule(
+ "resource:///modules/ipprotection/IPPSignInWatcher.sys.mjs"
+);
+
const { HttpServer, HTTP_403 } = ChromeUtils.importESModule(
"resource://testing-common/httpd.sys.mjs"
);
@@ -23,7 +27,6 @@ const { NimbusTestUtils } = ChromeUtils.importESModule(
ChromeUtils.defineESModuleGetters(this, {
sinon: "resource://testing-common/Sinon.sys.mjs",
- UIState: "resource://services-sync/UIState.sys.mjs",
ExperimentAPI: "resource://nimbus/ExperimentAPI.sys.mjs",
CustomizableUI:
"moz-src:///browser/components/customizableui/CustomizableUI.sys.mjs",
@@ -236,7 +239,6 @@ let DEFAULT_SERVICE_STATUS = {
/* exported DEFAULT_SERVICE_STATUS */
let STUBS = {
- UIState: undefined,
isLinkedToGuardian: undefined,
enroll: undefined,
fetchUserInfo: undefined,
@@ -267,7 +269,7 @@ add_setup(async function setupVPN() {
});
function setupStubs(stubs = STUBS) {
- stubs.UIState = setupSandbox.stub(UIState, "get");
+ stubs.isSignedIn = setupSandbox.stub(IPPSignInWatcher, "isSignedIn");
stubs.isLinkedToGuardian = setupSandbox.stub(
IPProtectionService.guardian,
"isLinkedToGuardian"
@@ -295,11 +297,7 @@ function setupService(
stubs = STUBS
) {
if (typeof isSignedIn != "undefined") {
- stubs.UIState.returns({
- status: isSignedIn
- ? UIState.STATUS_SIGNED_IN
- : UIState.STATUS_NOT_CONFIGURED,
- });
+ stubs.isSignedIn.get(() => isSignedIn);
}
if (typeof isEnrolled != "undefined") {
diff --git a/browser/components/ipprotection/tests/xpcshell/test_IPProtectionPanel.js b/browser/components/ipprotection/tests/xpcshell/test_IPProtectionPanel.js
@@ -3,15 +3,15 @@ https://creativecommons.org/publicdomain/zero/1.0/ */
"use strict";
-const { UIState } = ChromeUtils.importESModule(
- "resource://services-sync/UIState.sys.mjs"
-);
const { IPProtectionPanel } = ChromeUtils.importESModule(
"resource:///modules/ipprotection/IPProtectionPanel.sys.mjs"
);
const { IPProtectionService, IPProtectionStates } = ChromeUtils.importESModule(
"resource:///modules/ipprotection/IPProtectionService.sys.mjs"
);
+const { IPPSignInWatcher } = ChromeUtils.importESModule(
+ "resource:///modules/ipprotection/IPPSignInWatcher.sys.mjs"
+);
/**
* A class that mocks the IP Protection panel.
@@ -131,9 +131,7 @@ add_task(async function test_updateState() {
*/
add_task(async function test_IPProtectionPanel_signedIn() {
let sandbox = sinon.createSandbox();
- sandbox.stub(UIState, "get").returns({
- status: UIState.STATUS_SIGNED_IN,
- });
+ sandbox.stub(IPPSignInWatcher, "isSignedIn").get(() => true);
sandbox
.stub(IPProtectionService.guardian, "isLinkedToGuardian")
.resolves(true);
@@ -182,9 +180,7 @@ add_task(async function test_IPProtectionPanel_signedIn() {
*/
add_task(async function test_IPProtectionPanel_signedOut() {
let sandbox = sinon.createSandbox();
- sandbox.stub(UIState, "get").returns({
- status: UIState.STATUS_NOT_CONFIGURED,
- });
+ sandbox.stub(IPPSignInWatcher, "isSignedIn").get(() => false);
let ipProtectionPanel = new IPProtectionPanel();
let fakeElement = new FakeIPProtectionPanelElement();
@@ -227,9 +223,7 @@ add_task(async function test_IPProtectionPanel_started_stopped() {
fakeElement.isConnected = true;
let sandbox = sinon.createSandbox();
- sandbox.stub(UIState, "get").returns({
- status: UIState.STATUS_SIGNED_IN,
- });
+ sandbox.stub(IPPSignInWatcher, "isSignedIn").get(() => true);
sandbox
.stub(IPProtectionService.guardian, "isLinkedToGuardian")
.resolves(true);
diff --git a/browser/components/ipprotection/tests/xpcshell/test_IPProtectionService.js b/browser/components/ipprotection/tests/xpcshell/test_IPProtectionService.js
@@ -12,8 +12,8 @@ const { ExtensionTestUtils } = ChromeUtils.importESModule(
const { IPProtectionService, IPProtectionStates } = ChromeUtils.importESModule(
"resource:///modules/ipprotection/IPProtectionService.sys.mjs"
);
-const { UIState } = ChromeUtils.importESModule(
- "resource://services-sync/UIState.sys.mjs"
+const { IPPSignInWatcher } = ChromeUtils.importESModule(
+ "resource:///modules/ipprotection/IPPSignInWatcher.sys.mjs"
);
do_get_profile();
@@ -31,7 +31,7 @@ ExtensionTestUtils.init(this);
function setupStubs(
sandbox,
options = {
- signedIn: UIState.STATUS_SIGNED_IN,
+ signedIn: true,
isLinkedToGuardian: true,
validProxyPass: true,
entitlement: {
@@ -41,9 +41,7 @@ function setupStubs(
},
}
) {
- sandbox.stub(UIState, "get").returns({
- status: options.signedIn,
- });
+ sandbox.stub(IPPSignInWatcher, "isSignedIn").get(() => options.signedIn);
sandbox
.stub(IPProtectionService.guardian, "isLinkedToGuardian")
.resolves(options.isLinkedToGuardian);
@@ -174,7 +172,7 @@ add_task(async function test_IPProtectionService_updateState_signedIn() {
await signedInEventPromise;
- Assert.ok(IPProtectionService.isSignedIn, "Should be signed in after update");
+ Assert.ok(IPPSignInWatcher.isSignedIn, "Should be signed in after update");
IPProtectionService.uninit();
sandbox.restore();
@@ -189,9 +187,7 @@ add_task(async function test_IPProtectionService_updateState_signedOut() {
await IPProtectionService.init();
- UIState.get.returns({
- status: UIState.STATUS_NOT_CONFIGURED,
- });
+ sandbox.stub(IPPSignInWatcher, "isSignedIn").get(() => false);
let signedOutEventPromise = waitForEvent(
IPProtectionService,
@@ -205,7 +201,7 @@ add_task(async function test_IPProtectionService_updateState_signedOut() {
await signedOutEventPromise;
Assert.ok(
- !IPProtectionService.isSignedIn,
+ !IPPSignInWatcher.isSignedIn,
"Should not be signed in after update"
);
@@ -295,9 +291,7 @@ add_task(async function test_IPProtectionService_hasUpgraded_signed_out() {
await IPProtectionService.init();
- UIState.get.returns({
- status: UIState.STATUS_NOT_CONFIGURED,
- });
+ sandbox.stub(IPPSignInWatcher, "isSignedIn").get(() => false);
let signedOutEventPromise = waitForEvent(
IPProtectionService,
diff --git a/browser/components/ipprotection/tests/xpcshell/test_IPProtectionStates.js b/browser/components/ipprotection/tests/xpcshell/test_IPProtectionStates.js
@@ -6,8 +6,8 @@ https://creativecommons.org/publicdomain/zero/1.0/ */
const { IPProtectionService, IPProtectionStates } = ChromeUtils.importESModule(
"resource:///modules/ipprotection/IPProtectionService.sys.mjs"
);
-const { UIState } = ChromeUtils.importESModule(
- "resource://services-sync/UIState.sys.mjs"
+const { IPPSignInWatcher } = ChromeUtils.importESModule(
+ "resource:///modules/ipprotection/IPPSignInWatcher.sys.mjs"
);
do_get_profile();
@@ -53,9 +53,7 @@ add_task(async function test_IPProtectionStates_uninitialized() {
*/
add_task(async function test_IPProtectionStates_uninitialized() {
let sandbox = sinon.createSandbox();
- sandbox.stub(UIState, "get").returns({
- status: UIState.STATUS_STATUS_NOT_CONFIGURED,
- });
+ sandbox.stub(IPPSignInWatcher, "isSignedIn").get(() => false);
sandbox
.stub(IPProtectionService.guardian, "isLinkedToGuardian")
.resolves(false);
@@ -87,9 +85,7 @@ add_task(async function test_IPProtectionStates_uninitialized() {
*/
add_task(async function test_IPProtectionStates_unauthenticated() {
let sandbox = sinon.createSandbox();
- sandbox.stub(UIState, "get").returns({
- status: UIState.STATUS_SIGNED_IN,
- });
+ sandbox.stub(IPPSignInWatcher, "isSignedIn").get(() => true);
sandbox
.stub(IPProtectionService.guardian, "isLinkedToGuardian")
.resolves(false);
@@ -112,9 +108,7 @@ add_task(async function test_IPProtectionStates_unauthenticated() {
"IP Protection service should no longer be unauthenticated"
);
- UIState.get.returns({
- status: UIState.STATUS_STATUS_NOT_CONFIGURED,
- });
+ sandbox.stub(IPPSignInWatcher, "isSignedIn").get(() => false);
await IPProtectionService.updateState();
@@ -133,9 +127,7 @@ add_task(async function test_IPProtectionStates_unauthenticated() {
*/
add_task(async function test_IPProtectionStates_enrolling() {
let sandbox = sinon.createSandbox();
- sandbox.stub(UIState, "get").returns({
- status: UIState.STATUS_SIGNED_IN,
- });
+ sandbox.stub(IPPSignInWatcher, "isSignedIn").get(() => true);
sandbox
.stub(IPProtectionService.guardian, "isLinkedToGuardian")
.resolves(false);
@@ -174,9 +166,7 @@ add_task(async function test_IPProtectionStates_enrolling() {
*/
add_task(async function test_IPProtectionStates_ready() {
let sandbox = sinon.createSandbox();
- sandbox.stub(UIState, "get").returns({
- status: UIState.STATUS_SIGNED_IN,
- });
+ sandbox.stub(IPPSignInWatcher, "isSignedIn").get(() => true);
sandbox
.stub(IPProtectionService.guardian, "isLinkedToGuardian")
.resolves(true);
@@ -194,9 +184,7 @@ add_task(async function test_IPProtectionStates_ready() {
"IP Protection service should be ready"
);
- UIState.get.returns({
- status: UIState.STATUS_STATUS_NOT_CONFIGURED,
- });
+ sandbox.stub(IPPSignInWatcher, "isSignedIn").get(() => false);
await IPProtectionService.updateState();
@@ -215,9 +203,7 @@ add_task(async function test_IPProtectionStates_ready() {
*/
add_task(async function test_IPProtectionStates_active() {
let sandbox = sinon.createSandbox();
- sandbox.stub(UIState, "get").returns({
- status: UIState.STATUS_SIGNED_IN,
- });
+ sandbox.stub(IPPSignInWatcher, "isSignedIn").get(() => true);
sandbox
.stub(IPProtectionService.guardian, "isLinkedToGuardian")
.resolves(true);
@@ -268,9 +254,7 @@ add_task(async function test_IPProtectionStates_active() {
*/
add_task(async function test_IPProtectionStates_error() {
let sandbox = sinon.createSandbox();
- sandbox.stub(UIState, "get").returns({
- status: UIState.STATUS_SIGNED_IN,
- });
+ sandbox.stub(IPPSignInWatcher, "isSignedIn").get(() => true);
sandbox
.stub(IPProtectionService.guardian, "isLinkedToGuardian")
.resolves(true);
