Bug 1311739 - `PushDB.update` should reject instead of returning `undefined` for invalid records. r?dragana
MozReview-Commit-ID: HCLOSz4FHWO
--- a/dom/push/PushDB.jsm
+++ b/dom/push/PushDB.jsm
@@ -371,41 +371,39 @@ this.PushDB.prototype = {
return this._getAllByPushQuota(IDBKeyRange.only(0));
},
/**
* Updates an existing push registration.
*
* @param {String} aKeyID The registration ID.
* @param {Function} aUpdateFunc A function that receives the existing
- * registration record as its argument, and returns a new record. If the
- * function returns `null` or `undefined`, the record will not be updated.
- * If the record does not exist, the function will not be called.
- * @returns {Promise} A promise resolved with either the updated record, or
- * `undefined` if the record was not updated.
+ * registration record as its argument, and returns a new record.
+ * @returns {Promise} A promise resolved with either the updated record.
+ * Rejects if the record does not exist, or the function returns an invalid
+ * record.
*/
update: function(aKeyID, aUpdateFunc) {
return new Promise((resolve, reject) =>
this.newTxn(
"readwrite",
this._dbStoreName,
(aTxn, aStore) => {
aStore.get(aKeyID).onsuccess = aEvent => {
aTxn.result = undefined;
let record = aEvent.target.result;
if (!record) {
- console.error("update: Record does not exist", aKeyID);
- return;
+ throw new Error("Record " + aKeyID + " does not exist");
}
let newRecord = aUpdateFunc(this.toPushRecord(record));
if (!this.isValidRecord(newRecord)) {
console.error("update: Ignoring invalid update",
aKeyID, newRecord);
- return;
+ throw new Error("Invalid update for record " + aKeyID);
}
function putRecord() {
let req = aStore.put(newRecord);
req.onsuccess = aEvent => {
console.debug("update: Update successful", aKeyID, newRecord);
aTxn.result = newRecord;
};
}
--- a/dom/push/PushService.jsm
+++ b/dom/push/PushService.jsm
@@ -824,19 +824,16 @@ this.PushService = {
if (newRecord.isExpired()) {
return null;
}
newRecord.receivedPush(lastVisit);
return newRecord;
});
});
}).then(record => {
- if (!record) {
- throw new Error("Ignoring update for key ID " + keyID);
- }
gPushNotifier.notifySubscriptionModified(record.scope,
record.principal);
return record;
});
},
/**
* Decrypts an incoming message and notifies the associated service worker.
@@ -875,28 +872,26 @@ this.PushService = {
}
// If there are visible notifications, don't apply the quota penalty
// for the message.
if (record.uri && !this._visibleNotifications.has(record.uri.prePath)) {
record.reduceQuota();
}
return record;
}).then(record => {
- if (record) {
- if (record.isExpired()) {
- this._recordDidNotNotify(kDROP_NOTIFICATION_REASON_EXPIRED);
- // Drop the registration in the background. If the user returns to the
- // site, the service worker will be notified on the next `idle-daily`
- // event.
- this._backgroundUnregister(record,
- Ci.nsIPushErrorReporter.UNSUBSCRIBE_QUOTA_EXCEEDED);
- } else {
- gPushNotifier.notifySubscriptionModified(record.scope,
- record.principal);
- }
+ if (record.isExpired()) {
+ this._recordDidNotNotify(kDROP_NOTIFICATION_REASON_EXPIRED);
+ // Drop the registration in the background. If the user returns to the
+ // site, the service worker will be notified on the next `idle-daily`
+ // event.
+ this._backgroundUnregister(record,
+ Ci.nsIPushErrorReporter.UNSUBSCRIBE_QUOTA_EXCEEDED);
+ } else {
+ gPushNotifier.notifySubscriptionModified(record.scope,
+ record.principal);
}
if (this._updateQuotaTestCallback) {
// Callback so that test may be notified when the quota update is complete.
this._updateQuotaTestCallback();
}
}).catch(error => {
console.debug("updateQuota: Error while trying to update quota", error);
});
--- a/dom/push/test/xpcshell/test_registration_success_http2.js
+++ b/dom/push/test/xpcshell/test_registration_success_http2.js
@@ -1,23 +1,14 @@
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
'use strict';
Cu.import("resource://gre/modules/Services.jsm");
-Cu.import("resource://testing-common/PromiseTestUtils.jsm");
-
-///////////////////
-//
-// Whitelisting this test.
-// As part of bug 1077403, the leaking uncaught rejection should be fixed.
-//
-// Instances of the rejection "record is undefined" may or may not appear.
-PromiseTestUtils.thisTestLeaksUncaughtRejectionsAndShouldBeFixed();
const {PushDB, PushService, PushServiceHttp2} = serviceExports;
var prefs;
var serverPort = -1;
function run_test() {
--- a/dom/push/test/xpcshell/test_unregister_success_http2.js
+++ b/dom/push/test/xpcshell/test_unregister_success_http2.js
@@ -1,23 +1,14 @@
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
'use strict';
Cu.import("resource://gre/modules/Services.jsm");
-Cu.import("resource://testing-common/PromiseTestUtils.jsm");
-
-///////////////////
-//
-// Whitelisting this test.
-// As part of bug 1077403, the leaking uncaught rejection should be fixed.
-//
-// Instances of the rejection "record is undefined" may or may not appear.
-PromiseTestUtils.thisTestLeaksUncaughtRejectionsAndShouldBeFixed();
const {PushDB, PushService, PushServiceHttp2} = serviceExports;
var prefs;
var tlsProfile;
var pushEnabled;
var pushConnectionEnabled;