diff --git a/services/sync/modules/engines.js b/services/sync/modules/engines.js index cff46ffc5e58..2440f5987e01 100644 --- a/services/sync/modules/engines.js +++ b/services/sync/modules/engines.js @@ -49,7 +49,6 @@ const Cu = Components.utils; Cu.import("resource://services-sync/record.js"); Cu.import("resource://services-sync/constants.js"); Cu.import("resource://services-sync/ext/Observers.js"); -Cu.import("resource://services-sync/ext/Sync.js"); Cu.import("resource://services-sync/identity.js"); Cu.import("resource://services-sync/log4moz.js"); Cu.import("resource://services-sync/resource.js"); @@ -484,6 +483,10 @@ Engine.prototype = { function SyncEngine(name) { Engine.call(this, name || "SyncEngine"); this.loadToFetch(); + + Utils.lazy2(this, "_timer", function() { + return Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer); + }); } SyncEngine.prototype = { __proto__: Engine.prototype, @@ -575,6 +578,13 @@ SyncEngine.prototype = { return record; }, + _sleep: function _sleep(delay) { + let cb = Utils.makeSyncCallback(); + this._timer.initWithCallback({notify: cb}, delay, + Ci.nsITimer.TYPE_ONE_SHOT); + Utils.waitForSyncCallback(cb); + }, + // Any setup that needs to happen at the beginning of each sync. _syncStartup: function SyncEngine__syncStartup() { @@ -741,7 +751,7 @@ SyncEngine.prototype = { if (applyBatch.length == this.applyIncomingBatchSize) { doApplyBatch.call(this); } - Sync.sleep(0); + this._sleep(0); }); // Only bother getting data from the server if there's new things @@ -974,7 +984,7 @@ SyncEngine.prototype = { if ((++count % MAX_UPLOAD_RECORDS) == 0) doUpload((count - MAX_UPLOAD_RECORDS) + " - " + count + " out"); - Sync.sleep(0); + this._sleep(0); } // Final upload diff --git a/services/sync/modules/engines/bookmarks.js b/services/sync/modules/engines/bookmarks.js index 07ed922d8c4b..0b0f59338558 100644 --- a/services/sync/modules/engines/bookmarks.js +++ b/services/sync/modules/engines/bookmarks.js @@ -635,9 +635,43 @@ BookmarksStore.prototype = { return false; }, + // Turn a record's nsINavBookmarksService constant and other attributes into + // a granular type for comparison. + _recordType: function _recordType(itemId) { + let bms = this._bms; + let type = bms.getItemType(itemId); + + switch (type) { + case bms.TYPE_FOLDER: + if (this._ls.isLivemark(itemId)) + return "livemark"; + return "folder"; + + case bms.TYPE_BOOKMARK: + if (this._ms && this._ms.hasMicrosummary(itemId)) + return "microsummary"; + let bmkUri = bms.getBookmarkURI(itemId).spec; + if (bmkUri.search(/^place:/) == 0) + return "query"; + return "bookmark"; + + case bms.TYPE_SEPARATOR: + return "separator"; + + default: + return null; + } + }, + create: function BStore_create(record) { - // Default to unfiled if we don't have the parent yet - if (!record._parent) { + // Default to unfiled if we don't have the parent yet. + + // Valid parent IDs are all positive integers. Other values -- undefined, + // null, -1 -- all compare false for > 0, so this catches them all. We + // don't just use <= without the !, because undefined and null compare + // false for that, too! + if (!(record._parent > 0)) { + this._log.debug("Parent is " + record._parent + "; reparenting to unfiled."); record._parent = kSpecialIds.unfiled; } @@ -695,14 +729,27 @@ BookmarksStore.prototype = { break; case "livemark": let siteURI = null; + if (!record.feedUri) { + this._log.debug("No feed URI: skipping livemark record " + record.id); + return; + } + if (this._ls.isLivemark(record._parent)) { + this._log.debug("Invalid parent: skipping livemark record " + record.id); + return; + } + if (record.siteUri != null) siteURI = Utils.makeURI(record.siteUri); - newId = this._ls.createLivemark(record._parent, record.title, siteURI, - Utils.makeURI(record.feedUri), - Svc.Bookmark.DEFAULT_INDEX); - this._log.debug(["created livemark", newId, "under", record._parent, "as", - record.title, record.siteUri, record.feedUri].join(" ")); + // Use createLivemarkFolderOnly, not createLivemark, to avoid it + // automatically updating during a sync. + newId = this._ls.createLivemarkFolderOnly(record._parent, record.title, + siteURI, + Utils.makeURI(record.feedUri), + Svc.Bookmark.DEFAULT_INDEX); + this._log.debug("Created livemark " + newId + " under " + record._parent + + " as " + record.title + ", " + record.siteUri + ", " + + record.feedUri + ", GUID " + record.id); break; case "separator": newId = this._bms.insertSeparator(record._parent, @@ -722,26 +769,23 @@ BookmarksStore.prototype = { this._setGUID(newId, record.id); }, - remove: function BStore_remove(record) { - let itemId = this.idForGUID(record.id); - if (itemId <= 0) { - this._log.debug("Item " + record.id + " already removed"); - return; - } - var type = this._bms.getItemType(itemId); + // Factored out of `remove` to avoid redundant DB queries when the Places ID + // is already known. + removeById: function removeById(itemId, guid) { + let type = this._bms.getItemType(itemId); switch (type) { case this._bms.TYPE_BOOKMARK: - this._log.debug(" -> removing bookmark " + record.id); + this._log.debug(" -> removing bookmark " + guid); this._ts.untagURI(this._bms.getBookmarkURI(itemId), null); this._bms.removeItem(itemId); break; case this._bms.TYPE_FOLDER: - this._log.debug(" -> removing folder " + record.id); + this._log.debug(" -> removing folder " + guid); Svc.Bookmark.removeItem(itemId); break; case this._bms.TYPE_SEPARATOR: - this._log.debug(" -> removing separator " + record.id); + this._log.debug(" -> removing separator " + guid); this._bms.removeItem(itemId); break; default: @@ -750,6 +794,15 @@ BookmarksStore.prototype = { } }, + remove: function BStore_remove(record) { + let itemId = this.idForGUID(record.id); + if (itemId <= 0) { + this._log.debug("Item " + record.id + " already removed"); + return; + } + this.removeById(itemId, record.id); + }, + update: function BStore_update(record) { let itemId = this.idForGUID(record.id); @@ -758,6 +811,25 @@ BookmarksStore.prototype = { return; } + // Two items are the same type if they have the same ItemType in Places, + // and also share some key characteristics (e.g., both being livemarks). + // We figure this out by examining the item to find the equivalent granular + // (string) type. + // If they're not the same type, we can't just update attributes. Delete + // then recreate the record instead. + let localItemType = this._recordType(itemId); + let remoteRecordType = record.type; + this._log.trace("Local type: " + localItemType + ". " + + "Remote type: " + remoteRecordType + "."); + + if (localItemType != remoteRecordType) { + this._log.debug("Local record and remote record differ in type. " + + "Deleting and recreating."); + this.removeById(itemId, record.id); + this.create(record); + return; + } + this._log.trace("Updating " + record.id + " (" + itemId + ")"); // Move the bookmark to a new parent or new position if necessary @@ -1325,7 +1397,7 @@ BookmarksStore.prototype = { node.containerOpen = true; // Remember all the children GUIDs and recursively get more - for (var i = 0; i < node.childCount; i++) { + for (let i = 0; i < node.childCount; i++) { let child = node.getChild(i); items[this.GUIDForId(child.itemId)] = true; this._getChildren(child, items); diff --git a/services/sync/modules/engines/history.js b/services/sync/modules/engines/history.js index c2bde499943f..9da1c39f6bf7 100644 --- a/services/sync/modules/engines/history.js +++ b/services/sync/modules/engines/history.js @@ -54,7 +54,6 @@ Cu.import("resource://services-sync/engines.js"); Cu.import("resource://services-sync/record.js"); Cu.import("resource://services-sync/util.js"); Cu.import("resource://services-sync/log4moz.js"); -Cu.import("resource://services-sync/ext/Sync.js"); function HistoryRec(collection, id) { CryptoWrapper.call(this, collection, id); @@ -459,8 +458,7 @@ HistoryStore.prototype = { return failed; } - let [updatePlaces, cb] = Sync.withCb(this._asyncHistory.updatePlaces, - this._asyncHistory); + let cb = Utils.makeSyncCallback(); let onPlace = function onPlace(result, placeInfo) { if (!Components.isSuccessCode(result)) { failed.push(placeInfo.guid); @@ -471,7 +469,8 @@ HistoryStore.prototype = { cb(); }; Svc.Obs.add(TOPIC_UPDATEPLACES_COMPLETE, onComplete); - updatePlaces(placeInfos, onPlace); + this._asyncHistory.updatePlaces(placeInfos, onPlace); + Utils.waitForSyncCallback(cb); return failed; }, diff --git a/services/sync/modules/ext/Sync.js b/services/sync/modules/ext/Sync.js deleted file mode 100644 index 225033e481fe..000000000000 --- a/services/sync/modules/ext/Sync.js +++ /dev/null @@ -1,215 +0,0 @@ -/* ***** BEGIN LICENSE BLOCK ***** - * Version: MPL 1.1/GPL 2.0/LGPL 2.1 - * - * The contents of this file are subject to the Mozilla Public License Version - * 1.1 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * http://www.mozilla.org/MPL/ - * - * Software distributed under the License is distributed on an "AS IS" basis, - * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License - * for the specific language governing rights and limitations under the - * License. - * - * The Original Code is Weave. - * - * The Initial Developer of the Original Code is Mozilla. - * Portions created by the Initial Developer are Copyright (C) 2009 - * the Initial Developer. All Rights Reserved. - * - * Contributor(s): - * Edward Lee - * Dan Mills - * Myk Melez - * - * Alternatively, the contents of this file may be used under the terms of - * either the GNU General Public License Version 2 or later (the "GPL"), or - * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"), - * in which case the provisions of the GPL or the LGPL are applicable instead - * of those above. If you wish to allow use of your version of this file only - * under the terms of either the GPL or the LGPL, and not to allow others to - * use your version of this file under the terms of the MPL, indicate your - * decision by deleting the provisions above and replace them with the notice - * and other provisions required by the GPL or the LGPL. If you do not delete - * the provisions above, a recipient may use your version of this file under - * the terms of any one of the MPL, the GPL or the LGPL. - * - * ***** END LICENSE BLOCK ***** */ - -let EXPORTED_SYMBOLS = ["Sync"]; - -const Cc = Components.classes; -const Ci = Components.interfaces; -const Cr = Components.results; -const Cu = Components.utils; - -// Define some constants to specify various sync. callback states -const CB_READY = {}; -const CB_COMPLETE = {}; -const CB_FAIL = {}; - -// Share a secret only for functions in this file to prevent outside access -const SECRET = {}; - -/** - * Check if the app is ready (not quitting) - */ -function checkAppReady() { - // Watch for app-quit notification to stop any sync. calls - let os = Cc["@mozilla.org/observer-service;1"]. - getService(Ci.nsIObserverService); - os.addObserver({ - observe: function observe() { - // Now that the app is quitting, make checkAppReady throw - checkAppReady = function() { - throw Components.Exception("App. Quitting", Cr.NS_ERROR_ABORT); - }; - os.removeObserver(this, "quit-application"); - } - }, "quit-application", false); - - // In the common case, checkAppReady just returns true - return (checkAppReady = function() true)(); -}; - -/** - * Create a callback that remembers state like whether it's been called - */ -function makeCallback() { - // Initialize private callback data to prepare to be called - let _ = { - state: CB_READY, - value: null - }; - - // The main callback remembers the value it's passed and that it got data - let onComplete = function makeCallback_onComplete(data) { - _.state = CB_COMPLETE; - _.value = data; - }; - - // Only allow access to the private data if the secret matches - onComplete._ = function onComplete__(secret) secret == SECRET ? _ : {}; - - // Allow an alternate callback to trigger an exception to be thrown - onComplete.throw = function onComplete_throw(data) { - _.state = CB_FAIL; - _.value = data; - - // Cause the caller to get an exception and stop execution - throw data; - }; - - return onComplete; -} - -/** - * Make a synchronous version of the function object that will be called with - * the provided thisArg. - * - * @param func {Function} - * The asynchronous function to make a synchronous function - * @param thisArg {Object} [optional] - * The object that the function accesses with "this" - * @param callback {Function} [optional] [internal] - * The callback that will trigger the end of the async. call - * @usage let ret = Sync(asyncFunc, obj)(arg1, arg2); - * @usage let ret = Sync(ignoreThisFunc)(arg1, arg2); - * @usage let sync = Sync(async); let ret = sync(arg1, arg2); - */ -function Sync(func, thisArg, callback) { - return function syncFunc(/* arg1, arg2, ... */) { - // Grab the current thread so we can make it give up priority - let thread = Cc["@mozilla.org/thread-manager;1"].getService().currentThread; - - // Save the original arguments into an array - let args = Array.slice(arguments); - - let instanceCallback = callback; - // We need to create a callback and insert it if we weren't given one - if (instanceCallback == null) { - // Create a new callback for this invocation instance and pass it in - instanceCallback = makeCallback(); - args.unshift(instanceCallback); - } - - // Call the async function bound to thisArg with the passed args - func.apply(thisArg, args); - - // Keep waiting until our callback is triggered unless the app is quitting - let callbackData = instanceCallback._(SECRET); - while (checkAppReady() && callbackData.state == CB_READY) - thread.processNextEvent(true); - - // Reset the state of the callback to prepare for another call - let state = callbackData.state; - callbackData.state = CB_READY; - - // Throw the value the callback decided to fail with - if (state == CB_FAIL) - throw callbackData.value; - - // Return the value passed to the callback - return callbackData.value; - }; -} - -/** - * Make a synchronous version of an async. function and the callback to trigger - * the end of the async. call. - * - * @param func {Function} - * The asynchronous function to make a synchronous function - * @param thisArg {Object} [optional] - * The object that the function accesses with "this" - * @usage let [sync, cb] = Sync.withCb(async); let ret = sync(arg1, arg2, cb); - */ -Sync.withCb = function Sync_withCb(func, thisArg) { - let cb = makeCallback(); - return [Sync(func, thisArg, cb), cb]; -}; - -/** - * Set a timer, simulating the API for the window.setTimeout call. - * This only simulates the API for the version of the call that accepts - * a function as its first argument and no additional parameters, - * and it doesn't return the timeout ID. - * - * @param func {Function} - * the function to call after the delay - * @param delay {Number} - * the number of milliseconds to wait - */ -function setTimeout(func, delay) { - let timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer); - let callback = { - notify: function notify() { - // This line actually just keeps a reference to timer (prevent GC) - timer = null; - - // Call the function so that "this" is global - func(); - } - } - timer.initWithCallback(callback, delay, Ci.nsITimer.TYPE_ONE_SHOT); -} - -function sleep(callback, milliseconds) { - setTimeout(callback, milliseconds); -} - -/** - * Sleep the specified number of milliseconds, pausing execution of the caller - * without halting the current thread. - * For example, the following code pauses 1000ms between dumps: - * - * dump("Wait for it...\n"); - * Sync.sleep(1000); - * dump("Wait for it...\n"); - * Sync.sleep(1000); - * dump("What are you waiting for?!\n"); - * - * @param milliseconds {Number} - * The number of milliseconds to sleep - */ -Sync.sleep = Sync(sleep); diff --git a/services/sync/modules/identity.js b/services/sync/modules/identity.js index fd49e47cd7e2..cbaf26929088 100644 --- a/services/sync/modules/identity.js +++ b/services/sync/modules/identity.js @@ -42,7 +42,6 @@ const Cr = Components.results; const Cu = Components.utils; Cu.import("resource://services-sync/constants.js"); -Cu.import("resource://services-sync/ext/Sync.js"); Cu.import("resource://services-sync/log4moz.js"); Cu.import("resource://services-sync/util.js"); diff --git a/services/sync/modules/log4moz.js b/services/sync/modules/log4moz.js index 9d2e4a483e4c..7c1d421a181f 100644 --- a/services/sync/modules/log4moz.js +++ b/services/sync/modules/log4moz.js @@ -43,8 +43,6 @@ const Ci = Components.interfaces; const Cr = Components.results; const Cu = Components.utils; -Cu.import("resource://gre/modules/XPCOMUtils.jsm"); - const MODE_RDONLY = 0x01; const MODE_WRONLY = 0x02; const MODE_CREATE = 0x08; @@ -155,8 +153,6 @@ function LogMessage(loggerName, level, message){ this.time = Date.now(); } LogMessage.prototype = { - QueryInterface: XPCOMUtils.generateQI([Ci.nsISupports]), - get levelDesc() { if (this.level in Log4Moz.Level.Desc) return Log4Moz.Level.Desc[this.level]; @@ -178,14 +174,12 @@ function Logger(name, repository) { if (!repository) repository = Log4Moz.repository; this._name = name; - this._appenders = []; + this.children = []; + this.ownAppenders = []; + this.appenders = []; this._repository = repository; } Logger.prototype = { - QueryInterface: XPCOMUtils.generateQI([Ci.nsISupports]), - - parent: null, - get name() { return this._name; }, @@ -203,59 +197,97 @@ Logger.prototype = { this._level = level; }, - _appenders: null, - get appenders() { - if (!this.parent) - return this._appenders; - return this._appenders.concat(this.parent.appenders); + _parent: null, + get parent() this._parent, + set parent(parent) { + if (this._parent == parent) { + return; + } + // Remove ourselves from parent's children + if (this._parent) { + let index = this._parent.children.indexOf(this); + if (index != -1) { + this._parent.children.splice(index, 1); + } + } + this._parent = parent; + parent.children.push(this); + this.updateAppenders(); + }, + + updateAppenders: function updateAppenders() { + if (this._parent) { + let notOwnAppenders = this._parent.appenders.filter(function(appender) { + return this.ownAppenders.indexOf(appender) == -1; + }, this); + this.appenders = notOwnAppenders.concat(this.ownAppenders); + } else { + this.appenders = this.ownAppenders.slice(); + } + + // Update children's appenders. + for (let i = 0; i < this.children.length; i++) { + this.children[i].updateAppenders(); + } }, addAppender: function Logger_addAppender(appender) { - for (let i = 0; i < this._appenders.length; i++) { - if (this._appenders[i] == appender) - return; + if (this.ownAppenders.indexOf(appender) != -1) { + return; } - this._appenders.push(appender); + this.ownAppenders.push(appender); + this.updateAppenders(); }, removeAppender: function Logger_removeAppender(appender) { - let newAppenders = []; - for (let i = 0; i < this._appenders.length; i++) { - if (this._appenders[i] != appender) - newAppenders.push(this._appenders[i]); + let index = this.ownAppenders.indexOf(appender); + if (index == -1) { + return; } - this._appenders = newAppenders; + this.ownAppenders.splice(index, 1); + this.updateAppenders(); }, - log: function Logger_log(message) { - if (this.level > message.level) + log: function Logger_log(level, string) { + if (this.level > level) return; + + // Hold off on creating the message object until we actually have + // an appender that's responsible. + let message; let appenders = this.appenders; for (let i = 0; i < appenders.length; i++){ - appenders[i].append(message); + let appender = appenders[i]; + if (appender.level > level) + continue; + + if (!message) + message = new LogMessage(this._name, level, string); + + appender.append(message); } }, fatal: function Logger_fatal(string) { - this.log(new LogMessage(this._name, Log4Moz.Level.Fatal, string)); + this.log(Log4Moz.Level.Fatal, string); }, error: function Logger_error(string) { - this.log(new LogMessage(this._name, Log4Moz.Level.Error, string)); + this.log(Log4Moz.Level.Error, string); }, warn: function Logger_warn(string) { - this.log(new LogMessage(this._name, Log4Moz.Level.Warn, string)); + this.log(Log4Moz.Level.Warn, string); }, info: function Logger_info(string) { - this.log(new LogMessage(this._name, Log4Moz.Level.Info, string)); + this.log(Log4Moz.Level.Info, string); }, config: function Logger_config(string) { - this.log(new LogMessage(this._name, Log4Moz.Level.Config, string)); + this.log(Log4Moz.Level.Config, string); }, debug: function Logger_debug(string) { - this.log(new LogMessage(this._name, Log4Moz.Level.Debug, string)); + this.log(Log4Moz.Level.Debug, string); }, trace: function Logger_trace(string) { - this.log(new LogMessage(this._name, Log4Moz.Level.Trace, string)); + this.log(Log4Moz.Level.Trace, string); } }; @@ -266,8 +298,6 @@ Logger.prototype = { function LoggerRepository() {} LoggerRepository.prototype = { - QueryInterface: XPCOMUtils.generateQI([Ci.nsISupports]), - _loggers: {}, _rootLogger: null, @@ -278,10 +308,9 @@ LoggerRepository.prototype = { } return this._rootLogger; }, - // FIXME: need to update all parent values if we do this - //set rootLogger(logger) { - // this._rootLogger = logger; - //}, + set rootLogger(logger) { + throw "Cannot change the root logger"; + }, _updateParents: function LogRep__updateParents(name) { let pieces = name.split('.'); @@ -313,8 +342,6 @@ LoggerRepository.prototype = { }, getLogger: function LogRep_getLogger(name) { - if (!name) - name = this.getLogger.caller.name; if (name in this._loggers) return this._loggers[name]; this._loggers[name] = new Logger(name, this); @@ -332,11 +359,10 @@ LoggerRepository.prototype = { // Abstract formatter function Formatter() {} Formatter.prototype = { - QueryInterface: XPCOMUtils.generateQI([Ci.nsISupports]), format: function Formatter_format(message) {} }; -// FIXME: should allow for formatting the whole string, not just the date +// Basic formatter that doesn't do anything fancy function BasicFormatter(dateFormat) { if (dateFormat) this.dateFormat = dateFormat; @@ -344,32 +370,9 @@ function BasicFormatter(dateFormat) { BasicFormatter.prototype = { __proto__: Formatter.prototype, - _dateFormat: null, - - get dateFormat() { - if (!this._dateFormat) - this._dateFormat = "%Y-%m-%d %H:%M:%S"; - return this._dateFormat; - }, - - set dateFormat(format) { - this._dateFormat = format; - }, - format: function BF_format(message) { - // Pad a string to a certain length (20) with a character (space) - let pad = function BF__pad(str, len, chr) str + - new Array(Math.max((len || 20) - str.length + 1, 0)).join(chr || " "); - - // Generate a date string because toLocaleString doesn't work XXX 514803 - let z = function(n) n < 10 ? "0" + n : n; - let d = new Date(message.time); - let dateStr = [d.getFullYear(), "-", z(d.getMonth() + 1), "-", - z(d.getDate()), " ", z(d.getHours()), ":", z(d.getMinutes()), ":", - z(d.getSeconds())].join(""); - - return dateStr + "\t" + pad(message.loggerName) + " " + message.levelDesc + - "\t" + message.message + "\n"; + return message.time + "\t" + message.loggerName + "\t" + message.levelDesc + + "\t" + message.message + "\n"; } }; @@ -384,15 +387,10 @@ function Appender(formatter) { this._formatter = formatter? formatter : new BasicFormatter(); } Appender.prototype = { - QueryInterface: XPCOMUtils.generateQI([Ci.nsISupports]), - - _level: Log4Moz.Level.All, - get level() { return this._level; }, - set level(level) { this._level = level; }, + level: Log4Moz.Level.All, append: function App_append(message) { - if(this._level <= message.level) - this.doAppend(this._formatter.format(message)); + this.doAppend(this._formatter.format(message)); }, toString: function App_toString() { return this._name + " [level=" + this._level + diff --git a/services/sync/modules/resource.js b/services/sync/modules/resource.js index 9354d9ddee9c..3d0566251d92 100644 --- a/services/sync/modules/resource.js +++ b/services/sync/modules/resource.js @@ -48,7 +48,6 @@ const Cu = Components.utils; Cu.import("resource://services-sync/constants.js"); Cu.import("resource://services-sync/ext/Observers.js"); Cu.import("resource://services-sync/ext/Preferences.js"); -Cu.import("resource://services-sync/ext/Sync.js"); Cu.import("resource://services-sync/log4moz.js"); Cu.import("resource://services-sync/util.js"); @@ -403,7 +402,7 @@ Resource.prototype = { // is never called directly, but is used by the high-level // {{{get}}}, {{{put}}}, {{{post}}} and {{delete}} methods. _request: function Res__request(action, data) { - let [doRequest, cb] = Sync.withCb(this._doRequest, this); + let cb = Utils.makeSyncCallback(); function callback(error, ret) { if (error) cb.throw(error); @@ -412,7 +411,8 @@ Resource.prototype = { // The channel listener might get a failure code try { - return doRequest(action, data, callback); + this._doRequest(action, data, callback); + return Utils.waitForSyncCallback(cb); } catch(ex) { // Combine the channel stack with this request stack. Need to create // a new error object for that. diff --git a/services/sync/modules/service.js b/services/sync/modules/service.js index c9bfecdbfd40..9c762e3ba215 100644 --- a/services/sync/modules/service.js +++ b/services/sync/modules/service.js @@ -54,12 +54,13 @@ const CLUSTER_BACKOFF = 5 * 60 * 1000; // 5 minutes // How long a key to generate from an old passphrase. const PBKDF2_KEY_BYTES = 16; +const LOG_DATE_FORMAT = "%Y-%m-%d %H:%M:%S"; + Cu.import("resource://gre/modules/XPCOMUtils.jsm"); Cu.import("resource://services-sync/record.js"); Cu.import("resource://services-sync/constants.js"); Cu.import("resource://services-sync/engines.js"); Cu.import("resource://services-sync/engines/clients.js"); -Cu.import("resource://services-sync/ext/Sync.js"); Cu.import("resource://services-sync/ext/Preferences.js"); Cu.import("resource://services-sync/identity.js"); Cu.import("resource://services-sync/log4moz.js"); @@ -1661,7 +1662,8 @@ WeaveSvc.prototype = { }, sync: function sync() { - this._log.debug("In wrapping sync()."); + let dateStr = new Date().toLocaleFormat(LOG_DATE_FORMAT); + this._log.info("Starting sync at " + dateStr); this._catch(function () { // Make sure we're logged in. if (this._shouldLogin()) { @@ -1808,7 +1810,9 @@ WeaveSvc.prototype = { Svc.Prefs.set("lastSync", new Date().toString()); Status.sync = SYNC_SUCCEEDED; let syncTime = ((Date.now() - syncStartTime) / 1000).toFixed(2); - this._log.info("Sync completed successfully in " + syncTime + " secs."); + let dateStr = new Date().toLocaleFormat(LOG_DATE_FORMAT); + this._log.info("Sync completed successfully at " + dateStr + + " after " + syncTime + " secs."); } } finally { this._syncError = false; diff --git a/services/sync/modules/util.js b/services/sync/modules/util.js index a78b7b0b349a..66bb62d7a126 100644 --- a/services/sync/modules/util.js +++ b/services/sync/modules/util.js @@ -46,7 +46,6 @@ Cu.import("resource://services-sync/constants.js"); Cu.import("resource://services-sync/ext/Observers.js"); Cu.import("resource://services-sync/ext/Preferences.js"); Cu.import("resource://services-sync/ext/StringBundle.js"); -Cu.import("resource://services-sync/ext/Sync.js"); Cu.import("resource://services-sync/log4moz.js"); let NetUtil; @@ -58,6 +57,12 @@ try { // Firefox 3.5 :( } +// Constants for makeSyncCallback, waitForSyncCallback +const CB_READY = {}; +const CB_COMPLETE = {}; +const CB_FAIL = {}; + + /* * Utility functions */ @@ -230,8 +235,8 @@ let Utils = { names = names == null ? [] : [names]; // Synchronously asyncExecute fetching all results by name - let [exec, execCb] = Sync.withCb(query.executeAsync, query); - return exec({ + let execCb = Utils.makeSyncCallback(); + query.executeAsync({ items: [], handleResult: function handleResult(results) { let row; @@ -249,6 +254,7 @@ let Utils = { execCb(this.items); } }); + return Utils.waitForSyncCallback(execCb); }, byteArrayToString: function byteArrayToString(bytes) { @@ -361,7 +367,7 @@ let Utils = { * @param obj * Object to add properties to defer in its prototype * @param defer - * Hash property of obj to defer to (dot split each level) + * Property of obj to defer to * @param prop * Property name to defer (or an array of property names) */ @@ -369,45 +375,23 @@ let Utils = { if (Utils.isArray(prop)) return prop.map(function(prop) Utils.deferGetSet(obj, defer, prop)); - // Split the defer into each dot part for each level to dereference - let parts = defer.split("."); - let deref = function(base) Utils.deref(base, parts); - let prot = obj.prototype; // Create a getter if it doesn't exist yet if (!prot.__lookupGetter__(prop)) { - // Yes, this should be a one-liner, but there are errors if it's not - // broken out. *sigh* - // Errors are these: - // JavaScript strict warning: resource://services-sync/util.js, line 304: reference to undefined property deref(this)[prop] - // JavaScript strict warning: resource://services-sync/util.js, line 304: reference to undefined property deref(this)[prop] - let f = function() { - let d = deref(this); - if (!d) - return undefined; - let out = d[prop]; - return out; - } - prot.__defineGetter__(prop, f); + prot.__defineGetter__(prop, function () { + return this[defer][prop]; + }); } // Create a setter if it doesn't exist yet - if (!prot.__lookupSetter__(prop)) - prot.__defineSetter__(prop, function(val) deref(this)[prop] = val); + if (!prot.__lookupSetter__(prop)) { + prot.__defineSetter__(prop, function (val) { + this[defer][prop] = val; + }); + } }, - /** - * Dereference an array of properties starting from a base object - * - * @param base - * Base object to start dereferencing - * @param props - * Array of properties to dereference (one for each level) - */ - deref: function Utils_deref(base, props) props.reduce(function(curr, prop) - curr[prop], base), - /** * Determine if some value is an array * @@ -1487,8 +1471,8 @@ let Utils = { }, mpLocked: function mpLocked() { - let modules = Cc["@mozilla.org/security/pkcs11moduledb;1"]. - getService(Ci.nsIPKCS11ModuleDB); + let modules = Cc["@mozilla.org/security/pkcs11moduledb;1"] + .getService(Ci.nsIPKCS11ModuleDB); let sdrSlot = modules.findSlotByName(""); let status = sdrSlot.status; let slots = Ci.nsIPKCS11Slot; @@ -1507,13 +1491,13 @@ let Utils = { // If Master Password is enabled and locked, present a dialog to unlock it. // Return whether the system is unlocked. ensureMPUnlocked: function ensureMPUnlocked() { - sdr = Cc["@mozilla.org/security/sdr;1"].getService(Ci.nsISecretDecoderRing); - var ok = false; + let sdr = Cc["@mozilla.org/security/sdr;1"] + .getService(Ci.nsISecretDecoderRing); try { sdr.encryptString("bacon"); - ok = true; + return true; } catch(e) {} - return ok; + return false; }, __prefs: null, @@ -1525,6 +1509,77 @@ let Utils = { this.__prefs.QueryInterface(Ci.nsIPrefBranch2); } return this.__prefs; + }, + + /** + * Helpers for making asynchronous calls within a synchronous API possible. + * + * If you value your sanity, do not look closely at the following functions. + */ + + /** + * Check if the app is ready (not quitting) + */ + checkAppReady: function checkAppReady() { + // Watch for app-quit notification to stop any sync calls + Svc.Obs.add("quit-application", function() { + Utils.checkAppReady = function() { + throw Components.Exception("App. Quitting", Cr.NS_ERROR_ABORT); + }; + }); + // In the common case, checkAppReady just returns true + return (Utils.checkAppReady = function() true)(); + }, + + /** + * Create a sync callback that remembers state like whether it's been called + */ + makeSyncCallback: function makeSyncCallback() { + // The main callback remembers the value it's passed and that it got data + let onComplete = function onComplete(data) { + onComplete.state = CB_COMPLETE; + onComplete.value = data; + }; + + // Initialize private callback data to prepare to be called + onComplete.state = CB_READY; + onComplete.value = null; + + // Allow an alternate callback to trigger an exception to be thrown + onComplete.throw = function onComplete_throw(data) { + onComplete.state = CB_FAIL; + onComplete.value = data; + + // Cause the caller to get an exception and stop execution + throw data; + }; + + return onComplete; + }, + + /** + * Wait for a sync callback to finish + */ + waitForSyncCallback: function waitForSyncCallback(callback) { + // Grab the current thread so we can make it give up priority + let thread = Cc["@mozilla.org/thread-manager;1"].getService().currentThread; + + // Keep waiting until our callback is triggered unless the app is quitting + while (Utils.checkAppReady() && callback.state == CB_READY) { + thread.processNextEvent(true); + } + + // Reset the state of the callback to prepare for another call + let state = callback.state; + callback.state = CB_READY; + + // Throw the value the callback decided to fail with + if (state == CB_FAIL) { + throw callback.value; + } + + // Return the value passed to the callback + return callback.value; } }; diff --git a/services/sync/tests/unit/head_helpers.js b/services/sync/tests/unit/head_helpers.js index d378d35ae6a9..dbbc101b87d4 100644 --- a/services/sync/tests/unit/head_helpers.js +++ b/services/sync/tests/unit/head_helpers.js @@ -103,7 +103,8 @@ function initTestLogging(level) { log.level = Log4Moz.Level.Trace; appender.level = Log4Moz.Level.Trace; // Overwrite any other appenders (e.g. from previous incarnations) - log._appenders = [appender]; + log.ownAppenders = [appender]; + log.updateAppenders(); return logStats; } diff --git a/services/sync/tests/unit/test_bookmark_engine.js b/services/sync/tests/unit/test_bookmark_engine.js index 7a668c19b6f0..05a22ccb1a02 100644 --- a/services/sync/tests/unit/test_bookmark_engine.js +++ b/services/sync/tests/unit/test_bookmark_engine.js @@ -278,6 +278,91 @@ function test_restorePromptsReupload() { } } +// Bug 632287. +function test_mismatched_types() { + _("Ensure that handling a record that changes type causes deletion " + + "then re-adding."); + + function FakeRecord(constructor, r) { + constructor.call(this, "bookmarks", r.id); + for (let x in r) { + this[x] = r[x]; + } + } + + let oldRecord = { + "id": "l1nZZXfB8nC7", + "type":"folder", + "parentName":"Bookmarks Toolbar", + "title":"Innerst i Sneglehode", + "description":null, + "parentid": "toolbar" + }; + + let newRecord = { + "id": "l1nZZXfB8nC7", + "type":"livemark", + "siteUri":"http://sneglehode.wordpress.com/", + "feedUri":"http://sneglehode.wordpress.com/feed/", + "parentName":"Bookmarks Toolbar", + "title":"Innerst i Sneglehode", + "description":null, + "children": + ["HCRq40Rnxhrd", "YeyWCV1RVsYw", "GCceVZMhvMbP", "sYi2hevdArlF", + "vjbZlPlSyGY8", "UtjUhVyrpeG6", "rVq8WMG2wfZI", "Lx0tcy43ZKhZ", + "oT74WwV8_j4P", "IztsItWVSo3-"], + "parentid": "toolbar" + }; + + do_test_pending(); + Svc.Prefs.set("username", "foo"); + Service.serverURL = "http://localhost:8080/"; + Service.clusterURL = "http://localhost:8080/"; + + let collection = new ServerCollection({}, true); + + let engine = new BookmarksEngine(); + let store = engine._store; + let global = new ServerWBO('global', + {engines: {bookmarks: {version: engine.version, + syncID: engine.syncID}}}); + _("GUID: " + store.GUIDForId(6, true)); + let server = httpd_setup({ + "/1.0/foo/storage/meta/global": global.handler(), + "/1.0/foo/storage/bookmarks": collection.handler() + }); + + try { + let bms = store._bms; + let oldR = new FakeRecord(BookmarkFolder, oldRecord); + let newR = new FakeRecord(Livemark, newRecord); + oldR._parent = Svc.Bookmark.toolbarFolder; + newR._parent = Svc.Bookmark.toolbarFolder; + + store.applyIncoming(oldR); + _("Applied old. It's a folder."); + let oldID = store.idForGUID(oldR.id); + _("Old ID: " + oldID); + do_check_eq(bms.getItemType(oldID), bms.TYPE_FOLDER); + do_check_false(store._ls.isLivemark(oldID)); + + store.applyIncoming(newR); + let newID = store.idForGUID(newR.id); + _("New ID: " + newID); + + _("Applied new. It's a livemark."); + do_check_eq(bms.getItemType(newID), bms.TYPE_FOLDER); + do_check_true(store._ls.isLivemark(newID)); + + } finally { + store.wipe(); + server.stop(do_test_finished); + Svc.Prefs.resetBranch(""); + Records.clearCache(); + syncTesting = new SyncTestingInfrastructure(makeEngine); + } +} + function run_test() { initTestLogging("Trace"); Log4Moz.repository.getLogger("Engine.Bookmarks").level = Log4Moz.Level.Trace; @@ -286,5 +371,6 @@ function run_test() { test_processIncoming_error_orderChildren(); test_ID_caching(); + test_mismatched_types(); test_restorePromptsReupload(); } diff --git a/services/sync/tests/unit/test_bookmark_livemarks.js b/services/sync/tests/unit/test_bookmark_livemarks.js new file mode 100644 index 000000000000..993f9cd7c1f4 --- /dev/null +++ b/services/sync/tests/unit/test_bookmark_livemarks.js @@ -0,0 +1,147 @@ +Cu.import("resource://services-sync/log4moz.js"); +Cu.import("resource://services-sync/record.js"); +Cu.import("resource://services-sync/engines.js"); +Cu.import("resource://services-sync/engines/bookmarks.js"); +Cu.import("resource://services-sync/util.js"); + +Cu.import("resource://services-sync/service.js"); +try { + Cu.import("resource://gre/modules/PlacesUtils.jsm"); +} +catch(ex) { + Cu.import("resource://gre/modules/utils.js"); +} + +const DESCRIPTION_ANNO = "bookmarkProperties/description"; + +Engines.register(BookmarksEngine); +let engine = Engines.get("bookmarks"); +let store = engine._store; + +// Record borrowed from Bug 631361. +let record631361 = { + id: "M5bwUKK8hPyF", + index: 150, + modified: 1296768176.49, + payload: + {"id":"M5bwUKK8hPyF", + "type":"livemark", + "siteUri":"http://www.bbc.co.uk/go/rss/int/news/-/news/", + "feedUri":"http://fxfeeds.mozilla.com/en-US/firefox/headlines.xml", + "parentName":"Bookmarks Toolbar", + "parentid":"toolbar", + "title":"Latest Headlines", + "description":"", + "children": + ["7oBdEZB-8BMO", "SUd1wktMNCTB", "eZe4QWzo1BcY", "YNBhGwhVnQsN", + "92Aw2SMEkFg0", "uw0uKqrVFwd-", "x7mx2P3--8FJ", "d-jVF8UuC9Ye", + "DV1XVtKLEiZ5", "g4mTaTjr837Z", "1Zi5W3lwBw8T", "FEYqlUHtbBWS", + "qQd2u7LjosCB", "VUs2djqYfbvn", "KuhYnHocu7eg", "u2gcg9ILRg-3", + "hfK_RP-EC7Ol", "Aq5qsa4E5msH", "6pZIbxuJTn-K", "k_fp0iN3yYMR", + "59YD3iNOYO8O", "01afpSdAk2iz", "Cq-kjXDEPIoP", "HtNTjt9UwWWg", + "IOU8QRSrTR--", "HJ5lSlBx6d1D", "j2dz5R5U6Khc", "5GvEjrNR0yJl", + "67ozIBF5pNVP", "r5YB0cUx6C_w", "FtmFDBNxDQ6J", "BTACeZq9eEtw", + "ll4ozQ-_VNJe", "HpImsA4_XuW7", "nJvCUQPLSXwA", "94LG-lh6TUYe", + "WHn_QoOL94Os", "l-RvjgsZYlej", "LipQ8abcRstN", "74TiLvarE3n_", + "8fCiLQpQGK1P", "Z6h4WkbwfQFa", "GgAzhqakoS6g", "qyt92T8vpMsK", + "RyOgVCe2EAOE", "bgSEhW3w6kk5", "hWODjHKGD7Ph", "Cky673aqOHbT", + "gZCYT7nx3Nwu", "iJzaJxxrM58L", "rUHCRv68aY5L", "6Jc1hNJiVrV9", + "lmNgoayZ-ym8", "R1lyXsDzlfOd", "pinrXwDnRk6g", "Sn7TmZV01vMM", + "qoXyU6tcS1dd", "TRLanED-QfBK", "xHbhMeX_FYEA", "aPqacdRlAtaW", + "E3H04Wn2RfSi", "eaSIMI6kSrcz", "rtkRxFoG5Vqi", "dectkUglV0Dz", + "B4vUE0BE15No", "qgQFW5AQrgB0", "SxAXvwOhu8Zi", "0S6cRPOg-5Z2", + "zcZZBGeLnaWW", "B0at8hkQqVZQ", "sgPtgGulbP66", "lwtwGHSCPYaQ", + "mNTdpgoRZMbW", "-L8Vci6CbkJY", "bVzudKSQERc1", "Gxl9lb4DXsmL", + "3Qr13GucOtEh"]}, + collection: "bookmarks" +}; + +// Clean up after other tests. Only necessary in XULRunner. +store.wipe(); + +function makeLivemark(p, mintGUID) { + let b = new Livemark("bookmarks", p.id); + // Copy here, because tests mutate the contents. + b.cleartext = Utils.deepCopy(p); + + if (mintGUID) + b.id = Utils.makeGUID(); + + return b; +} + +function test_livemark_descriptions(next) { + let record = record631361.payload; + + function doRecord(r) { + store._childrenToOrder = {}; + store.applyIncoming(r); + store._orderChildren(); + delete store._childrenToOrder; + } + + // Attempt to provoke an error by messing around with the description. + record.description = null; + doRecord(makeLivemark(record)); + record.description = ""; + doRecord(makeLivemark(record)); + + // Attempt to provoke an error by adding a bad description anno. + let id = store.idForGUID(record.id); + Utils.anno(id, DESCRIPTION_ANNO, ""); + + next(); +} + +function test_livemark_invalid(next) { + _("Livemarks considered invalid by nsLivemarkService are skipped."); + + _("Parent is 0, which is invalid. Will be set to unfiled."); + let noParentRec = makeLivemark(record631361.payload, true); + noParentRec._parent = 0; + store.create(noParentRec); + let recID = store.idForGUID(noParentRec.id, true); + do_check_true(recID > 0); + do_check_eq(Svc.Bookmark.getFolderIdForItem(recID), Svc.Bookmark.unfiledBookmarksFolder); + + _("Parent is unknown. Will be set to unfiled."); + let lateParentRec = makeLivemark(record631361.payload, true); + let parentGUID = Utils.makeGUID(); + lateParentRec.parentid = parentGUID; + lateParentRec._parent = store.idForGUID(parentGUID); // Usually done by applyIncoming. + do_check_eq(-1, lateParentRec._parent); + + store.create(lateParentRec); + recID = store.idForGUID(lateParentRec.id, true); + do_check_true(recID > 0); + do_check_eq(Svc.Bookmark.getFolderIdForItem(recID), Svc.Bookmark.unfiledBookmarksFolder); + + _("No feed URI, which is invalid. Will be skipped."); + let noFeedURIRec = makeLivemark(record631361.payload, true); + delete noFeedURIRec.cleartext.feedUri; + store.create(noFeedURIRec); + // No exception, but no creation occurs. + do_check_eq(-1, store.idForGUID(noFeedURIRec.id, true)); + + _("Parent is a Livemark. Will be skipped."); + let lmParentRec = makeLivemark(record631361.payload, true); + lmParentRec._parent = recID; + store.create(lmParentRec); + // No exception, but no creation occurs. + do_check_eq(-1, store.idForGUID(lmParentRec.id, true)); + + // Clear event loop. + Utils.delay(next, 0); +} + +function run_test() { + do_test_pending(); + initTestLogging("Trace"); + Log4Moz.repository.getLogger("Engine.Bookmarks").level = Log4Moz.Level.Trace; + Log4Moz.repository.getLogger("Store.Bookmarks").level = Log4Moz.Level.Trace; + + asyncChainTests( + test_livemark_descriptions, + test_livemark_invalid, + do_test_finished)(); +} diff --git a/services/sync/tests/unit/test_resource.js b/services/sync/tests/unit/test_resource.js index 1c213a8f8745..1bb5fe283226 100644 --- a/services/sync/tests/unit/test_resource.js +++ b/services/sync/tests/unit/test_resource.js @@ -3,7 +3,6 @@ Cu.import("resource://services-sync/identity.js"); Cu.import("resource://services-sync/log4moz.js"); Cu.import("resource://services-sync/resource.js"); Cu.import("resource://services-sync/util.js"); -Cu.import("resource://services-sync/ext/Sync.js"); let logger; diff --git a/services/sync/tests/unit/test_syncengine.js b/services/sync/tests/unit/test_syncengine.js index 11f02b9825e2..f6752cd68d70 100644 --- a/services/sync/tests/unit/test_syncengine.js +++ b/services/sync/tests/unit/test_syncengine.js @@ -1,6 +1,5 @@ Cu.import("resource://services-sync/engines.js"); Cu.import("resource://services-sync/util.js"); -Cu.import("resource://services-sync/ext/Sync.js"); function makeSteamEngine() { return new SyncEngine('Steam'); @@ -83,7 +82,7 @@ function test_toFetch() { engine.toFetch = toFetch; do_check_eq(engine.toFetch, toFetch); // toFetch is written asynchronously - Sync.sleep(0); + engine._sleep(0); let fakefile = syncTesting.fakeFilesystem.fakeContents[filename]; do_check_eq(fakefile, JSON.stringify(toFetch)); diff --git a/services/sync/tests/unit/test_utils_deferGetSet.js b/services/sync/tests/unit/test_utils_deferGetSet.js index 9b6277baa286..55c0fcb0e670 100644 --- a/services/sync/tests/unit/test_utils_deferGetSet.js +++ b/services/sync/tests/unit/test_utils_deferGetSet.js @@ -31,13 +31,6 @@ function run_test() { do_check_eq(src.p2, "v2"); do_check_eq(src.dst.p2, "v2"); - _("handle dotted properties"); - src.dst.nest = {}; - Utils.deferGetSet(base, "dst.nest", "prop"); - src.prop = "val"; - do_check_eq(src.prop, "val"); - do_check_eq(src.dst.nest.prop, "val"); - _("make sure existing getter keeps its functionality"); Utils.deferGetSet(base, "dst", "a"); src.a = "not a";