Bug 1378766 - Fix the private browsing mode check in FxAccountsWebChannel.jsm. r=eoger, r=markh, a=jcristau
authorShane Tomlinson <stomlinson@mozilla.com>
Thu, 20 Jul 2017 10:35:47 +0100
changeset 414421 ed08245d52332cadf13498c0a88bcd1a0bd68d1d
parent 414420 f3b0db0e8b2d51eb79e7caa70a66276ba49271dc
child 414422 5ff8131b551228ca603a76e39925495d7d2a407e
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerseoger, markh, jcristau
bugs1378766
milestone55.0
Bug 1378766 - Fix the private browsing mode check in FxAccountsWebChannel.jsm. r=eoger, r=markh, a=jcristau 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";
@@ -251,16 +253,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...
@@ -322,25 +325,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");
 
@@ -649,29 +645,51 @@ add_task(function* test_helpers_shouldAl
   }
 
   let shouldAllowFxaStatus = helpers.shouldAllowFxaStatus("", mockSendingContext);
   do_check_false(shouldAllowFxaStatus);
   do_check_true(wasCalled.isPrivateBrowsingMode);
 });
 
 add_task(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(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(function* test_helpers_change_password() {
   let wasCalled = {
     updateUserAccountData: false,
     updateDeviceRegistration: false
   };
   let helpers = new FxAccountsWebChannelHelpers({