FIX: Add workaround for Safari's IndexedDb bug (#113)

FIX: Add workaround for Safari’s IndexedDb bug (#113)

  • DEV: Return a rejected promise without an identity

If no identity can be loaded from the local database a rejected Promise object should be returned instead of an empty identity. In the past this caused some processes to fail silently or later than expected.

Follow up to commits b273b0caeff467a9793e9c79a5202a9ac995d25d and 54d1f79a9eefa7e3a4e4a5b4b1303abe03ab72ed.

  • FIX: Add workaround for Safari’s IndexedDb bug

In Safari 14, indexedDB.open hangs if used immediately after page was loaded. Performing other operations as a warm up works around this bug.

  • FIX: Use workaround just for Safari 14
diff --git a/assets/javascripts/lib/database.js b/assets/javascripts/lib/database.js
index 5b16a06..606950a 100644
--- a/assets/javascripts/lib/database.js
+++ b/assets/javascripts/lib/database.js
@@ -21,13 +21,71 @@ export let useLocalStorage = false;
 /**
  * Force usage of local storage instead of IndexedDb.
  *
+ * Used in tests
+ *
  * @param {Boolean} value Whether to use local storage.
  */
-export function setUseLocalStorage(value) {
+export function _setUseLocalStorage(value) {
   useLocalStorage = value;
 }
 
 /**
+ * IndexedDb API used to store CryptoKey objects securely
+ */
+export let indexedDb = window.indexedDB;
+
+/**
+ * Sets IndexedDb backend
+ *
+ * Used in tests
+ *
+ * @param {Object} value
+ */
+export function _setIndexedDb(value) {
+  indexedDb = value;
+}
+
+/**
+ * Browser's user agent string
+ */
+export let userAgent = window.navigator.userAgent;
+
+/**
+ * Sets browser's user agent string
+ *
+ * Used in tests
+ *
+ * @param {String} value
+ */
+export function _setUserAgent(value) {
+  userAgent = value;
+}
+
+/**
+ * Warm up IndexedDB to ensure it works normally
+ *
+ * Used in Safari 14 to work around a bug. indexedDB.open hangs in
+ * Safari if used immediately after page was loaded:
+ * https://bugs.webkit.org/show_bug.cgi?id=226547
+ *
+ * @return {Promise<void>}
+ */
+function initIndexedDb() {
+  if (!userAgent.match(/Version\/14.+?Safari/)) {
+    return Promise.resolve();
+  }
+
+  let interval;
+  return new Promise((resolve, reject) => {
+    const tryIndexedDb = () => indexedDb.databases().then(resolve, reject);
+    interval = setInterval(tryIndexedDb, 100);
+    tryIndexedDb();
+  }).finally(() => {
+    clearInterval(interval);
+  });
+}
+
+/**
  * Opens plugin's Indexed DB.
  *
  * @param {Boolean} create Whether to create database if missing.
@@ -35,7 +93,7 @@ export function setUseLocalStorage(value) {
  * @return {IDBOpenDBRequest}
  */
 function openDb(create) {
-  const req = window.indexedDB.open(DB_NAME, 1);
+  const req = indexedDb.open(DB_NAME, 1);
 
   req.onupgradeneeded = (evt) => {
     if (!create) {
@@ -67,18 +125,6 @@ function saveIdentityToLocalStorage(identity) {
  * @return {Promise}
  */
 export function saveDbIdentity(identity) {
-  /*
-  if (
-    !useLocalStorage &&
-    Object.values(identity).any(
-      key => key instanceof CryptoKey && key.extractable
-    )
-  ) {
-    // eslint-disable-next-line no-console
-    console.warn("Saving an extractable key into the database.", identity);
-  }
-  */
-
   if (useLocalStorage) {
     return saveIdentityToLocalStorage(identity);
   }
@@ -114,7 +160,7 @@ function loadIdentityFromLocalStorage() {
   const exported = window.localStorage.getItem(DB_NAME);
   return exported && exported !== "true"
     ? importIdentity(exported)
-    : Promise.resolve(null);
+    : Promise.reject();
 }
 
 /**
@@ -127,48 +173,37 @@ export function loadDbIdentity() {
     return loadIdentityFromLocalStorage();
   }
 
-  return new Promise((resolve, reject) => {
-    const req = openDb(false);
-    // eslint-disable-next-line no-unused-vars
-    req.onerror = (evt) => {
-      loadIdentityFromLocalStorage().then(resolve, reject);
-    };
-
-    req.onsuccess = (evt) => {
-      const db = evt.target.result;
-      const tx = db.transaction("keys", "readonly");
-      const st = tx.objectStore("keys");
-
-      const dataReq = st.getAll();
-      dataReq.onsuccess = (dataEvt) => {
-        const identities = dataEvt.target.result;
-        db.close();
-
-        if (identities && identities.length > 0) {
-          const identity = identities[identities.length - 1];
-          resolve(identity);
-        } else {
-          resolve(null);
-        }
-      };
+  return initIndexedDb().then(() => {
+    return new Promise((resolve, reject) => {
+      const req = openDb(false);
       // eslint-disable-next-line no-unused-vars
-      dataReq.onerror = (dataEvt) => {
+      req.onerror = (evt) => {
         loadIdentityFromLocalStorage().then(resolve, reject);
       };
-    };
-  }).then((identity) => {
-    /*
-    if (
-      !useLocalStorage &&
-      Object.values(identity).any(
-        key => key instanceof CryptoKey && key.extractable
-      )
-    ) {
-      // eslint-disable-next-line no-console
-      console.warn("Loaded an extractable key from the database.", identity);
-    }
-    */
-    return identity;
+
+      req.onsuccess = (evt) => {
+        const db = evt.target.result;
+        const tx = db.transaction("keys", "readonly");
+        const st = tx.objectStore("keys");
+
+        const dataReq = st.getAll();
+        dataReq.onsuccess = (dataEvt) => {
+          const identities = dataEvt.target.result;
+          db.close();
+
+          if (identities && identities.length > 0) {
+            const identity = identities[identities.length - 1];
+            resolve(identity);
+          } else {
+            reject();
+          }
+        };
+        // eslint-disable-next-line no-unused-vars
+        dataReq.onerror = (dataEvt) => {
+          loadIdentityFromLocalStorage().then(resolve, reject);
+        };
+      };
+    });
   });
 }
 
@@ -181,11 +216,13 @@ export function deleteDb() {
   window.localStorage.removeItem(DB_NAME);
   window.localStorage.removeItem(DB_VERSION);
 
-  return new Promise((resolve) => {
-    const req = window.indexedDB.deleteDatabase(DB_NAME);
+  return initIndexedDb().then(() => {
+    return new Promise((resolve) => {
+      const req = indexedDb.deleteDatabase(DB_NAME);
 
-    req.onsuccess = (evt) => resolve(evt);
-    req.onerror = (evt) => resolve(evt);
-    req.onblocked = (evt) => resolve(evt);
+      req.onsuccess = (evt) => resolve(evt);
+      req.onerror = (evt) => resolve(evt);
+      req.onblocked = (evt) => resolve(evt);
+    });
   });
 }
diff --git a/test/javascripts/acceptance/encrypt-test.js b/test/javascripts/acceptance/encrypt-test.js
index ddb2899..d519acb 100644
--- a/test/javascripts/acceptance/encrypt-test.js
+++ b/test/javascripts/acceptance/encrypt-test.js
@@ -353,8 +353,7 @@ acceptance("Encrypt", function (needs) {
     await visit("/u/eviltrout/preferences/security");
     await wait(ENCRYPT_ENABLED, () => click(".encrypt button#deactivate"));
 
-    const identity = await loadDbIdentity();
-    assert.equal(identity, null);
+    assert.rejects(loadDbIdentity());
   });
 
   test("encrypt settings visible only to allowed groups", async (assert) => {
diff --git a/test/javascripts/lib/database-safari-test.js b/test/javascripts/lib/database-safari-test.js
new file mode 100644
index 0000000..fd07994
--- /dev/null
+++ b/test/javascripts/lib/database-safari-test.js
@@ -0,0 +1,46 @@
+import {
+  _setIndexedDb,
+  _setUserAgent,
+  deleteDb,
+  loadDbIdentity,
+} from "discourse/plugins/discourse-encrypt/lib/database";
+import { test } from "qunit";
+import { Promise } from "rsvp";
+
+let indexedDbCalls = 0;
+
+QUnit.module("discourse-encrypt:lib:database-safari", {
+  beforeEach() {
+    indexedDbCalls = 0;
+
+    _setIndexedDb({
+      open(name, version) {
+        return window.indexedDB.open(name, version);
+      },
+
+      databases() {
+        return indexedDbCalls++ > 3
+          ? window.indexedDB.databases()
+          : new Promise(() => {});
+      },
+
+      deleteDatabase(name) {
+        indexedDbCalls++;
+        return window.indexedDB.deleteDatabase(name);
+      },
+    });
+
+    _setUserAgent("iPhone");
+  },
+
+  afterEach() {
+    _setIndexedDb(window.indexedDB);
+    _setUserAgent(window.navigator.userAgent);
+  },
+});
+
+test("IndexedDB is initialized in Safari", async (assert) => {
+  await deleteDb();
+  assert.rejects(loadDbIdentity());
+  assert.ok(indexedDbCalls > 0);
+});
diff --git a/test/javascripts/lib/database-test.js b/test/javascripts/lib/database-test.js
index b674d0a..97a0b0c 100644
--- a/test/javascripts/lib/database-test.js
+++ b/test/javascripts/lib/database-test.js
@@ -1,25 +1,25 @@

[... diff too long, it was truncated ...]

GitHub sha: 37acd2ec63f179a2a0df35d1bb153d3b225425b3

This commit appears in #113 which was approved by pmusaraj. It was merged by SamSaffron.