Fix the private browsing mode check in FxAccountsWebChannel.jsm (
bug 1378766) r?markh, eoger
The private browsing mode check reached into the sendingContext's browser's docShell
for it's check, the Law of Demeter was shattered.
PrivateBrowsingUtils.jsm provides all the functionality needed for the check,
just call PrivateBrowsingUtils.isBrowserPrivate with the sendingContext's browser.
MozReview-Commit-ID: DRIU1fy94ml
***
Bug 1378766 - Remove the `sendingContext.browser` defined check
MozReview-Commit-ID: GWFFggOoItP
--- a/services/fxaccounts/FxAccountsWebChannel.jsm
+++ b/services/fxaccounts/FxAccountsWebChannel.jsm
@@ -19,16 +19,18 @@ Cu.import("resource://gre/modules/FxAcco
XPCOMUtils.defineLazyModuleGetter(this, "Services",
"resource://gre/modules/Services.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "WebChannel",
"resource://gre/modules/WebChannel.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "fxAccounts",
"resource://gre/modules/FxAccounts.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "FxAccountsStorageManagerCanStoreField",
"resource://gre/modules/FxAccountsStorage.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "PrivateBrowsingUtils",
+ "resource://gre/modules/PrivateBrowsingUtils.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "Weave",
"resource://services-sync/main.js");
const COMMAND_PROFILE_CHANGE = "profile:change";
const COMMAND_CAN_LINK_ACCOUNT = "fxaccounts:can_link_account";
const COMMAND_LOGIN = "fxaccounts:login";
const COMMAND_LOGOUT = "fxaccounts:logout";
const COMMAND_DELETE = "fxaccounts:delete";
@@ -256,16 +258,17 @@ this.FxAccountsWebChannel.prototype = {
log.debug("FxAccountsWebChannel registered: " + this._webChannelId + " with origin " + this._webChannelOrigin.prePath);
}
};
this.FxAccountsWebChannelHelpers = function(options) {
options = options || {};
this._fxAccounts = options.fxAccounts || fxAccounts;
+ this._privateBrowsingUtils = options.privateBrowsingUtils || PrivateBrowsingUtils;
};
this.FxAccountsWebChannelHelpers.prototype = {
// If the last fxa account used for sync isn't this account, we display
// a modal dialog checking they really really want to do this...
// (This is sync-specific, so ideally would be in sync's identity module,
// but it's a little more seamless to do here, and sync is currently the
// only fxa consumer, so...
@@ -338,25 +341,22 @@ this.FxAccountsWebChannelHelpers.prototy
return null;
});
},
/**
* Check if `sendingContext` is in private browsing mode.
*/
isPrivateBrowsingMode(sendingContext) {
- if (!sendingContext ||
- !sendingContext.browser ||
- !sendingContext.browser.docShell ||
- sendingContext.browser.docShell.usePrivateBrowsing === undefined) {
+ if (!sendingContext) {
log.error("Unable to check for private browsing mode, assuming true");
return true;
}
- const isPrivateBrowsing = sendingContext.browser.docShell.usePrivateBrowsing;
+ const isPrivateBrowsing = this._privateBrowsingUtils.isBrowserPrivate(sendingContext.browser);
log.debug("is private browsing", isPrivateBrowsing);
return isPrivateBrowsing;
},
/**
* Check whether sending fxa_status data should be allowed.
*/
shouldAllowFxaStatus(service, sendingContext) {
--- a/services/fxaccounts/tests/xpcshell/test_web_channel.js
+++ b/services/fxaccounts/tests/xpcshell/test_web_channel.js
@@ -5,21 +5,17 @@
Cu.import("resource://gre/modules/FxAccountsCommon.js");
const { FxAccountsWebChannel, FxAccountsWebChannelHelpers } =
Cu.import("resource://gre/modules/FxAccountsWebChannel.jsm", {});
const URL_STRING = "https://example.com";
const mockSendingContext = {
- browser: {
- docShell: {
- usePrivateBrowsing: false
- }
- },
+ browser: {},
principal: {},
eventTarget: {}
};
add_test(function() {
validationHelper(undefined,
"Error: Missing configuration options");
@@ -482,16 +478,19 @@ add_task(async function test_helpers_get
email: "testuser@testuser.com",
kA: "kA",
kb: "kB",
sessionToken: "sessionToken",
uid: "uid",
verified: true
});
}
+ },
+ privateBrowsingUtils: {
+ isBrowserPrivate: () => true
}
});
Services.prefs.setBoolPref("services.sync.engine.creditcards.available", true);
// Not defining "services.sync.engine.addresses.available" on purpose.
let fxaStatus = await helpers.getFxaStatus("sync", mockSendingContext);
ok(!!fxaStatus);
@@ -715,29 +714,51 @@ add_task(async function test_helpers_sho
}
let shouldAllowFxaStatus = helpers.shouldAllowFxaStatus("", mockSendingContext);
do_check_false(shouldAllowFxaStatus);
do_check_true(wasCalled.isPrivateBrowsingMode);
});
add_task(async function test_helpers_isPrivateBrowsingMode_private_browsing() {
- let helpers = new FxAccountsWebChannelHelpers({});
- mockSendingContext.browser.docShell.usePrivateBrowsing = true;
+ let wasCalled = {
+ isBrowserPrivate: false
+ };
+ let helpers = new FxAccountsWebChannelHelpers({
+ privateBrowsingUtils: {
+ isBrowserPrivate(browser) {
+ wasCalled.isBrowserPrivate = true;
+ do_check_eq(browser, mockSendingContext.browser);
+ return true;
+ }
+ }
+ });
let isPrivateBrowsingMode = helpers.isPrivateBrowsingMode(mockSendingContext);
do_check_true(isPrivateBrowsingMode);
+ do_check_true(wasCalled.isBrowserPrivate);
});
add_task(async function test_helpers_isPrivateBrowsingMode_private_browsing() {
- let helpers = new FxAccountsWebChannelHelpers({});
- mockSendingContext.browser.docShell.usePrivateBrowsing = false;
+ let wasCalled = {
+ isBrowserPrivate: false
+ };
+ let helpers = new FxAccountsWebChannelHelpers({
+ privateBrowsingUtils: {
+ isBrowserPrivate(browser) {
+ wasCalled.isBrowserPrivate = true;
+ do_check_eq(browser, mockSendingContext.browser);
+ return false;
+ }
+ }
+ });
let isPrivateBrowsingMode = helpers.isPrivateBrowsingMode(mockSendingContext);
do_check_false(isPrivateBrowsingMode);
+ do_check_true(wasCalled.isBrowserPrivate);
});
add_task(async function test_helpers_change_password() {
let wasCalled = {
updateUserAccountData: false,
updateDeviceRegistration: false
};
let helpers = new FxAccountsWebChannelHelpers({