Fix the private browsing mode check in FxAccountsWebChannel.jsm (bug 1378766) r=eoger,markh
authorShane Tomlinson <stomlinson@mozilla.com>
Thu, 20 Jul 2017 10:35:47 +0100
changeset 418742 b08d3ba207d5be19bed364f181ee23f3c71b644b
parent 418741 3a683a5acc3ff0946f373f1ecfdf8dd290cf97fe
child 418743 55070d5d1a59c5620a4b272040d241538910b78b
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerseoger, markh
bugs1378766
milestone56.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Fix the private browsing mode check in FxAccountsWebChannel.jsm (bug 1378766) r=eoger,markh 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
services/fxaccounts/FxAccountsWebChannel.jsm
services/fxaccounts/tests/xpcshell/test_web_channel.js
--- 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({