Bug 1691885 - Make CalDAV calendar notifications fire in the right sequence. r=mkmelin
This patch aims to make the network-based calendar providers consistently send notifications: all onAddItem and onModifyItem notifications should occur after onStartBatch and before onEndBatch, and onLoad must occur afterwards. ICS calendars with offline storage still don't behave correctly, but it isn't worse than it was, and I want to get more testing in place before making more invasive modifications to it. Differential Revision: https://phabricator.services.mozilla.com/D104651 --HG-- extra : moz-landing-system : lando
This commit is contained in:
Geoff Lankow
Родитель
24dc1d7a84
Коммит
71b3d5f7bf
|
|
@ -744,6 +744,7 @@ calCachedCalendar.prototype = {
|
|||
onGetResult(calendar, status, itemType, detail, items) {
|
||||
cal.ASSERT(false, "unexpected!");
|
||||
},
|
||||
// Returned Promise only available through wrappedJSObject.
|
||||
onOperationComplete(calendar, status, opType, id, detail) {
|
||||
if (isUnavailableCode(status)) {
|
||||
// The item couldn't be added to the (remote) location,
|
||||
|
|
@ -755,12 +756,24 @@ calCachedCalendar.prototype = {
|
|||
self.adoptOfflineItem(item, listener);
|
||||
} else if (Components.isSuccessCode(status)) {
|
||||
// On success, add the item to the cache.
|
||||
self.mCachedCalendar.addItem(detail, listener);
|
||||
return new Promise(resolve => {
|
||||
self.mCachedCalendar.addItem(detail, {
|
||||
onGetResult(...args) {
|
||||
cal.ASSERT(false, "unexpected!");
|
||||
},
|
||||
onOperationComplete(...args) {
|
||||
listener?.onOperationComplete(...args);
|
||||
resolve();
|
||||
},
|
||||
});
|
||||
});
|
||||
} else if (listener) {
|
||||
// Either an error occurred or this is a successful add
|
||||
// to a cached calendar. Forward the call to the listener
|
||||
listener.onOperationComplete(self, status, opType, id, detail);
|
||||
}
|
||||
|
||||
return Promise.resolve();
|
||||
},
|
||||
};
|
||||
|
||||
|
|
@ -823,6 +836,7 @@ calCachedCalendar.prototype = {
|
|||
* has performed the modification, see below: */
|
||||
let cacheListener = {
|
||||
onGetResult(calendar, status, itemType, detail, items) {},
|
||||
// Returned Promise only available through wrappedJSObject.
|
||||
onOperationComplete(calendar, status, opType, id, detail) {
|
||||
if (isUnavailableCode(status)) {
|
||||
// The item couldn't be modified at the (remote) location,
|
||||
|
|
@ -836,11 +850,21 @@ calCachedCalendar.prototype = {
|
|||
self.modifyOfflineItem(newItem, oldItem, listener);
|
||||
} else if (Components.isSuccessCode(status)) {
|
||||
// On success, modify the item in the cache
|
||||
self.mCachedCalendar.modifyItem(detail, oldItem, listener);
|
||||
return new Promise(resolve => {
|
||||
self.mCachedCalendar.modifyItem(detail, oldItem, {
|
||||
onGetResult(...args) {},
|
||||
onOperationComplete(...args) {
|
||||
listener?.onOperationComplete(...args);
|
||||
resolve();
|
||||
},
|
||||
});
|
||||
});
|
||||
} else if (listener) {
|
||||
// This happens on error, forward the error through the listener
|
||||
listener.onOperationComplete(self, status, opType, id, detail);
|
||||
}
|
||||
|
||||
return Promise.resolve();
|
||||
},
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -895,7 +895,7 @@ CalDavCalendar.prototype = {
|
|||
* @param aUri Base URI of the request
|
||||
* @param aListener Listener
|
||||
*/
|
||||
addTargetCalendarItem(path, calData, aUri, etag, aListener) {
|
||||
async addTargetCalendarItem(path, calData, aUri, etag, aListener) {
|
||||
let parser = Cc["@mozilla.org/calendar/ics-parser;1"].createInstance(Ci.calIIcsParser);
|
||||
// aUri.pathQueryRef may contain double slashes whereas path does not
|
||||
// this confuses our counting, so remove multiple successive slashes
|
||||
|
|
@ -965,7 +965,6 @@ CalDavCalendar.prototype = {
|
|||
this.mHrefIndex[path] = item.id;
|
||||
this.mItemInfoCache[item.id].etag = etag;
|
||||
|
||||
let needsAddModify = false;
|
||||
if (this.isCached) {
|
||||
this.setMetaData(item.id, path, etag, isInboxItem);
|
||||
|
||||
|
|
@ -974,29 +973,56 @@ CalDavCalendar.prototype = {
|
|||
// XXX This is quite fragile, but saves us a double modify/add
|
||||
|
||||
if (aListener) {
|
||||
// In the cached case, notifying operation complete will add the item to the cache
|
||||
if (this.mItemInfoCache[item.id].isNew) {
|
||||
this.notifyOperationComplete(aListener, Cr.NS_OK, cIOL.ADD, item.id, item);
|
||||
} else {
|
||||
this.notifyOperationComplete(aListener, Cr.NS_OK, cIOL.MODIFY, item.id, item);
|
||||
}
|
||||
} else {
|
||||
// No listener, we'll have to add it ourselves
|
||||
needsAddModify = true;
|
||||
await new Promise(resolve => {
|
||||
let wrappedListener = {
|
||||
onGetResult(...args) {
|
||||
aListener.onGetResult(...args);
|
||||
},
|
||||
onOperationComplete(...args) {
|
||||
// We must use wrappedJSObject to receive a returned Promise.
|
||||
let promise = aListener.wrappedJSObject.onOperationComplete(...args);
|
||||
if (promise) {
|
||||
promise.then(resolve);
|
||||
} else {
|
||||
resolve();
|
||||
}
|
||||
},
|
||||
};
|
||||
|
||||
// In the cached case, notifying operation complete will add the item to the cache
|
||||
if (this.mItemInfoCache[item.id].isNew) {
|
||||
this.notifyOperationComplete(wrappedListener, Cr.NS_OK, cIOL.ADD, item.id, item);
|
||||
} else {
|
||||
this.notifyOperationComplete(wrappedListener, Cr.NS_OK, cIOL.MODIFY, item.id, item);
|
||||
}
|
||||
});
|
||||
return;
|
||||
}
|
||||
} else {
|
||||
// In the uncached case, we need to do so ourselves
|
||||
needsAddModify = true;
|
||||
}
|
||||
|
||||
// Now take care of the add/modify if needed.
|
||||
if (needsAddModify) {
|
||||
// Either there's no listener, or we're uncached.
|
||||
|
||||
await new Promise(resolve => {
|
||||
let listener = {
|
||||
onGetResult(...args) {
|
||||
if (aListener) {
|
||||
aListener.onGetResult(...args);
|
||||
}
|
||||
},
|
||||
onOperationComplete(...args) {
|
||||
if (aListener) {
|
||||
aListener.onOperationComplete(...args);
|
||||
}
|
||||
resolve();
|
||||
},
|
||||
};
|
||||
|
||||
if (this.mItemInfoCache[item.id].isNew) {
|
||||
this.mOfflineStorage.adoptItem(item, aListener);
|
||||
this.mOfflineStorage.adoptItem(item, listener);
|
||||
} else {
|
||||
this.mOfflineStorage.modifyItem(item, null, aListener);
|
||||
this.mOfflineStorage.modifyItem(item, null, listener);
|
||||
}
|
||||
}
|
||||
});
|
||||
},
|
||||
|
||||
/**
|
||||
|
|
@ -1045,6 +1071,8 @@ CalDavCalendar.prototype = {
|
|||
if (this.isCached) {
|
||||
if (aChangeLogListener) {
|
||||
aChangeLogListener.onResult({ status: Cr.NS_OK }, Cr.NS_OK);
|
||||
} else {
|
||||
this.mObservers.notify("onLoad", [this]);
|
||||
}
|
||||
} else {
|
||||
this.mObservers.notify("onLoad", [this]);
|
||||
|
|
|
|||
|
|
@ -4,7 +4,6 @@
|
|||
|
||||
var { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
|
||||
var { cal } = ChromeUtils.import("resource:///modules/calendar/calUtils.jsm");
|
||||
var { setTimeout } = ChromeUtils.import("resource://gre/modules/Timer.jsm");
|
||||
|
||||
var { CalDavLegacySAXRequest } = ChromeUtils.import("resource:///modules/caldav/CalDavRequest.jsm");
|
||||
|
||||
|
|
@ -46,7 +45,7 @@ class XMLResponseHandler {
|
|||
* Parse this._xmlString with DOMParser, then create a TreeWalker and start
|
||||
* walking the node tree.
|
||||
*/
|
||||
handleResponse() {
|
||||
async handleResponse() {
|
||||
let parser = new DOMParser();
|
||||
let doc;
|
||||
try {
|
||||
|
|
@ -58,8 +57,8 @@ class XMLResponseHandler {
|
|||
|
||||
let treeWalker = doc.createTreeWalker(doc.documentElement, NodeFilter.SHOW_ELEMENT);
|
||||
this.startDocument();
|
||||
this._walk(treeWalker);
|
||||
this.endDocument();
|
||||
await this._walk(treeWalker);
|
||||
await this.endDocument();
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -85,7 +84,7 @@ class XMLResponseHandler {
|
|||
/**
|
||||
* Walk the tree node by node, call startElement and endElement when appropriate.
|
||||
*/
|
||||
_walk(treeWalker) {
|
||||
async _walk(treeWalker) {
|
||||
let currentNode = treeWalker.currentNode;
|
||||
if (currentNode) {
|
||||
this.startElement("", currentNode.localName, currentNode.nodeName, "");
|
||||
|
|
@ -93,25 +92,25 @@ class XMLResponseHandler {
|
|||
// Traverse children first.
|
||||
let firstChild = treeWalker.firstChild();
|
||||
if (firstChild) {
|
||||
this._walk(treeWalker);
|
||||
await this._walk(treeWalker);
|
||||
// TreeWalker has reached a leaf node, reset the cursor to continue the traversal.
|
||||
treeWalker.currentNode = firstChild;
|
||||
} else {
|
||||
this.characters(currentNode.textContent);
|
||||
this.endElement("", currentNode.localName, currentNode.nodeName);
|
||||
await this.endElement("", currentNode.localName, currentNode.nodeName);
|
||||
return;
|
||||
}
|
||||
|
||||
// Traverse siblings next.
|
||||
let nextSibling = treeWalker.nextSibling();
|
||||
while (nextSibling) {
|
||||
this._walk(treeWalker);
|
||||
await this._walk(treeWalker);
|
||||
// TreeWalker has reached a leaf node, reset the cursor to continue the traversal.
|
||||
treeWalker.currentNode = nextSibling;
|
||||
nextSibling = treeWalker.nextSibling();
|
||||
}
|
||||
|
||||
this.endElement("", currentNode.localName, currentNode.nodeName);
|
||||
await this.endElement("", currentNode.localName, currentNode.nodeName);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -180,7 +179,7 @@ class CalDavEtagsHandler extends XMLResponseHandler {
|
|||
if (this.calendar.verboseLogging()) {
|
||||
cal.LOG("CalDAV: recv: " + this.logXML);
|
||||
}
|
||||
this.handleResponse();
|
||||
await this.handleResponse();
|
||||
|
||||
// Now that we are done, check which items need fetching.
|
||||
this.calendar.superCalendar.startBatch();
|
||||
|
|
@ -484,12 +483,12 @@ class CalDavWebDavSyncHandler extends XMLResponseHandler {
|
|||
}
|
||||
}
|
||||
|
||||
onStopRequest(request, statusCode) {
|
||||
async onStopRequest(request, statusCode) {
|
||||
if (this.calendar.verboseLogging()) {
|
||||
cal.LOG("CalDAV: recv: " + this.logXML);
|
||||
}
|
||||
|
||||
this.handleResponse();
|
||||
await this.handleResponse();
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -516,7 +515,7 @@ class CalDavWebDavSyncHandler extends XMLResponseHandler {
|
|||
this.calendar.superCalendar.startBatch();
|
||||
}
|
||||
|
||||
endDocument() {
|
||||
async endDocument() {
|
||||
if (this.unhandledErrors) {
|
||||
this.calendar.superCalendar.endBatch();
|
||||
this.calendar.reportDavError(Ci.calIErrors.DAV_REPORT_ERROR);
|
||||
|
|
@ -532,7 +531,7 @@ class CalDavWebDavSyncHandler extends XMLResponseHandler {
|
|||
// sync
|
||||
for (let path in this.calendar.mHrefIndex) {
|
||||
if (!this.itemsReported[path]) {
|
||||
this.calendar.deleteTargetCalendarItem(path);
|
||||
await this.calendar.deleteTargetCalendarItem(path);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -590,7 +589,7 @@ class CalDavWebDavSyncHandler extends XMLResponseHandler {
|
|||
}
|
||||
}
|
||||
|
||||
endElement(aUri, aLocalName, aQName) {
|
||||
async endElement(aUri, aLocalName, aQName) {
|
||||
switch (aLocalName) {
|
||||
case "response": // WebDAV Sync draft 3
|
||||
case "sync-response": {
|
||||
|
|
@ -622,7 +621,7 @@ class CalDavWebDavSyncHandler extends XMLResponseHandler {
|
|||
) {
|
||||
if (this.calendar.mHrefIndex[resp.href]) {
|
||||
this.changeCount++;
|
||||
this.calendar.deleteTargetCalendarItem(resp.href);
|
||||
await this.calendar.deleteTargetCalendarItem(resp.href);
|
||||
} else {
|
||||
cal.LOG("CalDAV: skipping unfound deleted item : " + resp.href);
|
||||
}
|
||||
|
|
@ -849,7 +848,7 @@ class CalDavMultigetSyncHandler extends XMLResponseHandler {
|
|||
}
|
||||
}
|
||||
|
||||
onStopRequest(request, statusCode) {
|
||||
async onStopRequest(request, statusCode) {
|
||||
if (this.calendar.verboseLogging()) {
|
||||
cal.LOG("CalDAV: recv: " + this.logXML);
|
||||
}
|
||||
|
|
@ -864,21 +863,8 @@ class CalDavMultigetSyncHandler extends XMLResponseHandler {
|
|||
this.calendar.saveCalendarProperties();
|
||||
cal.LOG("CalDAV: New webdav-sync Token: " + this.calendar.mWebdavSyncToken);
|
||||
}
|
||||
|
||||
if (this.additionalSyncNeeded) {
|
||||
setTimeout(() => {
|
||||
let wds = new CalDavWebDavSyncHandler(
|
||||
this.calendar,
|
||||
this.baseUri,
|
||||
this.changeLogListener
|
||||
);
|
||||
wds.doWebDAVSync();
|
||||
}, 0);
|
||||
} else {
|
||||
this.calendar.finalizeUpdatedItems(this.changeLogListener, this.baseUri);
|
||||
}
|
||||
}
|
||||
this.handleResponse();
|
||||
await this.handleResponse();
|
||||
if (this.itemsNeedFetching.length > 0) {
|
||||
cal.LOG("CalDAV: Still need to fetch " + this.itemsNeedFetching.length + " elements.");
|
||||
this.resetXMLResponseHandler();
|
||||
|
|
@ -891,6 +877,11 @@ class CalDavMultigetSyncHandler extends XMLResponseHandler {
|
|||
};
|
||||
this.timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
|
||||
this.timer.initWithCallback(timerCallback, 0, Ci.nsITimer.TYPE_ONE_SHOT);
|
||||
} else if (this.additionalSyncNeeded) {
|
||||
let wds = new CalDavWebDavSyncHandler(this.calendar, this.baseUri, this.changeLogListener);
|
||||
wds.doWebDAVSync();
|
||||
} else {
|
||||
this.calendar.finalizeUpdatedItems(this.changeLogListener, this.baseUri);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -955,7 +946,7 @@ class CalDavMultigetSyncHandler extends XMLResponseHandler {
|
|||
}
|
||||
}
|
||||
|
||||
endElement(aUri, aLocalName, aQName) {
|
||||
async endElement(aUri, aLocalName, aQName) {
|
||||
switch (aLocalName) {
|
||||
case "response": {
|
||||
let resp = this.currentResponse;
|
||||
|
|
@ -970,7 +961,7 @@ class CalDavMultigetSyncHandler extends XMLResponseHandler {
|
|||
resp.status.indexOf(" 404") > 0
|
||||
) {
|
||||
if (this.calendar.mHrefIndex[resp.href]) {
|
||||
this.calendar.deleteTargetCalendarItem(resp.href);
|
||||
await this.calendar.deleteTargetCalendarItem(resp.href);
|
||||
} else {
|
||||
cal.LOG("CalDAV: skipping unfound deleted item : " + resp.href);
|
||||
}
|
||||
|
|
@ -991,7 +982,7 @@ class CalDavMultigetSyncHandler extends XMLResponseHandler {
|
|||
oldEtag = null;
|
||||
}
|
||||
if (!oldEtag || oldEtag != resp.getetag) {
|
||||
this.calendar.addTargetCalendarItem(
|
||||
await this.calendar.addTargetCalendarItem(
|
||||
resp.href,
|
||||
resp.calendardata,
|
||||
this.baseUri,
|
||||
|
|
|
|||
|
|
@ -38,7 +38,7 @@ calCompositeCalendarObserverHelper.prototype = {
|
|||
// any refreshed dependent calendar logically refreshes
|
||||
// this composite calendar, thus we send out an onLoad
|
||||
// for this composite calendar:
|
||||
this.compCalendar.mObservers.notify("onLoad", [this.compCalendar]);
|
||||
this.compCalendar.mObservers.notify("onLoad", [calendar]);
|
||||
}
|
||||
},
|
||||
|
||||
|
|
|
|||
|
|
@ -261,6 +261,7 @@ CalICSCalendar.prototype = {
|
|||
this.createMemoryCalendar();
|
||||
|
||||
this.mObserver.onStartBatch();
|
||||
this.mMemoryCalendar.addObserver(this.mObserver);
|
||||
|
||||
// Wrap parsing in a try block. Will ignore errors. That's a good thing
|
||||
// for non-existing or empty files, but not good for invalid files.
|
||||
|
|
@ -289,7 +290,6 @@ CalICSCalendar.prototype = {
|
|||
// we should add ourselves as observer. It is important that this
|
||||
// happens *after* the calls to adoptItem in the above loop to prevent
|
||||
// the views from being notified.
|
||||
self.mMemoryCalendar.addObserver(self.mObserver);
|
||||
self.unlock();
|
||||
},
|
||||
};
|
||||
|
|
@ -462,6 +462,7 @@ CalICSCalendar.prototype = {
|
|||
|
||||
this.mHooks.onAfterPut(request, () => {
|
||||
this.unlock();
|
||||
this.mObserver.onLoad(this);
|
||||
Services.startup.exitLastWindowClosingSurvivalArea();
|
||||
});
|
||||
},
|
||||
|
|
@ -476,14 +477,17 @@ CalICSCalendar.prototype = {
|
|||
if (this.readOnly) {
|
||||
throw calIErrors.CAL_IS_READONLY;
|
||||
}
|
||||
this.startBatch();
|
||||
this.queue.push({ action: "add", item: aItem, listener: aListener });
|
||||
this.processQueue();
|
||||
this.endBatch();
|
||||
},
|
||||
|
||||
modifyItem(aNewItem, aOldItem, aListener) {
|
||||
if (this.readOnly) {
|
||||
throw calIErrors.CAL_IS_READONLY;
|
||||
}
|
||||
this.startBatch();
|
||||
this.queue.push({
|
||||
action: "modify",
|
||||
oldItem: aOldItem,
|
||||
|
|
@ -491,6 +495,7 @@ CalICSCalendar.prototype = {
|
|||
listener: aListener,
|
||||
});
|
||||
this.processQueue();
|
||||
this.endBatch();
|
||||
},
|
||||
|
||||
deleteItem(aItem, aListener) {
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче