Remove security origin support from DOM Storage code as it was buggy and unnecessary.

Bug: 1347831
Change-Id: Icf1c52bc8f765123b473c2cba738f88f6f552b63
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4127160
Commit-Queue: Danil Somsikov <dsv@chromium.org>
Auto-Submit: Danil Somsikov <dsv@chromium.org>
Reviewed-by: Kateryna Prokopenko <kprokopenko@chromium.org>

front_end/panels/application/ApplicationPanelSidebar.ts

@@ -729,7 +729,7 @@ export class ApplicationPanelSidebar extends UI.Widget.VBox implements SDK.Targe
 
   private addDOMStorage(domStorage: DOMStorage): void {
     console.assert(!this.domStorageTreeElements.get(domStorage));
-    console.assert(Boolean(domStorage.storageKey) || Boolean(domStorage.securityOrigin));
+    console.assert(Boolean(domStorage.storageKey));
 
     const domStorageTreeElement = new DOMStorageTreeElement(this.panel, domStorage);
     this.domStorageTreeElements.set(domStorage, domStorageTreeElement);
@@ -1665,19 +1665,15 @@ export class IDBIndexTreeElement extends ApplicationPanelTreeElement {
 export class DOMStorageTreeElement extends ApplicationPanelTreeElement {
   private readonly domStorage: DOMStorage;
   constructor(storagePanel: ResourcesPanel, domStorage: DOMStorage) {
-    super(
-        storagePanel,
-        domStorage.securityOrigin ? domStorage.securityOrigin :
-                                    (domStorage.storageKey ? domStorage.storageKey : i18nString(UIStrings.localFiles)),
-        false);
+    super(storagePanel, domStorage.storageKey ? domStorage.storageKey : i18nString(UIStrings.localFiles), false);
     this.domStorage = domStorage;
     const icon = UI.Icon.Icon.create('mediumicon-table', 'resource-tree-item');
     this.setLeadingIcons([icon]);
   }
 
   get itemURL(): Platform.DevToolsPath.UrlString {
-    return 'storage://' + this.domStorage.securityOrigin + '/' +
-        (this.domStorage.isLocalStorage ? 'local' : 'session') as Platform.DevToolsPath.UrlString;
+    return 'storage://' + this.domStorage.storageKey + '/' + (this.domStorage.isLocalStorage ? 'local' : 'session') as
+        Platform.DevToolsPath.UrlString;
   }
 
   onselect(selectedByUser?: boolean): boolean {

front_end/panels/application/DOMStorageModel.ts

@@ -38,55 +38,22 @@ import type * as ProtocolProxyApi from '../../generated/protocol-proxy-api.js';
 
 export class DOMStorage extends Common.ObjectWrapper.ObjectWrapper<DOMStorage.EventTypes> {
   private readonly model: DOMStorageModel;
-  private readonly securityOriginInternal: string|null;
-  private readonly storageKeyInternal: string|null;
+  private readonly storageKeyInternal: string;
   private readonly isLocalStorageInternal: boolean;
 
-  constructor(model: DOMStorageModel, securityOrigin: string, storageKey: string, isLocalStorage: boolean) {
+  constructor(model: DOMStorageModel, storageKey: string, isLocalStorage: boolean) {
     super();
     this.model = model;
-    this.securityOriginInternal = securityOrigin;
     this.storageKeyInternal = storageKey;
     this.isLocalStorageInternal = isLocalStorage;
   }
 
-  static storageId(securityOrigin: string, isLocalStorage: boolean): Protocol.DOMStorage.StorageId {
-    return {securityOrigin: securityOrigin, isLocalStorage: isLocalStorage};
-  }
-
-  static storageIdWithSecurityOrigin(securityOrigin: string, isLocalStorage: boolean): Protocol.DOMStorage.StorageId {
-    return {securityOrigin: securityOrigin, isLocalStorage: isLocalStorage};
-  }
-
-  static storageIdWithStorageKey(storageKey: string, isLocalStorage: boolean): Protocol.DOMStorage.StorageId {
+  static storageId(storageKey: string, isLocalStorage: boolean): Protocol.DOMStorage.StorageId {
     return {storageKey: storageKey, isLocalStorage: isLocalStorage};
   }
 
-  get idWithSecurityOrigin(): Protocol.DOMStorage.StorageId {
-    let securityOrigin = '';
-    if (this.securityOriginInternal) {
-      securityOrigin = this.securityOriginInternal;
-    }
-    return DOMStorage.storageIdWithSecurityOrigin(securityOrigin, this.isLocalStorageInternal);
-  }
-
-  get idWithStorageKey(): Protocol.DOMStorage.StorageId {
-    let storageKey = '';
-    if (this.storageKeyInternal) {
-      storageKey = this.storageKeyInternal;
-    }
-    return DOMStorage.storageIdWithStorageKey(storageKey, this.isLocalStorageInternal);
-  }
-
   get id(): Protocol.DOMStorage.StorageId {
-    if (this.storageKeyInternal) {
-      return this.idWithStorageKey;
-    }
-    return this.idWithSecurityOrigin;
-  }
-
-  get securityOrigin(): string|null {
-    return this.securityOriginInternal;
+    return DOMStorage.storageId(this.storageKeyInternal, this.isLocalStorageInternal);
   }
 
   get storageKey(): string|null {
@@ -148,7 +115,6 @@ export namespace DOMStorage {
 }
 
 export class DOMStorageModel extends SDK.SDKModel.SDKModel<EventTypes> {
-  private readonly securityOriginManager: SDK.SecurityOriginManager.SecurityOriginManager|null;
   private readonly storageKeyManagerInternal: SDK.StorageKeyManager.StorageKeyManager|null;
   private storagesInternal: {
     [x: string]: DOMStorage,
@@ -159,7 +125,6 @@ export class DOMStorageModel extends SDK.SDKModel.SDKModel<EventTypes> {
   constructor(target: SDK.Target.Target) {
     super(target);
 
-    this.securityOriginManager = target.model(SDK.SecurityOriginManager.SecurityOriginManager);
     this.storageKeyManagerInternal = target.model(SDK.StorageKeyManager.StorageKeyManager);
     this.storagesInternal = {};
     this.agent = target.domstorageAgent();
@@ -185,43 +150,17 @@ export class DOMStorageModel extends SDK.SDKModel.SDKModel<EventTypes> {
         this.addStorageKey(storageKey);
       }
     }
-    if (this.securityOriginManager) {
-      this.securityOriginManager.addEventListener(
-          SDK.SecurityOriginManager.Events.SecurityOriginAdded, this.securityOriginAdded, this);
-      this.securityOriginManager.addEventListener(
-          SDK.SecurityOriginManager.Events.SecurityOriginRemoved, this.securityOriginRemoved, this);
-
-      for (const securityOrigin of this.securityOriginManager.securityOrigins()) {
-        this.addOrigin(securityOrigin);
-      }
-    }
     void this.agent.invoke_enable();
 
     this.enabled = true;
   }
 
-  clearForOrigin(origin: string): void {
-    if (!this.enabled) {
-      return;
-    }
-    for (const isLocal of [true, false]) {
-      const key = this.keyForSecurityOrigin(origin, isLocal);
-      const storage = this.storagesInternal[key];
-      if (!storage) {
-        return;
-      }
-      storage.clear();
-    }
-    this.removeOrigin(origin);
-    this.addOrigin(origin);
-  }
-
   clearForStorageKey(storageKey: string): void {
     if (!this.enabled) {
       return;
     }
     for (const isLocal of [true, false]) {
-      const key = this.keyForStorageKey(storageKey, isLocal);
+      const key = this.storageKey(storageKey, isLocal);
       const storage = this.storagesInternal[key];
       if (!storage) {
         return;
@@ -232,83 +171,27 @@ export class DOMStorageModel extends SDK.SDKModel.SDKModel<EventTypes> {
     this.addStorageKey(storageKey);
   }
 
-  private securityOriginAdded(event: Common.EventTarget.EventTargetEvent<string>): void {
-    this.addOrigin(event.data);
-  }
-
   private storageKeyAdded(event: Common.EventTarget.EventTargetEvent<string>): void {
     this.addStorageKey(event.data);
   }
 
-  private addOrigin(securityOrigin: string): void {
-    const parsed = new Common.ParsedURL.ParsedURL(securityOrigin);
-    // These are "opaque" origins which are not supposed to support DOM storage.
-    if (!parsed.isValid || parsed.scheme === 'data' || parsed.scheme === 'about' || parsed.scheme === 'javascript') {
-      return;
-    }
-
-    for (const isLocal of [true, false]) {
-      const key = this.keyForSecurityOrigin(securityOrigin, isLocal);
-      console.assert(!this.storagesInternal[key]);
-      if (this.duplicateExists(key)) {
-        continue;
-      }
-      const storage = new DOMStorage(this, securityOrigin, '', isLocal);
-      this.storagesInternal[key] = storage;
-      this.dispatchEventToListeners(Events.DOMStorageAdded, storage);
-    }
-  }
-
   private addStorageKey(storageKey: string): void {
     for (const isLocal of [true, false]) {
-      const key = this.keyForStorageKey(storageKey, isLocal);
+      const key = this.storageKey(storageKey, isLocal);
       console.assert(!this.storagesInternal[key]);
-      if (this.duplicateExists(key)) {
-        continue;
-      }
-      const storage = new DOMStorage(this, '', storageKey, isLocal);
+      const storage = new DOMStorage(this, storageKey, isLocal);
       this.storagesInternal[key] = storage;
       this.dispatchEventToListeners(Events.DOMStorageAdded, storage);
     }
   }
 
-  private duplicateExists(key: string): boolean {
-    const parsedKey = JSON.parse(key);
-    for (const storageInternal in this.storagesInternal) {
-      const parsedStorageInternalKey = JSON.parse(storageInternal);
-      if (parsedKey.isLocalStorage === parsedStorageInternalKey.isLocalStorage) {
-        if (parsedKey.storageKey?.slice(0, -1) === parsedStorageInternalKey.securityOrigin ||
-            parsedKey.securityOrigin === parsedStorageInternalKey.storageKey?.slice(0, -1)) {
-          return true;
-        }
-      }
-    }
-    return false;
-  }
-
-  private securityOriginRemoved(event: Common.EventTarget.EventTargetEvent<string>): void {
-    this.removeOrigin(event.data);
-  }
-
   private storageKeyRemoved(event: Common.EventTarget.EventTargetEvent<string>): void {
     this.removeStorageKey(event.data);
   }
 
-  private removeOrigin(securityOrigin: string): void {
-    for (const isLocal of [true, false]) {
-      const key = this.keyForSecurityOrigin(securityOrigin, isLocal);
-      const storage = this.storagesInternal[key];
-      if (!storage) {
-        continue;
-      }
-      delete this.storagesInternal[key];
-      this.dispatchEventToListeners(Events.DOMStorageRemoved, storage);
-    }
-  }
-
   private removeStorageKey(storageKey: string): void {
     for (const isLocal of [true, false]) {
-      const key = this.keyForStorageKey(storageKey, isLocal);
+      const key = this.storageKey(storageKey, isLocal);
       const storage = this.storagesInternal[key];
       if (!storage) {
         continue;
@@ -318,23 +201,8 @@ export class DOMStorageModel extends SDK.SDKModel.SDKModel<EventTypes> {
     }
   }
 
-  private storageKey(securityOrigin: string|undefined, storageKey: string|undefined, isLocalStorage: boolean): string {
-    console.assert(Boolean(securityOrigin) || Boolean(storageKey));
-    if (storageKey) {
-      return JSON.stringify(DOMStorage.storageIdWithStorageKey(storageKey, isLocalStorage));
-    }
-    if (securityOrigin) {
-      return JSON.stringify(DOMStorage.storageIdWithSecurityOrigin(securityOrigin, isLocalStorage));
-    }
-    throw new Error('Either securityOrigin or storageKey is required');
-  }
-
-  private keyForSecurityOrigin(securityOrigin: string, isLocalStorage: boolean): string {
-    return this.storageKey(securityOrigin, '', isLocalStorage);
-  }
-
-  private keyForStorageKey(storageKey: string, isLocalStorage: boolean): string {
-    return this.storageKey('', storageKey, isLocalStorage);
+  private storageKey(storageKey: string, isLocalStorage: boolean): string {
+    return JSON.stringify(DOMStorage.storageId(storageKey, isLocalStorage));
   }
 
   domStorageItemsCleared(storageId: Protocol.DOMStorage.StorageId): void {
@@ -377,8 +245,8 @@ export class DOMStorageModel extends SDK.SDKModel.SDKModel<EventTypes> {
   }
 
   storageForId(storageId: Protocol.DOMStorage.StorageId): DOMStorage {
-    return this
-        .storagesInternal[this.storageKey(storageId.securityOrigin, storageId.storageKey, storageId.isLocalStorage)];
+    console.assert(Boolean(storageId.storageKey));
+    return this.storagesInternal[this.storageKey(storageId.storageKey || '', storageId.isLocalStorage)];
   }
 
   storages(): DOMStorage[] {

front_end/panels/application/StorageView.ts

@@ -406,12 +406,8 @@ export class StorageView extends UI.ThrottledWidget.ThrottledWidget {
 
     if (this.target) {
       const includeThirdPartyCookies = this.includeThirdPartyCookiesSetting.get();
-      if (this.storageKey) {
-        StorageView.clearByStorageKey(
-            this.target, this.storageKey, this.securityOrigin, selectedStorageTypes, includeThirdPartyCookies);
-      } else if (this.securityOrigin) {
-        StorageView.clear(this.target, this.securityOrigin, selectedStorageTypes, includeThirdPartyCookies);
-      }
+      StorageView.clear(
+          this.target, this.storageKey, this.securityOrigin, selectedStorageTypes, includeThirdPartyCookies);
     }
 
     this.clearButton.disabled = true;
@@ -425,39 +421,12 @@ export class StorageView extends UI.ThrottledWidget.ThrottledWidget {
   }
 
   static clear(
-      target: SDK.Target.Target, securityOrigin: string, selectedStorageTypes: string[],
+      target: SDK.Target.Target, storageKey: string|null, originForCookies: string|null, selectedStorageTypes: string[],
       includeThirdPartyCookies: boolean): void {
-    void target.storageAgent().invoke_clearDataForOrigin(
-        {origin: securityOrigin, storageTypes: selectedStorageTypes.join(',')});
-
-    const set = new Set(selectedStorageTypes);
-    const hasAll = set.has(Protocol.Storage.StorageType.All);
-    if (set.has(Protocol.Storage.StorageType.Cookies) || hasAll) {
-      const cookieModel = target.model(SDK.CookieModel.CookieModel);
-      if (cookieModel) {
-        void cookieModel.clear(undefined, includeThirdPartyCookies ? undefined : securityOrigin);
-      }
+    console.assert(Boolean(storageKey));
+    if (!storageKey) {
+      return;
     }
-
-    if (set.has(Protocol.Storage.StorageType.Local_storage) || hasAll) {
-      const storageModel = target.model(DOMStorageModel);
-      if (storageModel) {
-        storageModel.clearForOrigin(securityOrigin);
-      }
-    }
-
-    if (set.has(Protocol.Storage.StorageType.Websql) || hasAll) {
-      const databaseModel = target.model(DatabaseModel);
-      if (databaseModel) {
-        databaseModel.disable();
-        databaseModel.enable();
-      }
-    }
-  }
-
-  static clearByStorageKey(
-      target: SDK.Target.Target, storageKey: string, originForCookies: string|undefined, selectedStorageTypes: string[],
-      includeThirdPartyCookies: boolean): void {
     void target.storageAgent().invoke_clearDataForStorageKey(
         {storageKey, storageTypes: selectedStorageTypes.join(',')});
 
@@ -637,12 +606,7 @@ export class ActionDelegate implements UI.ActionRegistration.ActionDelegate {
     }
     const securityOrigin = resourceTreeModel.getMainSecurityOrigin();
     resourceTreeModel.getMainStorageKey().then(storageKey => {
-      if (storageKey) {
-        StorageView.clearByStorageKey(
-            target, storageKey, securityOrigin ? securityOrigin : undefined, AllStorageTypes, includeThirdPartyCookies);
-      } else if (securityOrigin) {
-        StorageView.clear(target, securityOrigin, AllStorageTypes, includeThirdPartyCookies);
-      }
+      StorageView.clear(target, storageKey, securityOrigin, AllStorageTypes, includeThirdPartyCookies);
     }, _ => {});
     return true;
   }

test/unittests/front_end/panels/application/DOMStorageModel_test.ts

@@ -20,7 +20,7 @@ describeWithMockConnection('DOMStorageModel', () => {
   beforeEach(() => {
     target = createTarget();
     domStorageModel = new Resources.DOMStorageModel.DOMStorageModel(target);
-    domStorage = new Resources.DOMStorageModel.DOMStorage(domStorageModel, 'testOrigin', initKey, true);
+    domStorage = new Resources.DOMStorageModel.DOMStorage(domStorageModel, initKey, true);
   });
 
   it('DOMStorage is instantiated correctly', () => {
@@ -44,15 +44,4 @@ describeWithMockConnection('DOMStorageModel', () => {
     manager.dispatchEventToListeners(SDK.StorageKeyManager.Events.StorageKeyRemoved, testKey);
     assert.isUndefined(domStorageModel.storageForId(testId));
   });
-
-  it('DOMStorage prioritises storage key over security origin if both set', () => {
-    const storageId = {storageKey: initKey, isLocalStorage: true};
-    const getItemsSpy = sinon.spy(target.domstorageAgent(), 'invoke_getDOMStorageItems');
-
-    void domStorage.getItems();
-
-    assert.isNotEmpty(domStorage.securityOrigin);
-    assert.isNotEmpty(domStorage.storageKey);
-    assert.isTrue(getItemsSpy.calledOnceWithExactly({storageId}));
-  });
 });

test/unittests/front_end/panels/application/StorageView_test.ts

@@ -27,7 +27,7 @@ describeWithMockConnection('StorageView', () => {
       storageKeyManager = target.model(SDK.StorageKeyManager.StorageKeyManager);
     });
 
-    it('emits correct events on clearStorageByStorageKey', () => {
+    it('emits correct events on clear', () => {
       const testId = {storageKey: testKey, isLocalStorage: true} as Protocol.DOMStorage.StorageId;
 
       assertNotNullOrUndefined(domStorageModel);
@@ -38,8 +38,7 @@ describeWithMockConnection('StorageView', () => {
 
       const dispatcherSpy = sinon.spy(domStorageModel, 'dispatchEventToListeners');
       const spyClearDataForStorageKey = sinon.stub(target.storageAgent(), 'invoke_clearDataForStorageKey');
-      Resources.StorageView.StorageView.clearByStorageKey(
-          target, testKey, undefined, [Protocol.Storage.StorageType.All], false);
+      Resources.StorageView.StorageView.clear(target, testKey, null, [Protocol.Storage.StorageType.All], false);
       // must be called 4 times, twice with DOMStorageRemoved for local and non-local storage and twice with DOMStorageAdded
       assert.isTrue(spyClearDataForStorageKey.calledOnce);
       assert.strictEqual(dispatcherSpy.callCount, 4);
@@ -61,20 +60,19 @@ describeWithMockConnection('StorageView', () => {
       assert.strictEqual(subtitle?.textContent, testKey);
     });
 
-    it('also clears cookies on clearByStorageKey', () => {
+    it('also clears cookies on clear', () => {
       /* eslint-disable-next-line @typescript-eslint/no-non-null-assertion */
       const cookieModel = target.model(SDK.CookieModel.CookieModel)!;
       const clearByOriginSpy = sinon.spy(target.storageAgent(), 'invoke_clearDataForOrigin');
       const cookieClearSpy = sinon.spy(cookieModel, 'clear');
 
-      Resources.StorageView.StorageView.clearByStorageKey(
-          target, testKey, testOrigin, [Protocol.Storage.StorageType.All], false);
+      Resources.StorageView.StorageView.clear(target, testKey, testOrigin, [Protocol.Storage.StorageType.All], false);
 
       assert.isTrue(clearByOriginSpy.calledOnceWithExactly({origin: testOrigin, storageTypes: 'cookies'}));
       assert.isTrue(cookieClearSpy.calledOnceWithExactly(undefined, testOrigin));
     });
 
-    it('also clears WebSQL on clearByStorageKey', async () => {
+    it('also clears WebSQL on clear', async () => {
       const databaseModel = target.model(Resources.DatabaseModel.DatabaseModel);
       assertNotNullOrUndefined(databaseModel);
       const databaseRemoved = new Promise(resolve => {
@@ -86,8 +84,7 @@ describeWithMockConnection('StorageView', () => {
       databaseModel.addDatabase(testDatabase);
       assert.deepEqual(databaseModel.databases()[0], testDatabase);
 
-      Resources.StorageView.StorageView.clearByStorageKey(
-          target, testKey, '', [Protocol.Storage.StorageType.All], false);
+      Resources.StorageView.StorageView.clear(target, testKey, '', [Protocol.Storage.StorageType.All], false);
 
       await databaseRemoved;
       assert.isEmpty(databaseModel.databases());
@@ -121,7 +118,7 @@ describeWithMockConnection('StorageView', () => {
       assert.isEmpty(databaseModel.databases());
     });
 
-    it('clears cache on clearByStorageKey', async () => {
+    it('clears cache on clear', async () => {
       const cacheStorageModel = target.model(SDK.ServiceWorkerCacheModel.ServiceWorkerCacheModel);
       assertNotNullOrUndefined(cacheStorageModel);
       let caches = [
@@ -149,8 +146,7 @@ describeWithMockConnection('StorageView', () => {
       await cacheAddedPromise;
       caches = [];
 
-      Resources.StorageView.StorageView.clearByStorageKey(
-          target, testKey, '', [Protocol.Storage.StorageType.Cache_storage], false);
+      Resources.StorageView.StorageView.clear(target, testKey, '', [Protocol.Storage.StorageType.Cache_storage], false);
 
       assert.isEmpty(cacheStorageModel.caches());
     });