diff --git a/app/scripts/controllers/permissions/index.js b/app/scripts/controllers/permissions/index.js index 1a6f7087c2ac..e1a756ea8921 100644 --- a/app/scripts/controllers/permissions/index.js +++ b/app/scripts/controllers/permissions/index.js @@ -535,20 +535,13 @@ export class PermissionsController { // attempt to pop metadata of a single domain without permissions if the // store is too large overall - if (this._newMetadataOrigins.size >= METADATA_CACHE_MAX_SIZE) { + if (this._pendingSiteMetadata.size >= METADATA_CACHE_MAX_SIZE) { const permissionsDomains = this.permissions.getDomains() - for (const origin of this._newMetadataOrigins.values()) { - - // We can stop keeping track of all domains we iterate over, because - // we will either delete 'origin' now if it does not have permissions, - // or we won't delete 'origin' until the next time MetaMask boots. - this._newMetadataOrigins.delete(origin) - - if (!permissionsDomains[origin]) { - delete newMetadataState[origin] - break - } + const oldOrigin = this._pendingSiteMetadata.values().next().value + this._pendingSiteMetadata.delete(oldOrigin) + if (!permissionsDomains[oldOrigin]) { + delete newMetadataState[oldOrigin] } } @@ -558,7 +551,7 @@ export class PermissionsController { ...metadata, lastUpdated: Date.now(), } - this._newMetadataOrigins.add(origin) + this._pendingSiteMetadata.add(origin) this._setDomainMetadata(newMetadataState) } @@ -573,7 +566,7 @@ export class PermissionsController { const metadataState = restoredState[METADATA_STORE_KEY] || {} const newMetadataState = this._trimDomainMetadata(metadataState) - this._newMetadataOrigins = new Set() + this._pendingSiteMetadata = new Set() this._setDomainMetadata(newMetadataState) } diff --git a/test/unit/app/controllers/permissions/permissions-controller-test.js b/test/unit/app/controllers/permissions/permissions-controller-test.js index 401fdff290af..981330872380 100644 --- a/test/unit/app/controllers/permissions/permissions-controller-test.js +++ b/test/unit/app/controllers/permissions/permissions-controller-test.js @@ -1419,7 +1419,9 @@ describe('permissions controller', function () { ) }) - it('pops metadata when the metadata store is too large', function () { + it('pops metadata on add when too many origins are pending', function () { + + sinon.spy(permController._pendingSiteMetadata, 'delete') const mockMetadata = getMockMetadata(METADATA_CACHE_MAX_SIZE) const expectedDeletedOrigin = Object.keys(mockMetadata)[0] @@ -1428,10 +1430,10 @@ describe('permissions controller', function () { [METADATA_STORE_KEY]: { ...mockMetadata }, }) - // populate permController.newMetadataOrigins, as though these origins + // populate permController._pendingSiteMetadata, as though these origins // were actually added Object.keys(mockMetadata).forEach((origin) => { - permController._newMetadataOrigins.add(origin) + permController._pendingSiteMetadata.add(origin) }) permController.addDomainMetadata(ORIGINS.a, { foo: 'bar' }) @@ -1450,6 +1452,10 @@ describe('permissions controller', function () { } delete expectedMetadata[expectedDeletedOrigin] + assert.ok( + permController._pendingSiteMetadata.delete.calledOnceWithExactly(expectedDeletedOrigin), + 'should have called _pendingSiteMetadata.delete once' + ) assert.equal( permController._setDomainMetadata.getCalls().length, 1, 'should have called _setDomainMetadata once'