Remove usages of chrome.storage (#8664)

* Enable clearing and retrieving all values from local storage

I didn't add these methods to the base abstract class because we don't currently have a use case for them. If we develop one, we can just lift it up.

* Use new browser local storage methods for reseed task

* Remove the now dangerous methods enabling usage of `chrome.storage`

Any direct reference to chrome storage needs to handle serialization tags, which is best dealt with through the classes implementing `AbstractChromeStorageService`

apps/browser/src/autofill/utils/index.spec.ts

@@ -8,7 +8,6 @@ import {
   generateRandomCustomElementName,
   sendExtensionMessage,
   setElementStyles,
-  getFromLocalStorage,
   setupExtensionDisconnectAction,
   setupAutofillInitDisconnectAction,
 } from "./index";
@@ -124,33 +123,6 @@ describe("setElementStyles", () => {
   });
 });
 
-describe("getFromLocalStorage", () => {
-  it("returns a promise with the storage object pulled from the extension storage api", async () => {
-    const localStorage: Record<string, any> = {
-      testValue: "test",
-      another: "another",
-    };
-    jest.spyOn(chrome.storage.local, "get").mockImplementation((keys, callback) => {
-      const localStorageObject: Record<string, string> = {};
-
-      if (typeof keys === "string") {
-        localStorageObject[keys] = localStorage[keys];
-      } else if (Array.isArray(keys)) {
-        for (const key of keys) {
-          localStorageObject[key] = localStorage[key];
-        }
-      }
-
-      callback(localStorageObject);
-    });
-
-    const returnValue = await getFromLocalStorage("testValue");
-
-    expect(chrome.storage.local.get).toHaveBeenCalled();
-    expect(returnValue).toEqual({ testValue: "test" });
-  });
-});
-
 describe("setupExtensionDisconnectAction", () => {
   afterEach(() => {
     jest.clearAllMocks();

apps/browser/src/autofill/utils/index.ts

@@ -106,18 +106,6 @@ function setElementStyles(
   }
 }
 
-/**
- * Get data from local storage based on the keys provided.
- *
- * @param keys - String or array of strings of keys to get from local storage
- * @deprecated Do not call this, use state-relevant services instead
- */
-async function getFromLocalStorage(keys: string | string[]): Promise<Record<string, any>> {
-  return new Promise((resolve) => {
-    chrome.storage.local.get(keys, (storage: Record<string, any>) => resolve(storage));
-  });
-}
-
 /**
  * Sets up a long-lived connection with the extension background
  * and triggers an onDisconnect event if the extension context
@@ -278,7 +266,6 @@ export {
   buildSvgDomElement,
   sendExtensionMessage,
   setElementStyles,
-  getFromLocalStorage,
   setupExtensionDisconnectAction,
   setupAutofillInitDisconnectAction,
   elementIsFillableFormField,

apps/browser/src/background/main.background.ts

@@ -234,7 +234,7 @@ import RuntimeBackground from "./runtime.background";
 
 export default class MainBackground {
   messagingService: MessagingServiceAbstraction;
-  storageService: AbstractStorageService & ObservableStorageService;
+  storageService: BrowserLocalStorageService;
   secureStorageService: AbstractStorageService;
   memoryStorageService: AbstractMemoryStorageService;
   memoryStorageForStateProviders: AbstractMemoryStorageService & ObservableStorageService;
@@ -1255,18 +1255,8 @@ export default class MainBackground {
       return;
     }
 
-    const getStorage = (): Promise<any> =>
+    const storage = await this.storageService.getAll();
-      new Promise((resolve) => {
+    await this.storageService.clear();
-        chrome.storage.local.get(null, (o: any) => resolve(o));
-      });
-
-    const clearStorage = (): Promise<void> =>
-      new Promise((resolve) => {
-        chrome.storage.local.clear(() => resolve());
-      });
-
-    const storage = await getStorage();
-    await clearStorage();
 
     for (const key in storage) {
       // eslint-disable-next-line

apps/browser/src/platform/services/abstractions/abstract-chrome-storage-api.service.ts

@@ -10,6 +10,8 @@ import { fromChromeEvent } from "../../browser/from-chrome-event";
 
 export const serializationIndicator = "__json__";
 
+type serializedObject = { [serializationIndicator]: true; value: string };
+
 export const objToStore = (obj: any) => {
   if (obj == null) {
     return null;
@@ -61,11 +63,7 @@ export default abstract class AbstractChromeStorageService
     return new Promise((resolve) => {
       this.chromeStorageApi.get(key, (obj: any) => {
         if (obj != null && obj[key] != null) {
-          let value = obj[key];
+          resolve(this.processGetObject(obj[key]));
-          if (value[serializationIndicator] && typeof value.value === "string") {
-            value = JSON.parse(value.value);
-          }
-          resolve(value as T);
           return;
         }
         resolve(null);
@@ -95,4 +93,22 @@ export default abstract class AbstractChromeStorageService
       });
     });
   }
+
+  /** Backwards compatible resolution of retrieved object with new serialized storage */
+  protected processGetObject<T>(obj: T | serializedObject): T | null {
+    if (this.isSerialized(obj)) {
+      obj = JSON.parse(obj.value);
+    }
+    return obj as T;
+  }
+
+  /** Type guard for whether an object is tagged as serialized */
+  protected isSerialized<T>(value: T | serializedObject): value is serializedObject {
+    const asSerialized = value as serializedObject;
+    return (
+      asSerialized != null &&
+      asSerialized[serializationIndicator] &&
+      typeof asSerialized.value === "string"
+    );
+  }
 }

apps/browser/src/platform/services/abstractions/chrome-storage-api.service.spec.ts

@@ -66,6 +66,7 @@ describe("ChromeStorageApiService", () => {
 
   describe("get", () => {
     let getMock: jest.Mock;
+    const key = "key";
 
     beforeEach(() => {
       // setup get
@@ -76,7 +77,6 @@ describe("ChromeStorageApiService", () => {
     });
 
     it("returns a stored value when it is serialized", async () => {
-      const key = "key";
       const value = { key: "value" };
       store[key] = objToStore(value);
       const result = await service.get(key);
@@ -84,7 +84,6 @@ describe("ChromeStorageApiService", () => {
     });
 
     it("returns a stored value when it is not serialized", async () => {
-      const key = "key";
       const value = "value";
       store[key] = value;
       const result = await service.get(key);
@@ -95,5 +94,12 @@ describe("ChromeStorageApiService", () => {
       const result = await service.get("key");
       expect(result).toBeNull();
     });
+
+    it("returns null when the stored object is null", async () => {
+      store[key] = null;
+
+      const result = await service.get(key);
+      expect(result).toBeNull();
+    });
   });
 });

apps/browser/src/platform/services/browser-local-storage.service.spec.ts

@@ -0,0 +1,89 @@
+import { objToStore } from "./abstractions/abstract-chrome-storage-api.service";
+import BrowserLocalStorageService from "./browser-local-storage.service";
+
+describe("BrowserLocalStorageService", () => {
+  let service: BrowserLocalStorageService;
+  let store: Record<any, any>;
+
+  beforeEach(() => {
+    store = {};
+
+    service = new BrowserLocalStorageService();
+  });
+
+  describe("clear", () => {
+    let clearMock: jest.Mock;
+
+    beforeEach(() => {
+      clearMock = chrome.storage.local.clear as jest.Mock;
+    });
+
+    it("uses the api to clear", async () => {
+      await service.clear();
+
+      expect(clearMock).toHaveBeenCalledTimes(1);
+    });
+  });
+
+  describe("getAll", () => {
+    let getMock: jest.Mock;
+
+    beforeEach(() => {
+      // setup get
+      getMock = chrome.storage.local.get as jest.Mock;
+      getMock.mockImplementation((key, callback) => {
+        if (key == null) {
+          callback(store);
+        } else {
+          callback({ [key]: store[key] });
+        }
+      });
+    });
+
+    it("returns all values", async () => {
+      store["key1"] = "string";
+      store["key2"] = 0;
+      const result = await service.getAll();
+
+      expect(result).toEqual(store);
+    });
+
+    it("handles empty stores", async () => {
+      const result = await service.getAll();
+
+      expect(result).toEqual({});
+    });
+
+    it("handles stores with null values", async () => {
+      store["key"] = null;
+
+      const result = await service.getAll();
+      expect(result).toEqual(store);
+    });
+
+    it("handles values processed for storage", async () => {
+      const obj = { test: 2 };
+      const key = "key";
+      store[key] = objToStore(obj);
+
+      const result = await service.getAll();
+
+      expect(result).toEqual({
+        [key]: obj,
+      });
+    });
+
+    // This is a test of backwards compatibility before local storage was serialized.
+    it("handles values that were stored without processing for storage", async () => {
+      const obj = { test: 2 };
+      const key = "key";
+      store[key] = obj;
+
+      const result = await service.getAll();
+
+      expect(result).toEqual({
+        [key]: obj,
+      });
+    });
+  });
+});

apps/browser/src/platform/services/browser-local-storage.service.ts

@@ -4,4 +4,32 @@ export default class BrowserLocalStorageService extends AbstractChromeStorageSer
   constructor() {
     super(chrome.storage.local);
   }
+
+  /**
+   * Clears local storage
+   */
+  async clear() {
+    await chrome.storage.local.clear();
+  }
+
+  /**
+   * Retrieves all objects stored in local storage.
+   *
+   * @remarks This method processes values prior to resolving, do not use `chrome.storage.local` directly
+   * @returns Promise resolving to keyed object of all stored data
+   */
+  async getAll(): Promise<Record<string, unknown>> {
+    return new Promise((resolve) => {
+      this.chromeStorageApi.get(null, (allStorage) => {
+        const resolved = Object.entries(allStorage).reduce(
+          (agg, [key, value]) => {
+            agg[key] = this.processGetObject(value);
+            return agg;
+          },
+          {} as Record<string, unknown>,
+        );
+        resolve(resolved);
+      });
+    });
+  }
 }