put private tabs in different sessions

partial fix for #6684
and #8963

TODO: Either the private tab sessions should be cleared and revert back to private session 1 when the last private tab is closed, or we need a private tab session manager menu. Currently the session will keep incrementing until the browser restarts.

Test Plan:
1. open private tab
2. in the private tab, go to a site that requires login
3. login, then open a link from that site in a new private tab
4. you should also be logged in in the second private tab
5. open a new private tab
6. go to the same site. you should not be logged in.

Author: diracdeltas

app/browser/tabs.js

@@ -13,7 +13,7 @@ const {app, BrowserWindow, extensions, session, ipcMain} = require('electron')
 const {makeImmutable, makeJS} = require('../common/state/immutableUtil')
 const {getTargetAboutUrl, getSourceAboutUrl, isSourceAboutUrl, newFrameUrl, isTargetAboutUrl, isIntermediateAboutPage, isTargetMagnetUrl, getSourceMagnetUrl} = require('../../js/lib/appUrlUtil')
 const {isURL, getUrlFromInput, toPDFJSLocation, getDefaultFaviconUrl, isHttpOrHttps, getLocationIfPDF} = require('../../js/lib/urlutil')
-const {isSessionPartition} = require('../../js/state/frameStateUtil')
+const {isSessionPartition, getPartitionNumber, getPartitionFromNumber} = require('../../js/state/frameStateUtil')
 const {getOrigin} = require('../../js/lib/urlutil')
 const {getSetting} = require('../../js/settings')
 const settings = require('../../js/constants/settings')
@@ -39,7 +39,10 @@ const ledgerState = require('../common/state/ledgerState')
 const {getWindow} = require('./windows')
 
 let currentPartitionNumber = 0
-const incrementPartitionNumber = () => ++currentPartitionNumber
+let currentPrivatePartitionNumber = 0
+const incrementPartitionNumber = (isPrivate) => isPrivate
+  ? ++currentPrivatePartitionNumber
+  : ++currentPartitionNumber
 
 const normalizeUrl = function (url) {
   if (isSourceAboutUrl(url)) {
@@ -74,28 +77,26 @@ const updateTab = (tabId, changeInfo = {}) => {
   }
 }
 
-const getPartitionNumber = (partition) => {
-  return Number(partition.split('persist:partition-')[1] || 0)
-}
-
 /**
  * Obtains the current partition.
  * Warning: This function has global side effects in that it increments the
  * global next partition number if isPartitioned is passed into the create options.
  */
 const getPartition = (createProperties) => {
-  let partition = session.defaultSession.partition
   if (createProperties.partition) {
-    partition = createProperties.partition
-  } else if (createProperties.isPrivate) {
-    partition = 'default'
-  } else if (createProperties.isPartitioned) {
-    partition = `persist:partition-${incrementPartitionNumber()}`
-  } else if (createProperties.partitionNumber) {
-    partition = `persist:partition-${createProperties.partitionNumber}`
+    return createProperties.partition
   }
-
-  return partition
+  if (createProperties.partitionNumber) {
+    return getPartitionFromNumber(createProperties.partitionNumber,
+      createProperties.isPrivate)
+  }
+  if (createProperties.isPrivate) {
+    return getPartitionFromNumber(incrementPartitionNumber(true), true)
+  }
+  if (createProperties.isPartitioned) {
+    return getPartitionFromNumber(incrementPartitionNumber(false), false)
+  }
+  return session.defaultSession.partition
 }
 
 const needsPartitionAssigned = (createProperties) => {

app/common/state/tabContentState/partitionState.js

@@ -10,13 +10,7 @@ const frameStateUtil = require('../../../../js/state/frameStateUtil')
 const {tabs} = require('../../../../js/constants/config')
 
 module.exports.isPartitionTab = (state, frameKey) => {
-  const frame = frameStateUtil.getFrameByKey(state, frameKey)
-
-  if (frame == null) {
-    return false
-  }
-
-  return !!frame.get('partitionNumber')
+  return module.exports.getPartitionNumber(state, frameKey) > 0
 }
 
 module.exports.getPartitionNumber = (state, frameKey) => {
@@ -28,9 +22,9 @@ module.exports.getPartitionNumber = (state, frameKey) => {
 
   const partitionNumber = frame.get('partitionNumber')
   if (typeof partitionNumber === 'string') {
-    return partitionNumber.replace(/^partition-/i, '')
+    return Number(partitionNumber.replace(/^partition-/i, ''))
   }
-  return partitionNumber
+  return Number(partitionNumber)
 }
 
 module.exports.getMaxAllowedPartitionNumber = (state, frameKey) => {

app/renderer/components/main/siteInfo.js

@@ -105,7 +105,7 @@ class SiteInfo extends React.Component {
   }
 
   get partitionInfo () {
-    if (this.props.partitionNumber) {
+    if (this.props.partitionNumber > 0) {
       // Figure out the partition info display
       let l10nArgs = {
         partitionNumber: this.props.partitionNumber

js/state/frameStateUtil.js

@@ -289,31 +289,42 @@ function isAncestorFrameKey (state, frame, parentFrameKey) {
   return isAncestorFrameKey(state, parentFrame, parentFrameKey)
 }
 
+/**
+ * @param {string} partition - the name of the partition
+ * @returns {number}
+ */
 function getPartitionNumber (partition) {
-  const regex = /(?:persist:)?partition-(\d+)/
+  const regex = /^(?:persist:)?partition-(\d+)$/
   const matches = regex.exec(partition)
-  return Number((matches && matches[1]) || 0)
+  const partitionNumber = Number((matches && matches[1]) || 0)
+  // Return negative numbers for private sessions to avoid conflicts with
+  // regular sessions
+  return isPrivatePartition(partition) ? -partitionNumber : partitionNumber
 }
 
 function isPrivatePartition (partition) {
-  return partition && !partition.startsWith('persist:')
+  if (partition === '') {
+    return true
+  }
+  if (partition) {
+    return !partition.startsWith('persist:')
+  }
+  return false
 }
 
 function isSessionPartition (partition) {
-  return partition && partition.startsWith('persist:partition-')
+  return partition &&
+    (partition.startsWith('persist:partition-') || partition.startsWith('partition-'))
 }
 
 function getPartition (frameOpts) {
   return getPartitionFromNumber(frameOpts.get('partitionNumber'), frameOpts.get('isPrivate'))
 }
 
 function getPartitionFromNumber (partitionNumber, incognito) {
-  if (!partitionNumber && !incognito) {
-    return 'persist:default'
-  } else if (incognito) {
-    return 'default'
-  }
-  return `persist:partition-${partitionNumber}`
+  const partition = partitionNumber ? `partition-${Math.abs(partitionNumber)}` : 'default'
+  const prefix = incognito ? '' : 'persist:'
+  return `${prefix}${partition}`
 }
 
 const frameOptsFromFrame = (frame) => {

test/unit/app/common/state/tabContentStateTest/partitionStateTest.js

@@ -52,7 +52,7 @@ describe('partitionState unit tests', function () {
       assert.equal(partitionState.isPartitionTab(), false)
     })
 
-    it('returns true if partition number is defined', function * () {
+    it('returns true if partition number is positive', function * () {
       const partitionNumber = 1337
       const state = defaultState
         .setIn(['frames', index, 'partitionNumber'], partitionNumber)
@@ -65,15 +65,33 @@ describe('partitionState unit tests', function () {
       const result = partitionState.isPartitionTab(state, frameKey)
       assert.equal(result, false)
     })
+
+    it('returns false if partition number is 0', function * () {
+      const partitionNumber = 0
+      const state = defaultState
+        .setIn(['frames', index, 'partitionNumber'], partitionNumber)
+      const result = partitionState.isPartitionTab(state, frameKey)
+      assert.equal(result, false)
+    })
+
+    it('returns false if partition number is negative', function * () {
+      const partitionNumber = -10
+      const state = defaultState
+        .setIn(['frames', index, 'partitionNumber'], partitionNumber)
+      const result = partitionState.isPartitionTab(state, frameKey)
+      assert.equal(result, false)
+    })
   })
 
   describe('getPartitionNumber', function () {
     it('returns zero if frame is null/undefined', function * () {
       assert.equal(partitionState.getPartitionNumber(), 0)
     })
 
-    it('returns zero if frame is null/undefined', function * () {
-      assert.equal(partitionState.getPartitionNumber(), 0)
+    it('returns zero if partition number is null/undefined', function * () {
+      const state = defaultState
+        .setIn(['frames', index, 'partitionNumber'], null)
+      assert.equal(partitionState.getPartitionNumber(state, frameKey), 0)
     })
 
     it('can remove _partition_ string and keep the partition number', function * () {
@@ -95,8 +113,14 @@ describe('partitionState unit tests', function () {
   })
 
   describe('getMaxAllowedPartitionNumber', function () {
-    it('returns false if frame is null/undefined', function () {
-      assert.equal(partitionState.getMaxAllowedPartitionNumber(), false)
+    it('returns zero if frame is null/undefined', function () {
+      assert.equal(partitionState.getMaxAllowedPartitionNumber(), 0)
+    })
+
+    it('returns zero if partition number is null/undefined', function * () {
+      const state = defaultState
+        .setIn(['frames', index, 'partitionNumber'], null)
+      assert.equal(partitionState.getMaxAllowedPartitionNumber(state, frameKey), 0)
     })
 
     it('returns partition number', function * () {

test/unit/state/frameStateUtilTest.js

@@ -444,4 +444,40 @@ describe('frameStateUtil', function () {
       assert.equal(result, true)
     })
   })
+
+  describe('partition utils', function () {
+    it('getPartitionNumber', function () {
+      const getPartitionNumber = frameStateUtil.getPartitionNumber
+      assert.equal(getPartitionNumber(''), 0)
+      assert.equal(getPartitionNumber('1'), 0)
+      assert.equal(getPartitionNumber('partition-1'), -1)
+      assert.equal(getPartitionNumber('partition-222'), -222)
+      assert.equal(getPartitionNumber('partition-222.1'), 0)
+      assert.equal(getPartitionNumber('persist:partition-1'), 1)
+      assert.equal(getPartitionNumber('unpersist:partition-1'), 0)
+      assert.equal(getPartitionNumber('persist:partition-10'), 10)
+      assert.equal(getPartitionNumber('persist:partition'), 0)
+    })
+    it('isPrivatePartition', function () {
+      const isPrivatePartition = frameStateUtil.isPrivatePartition
+      assert.equal(isPrivatePartition(''), true)
+      assert.equal(isPrivatePartition('partition-1'), true)
+      assert.equal(isPrivatePartition('persist:partition-1'), false)
+    })
+    it('isSessionPartition', function () {
+      const isSessionPartition = frameStateUtil.isSessionPartition
+      assert.equal(isSessionPartition(''), false)
+      assert.equal(isSessionPartition('default'), false)
+      assert.equal(isSessionPartition('partition-1'), true)
+      assert.equal(isSessionPartition('persist:partition-1'), true)
+    })
+    it('getPartitionFromNumber', function () {
+      const getPartitionFromNumber = frameStateUtil.getPartitionFromNumber
+      assert.equal(getPartitionFromNumber(0, true), 'default')
+      assert.equal(getPartitionFromNumber(0, false), 'persist:default')
+      assert.equal(getPartitionFromNumber(1, true), 'partition-1')
+      assert.equal(getPartitionFromNumber(-10, true), 'partition-10')
+      assert.equal(getPartitionFromNumber(1, false), 'persist:partition-1')
+    })
+  })
 })