From b0d23b044f5ce9a5d59d756580315f139037fb2d Mon Sep 17 00:00:00 2001 From: Richard Newman Date: Thu, 20 Jun 2013 09:56:49 -0700 Subject: [PATCH] Bug 878677 - Part 2: Sync changes to support FormHistory.jsm. r=rnewman --- services/sync/modules/engines/forms.js | 247 +++++++----------- services/sync/modules/util.js | 4 +- services/sync/tests/unit/head_appinfo.js | 8 + services/sync/tests/unit/test_forms_store.js | 10 +- .../sync/tests/unit/test_forms_tracker.js | 29 +- 5 files changed, 133 insertions(+), 165 deletions(-) diff --git a/services/sync/modules/engines/forms.js b/services/sync/modules/engines/forms.js index fbbafac87b2..2eca83ef6e5 100644 --- a/services/sync/modules/engines/forms.js +++ b/services/sync/modules/engines/forms.js @@ -33,120 +33,63 @@ Utils.deferGetSet(FormRec, "cleartext", ["name", "value"]); let FormWrapper = { _log: Log4Moz.repository.getLogger("Sync.Engine.Forms"), - _getEntryCols: ["name", "value"], + _getEntryCols: ["fieldname", "value"], _guidCols: ["guid"], - _stmts: {}, - _getStmt: function _getStmt(query) { - if (query in this._stmts) { - return this._stmts[query]; - } - - this._log.trace("Creating SQL statement: " + query); - let db = Svc.Form.DBConnection; - return this._stmts[query] = db.createAsyncStatement(query); + // Do a "sync" search by spinning the event loop until it completes. + _searchSpinningly: function(terms, searchData) { + let results = []; + let cb = Async.makeSpinningCallback(); + let callbacks = { + handleResult: function(result) { + results.push(result); + }, + handleCompletion: function(reason) { + cb(null, results); + } + }; + Svc.FormHistory.search(terms, searchData, callbacks); + return cb.wait(); }, - _finalize : function () { - for each (let stmt in FormWrapper._stmts) { - stmt.finalize(); - } - FormWrapper._stmts = {}; + _updateSpinningly: function(changes) { + let cb = Async.makeSpinningCallback(); + let callbacks = { + handleCompletion: function(reason) { + cb(); + } + }; + Svc.FormHistory.update(changes, callbacks); + return cb.wait(); }, - get _getAllEntriesStmt() { - const query = - "SELECT fieldname name, value FROM moz_formhistory " + - "ORDER BY 1.0 * (lastUsed - (SELECT lastUsed FROM moz_formhistory ORDER BY lastUsed ASC LIMIT 1)) / " + - "((SELECT lastUsed FROM moz_formhistory ORDER BY lastUsed DESC LIMIT 1) - (SELECT lastUsed FROM moz_formhistory ORDER BY lastUsed ASC LIMIT 1)) * " + - "timesUsed / (SELECT timesUsed FROM moz_formhistory ORDER BY timesUsed DESC LIMIT 1) DESC " + - "LIMIT 500"; - return this._getStmt(query); - }, - - get _getEntryStmt() { - const query = "SELECT fieldname name, value FROM moz_formhistory " + - "WHERE guid = :guid"; - return this._getStmt(query); - }, - - get _getGUIDStmt() { - const query = "SELECT guid FROM moz_formhistory " + - "WHERE fieldname = :name AND value = :value"; - return this._getStmt(query); - }, - - get _setGUIDStmt() { - const query = "UPDATE moz_formhistory SET guid = :guid " + - "WHERE fieldname = :name AND value = :value"; - return this._getStmt(query); - }, - - get _hasGUIDStmt() { - const query = "SELECT guid FROM moz_formhistory WHERE guid = :guid LIMIT 1"; - return this._getStmt(query); - }, - - get _replaceGUIDStmt() { - const query = "UPDATE moz_formhistory SET guid = :newGUID " + - "WHERE guid = :oldGUID"; - return this._getStmt(query); - }, - - getAllEntries: function getAllEntries() { - return Async.querySpinningly(this._getAllEntriesStmt, this._getEntryCols); - }, - - getEntry: function getEntry(guid) { - let stmt = this._getEntryStmt; - stmt.params.guid = guid; - return Async.querySpinningly(stmt, this._getEntryCols)[0]; - }, - - getGUID: function getGUID(name, value) { - // Query for the provided entry. - let getStmt = this._getGUIDStmt; - getStmt.params.name = name; - getStmt.params.value = value; - - // Give the GUID if we found one. - let item = Async.querySpinningly(getStmt, this._guidCols)[0]; - - if (!item) { - // Shouldn't happen, but Bug 597400... - // Might as well just return. - this._log.warn("GUID query returned " + item + "; turn on Trace logging for details."); - this._log.trace("getGUID(" + JSON.stringify(name) + ", " + - JSON.stringify(value) + ") => " + item); + getEntry: function (guid) { + let results = this._searchSpinningly(this._getEntryCols, {guid: guid}); + if (!results.length) { return null; } + return {name: results[0].fieldname, value: results[0].value}; + }, - if (item.guid != null) { - return item.guid; + getGUID: function (name, value) { + // Query for the provided entry. + let query = { fieldname: name, value: value }; + let results = this._searchSpinningly(this._guidCols, query); + return results.length ? results[0].guid : null; + }, + + hasGUID: function (guid) { + // We could probably use a count function here, but searchSpinningly exists... + return this._searchSpinningly(this._guidCols, {guid: guid}).length != 0; + }, + + replaceGUID: function (oldGUID, newGUID) { + let changes = { + op: "update", + guid: oldGUID, + newGuid: newGUID, } - - // We need to create a GUID for this entry. - let setStmt = this._setGUIDStmt; - let guid = Utils.makeGUID(); - setStmt.params.guid = guid; - setStmt.params.name = name; - setStmt.params.value = value; - Async.querySpinningly(setStmt); - - return guid; - }, - - hasGUID: function hasGUID(guid) { - let stmt = this._hasGUIDStmt; - stmt.params.guid = guid; - return Async.querySpinningly(stmt, this._guidCols).length == 1; - }, - - replaceGUID: function replaceGUID(oldGUID, newGUID) { - let stmt = this._replaceGUIDStmt; - stmt.params.oldGUID = oldGUID; - stmt.params.newGUID = newGUID; - Async.querySpinningly(stmt); + this._updateSpinningly(changes); } }; @@ -164,9 +107,7 @@ FormEngine.prototype = { get prefName() "history", _findDupe: function _findDupe(item) { - if (Svc.Form.entryExists(item.name, item.value)) { - return FormWrapper.getGUID(item.name, item.value); - } + return FormWrapper.getGUID(item.name, item.value); } }; @@ -176,34 +117,47 @@ function FormStore(name, engine) { FormStore.prototype = { __proto__: Store.prototype, - applyIncomingBatch: function applyIncomingBatch(records) { - return Utils.runInTransaction(Svc.Form.DBConnection, function() { - return Store.prototype.applyIncomingBatch.call(this, records); - }, this); + _processChange: function (change) { + // If this._changes is defined, then we are applying a batch, so we + // can defer it. + if (this._changes) { + this._changes.push(change); + return; + } + + // Otherwise we must handle the change synchronously, right now. + FormWrapper._updateSpinningly(change); }, - applyIncoming: function applyIncoming(record) { - Store.prototype.applyIncoming.call(this, record); - this._sleep(0); // Yield back to main thread after synchronous operation. + applyIncomingBatch: function (records) { + // We collect all the changes to be made then apply them all at once. + this._changes = []; + let failures = Store.prototype.applyIncomingBatch.call(this, records); + if (this._changes.length) { + FormWrapper._updateSpinningly(this._changes); + } + delete this._changes; + return failures; }, - getAllIDs: function FormStore_getAllIDs() { + getAllIDs: function () { + let results = FormWrapper._searchSpinningly(["guid"], []) let guids = {}; - for each (let {name, value} in FormWrapper.getAllEntries()) { - guids[FormWrapper.getGUID(name, value)] = true; + for (let result of results) { + guids[result.guid] = true; } return guids; }, - changeItemID: function FormStore_changeItemID(oldID, newID) { + changeItemID: function (oldID, newID) { FormWrapper.replaceGUID(oldID, newID); }, - itemExists: function FormStore_itemExists(id) { + itemExists: function (id) { return FormWrapper.hasGUID(id); }, - createRecord: function createRecord(id, collection) { + createRecord: function (id, collection) { let record = new FormRec(collection, id); let entry = FormWrapper.getEntry(id); if (entry != null) { @@ -215,29 +169,34 @@ FormStore.prototype = { return record; }, - create: function FormStore_create(record) { + create: function (record) { this._log.trace("Adding form record for " + record.name); - Svc.Form.addEntry(record.name, record.value); + let change = { + op: "add", + fieldname: record.name, + value: record.value + }; + this._processChange(change); }, - remove: function FormStore_remove(record) { + remove: function (record) { this._log.trace("Removing form record: " + record.id); - - // Just skip remove requests for things already gone - let entry = FormWrapper.getEntry(record.id); - if (entry == null) { - return; - } - - Svc.Form.removeEntry(entry.name, entry.value); + let change = { + op: "remove", + guid: record.id + }; + this._processChange(change); }, - update: function FormStore_update(record) { + update: function (record) { this._log.trace("Ignoring form record update request!"); }, - wipe: function FormStore_wipe() { - Svc.Form.removeAllEntries(); + wipe: function () { + let change = { + op: "remove" + }; + FormWrapper._updateSpinningly(change); } }; @@ -255,13 +214,8 @@ FormTracker.prototype = { Ci.nsIObserver, Ci.nsISupportsWeakReference]), - trackEntry: function trackEntry(name, value) { - this.addChangedID(FormWrapper.getGUID(name, value)); - this.score += SCORE_INCREMENT_MEDIUM; - }, - _enabled: false, - observe: function observe(subject, topic, data) { + observe: function (subject, topic, data) { switch (topic) { case "weave:engine:start-tracking": if (!this._enabled) { @@ -287,13 +241,10 @@ FormTracker.prototype = { } break; case "satchel-storage-changed": - if (data == "addEntry" || data == "before-removeEntry") { - subject = subject.QueryInterface(Ci.nsIArray); - let name = subject.queryElementAt(0, Ci.nsISupportsString) - .toString(); - let value = subject.queryElementAt(1, Ci.nsISupportsString) - .toString(); - this.trackEntry(name, value); + if (data == "formhistory-add" || data == "formhistory-remove") { + let guid = subject.QueryInterface(Ci.nsISupportsString).toString(); + this.addChangedID(guid); + this.score += SCORE_INCREMENT_MEDIUM; } break; case "profile-change-teardown": @@ -302,7 +253,7 @@ FormTracker.prototype = { } }, - notify: function FormTracker_notify(formElement, aWindow, actionURI) { + notify: function (formElement, aWindow, actionURI) { if (this.ignoreAll) { return; } diff --git a/services/sync/modules/util.js b/services/sync/modules/util.js index 32d85b4c316..041154c19b5 100644 --- a/services/sync/modules/util.js +++ b/services/sync/modules/util.js @@ -621,13 +621,15 @@ let _sessionCID = Services.appinfo.ID == SEAMONKEY_ID ? "@mozilla.org/suite/sessionstore;1" : "@mozilla.org/browser/sessionstore;1"; -[["Form", "@mozilla.org/satchel/form-history;1", "nsIFormHistory2"], +[ ["Idle", "@mozilla.org/widget/idleservice;1", "nsIIdleService"], ["Session", _sessionCID, "nsISessionStore"] ].forEach(function([name, contract, iface]) { XPCOMUtils.defineLazyServiceGetter(Svc, name, contract, iface); }); +XPCOMUtils.defineLazyModuleGetter(Svc, "FormHistory", "resource://gre/modules/FormHistory.jsm"); + Svc.__defineGetter__("Crypto", function() { let cryptoSvc; let ns = {}; diff --git a/services/sync/tests/unit/head_appinfo.js b/services/sync/tests/unit/head_appinfo.js index 20f9a98e92d..970adb65d61 100644 --- a/services/sync/tests/unit/head_appinfo.js +++ b/services/sync/tests/unit/head_appinfo.js @@ -3,10 +3,18 @@ const {classes: Cc, interfaces: Ci, results: Cr, utils: Cu} = Components; +Cu.import("resource://gre/modules/Services.jsm"); + let gSyncProfile; gSyncProfile = do_get_profile(); +// Init FormHistoryStartup and pretend we opened a profile. +let fhs = Cc["@mozilla.org/satchel/form-history-startup;1"] + .getService(Ci.nsIObserver); +fhs.observe(null, "profile-after-change", null); + + Cu.import("resource://gre/modules/XPCOMUtils.jsm"); // Make sure to provide the right OS so crypto loads the right binaries diff --git a/services/sync/tests/unit/test_forms_store.js b/services/sync/tests/unit/test_forms_store.js index 5dc5f3e9bc2..865ba4d63aa 100644 --- a/services/sync/tests/unit/test_forms_store.js +++ b/services/sync/tests/unit/test_forms_store.js @@ -8,7 +8,8 @@ Cu.import("resource://services-sync/util.js"); function run_test() { let baseuri = "http://fake/uri/"; - let store = new FormEngine(Service)._store; + let engine = new FormEngine(Service); + let store = engine._store; function applyEnsureNoFailures(records) { do_check_eq(store.applyIncomingBatch(records).length, 0); @@ -37,6 +38,9 @@ function run_test() { } do_check_true(store.itemExists(id)); + _("Should be able to find this entry as a dupe"); + do_check_eq(engine._findDupe({name: "name!!", value: "value??"}), id); + let rec = store.createRecord(id); _("Got record for id", id, rec); do_check_eq(rec.name, "name!!"); @@ -120,9 +124,7 @@ function run_test() { value: "entry" }]); - Utils.runInTransaction(Svc.Form.DBConnection, function() { - store.wipe(); - }); + store.wipe(); for (let id in store.getAllIDs()) { do_throw("Shouldn't get any ids!"); diff --git a/services/sync/tests/unit/test_forms_tracker.js b/services/sync/tests/unit/test_forms_tracker.js index bc943e01385..3d0139962e9 100644 --- a/services/sync/tests/unit/test_forms_tracker.js +++ b/services/sync/tests/unit/test_forms_tracker.js @@ -8,46 +8,51 @@ Cu.import("resource://services-sync/util.js"); function run_test() { _("Verify we've got an empty tracker to work with."); - let tracker = new FormEngine(Service)._tracker; + let engine = new FormEngine(Service); + let tracker = engine._tracker; // Don't do asynchronous writes. tracker.persistChangedIDs = false; do_check_empty(tracker.changedIDs); Log4Moz.repository.rootLogger.addAppender(new Log4Moz.DumpAppender()); + function addEntry(name, value) { + engine._store.create({name: name, value: value}); + } + function removeEntry(name, value) { + guid = engine._findDupe({name: name, value: value}); + engine._store.remove({id: guid}); + } + try { _("Create an entry. Won't show because we haven't started tracking yet"); - Svc.Form.addEntry("name", "John Doe"); + addEntry("name", "John Doe"); do_check_empty(tracker.changedIDs); _("Tell the tracker to start tracking changes."); Svc.Obs.notify("weave:engine:start-tracking"); - Svc.Form.removeEntry("name", "John Doe"); - Svc.Form.addEntry("email", "john@doe.com"); + removeEntry("name", "John Doe"); + addEntry("email", "john@doe.com"); do_check_attribute_count(tracker.changedIDs, 2); _("Notifying twice won't do any harm."); Svc.Obs.notify("weave:engine:start-tracking"); - Svc.Form.addEntry("address", "Memory Lane"); + addEntry("address", "Memory Lane"); do_check_attribute_count(tracker.changedIDs, 3); _("Let's stop tracking again."); tracker.clearChangedIDs(); Svc.Obs.notify("weave:engine:stop-tracking"); - Svc.Form.removeEntry("address", "Memory Lane"); + removeEntry("address", "Memory Lane"); do_check_empty(tracker.changedIDs); _("Notifying twice won't do any harm."); Svc.Obs.notify("weave:engine:stop-tracking"); - Svc.Form.removeEntry("email", "john@doe.com"); + removeEntry("email", "john@doe.com"); do_check_empty(tracker.changedIDs); - _("Test error detection."); - // This throws an exception without the fix for Bug 597400. - tracker.trackEntry("foo", "bar"); - } finally { _("Clean up."); - Svc.Form.removeAllEntries(); + engine._store.wipe(); } }