Add Notifier.queue()

Notifier.trigger() needs to be async, since if it actually runs it waits for
promises returned from observers. But the vast majority of trigger() calls are
in transactions where they just queue and can therefore be synchronous. This
replaces all such calls with Notifier.queue().

This should fix a race condition that was causing the emptyTrash() test to fail
intermittently.

chrome/content/zotero/fileInterface.js

@@ -341,6 +341,7 @@ var Zotero_File_Interface = new function() {
 				Zotero.UnresponsiveScriptIndicator.enable();
 				
 				if(importCollection) {
+					// TODO: yield or change to .queue()
 					Zotero.Notifier.trigger('refresh', 'collection', importCollection.id);
 				}
 				if (!worked) {

chrome/content/zotero/xpcom/data/collection.js

@@ -292,7 +292,7 @@ Zotero.Collection.prototype._saveData = Zotero.Promise.coroutine(function* (env)
 			));
 		}
 		if (!isNew) {
-			Zotero.Notifier.trigger('move', 'collection', this.id);
+			Zotero.Notifier.queue('move', 'collection', this.id);
 		}
 		env.parentIDs = parentIDs;
 	}
@@ -307,10 +307,10 @@ Zotero.Collection.prototype._finalizeSave = Zotero.Promise.coroutine(function* (
 	
 	if (!env.options.skipNotifier) {
 		if (env.isNew) {
-			Zotero.Notifier.trigger('add', 'collection', this.id, env.notifierData);
+			Zotero.Notifier.queue('add', 'collection', this.id, env.notifierData);
 		}
 		else  {
-			Zotero.Notifier.trigger('modify', 'collection', this.id, env.notifierData);
+			Zotero.Notifier.queue('modify', 'collection', this.id, env.notifierData);
 		}
 	}
 	
@@ -611,7 +611,7 @@ Zotero.Collection.prototype._eraseData = Zotero.Promise.coroutine(function* (env
 	//return Zotero.Collections.reloadAll();
 	
 	if (!env.skipNotifier) {
-		Zotero.Notifier.trigger('delete', 'collection', collections, notifierData);
+		Zotero.Notifier.queue('delete', 'collection', collections, notifierData);
 	}
 });
 

chrome/content/zotero/xpcom/data/group.js

@@ -245,10 +245,10 @@ Zotero.Group.prototype.save = Zotero.Promise.coroutine(function* () {
 				yield this.load();
 				Zotero.Groups.register(this)
 			}.bind(this)));
-			Zotero.Notifier.trigger('add', 'group', this.id);
+			Zotero.Notifier.queue('add', 'group', this.id);
 		}
 		else {
-			Zotero.Notifier.trigger('modify', 'group', this.id);
+			Zotero.Notifier.queue('modify', 'group', this.id);
 		}
 	}.bind(this));
 });
@@ -293,7 +293,7 @@ Zotero.Group.prototype.erase = Zotero.Promise.coroutine(function* () {
 		yield Zotero.DB.queryAsync(sql, this.libraryID)
 		
 		Zotero.Groups.unregister(this.id);
-		Zotero.Notifier.trigger('delete', 'group', this.id, notifierData);
+		Zotero.Notifier.queue('delete', 'group', this.id, notifierData);
 	}.bind(this));
 	
 	yield Zotero.purgeDataObjects();

chrome/content/zotero/xpcom/data/item.js

@@ -1231,7 +1231,7 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 		}
 		
 		if (!env.options.skipNotifier) {
-			Zotero.Notifier.trigger('add', 'item', itemID, env.notifierData);
+			Zotero.Notifier.queue('add', 'item', itemID, env.notifierData);
 		}
 	}
 	else {
@@ -1240,7 +1240,7 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 		yield Zotero.DB.queryAsync(sql, env.sqlValues);
 		
 		if (!env.options.skipNotifier) {
-			Zotero.Notifier.trigger('modify', 'item', itemID, env.notifierData);
+			Zotero.Notifier.queue('modify', 'item', itemID, env.notifierData);
 		}
 	}
 	
@@ -1349,7 +1349,7 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 			
 			let newParentItemNotifierData = {};
 			//newParentItemNotifierData[newParentItem.id] = {};
-			Zotero.Notifier.trigger('modify', 'item', parentItemID, newParentItemNotifierData);
+			Zotero.Notifier.queue('modify', 'item', parentItemID, newParentItemNotifierData);
 			
 			switch (Zotero.ItemTypes.getName(itemTypeID)) {
 				case 'note':
@@ -1373,7 +1373,7 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 				
 				let newParentItemNotifierData = {};
 				//newParentItemNotifierData[newParentItem.id] = {};
-				Zotero.Notifier.trigger('modify', 'item', parentItemID, newParentItemNotifierData);
+				Zotero.Notifier.queue('modify', 'item', parentItemID, newParentItemNotifierData);
 			}
 			
 			let oldParentKey = this._previousData.parentKey;
@@ -1383,7 +1383,7 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 				if (oldParentItemID) {
 					let oldParentItemNotifierData = {};
 					//oldParentItemNotifierData[oldParentItemID] = {};
-					Zotero.Notifier.trigger('modify', 'item', oldParentItemID, oldParentItemNotifierData);
+					Zotero.Notifier.queue('modify', 'item', oldParentItemID, oldParentItemNotifierData);
 				}
 				else {
 					Zotero.debug("Old source item " + oldParentKey
@@ -1406,7 +1406,7 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 						yield this.loadCollections();
 						this.removeFromCollection(changedCollections[i]);
 						
-						Zotero.Notifier.trigger(
+						Zotero.Notifier.queue(
 							'remove',
 							'collection-item',
 							changedCollections[i] + '-' + this.id
@@ -1472,9 +1472,9 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 		yield Zotero.DB.queryAsync(sql, itemID);
 		
 		// Refresh trash
-		Zotero.Notifier.trigger('refresh', 'trash', this.libraryID);
+		Zotero.Notifier.queue('refresh', 'trash', this.libraryID);
 		if (this._deleted) {
-			Zotero.Notifier.trigger('trash', 'item', this.id);
+			Zotero.Notifier.queue('trash', 'item', this.id);
 		}
 		
 		if (parentItemID) {
@@ -1594,7 +1594,7 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 			// "OR REPLACE" allows changing type
 			let sql = "INSERT OR REPLACE INTO itemTags (itemID, tagID, type) VALUES (?, ?, ?)";
 			yield Zotero.DB.queryAsync(sql, [this.id, tagID, tag.type ? tag.type : 0]);
-			Zotero.Notifier.trigger('add', 'item-tag', this.id + '-' + tag.tag);
+			Zotero.Notifier.queue('add', 'item-tag', this.id + '-' + tag.tag);
 		}
 		
 		if (toRemove.length) {
@@ -1604,7 +1604,7 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 				let tagID = Zotero.Tags.getID(this.libraryID, tag.tag);
 				let sql = "DELETE FROM itemTags WHERE itemID=? AND tagID=? AND type=?";
 				yield Zotero.DB.queryAsync(sql, [this.id, tagID, tag.type ? tag.type : 0]);
-				Zotero.Notifier.trigger('remove', 'item-tag', this.id + '-' + tag.tag);
+				Zotero.Notifier.queue('remove', 'item-tag', this.id + '-' + tag.tag);
 			}
 			Zotero.Prefs.set('purge.tags', true);
 		}
@@ -1634,7 +1634,7 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 			yield Zotero.DB.queryAsync(sql, [collectionID, this.id, orderIndex]);
 			
 			yield this.ContainerObjectsClass.refreshChildItems(collectionID);
-			Zotero.Notifier.trigger('add', 'collection-item', collectionID + '-' + this.id);
+			Zotero.Notifier.queue('add', 'collection-item', collectionID + '-' + this.id);
 		}
 		
 		if (toRemove.length) {
@@ -1646,7 +1646,7 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 			for (let i=0; i<toRemove.length; i++) {
 				let collectionID = toRemove[i];
 				yield this.ContainerObjectsClass.refreshChildItems(collectionID);
-				Zotero.Notifier.trigger('remove', 'collection-item', collectionID + '-' + this.id);
+				Zotero.Notifier.queue('remove', 'collection-item', collectionID + '-' + this.id);
 			}
 		}
 	}
@@ -1714,7 +1714,7 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 			}
 		}
 		
-		Zotero.Notifier.trigger('modify', 'item', removed.concat(newids));
+		Zotero.Notifier.queue('modify', 'item', removed.concat(newids));
 	}
 	
 	// Update child item counts and contents
@@ -3977,7 +3977,7 @@ Zotero.Item.prototype._erasePreCommit = Zotero.Promise.coroutine(function* (env)
 	this.ObjectsClass.unload(this.id);
 	
 	if (!env.skipNotifier) {
-		Zotero.Notifier.trigger('delete', 'item', this.id, env.deletedItemNotifierData);
+		Zotero.Notifier.queue('delete', 'item', this.id, env.deletedItemNotifierData);
 	}
 	
 	Zotero.Prefs.set('purge.items', true);

chrome/content/zotero/xpcom/data/items.js

@@ -485,7 +485,7 @@ Zotero.Items = function() {
 				let item = yield this.getAsync(id);
 				if (!item) {
 					Zotero.debug('Item ' + id + ' does not exist in Items.trash()!', 1);
-					Zotero.Notifier.trigger('delete', 'item', id);
+					Zotero.Notifier.queue('delete', 'item', id);
 					continue;
 				}
 				item.deleted = true;
@@ -515,7 +515,7 @@ Zotero.Items = function() {
 		if (deletedIDs.length) {
 			yield Zotero.Utilities.Internal.forEachChunkAsync(deletedIDs, 50, function* (chunk) {
 				yield this.erase(chunk);
-				Zotero.Notifier.trigger('refresh', 'trash', libraryID);
+				yield Zotero.Notifier.trigger('refresh', 'trash', libraryID);
 			}.bind(this));
 		}
 		

chrome/content/zotero/xpcom/data/tags.js

@@ -274,7 +274,7 @@ Zotero.Tags = new function() {
 					tag: oldName
 				}
 			};
-			Zotero.Notifier.trigger(
+			Zotero.Notifier.queue(
 				'modify',
 				'item-tag',
 				oldItemIDs.map(function (itemID) itemID + '-' + newName),
@@ -425,7 +425,7 @@ Zotero.Tags = new function() {
 		sql = "DROP TABLE tagDelete";
 		yield Zotero.DB.queryAsync(sql);
 		
-		Zotero.Notifier.trigger('delete', 'tag', toDelete, notifierData);
+		Zotero.Notifier.queue('delete', 'tag', toDelete, notifierData);
 		
 		Zotero.Prefs.set('purge.tags', false);
 	});
@@ -610,7 +610,7 @@ Zotero.Tags = new function() {
 			};
 			
 			if (affectedItems.length) {
-				Zotero.Notifier.trigger('redraw', 'item', affectedItems, { column: 'title' });
+				yield Zotero.Notifier.trigger('redraw', 'item', affectedItems, { column: 'title' });
 			}
 		}
 	});

chrome/content/zotero/xpcom/fulltext.js

@@ -424,6 +424,8 @@ Zotero.Fulltext = new function(){
 			var sql = "REPLACE INTO fulltextContent (itemID, textContent) VALUES (?,?)";
 			Zotero.DB.query(sql, [itemID, {string:text}]);
 			*/
+			
+			Zotero.Notifier.queue('refresh', 'item', itemID);
 		}.bind(this));
 		
 		// If there's a processor cache file, delete it (whether or not we just used it)
@@ -432,8 +434,6 @@ Zotero.Fulltext = new function(){
 		if (cacheFile.exists()) {
 			cacheFile.remove(false);
 		}
-		
-		Zotero.Notifier.trigger('refresh', 'item', itemID);
 	}.bind(this));
 	
 	

chrome/content/zotero/xpcom/notifier.js

@@ -88,6 +88,7 @@ Zotero.Notifier = new function(){
 		delete _observers[id];
 	}
 	
+	
 	/**
 	* Trigger a notification to the appropriate observers
 	*
@@ -106,37 +107,78 @@ Zotero.Notifier = new function(){
 	* - New events and types should be added to the order arrays in commit()
 	**/
 	this.trigger = Zotero.Promise.coroutine(function* (event, type, ids, extraData, force) {
+		if (_inTransaction && !force) {
+			return this.queue(event, type, ids, extraData);
+		}
+		
 		if (_disabled){
 			Zotero.debug("Notifications are disabled");
 			return false;
 		}
 		
-		if (_types && _types.indexOf(type) == -1){
+		if (_types && _types.indexOf(type) == -1) {
-			throw ('Invalid type ' + type + ' in Notifier.trigger()');
+			throw new Error("Invalid type '" + type + "'");
 		}
 		
 		ids = Zotero.flattenArguments(ids);
 		
-		var queue = _inTransaction && !force;
+		if (Zotero.Debug.enabled) {
+			_logTrigger(event, type, ids, extraData);
+		}
+		
+		var order = _getObserverOrder(type);
+		for (let id of order) {
+			Zotero.debug("Calling notify() with " + event + "/" + type
+				+ " on observer with id '" + id + "'", 4);
+			
+			if (!_observers[id]) {
+				Zotero.debug("Observer no longer exists");
+				continue;
+			}
+			
+			// Catch exceptions so all observers get notified even if
+			// one throws an error
+			try {
+				yield Zotero.Promise.resolve(_observers[id].ref.notify(event, type, ids, extraData));
+			}
+			catch (e) {
+				Zotero.debug(e);
+				Components.utils.reportError(e);
+			}
+		}
+		
+		return true;
+	});
+	
+	
+	/**
+	 * Queue an event until the end of the current notifier transaction
+	 *
+	 * Takes the same parameters as trigger()
+	 *
+	 * @throws If a notifier transaction isn't currently open
+	 */
+	this.queue = function (event, type, ids, extraData) {
+		if (_disabled){
+			Zotero.debug("Notifications are disabled");
+			return false;
+		}
+		
+		if (_types && _types.indexOf(type) == -1) {
+			throw new Error("Invalid type '" + type + "'");
+		}
+		
+		ids = Zotero.flattenArguments(ids);
 		
 		if (Zotero.Debug.enabled) {
-			Zotero.debug("Notifier.trigger("
+			_logTrigger(event, type, ids, extraData, true);
-				+ "'" + event + "', "
-				+ "'" + type + "', "
-				+ "[" + ids.join() + "]"
-				+ (extraData ? ", " + extraData : "")
-				+ ")"
-				+ (queue
-					? " queued"
-					: " called " + "[observers: " + _countObserversForType(type) + "]")
-			);
-			if (extraData) {
-				Zotero.debug(extraData);
 		}
+		
+		if (!_inTransaction) {
+			throw new Error("Can't queue event outside of a transaction");
 		}
 		
 		// Merge with existing queue
-		if (queue) {
 		if (!_queue[type]) {
 			_queue[type] = [];
 		}
@@ -173,30 +215,24 @@ Zotero.Notifier = new function(){
 		return true;
 	}
 	
-		var order = _getObserverOrder(type);
-		for (let id of order) {
-			Zotero.debug("Calling notify() with " + event + "/" + type
-				+ " on observer with id '" + id + "'", 4);
 	
-			if (!_observers[id]) {
+	function _logTrigger(event, type, ids, extraData, queueing) {
-				Zotero.debug("Observer no longer exists");
+		Zotero.debug("Notifier.trigger("
-				continue;
+			+ "'" + event + "', "
-			}
+			+ "'" + type + "', "
-			
+			+ "[" + ids.join() + "]"
-			// Catch exceptions so all observers get notified even if
+			+ (extraData ? ", " + extraData : "")
-			// one throws an error
+			+ ")"
-			try {
+			+ (queueing
-				yield Zotero.Promise.resolve(_observers[id].ref.notify(event, type, ids, extraData));
+				? " queued "
-			}
+				: " called "
-			catch (e) {
+			+ "[observers: " + _countObserversForType(type) + "]")
-				Zotero.debug(e);
+		);
-				Components.utils.reportError(e);
+		if (extraData) {
+			Zotero.debug(extraData);
 		}
 	}
 	
-		return true;
-	});
-	
 	
 	/**
 	 * Get order of observer by priority, with lower numbers having higher priority.

chrome/content/zotero/xpcom/search.js

@@ -172,10 +172,10 @@ Zotero.Search.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 
 Zotero.Search.prototype._finalizeSave = Zotero.Promise.coroutine(function* (env) {
 	if (env.isNew) {
-		Zotero.Notifier.trigger('add', 'search', this.id, env.notifierData);
+		Zotero.Notifier.queue('add', 'search', this.id, env.notifierData);
 	}
 	else if (!env.options.skipNotifier) {
-		Zotero.Notifier.trigger('modify', 'search', this.id, env.notifierData);
+		Zotero.Notifier.queue('modify', 'search', this.id, env.notifierData);
 	}
 	
 	if (env.isNew && Zotero.Libraries.isGroupLibrary(this.libraryID)) {
@@ -233,7 +233,7 @@ Zotero.Search.prototype._eraseData = Zotero.Promise.coroutine(function* (env) {
 	yield Zotero.DB.queryAsync(sql, this.id);
 	
 	if (!env.skipNotifier) {
-		Zotero.Notifier.trigger('delete', 'search', this.id, notifierData);
+		Zotero.Notifier.queue('delete', 'search', this.id, notifierData);
 	}
 });
 

chrome/content/zotero/xpcom/storage.js

@@ -1220,6 +1220,7 @@ Zotero.Sync.Storage = new function () {
 		var libraryID, key;
 		[libraryID, key] = libraryKey.split("/");
 		var item = Zotero.Items.getByLibraryAndKey(libraryID, key);
+		// TODO: yield or switch to queue
 		Zotero.Notifier.trigger('redraw', 'item', item.id, { column: "hasAttachment" });
 		
 		var parent = item.parentItemKey;

chrome/content/zotero/xpcom/syncedSettings.js

@@ -75,7 +75,7 @@ Zotero.SyncedSettings = (function () {
 				var sql = "DELETE FROM syncedSettings WHERE setting=? AND libraryID=?";
 				yield Zotero.DB.queryAsync(sql, [setting, libraryID]);
 				
-				Zotero.Notifier.trigger('delete', 'setting', [id], extraData);
+				yield Zotero.Notifier.trigger('delete', 'setting', [id], extraData);
 				return true;
 			}
 			
@@ -100,7 +100,7 @@ Zotero.SyncedSettings = (function () {
 					+ "(setting, libraryID, value, synced) VALUES (?, ?, ?, ?)";
 				yield Zotero.DB.queryAsync(sql, [setting, libraryID, JSON.stringify(value), synced]);
 			}
-			Zotero.Notifier.trigger(event, 'setting', [id], extraData);
+			yield Zotero.Notifier.trigger(event, 'setting', [id], extraData);
 			return true;
 		})
 	};