From e72a3e777972ecb757a9ad288904ed08bff4271e Mon Sep 17 00:00:00 2001 From: Jan Varga Date: Thu, 11 Jun 2020 06:36:55 +0000 Subject: [PATCH] Bug 1621916 - Provide a usable stack when requestFinished throws; r=dom-workers-and-storage-reviewers,sg,asuth Differential Revision: https://phabricator.services.mozilla.com/D77217 --- .../test/unit/xpcshell-head-parent-process.js | 27 +++++++++++++------ dom/localstorage/test/helpers.js | 27 +++++++++++++------ dom/localstorage/test/unit/head.js | 27 +++++++++++++------ dom/quota/QuotaRequests.cpp | 12 +++++++++ dom/quota/nsIQuotaRequests.idl | 2 ++ dom/quota/test/common/content.js | 18 +++++++------ dom/quota/test/common/global.js | 9 +++++++ dom/quota/test/common/system.js | 18 +++++++------ .../test/xpcshell/test_persist_globalLimit.js | 7 +++-- .../test/xpcshell/test_persist_groupLimit.js | 7 +++-- .../test/xpcshell/test_storagePressure.js | 7 +++-- 11 files changed, 115 insertions(+), 46 deletions(-) diff --git a/dom/indexedDB/test/unit/xpcshell-head-parent-process.js b/dom/indexedDB/test/unit/xpcshell-head-parent-process.js index 17e2913ceb4a..8b4df69cb03a 100644 --- a/dom/indexedDB/test/unit/xpcshell-head-parent-process.js +++ b/dom/indexedDB/test/unit/xpcshell-head-parent-process.js @@ -542,16 +542,27 @@ function getPrincipal(url) { return Services.scriptSecurityManager.createContentPrincipal(uri, {}); } -function requestFinished(request) { - return new Promise(function(resolve, reject) { - request.callback = function(req) { - if (req.resultCode == Cr.NS_OK) { - resolve(req.result); - } else { - reject(req.resultCode); - } +class RequestError extends Error { + constructor(resultCode, resultName) { + super(`Request failed (code: ${resultCode}, name: ${resultName})`); + this.name = "RequestError"; + this.resultCode = resultCode; + this.resultName = resultName; + } +} + +async function requestFinished(request) { + await new Promise(function(resolve) { + request.callback = function() { + resolve(); }; }); + + if (request.resultCode !== Cr.NS_OK) { + throw new RequestError(request.resultCode, request.resultName); + } + + return request.result; } // TODO: Rename to openDBRequestSucceeded ? diff --git a/dom/localstorage/test/helpers.js b/dom/localstorage/test/helpers.js index ca3d383a2b82..69281b09df9c 100644 --- a/dom/localstorage/test/helpers.js +++ b/dom/localstorage/test/helpers.js @@ -62,14 +62,25 @@ function getLocalStorage() { return localStorage; } -function requestFinished(request) { - return new Promise(function(resolve, reject) { - request.callback = SpecialPowers.wrapCallback(function(requestInner) { - if (requestInner.resultCode === SpecialPowers.Cr.NS_OK) { - resolve(requestInner.result); - } else { - reject(requestInner.resultCode); - } +class RequestError extends Error { + constructor(resultCode, resultName) { + super(`Request failed (code: ${resultCode}, name: ${resultName})`); + this.name = "RequestError"; + this.resultCode = resultCode; + this.resultName = resultName; + } +} + +async function requestFinished(request) { + await new Promise(function(resolve) { + request.callback = SpecialPowers.wrapCallback(function() { + resolve(); }); }); + + if (request.resultCode !== SpecialPowers.Cr.NS_OK) { + throw new RequestError(request.resultCode, request.resultName); + } + + return request.result; } diff --git a/dom/localstorage/test/unit/head.js b/dom/localstorage/test/unit/head.js index 0556c7b234df..234fbbb8452e 100644 --- a/dom/localstorage/test/unit/head.js +++ b/dom/localstorage/test/unit/head.js @@ -289,16 +289,27 @@ function getLocalStorage(principal) { ); } -function requestFinished(request) { - return new Promise(function(resolve, reject) { - request.callback = function(requestInner) { - if (requestInner.resultCode == Cr.NS_OK) { - resolve(requestInner.result); - } else { - reject(requestInner.resultCode); - } +class RequestError extends Error { + constructor(resultCode, resultName) { + super(`Request failed (code: ${resultCode}, name: ${resultName})`); + this.name = "RequestError"; + this.resultCode = resultCode; + this.resultName = resultName; + } +} + +async function requestFinished(request) { + await new Promise(function(resolve) { + request.callback = function() { + resolve(); }; }); + + if (request.resultCode !== Cr.NS_OK) { + throw new RequestError(request.resultCode, request.resultName); + } + + return request.result; } function loadSubscript(path) { diff --git a/dom/quota/QuotaRequests.cpp b/dom/quota/QuotaRequests.cpp index d03d7131334d..8f4c86bc25ed 100644 --- a/dom/quota/QuotaRequests.cpp +++ b/dom/quota/QuotaRequests.cpp @@ -74,6 +74,18 @@ RequestBase::GetResultCode(nsresult* aResultCode) { return NS_OK; } +NS_IMETHODIMP +RequestBase::GetResultName(nsACString& aResultName) { + AssertIsOnOwningThread(); + + if (!mHaveResultOrErrorCode) { + return NS_ERROR_FAILURE; + } + + mozilla::GetErrorName(mResultCode, aResultName); + return NS_OK; +} + UsageRequest::UsageRequest(nsIQuotaUsageCallback* aCallback) : mCallback(aCallback), mBackgroundActor(nullptr), mCanceled(false) { AssertIsOnOwningThread(); diff --git a/dom/quota/nsIQuotaRequests.idl b/dom/quota/nsIQuotaRequests.idl index 0a8f606ffc6c..8b05717e834d 100644 --- a/dom/quota/nsIQuotaRequests.idl +++ b/dom/quota/nsIQuotaRequests.idl @@ -17,6 +17,8 @@ interface nsIQuotaRequestBase : nsISupports readonly attribute nsIPrincipal principal; [must_use] readonly attribute nsresult resultCode; + + [must_use] readonly attribute ACString resultName; }; [scriptable, uuid(166e28e6-cf6d-4927-a6d7-b51bca9d3469)] diff --git a/dom/quota/test/common/content.js b/dom/quota/test/common/content.js index 229a272aa2bd..d3927d649a00 100644 --- a/dom/quota/test/common/content.js +++ b/dom/quota/test/common/content.js @@ -47,14 +47,16 @@ function getSimpleDatabase() { return connection; } -function requestFinished(request) { - return new Promise(function(resolve, reject) { - request.callback = SpecialPowers.wrapCallback(function(req) { - if (req.resultCode === SpecialPowers.Cr.NS_OK) { - resolve(req.result); - } else { - reject(req.resultCode); - } +async function requestFinished(request) { + await new Promise(function(resolve) { + request.callback = SpecialPowers.wrapCallback(function() { + resolve(); }); }); + + if (request.resultCode != SpecialPowers.Cr.NS_OK) { + throw new RequestError(request.resultCode, request.resultName); + } + + return request.result; } diff --git a/dom/quota/test/common/global.js b/dom/quota/test/common/global.js index 35bae1a1f3c9..ad73ce1d8bd1 100644 --- a/dom/quota/test/common/global.js +++ b/dom/quota/test/common/global.js @@ -3,6 +3,15 @@ * http://creativecommons.org/publicdomain/zero/1.0/ */ +class RequestError extends Error { + constructor(resultCode, resultName) { + super(`Request failed (code: ${resultCode}, name: ${resultName})`); + this.name = "RequestError"; + this.resultCode = resultCode; + this.resultName = resultName; + } +} + function openDBRequestUpgradeNeeded(request) { return new Promise(function(resolve, reject) { request.onerror = function(event) { diff --git a/dom/quota/test/common/system.js b/dom/quota/test/common/system.js index 801ea81fdad5..36130725c56a 100644 --- a/dom/quota/test/common/system.js +++ b/dom/quota/test/common/system.js @@ -57,14 +57,16 @@ function getSimpleDatabase(principal, persistence) { return connection; } -function requestFinished(request) { - return new Promise(function(resolve, reject) { - request.callback = function(req) { - if (req.resultCode == Cr.NS_OK) { - resolve(req.result); - } else { - reject(req.resultCode); - } +async function requestFinished(request) { + await new Promise(function(resolve) { + request.callback = function() { + resolve(); }; }); + + if (request.resultCode !== Cr.NS_OK) { + throw new RequestError(request.resultCode, request.resultName); + } + + return request.result; } diff --git a/dom/quota/test/xpcshell/test_persist_globalLimit.js b/dom/quota/test/xpcshell/test_persist_globalLimit.js index 56d875eba30a..8b15c26eaeb3 100644 --- a/dom/quota/test/xpcshell/test_persist_globalLimit.js +++ b/dom/quota/test/xpcshell/test_persist_globalLimit.js @@ -56,9 +56,12 @@ async function testSteps() { await requestFinished(request); ok(false, "Should have thrown"); - } catch (ex) { + } catch (e) { ok(true, "Should have thrown"); - ok(ex === NS_ERROR_FILE_NO_DEVICE_SPACE, "Threw right code"); + ok( + e.resultCode === NS_ERROR_FILE_NO_DEVICE_SPACE, + "Threw right result code" + ); } info("Closing the database and clearing"); diff --git a/dom/quota/test/xpcshell/test_persist_groupLimit.js b/dom/quota/test/xpcshell/test_persist_groupLimit.js index d0cc86e04574..202726299f04 100644 --- a/dom/quota/test/xpcshell/test_persist_groupLimit.js +++ b/dom/quota/test/xpcshell/test_persist_groupLimit.js @@ -67,9 +67,12 @@ async function testSteps() { request = databases[index].write(new ArrayBuffer(1)); await requestFinished(request); ok(false, "Should have thrown"); - } catch (ex) { + } catch (e) { ok(true, "Should have thrown"); - ok(ex == NS_ERROR_FILE_NO_DEVICE_SPACE, "Threw right code"); + ok( + e.resultCode == NS_ERROR_FILE_NO_DEVICE_SPACE, + "Threw right result code" + ); } } diff --git a/dom/quota/test/xpcshell/test_storagePressure.js b/dom/quota/test/xpcshell/test_storagePressure.js index 9a1f1c9706b3..2e95b2d749f5 100644 --- a/dom/quota/test/xpcshell/test_storagePressure.js +++ b/dom/quota/test/xpcshell/test_storagePressure.js @@ -84,9 +84,12 @@ async function testSteps() { await requestFinished(request); ok(false, "Should have thrown"); - } catch (ex) { + } catch (e) { ok(true, "Should have thrown"); - ok(ex === NS_ERROR_FILE_NO_DEVICE_SPACE, "Threw right code"); + ok( + e.resultCode === NS_ERROR_FILE_NO_DEVICE_SPACE, + "Threw right result code" + ); } info("Checking the storage pressure event");