Refactor background to use new per domain isolation structure (stoically#240)

stoically · stoically · commit 7c5a7db50b2b · 2019-06-29T02:11:55.000+02:00
diff --git a/.eslintrc b/.eslintrc
@@ -7,7 +7,7 @@
     },
     "extends": "eslint:recommended",
     "parserOptions": {
-      "ecmaVersion": 8
+      "ecmaVersion": 2018
     },
     "rules": {
         "indent": [
diff --git a/package.json b/package.json
@@ -8,7 +8,7 @@
     "build-sign": "rm -rf web-ext-artifacts .cache dist && node build-sign.js && npm run lint && npm test && parcel build src/ui/options.html src/ui/popup.html --no-minify --no-source-maps && cp README.md LICENSE dist && web-ext sign --channel unlisted -s dist && git checkout -- src/manifest.json && ncu",
     "lint": "eslint src/**/*.{js,vue}",
     "test": "nyc --reporter=html --reporter=text mocha --reporter=dot ./test/setup.js test/*.test.js --timeout 60000",
-    "test-watch": "mocha ./test/setup.js test/*.test.js --watch --tmp-debug --inspect",
+    "test-watch": "mocha ./test/setup.js test/*.test.js -b --watch --tmp-debug --inspect",
     "test-functional": "mocha ./test/functional/*.test.js --timeout 10000",
     "check-dependencies": "ncu",
     "watch": "rm -rf dist && parcel src/ui/options.html src/ui/popup.html --no-hmr"
diff --git a/src/background/container.js b/src/background/container.js
@@ -313,7 +313,7 @@ class Container {
 
   async tryToRemoveQueue(queue) {
     debug('[tryToRemoveQueue] removal queue', queue);
-    for (let cookieStoreId of queue) {
+    for (const cookieStoreId of queue) {
       if (!this.storage.local.tempContainers[cookieStoreId]) {
         debug('[tryToRemoveQueue] unknown container, probably already removed', cookieStoreId);
         continue;
@@ -597,7 +597,7 @@ class Container {
     const containersOptions = Object.values(this.storage.local.tempContainers);
     const assignedColors = {};
     let maxColors = 0;
-    for (let containerOptions of containersOptions) {
+    for (const containerOptions of containersOptions) {
       if (typeof containerOptions !== 'object') {
         continue;
       }
@@ -610,7 +610,7 @@ class Container {
       }
     }
 
-    for (let color of this.containerColors) {
+    for (const color of this.containerColors) {
       if (!assignedColors[color] || assignedColors[color] < maxColors) {
         availableColors.push(color);
       }
diff --git a/src/background/cookies.js b/src/background/cookies.js
@@ -29,7 +29,7 @@ class Cookies {
       let cookieHeader;
       let cookiesHeader = {};
       let cookieHeaderChanged = false;
-      for (let domainPattern in this.storage.local.preferences.cookies.domain) {
+      for (const domainPattern in this.storage.local.preferences.cookies.domain) {
         if (!this.isolation.matchDomainPattern(details.url, domainPattern)) {
           continue;
         }
@@ -53,7 +53,7 @@ class Cookies {
           debug('[maybeAddCookiesToHeader] found temp tab and header', details, cookieHeader, cookiesHeader);
         }
 
-        for (let cookie of this.storage.local.preferences.cookies.domain[domainPattern]) {
+        for (const cookie of this.storage.local.preferences.cookies.domain[domainPattern]) {
           if (!cookie) {
             continue;
           }
diff --git a/src/background/isolation.js b/src/background/isolation.js
@@ -151,8 +151,9 @@ class Isolation {
     const parsedURL = new URL(url);
     const parsedRequestURL = new URL(request.url);
 
-    for (let domainPattern in this.storage.local.preferences.isolation.domain) {
-      const patternPreferences = this.storage.local.preferences.isolation.domain[domainPattern];
+    for (const patternPreferences of this.storage.local.preferences.isolation.domain) {
+      const domainPattern = patternPreferences.pattern;
+
       if (!this.matchDomainPattern(
         tab.url === 'about:blank' && openerTab && openerTab.url || tab.url, domainPattern
       )) {
@@ -202,8 +203,9 @@ class Isolation {
       return false;
     }
 
-    for (let domainPattern in this.storage.local.preferences.isolation.domain) {
-      const patternPreferences = this.storage.local.preferences.isolation.domain[domainPattern];
+    for (const patternPreferences of this.storage.local.preferences.isolation.domain) {
+      const domainPattern = patternPreferences.pattern;
+
       if (!this.matchDomainPattern(request.url, domainPattern)) {
         continue;
       }
diff --git a/src/background/mouseclick.js b/src/background/mouseclick.js
@@ -101,15 +101,15 @@ class MouseClick {
     const parsedClickedURL = new URL(message.href);
     debug('[checkClick] checking click', type, message, sender);
 
-    for (let domainPattern in this.storage.local.preferences.isolation.domain) {
+    for (const domainPatternPreferences of this.storage.local.preferences.isolation.domain) {
+      const domainPattern = domainPatternPreferences.pattern;
       if (!this.isolation.matchDomainPattern(sender.tab.url, domainPattern)) {
         continue;
       }
-      const domainPatternPreferences = this.storage.local.preferences.isolation.domain[domainPattern].mouseClick;
-      if (!domainPatternPreferences[type]) {
+      if (!domainPatternPreferences.mouseClick[type]) {
         continue;
       }
-      const preferences = domainPatternPreferences[type];
+      const preferences = domainPatternPreferences.mouseClick[type];
       debug('[checkClick] per website pattern found', );
       if (preferences.action === 'global') {
         debug('[checkClick] breaking because "global"');
diff --git a/src/ui/components/isolation/perdomain.vue b/src/ui/components/isolation/perdomain.vue
@@ -118,6 +118,7 @@ export default {
           this.editing = false;
         } else {
           this.preferences.isolation.domain.push(JSON.parse(JSON.stringify(this.domain)));
+          this.reset();
         }
       }
     });
diff --git a/test/background.alwaysopenin.test.js b/test/background.alwaysopenin.test.js
@@ -3,20 +3,22 @@ preferencesTestSet.map(preferences => { describe(`preferences: ${JSON.stringify(
   describe('Always Open In', () => {
     beforeEach(async () => {
       global.background = await loadBackground(preferences);
-      background.storage.local.preferences.isolation.domain = {
-        'example.com': {
+      background.storage.local.preferences.isolation.domain = [
+        {
+          pattern: 'example.com',
           always: {
             action: 'enabled',
             allowedInPermanent: false
           }
         },
-        '*.notexample.com': {
+        {
+          pattern: '*.notexample.com',
           always: {
             action: 'enabled',
             allowedInPermanent: false
           }
         }
-      };
+      ];
     });
 
     it('should open in a new temporary container', async () => {
@@ -57,14 +59,15 @@ preferencesTestSet.map(preferences => { describe(`preferences: ${JSON.stringify(
     });
 
     it('should not open in a new temporary container if its allowed in permanent container', async () => {
-      background.storage.local.preferences.isolation.domain = {
-        'example.com': {
+      background.storage.local.preferences.isolation.domain = [
+        {
+          pattern: 'example.com',
           always: {
             action: 'enabled',
             allowedInPermanent: true
           }
         }
-      };
+      ];
       await helper.browser.request({
         tabId: 2,
         tabUrl: 'about:newtab',
diff --git a/test/background.isolation.test.js b/test/background.isolation.test.js
@@ -81,8 +81,13 @@ preferencesTestSet.map(preferences => { describe(`preferences: ${JSON.stringify(
 
             case 'sametab.perdomain':
             case 'newtab.perdomain':
-              background.storage.local.preferences.isolation.domain['example.com'] = defaultIsolationDomainPreferences;
-              background.storage.local.preferences.isolation.domain['example.com'].navigation.action = 'never';
+              background.storage.local.preferences.isolation.domain = [{
+                pattern: 'example.com',
+                ...defaultIsolationDomainPreferences,
+                navigation: {
+                  action: 'never'
+                }
+              }];
               break;
             }
           });
@@ -128,8 +133,13 @@ preferencesTestSet.map(preferences => { describe(`preferences: ${JSON.stringify(
 
             case 'sametab.perdomain':
             case 'newtab.perdomain':
-              background.storage.local.preferences.isolation.domain['example.com'] = defaultIsolationDomainPreferences;
-              background.storage.local.preferences.isolation.domain['example.com'].navigation.action = 'always';
+              background.storage.local.preferences.isolation.domain = [{
+                pattern: 'example.com',
+                ...defaultIsolationDomainPreferences,
+                navigation: {
+                  action: 'always'
+                }
+              }];
               break;
             }
           });
@@ -174,8 +184,13 @@ preferencesTestSet.map(preferences => { describe(`preferences: ${JSON.stringify(
 
               case 'sametab.perdomain':
               case 'newtab.perdomain':
-                background.storage.local.preferences.isolation.domain['example.com'] = defaultIsolationDomainPreferences;
-                background.storage.local.preferences.isolation.domain['example.com'].excluded['excluded.com'] = {};
+                background.storage.local.preferences.isolation.domain = [{
+                  pattern: 'example.com',
+                  ...defaultIsolationDomainPreferences,
+                  excluded: {
+                    'excluded.com': {}
+                  }
+                }];
                 break;
               }
 
@@ -198,8 +213,13 @@ preferencesTestSet.map(preferences => { describe(`preferences: ${JSON.stringify(
 
             case 'sametab.perdomain':
             case 'newtab.perdomain':
-              background.storage.local.preferences.isolation.domain['example.com'] = defaultIsolationDomainPreferences;
-              background.storage.local.preferences.isolation.domain['example.com'].navigation.action = 'notsamedomain';
+              background.storage.local.preferences.isolation.domain = [{
+                pattern: 'example.com',
+                ...defaultIsolationDomainPreferences,
+                navigation: {
+                  action: 'notsamedomain'
+                }
+              }];
               break;
             }
           });
@@ -258,8 +278,13 @@ preferencesTestSet.map(preferences => { describe(`preferences: ${JSON.stringify(
 
             case 'sametab.perdomain':
             case 'newtab.perdomain':
-              background.storage.local.preferences.isolation.domain['example.com'] = defaultIsolationDomainPreferences;
-              background.storage.local.preferences.isolation.domain['example.com'].navigation.action = 'notsamedomainexact';
+              background.storage.local.preferences.isolation.domain = [{
+                pattern: 'example.com',
+                ...defaultIsolationDomainPreferences,
+                navigation: {
+                  action: 'notsamedomainexact'
+                }
+              }];
               break;
             }
           });
@@ -399,14 +424,13 @@ preferencesTestSet.map(preferences => { describe(`preferences: ${JSON.stringify(
         });
 
         it('should not open in a new temporary container if the opener tab url belonging to the request matches the pattern', async () => {
-          background.storage.local.preferences.isolation.domain = {
-            'example.com': {
-              always: {
-                action: 'enabled',
-                allowedInPermanent: false
-              }
+          background.storage.local.preferences.isolation.domain = [{
+            pattern: 'example.com',
+            always: {
+              action: 'enabled',
+              allowedInPermanent: false
             }
-          };
+          }];
 
           await browser.tabs._create({
             url: 'https://example.com',
diff --git a/test/background.mouseclicks.test.js b/test/background.mouseclicks.test.js
diff --git a/test/background.storage.test.js b/test/background.storage.test.js
diff --git a/test/background.test.js b/test/background.test.js

Original file line number	Diff line number	Diff line change
`@@ -313,7 +313,7 @@ class Container {`
`313`	`313`
`314`	`314`	`async tryToRemoveQueue(queue) {`
`315`	`315`	`debug('[tryToRemoveQueue] removal queue', queue);`
`316`		`- for (let cookieStoreId of queue) {`
	`316`	`+ for (const cookieStoreId of queue) {`
`317`	`317`	`if (!this.storage.local.tempContainers[cookieStoreId]) {`
`318`	`318`	`debug('[tryToRemoveQueue] unknown container, probably already removed', cookieStoreId);`
`319`	`319`	`continue;`
`@@ -597,7 +597,7 @@ class Container {`
`597`	`597`	`const containersOptions = Object.values(this.storage.local.tempContainers);`
`598`	`598`	`const assignedColors = {};`
`599`	`599`	`let maxColors = 0;`
`600`		`- for (let containerOptions of containersOptions) {`
	`600`	`+ for (const containerOptions of containersOptions) {`
`601`	`601`	`if (typeof containerOptions !== 'object') {`
`602`	`602`	`continue;`
`603`	`603`	`}`
`@@ -610,7 +610,7 @@ class Container {`
`610`	`610`	`}`
`611`	`611`	`}`
`612`	`612`
`613`		`- for (let color of this.containerColors) {`
	`613`	`+ for (const color of this.containerColors) {`
`614`	`614`	`if (!assignedColors[color] \|\| assignedColors[color] < maxColors) {`
`615`	`615`	`availableColors.push(color);`
`616`	`616`	`}`
Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,7 @@ class Cookies {`
`29`	`29`	`let cookieHeader;`
`30`	`30`	`let cookiesHeader = {};`
`31`	`31`	`let cookieHeaderChanged = false;`
`32`		`- for (let domainPattern in this.storage.local.preferences.cookies.domain) {`
	`32`	`+ for (const domainPattern in this.storage.local.preferences.cookies.domain) {`
`33`	`33`	`if (!this.isolation.matchDomainPattern(details.url, domainPattern)) {`
`34`	`34`	`continue;`
`35`	`35`	`}`
`@@ -53,7 +53,7 @@ class Cookies {`
`53`	`53`	`debug('[maybeAddCookiesToHeader] found temp tab and header', details, cookieHeader, cookiesHeader);`
`54`	`54`	`}`
`55`	`55`
`56`		`- for (let cookie of this.storage.local.preferences.cookies.domain[domainPattern]) {`
	`56`	`+ for (const cookie of this.storage.local.preferences.cookies.domain[domainPattern]) {`
`57`	`57`	`if (!cookie) {`
`58`	`58`	`continue;`
`59`	`59`	`}`
Original file line number	Diff line number	Diff line change
`@@ -118,6 +118,7 @@ export default {`
`118`	`118`	`this.editing = false;`
`119`	`119`	`} else {`
`120`	`120`	`this.preferences.isolation.domain.push(JSON.parse(JSON.stringify(this.domain)));`
	`121`	`+ this.reset();`
`121`	`122`	`}`
`122`	`123`	`}`
`123`	`124`	`});`