Backed out changeset 80fce16cfb57 (bug 1317223)

--HG--
extra : rebase_source : 0d4ef9da6dc87d6868506dc2950c3802bcae300c
This commit is contained in:
Carsten "Tomcat" Book 2017-03-02 13:08:40 +01:00
Родитель 90603e0461
Коммит 1b0724ee1d
6 изменённых файлов: 2 добавлений и 702 удалений

Просмотреть файл

@ -6,14 +6,11 @@
const Cu = Components.utils;
this.EXPORTED_SYMBOLS = ["BookmarkRepairRequestor", "BookmarkRepairResponder"];
this.EXPORTED_SYMBOLS = ["BookmarkRepairRequestor"];
Cu.import("resource://gre/modules/Services.jsm");
Cu.import("resource://gre/modules/Preferences.jsm");
Cu.import("resource://gre/modules/Log.jsm");
Cu.import("resource://gre/modules/PlacesUtils.jsm");
Cu.import("resource://gre/modules/PlacesSyncUtils.jsm");
Cu.import("resource://services-sync/util.js");
Cu.import("resource://services-sync/collection_repair.js");
Cu.import("resource://services-sync/constants.js");
Cu.import("resource://services-sync/resource.js");
@ -534,158 +531,3 @@ class BookmarkRepairRequestor extends CollectionRepairRequestor {
return AsyncResource.serverTime;
}
}
/* An object that responds to repair requests initiated by some other device.
*/
class BookmarkRepairResponder extends CollectionRepairResponder {
async repair(request, rawCommand) {
if (request.request != "upload") {
this._abortRepair(request, rawCommand,
`Don't understand request type '${request.request}'`);
return;
}
// Note that we don't try and guard against multiple repairs being in
// progress as we don't do anything too smart that could cause problems,
// but just upload items. If we get any smarter we should re-think this
// (but when we do, note that checking this._currentState isn't enough as
// this responder is not a singleton)
let engine = this.service.engineManager.get("bookmarks");
// Some items have been requested, but we need to be careful about how we
// handle them:
// * The item exists locally, but isn't in the tree of items we sync (eg, it
// might be a left-pane item or similar.) We write a tombstone for these.
// * The item exists locally as a folder - and the children of the folder
// also don't exist on the server - just uploading the folder isn't going
// to help. (Note that we assume the parents *do* exist, otherwise the
// device requesting the item be uploaded wouldn't be aware it exists)
// Bug 1343101 covers additional issues we might repair in the future.
let allIDs = new Set(); // all items we discovered inspecting the requested IDs.
let maybeToDelete = new Set(); // items we *may* delete.
let toUpload = new Set(); // items we will upload.
let results = await PlacesSyncUtils.bookmarks.fetchSyncIdsForRepair(request.ids);
for (let { syncId: id, syncable } of results) {
allIDs.add(id);
if (syncable) {
toUpload.add(id);
} else {
log.debug(`repair request to upload item ${id} but it isn't under a syncable root`);
maybeToDelete.add(id);
}
}
if (log.level <= Log.Level.Debug) {
let missingItems = request.ids.filter(id =>
!toUpload.has(id) && !maybeToDelete.has(id)
);
if (missingItems.length) {
log.debug("repair request to upload items that don't exist locally",
missingItems);
}
}
// So we've now got items we know should potentially be uploaded or deleted.
// Query the server to find out what it actually has.
let existsRemotely = new Set(); // items we determine already exist on the server
let itemSource = engine.itemSource();
itemSource.ids = Array.from(allIDs);
log.trace(`checking the server for items`, itemSource.ids);
for (let remoteID of JSON.parse(itemSource.get())) {
log.trace(`the server has "${remoteID}"`);
existsRemotely.add(remoteID);
// This item exists on the server, so remove it from toUpload if it wasn't
// explicitly requested (ie, if it's just a child of a requested item and
// it exists then there's no need to upload it, but if it was explicitly
// requested, that may be due to the requestor believing it is corrupt.
if (request.ids.indexOf(remoteID) == -1) {
toUpload.delete(remoteID);
}
}
// We only need to flag as deleted items that actually are on the server.
let toDelete = CommonUtils.difference(maybeToDelete, existsRemotely);
// whew - now add these items to the tracker "weakly" (ie, they will not
// persist in the case of a restart, but that's OK - we'll then end up here
// again.)
log.debug(`repair request will upload ${toUpload.size} items and delete ${toDelete.size} items`);
for (let id of toUpload) {
engine._modified.setWeak(id, { tombstone: false });
}
for (let id of toDelete) {
engine._modified.setWeak(id, { tombstone: true });
}
// Add an observer for the engine sync being complete.
this._currentState = {
request,
rawCommand,
toUpload,
toDelete,
}
if (toUpload.size || toDelete.size) {
// We have arranged for stuff to be uploaded, so wait until that's done.
Svc.Obs.add("weave:engine:sync:uploaded", this.onUploaded, this);
// and record in telemetry that we got this far - just incase we never
// end up doing the upload for some obscure reason.
let eventExtra = {
flowID: request.flowID,
numIDs: (toUpload.size + toDelete.size).toString(),
};
this.service.recordTelemetryEvent("repairResponse", "uploading", undefined, eventExtra);
} else {
// We were unable to help with the repair, so report that we are done.
this._finishRepair();
}
}
onUploaded(subject, data) {
if (data != "bookmarks") {
return;
}
Svc.Obs.remove("weave:engine:sync:uploaded", this.onUploaded, this);
log.debug(`bookmarks engine has uploaded stuff - creating a repair response`);
this._finishRepair();
}
_finishRepair() {
let clientsEngine = this.service.clientsEngine;
let flowID = this._currentState.request.flowID;
let response = {
request: this._currentState.request.request,
collection: "bookmarks",
clientID: clientsEngine.localID,
flowID,
ids: [],
}
for (let id of this._currentState.toUpload) {
response.ids.push(id);
}
for (let id of this._currentState.toDelete) {
response.ids.push(id);
}
let clientID = this._currentState.request.requestor;
clientsEngine.sendCommand("repairResponse", [response], clientID, { flowID });
// and nuke the request from our client.
clientsEngine.removeLocalCommand(this._currentState.rawCommand);
let eventExtra = {
flowID,
numIDs: response.ids.length.toString(),
}
this.service.recordTelemetryEvent("repairResponse", "finished", undefined, eventExtra);
this._currentState = null;
}
_abortRepair(request, rawCommand, why) {
log.warn(`aborting repair request: ${why}`);
this.service.clientsEngine.removeLocalCommand(rawCommand);
// record telemetry for this.
let eventExtra = {
flowID: request.flowID,
reason: why,
};
this.service.recordTelemetryEvent("repairResponse", "aborted", undefined, eventExtra);
// We could also consider writing a response here so the requestor can take
// some immediate action rather than timing out, but we abort only in cases
// that should be rare, so let's wait and see what telemetry tells us.
}
}

Просмотреть файл

@ -22,7 +22,6 @@ const REQUESTORS = {
}
const RESPONDERS = {
bookmarks: ["bookmark_repair.js", "BookmarkRepairResponder"],
}
// Should we maybe enforce the requestors being a singleton?

Просмотреть файл

@ -1,498 +0,0 @@
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
Cu.import("resource://gre/modules/PlacesUtils.jsm");
Cu.import("resource://gre/modules/PlacesSyncUtils.jsm");
Cu.import("resource://testing-common/PlacesTestUtils.jsm");
Cu.import("resource:///modules/PlacesUIUtils.jsm");
Cu.import("resource://gre/modules/Log.jsm");
Cu.import("resource://services-sync/engines/bookmarks.js");
Cu.import("resource://services-sync/service.js");
Cu.import("resource://services-sync/bookmark_repair.js");
Cu.import("resource://testing-common/services/sync/utils.js");
initTestLogging("Trace");
Log.repository.getLogger("Sync.Engine.Bookmarks").level = Log.Level.Trace;
// stub telemetry so we can easily check the right things are recorded.
var recordedEvents = [];
Service.recordTelemetryEvent = (object, method, value, extra = undefined) => {
recordedEvents.push({ object, method, value, extra });
}
function checkRecordedEvents(expected) {
deepEqual(recordedEvents, expected);
// and clear the list so future checks are easier to write.
recordedEvents = [];
}
function getServerBookmarks(server) {
return server.user("foo").collection("bookmarks");
}
async function setup() {
let clientsEngine = Service.clientsEngine;
let bookmarksEngine = Service.engineManager.get("bookmarks");
let server = serverForUsers({"foo": "password"}, {
meta: {
global: {
syncID: Service.syncID,
storageVersion: STORAGE_VERSION,
engines: {
clients: {
version: clientsEngine.version,
syncID: clientsEngine.syncID,
},
bookmarks: {
version: bookmarksEngine.version,
syncID: bookmarksEngine.syncID,
},
},
},
},
crypto: {
keys: encryptPayload({
id: "keys",
// Generate a fake default key bundle to avoid resetting the client
// before the first sync.
default: [
Svc.Crypto.generateRandomKey(),
Svc.Crypto.generateRandomKey(),
],
}),
},
});
await SyncTestingInfrastructure(server);
// Disable validation so that we don't try to automatically repair the server
// when we sync.
Svc.Prefs.set("engine.bookmarks.validation.enabled", false);
return server;
}
async function cleanup(server) {
await promiseStopServer(server);
await PlacesSyncUtils.bookmarks.wipe();
Svc.Prefs.reset("engine.bookmarks.validation.enabled");
}
add_task(async function test_responder_no_items() {
let server = await setup();
let request = {
request: "upload",
ids: [Utils.makeGUID()],
flowID: Utils.makeGUID(),
}
let responder = new BookmarkRepairResponder();
await responder.repair(request, null);
checkRecordedEvents([
{ object: "repairResponse",
method: "finished",
value: undefined,
extra: {flowID: request.flowID, numIDs: "0"},
},
]);
await cleanup(server);
});
// One item requested and we have it locally, but it's not yet on the server.
add_task(async function test_responder_upload() {
let server = await setup();
// Pretend we've already synced this bookmark, so that we can ensure it's
// uploaded in response to our repair request.
let bm = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
title: "Get Firefox",
url: "http://getfirefox.com/",
source: PlacesUtils.bookmarks.SOURCES.SYNC });
Service.sync();
deepEqual(getServerBookmarks(server).keys().sort(), [
"menu",
"mobile",
"toolbar",
"unfiled",
], "Should only upload roots on first sync");
let request = {
request: "upload",
ids: [bm.guid],
flowID: Utils.makeGUID(),
}
let responder = new BookmarkRepairResponder();
await responder.repair(request, null);
checkRecordedEvents([
{ object: "repairResponse",
method: "uploading",
value: undefined,
extra: {flowID: request.flowID, numIDs: "1"},
},
]);
Service.sync();
deepEqual(getServerBookmarks(server).keys().sort(), [
"menu",
"mobile",
"toolbar",
"unfiled",
bm.guid,
].sort(), "Should upload requested bookmark on second sync");
checkRecordedEvents([
{ object: "repairResponse",
method: "finished",
value: undefined,
extra: {flowID: request.flowID, numIDs: "1"},
},
]);
await cleanup(server);
});
// One item requested and we have it locally and it's already on the server.
// As it was explicitly requested, we should upload it.
add_task(async function test_responder_item_exists_locally() {
let server = await setup();
let bm = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
title: "Get Firefox",
url: "http://getfirefox.com/" });
// first sync to get the item on the server.
_("Syncing to get item on the server");
Service.sync();
// issue a repair request for it.
let request = {
request: "upload",
ids: [bm.guid],
flowID: Utils.makeGUID(),
}
let responder = new BookmarkRepairResponder();
await responder.repair(request, null);
// We still re-upload the item.
checkRecordedEvents([
{ object: "repairResponse",
method: "uploading",
value: undefined,
extra: {flowID: request.flowID, numIDs: "1"},
},
]);
_("Syncing to do the upload.");
Service.sync();
checkRecordedEvents([
{ object: "repairResponse",
method: "finished",
value: undefined,
extra: {flowID: request.flowID, numIDs: "1"},
},
]);
await cleanup(server);
});
add_task(async function test_responder_tombstone() {
let server = await setup();
// TODO: Request an item for which we have a tombstone locally. Decide if
// we want to store tombstones permanently for this. In the integration
// test, we can also try requesting a deleted child or ancestor.
// For now, we'll handle this identically to `test_responder_missing_items`.
// Bug 1343103 is a follow-up to better handle this.
await cleanup(server);
});
add_task(async function test_responder_missing_items() {
let server = await setup();
let fxBmk = await PlacesUtils.bookmarks.insert({
parentGuid: PlacesUtils.bookmarks.unfiledGuid,
title: "Get Firefox",
url: "http://getfirefox.com/",
});
let tbBmk = await PlacesUtils.bookmarks.insert({
parentGuid: PlacesUtils.bookmarks.unfiledGuid,
title: "Get Thunderbird",
url: "http://getthunderbird.com/",
// Pretend we've already synced Thunderbird.
source: PlacesUtils.bookmarks.SOURCES.SYNC,
});
Service.sync();
deepEqual(getServerBookmarks(server).keys().sort(), [
"menu",
"mobile",
"toolbar",
"unfiled",
fxBmk.guid,
].sort(), "Should upload roots and Firefox on first sync");
_("Request Firefox, Thunderbird, and nonexistent GUID");
let request = {
request: "upload",
ids: [fxBmk.guid, tbBmk.guid, Utils.makeGUID()],
flowID: Utils.makeGUID(),
}
let responder = new BookmarkRepairResponder();
await responder.repair(request, null);
checkRecordedEvents([
{ object: "repairResponse",
method: "uploading",
value: undefined,
extra: {flowID: request.flowID, numIDs: "2"},
},
]);
_("Sync after requesting IDs");
Service.sync();
deepEqual(getServerBookmarks(server).keys().sort(), [
"menu",
"mobile",
"toolbar",
"unfiled",
fxBmk.guid,
tbBmk.guid,
].sort(), "Second sync should upload Thunderbird; skip nonexistent");
checkRecordedEvents([
{ object: "repairResponse",
method: "finished",
value: undefined,
extra: {flowID: request.flowID, numIDs: "2"},
},
]);
await cleanup(server);
});
add_task(async function test_non_syncable() {
let server = await setup();
// Creates the left pane queries as a side effect.
let leftPaneId = PlacesUIUtils.leftPaneFolderId;
_(`Left pane root ID: ${leftPaneId}`);
await PlacesTestUtils.promiseAsyncUpdates();
// A child folder of the left pane root, containing queries for the menu,
// toolbar, and unfiled queries.
let allBookmarksId = PlacesUIUtils.leftPaneQueries.AllBookmarks;
let allBookmarksGuid = await PlacesUtils.promiseItemGuid(allBookmarksId);
// Explicitly request the unfiled query; we should also upload tombstones
// for the menu and toolbar queries.
let unfiledQueryId = PlacesUIUtils.leftPaneQueries.UnfiledBookmarks;
let unfiledQueryGuid = await PlacesUtils.promiseItemGuid(unfiledQueryId);
let request = {
request: "upload",
ids: [allBookmarksGuid, unfiledQueryGuid],
flowID: Utils.makeGUID(),
}
let responder = new BookmarkRepairResponder();
await responder.repair(request, null);
checkRecordedEvents([
{ object: "repairResponse",
method: "uploading",
value: undefined,
// Tombstones for the folder and its 3 children.
extra: {flowID: request.flowID, numIDs: "4"},
},
]);
_("Sync to upload tombstones for items");
Service.sync();
let toolbarQueryId = PlacesUIUtils.leftPaneQueries.BookmarksToolbar;
let menuQueryId = PlacesUIUtils.leftPaneQueries.BookmarksMenu;
let queryGuids = [
allBookmarksGuid,
await PlacesUtils.promiseItemGuid(toolbarQueryId),
await PlacesUtils.promiseItemGuid(menuQueryId),
unfiledQueryGuid,
];
let collection = getServerBookmarks(server);
deepEqual(collection.keys().sort(), [
// We always upload roots on the first sync.
"menu",
"mobile",
"toolbar",
"unfiled",
...queryGuids,
].sort(), "Should upload roots and queries on first sync");
for (let guid of queryGuids) {
let wbo = collection.wbo(guid);
let payload = JSON.parse(JSON.parse(wbo.payload).ciphertext);
ok(payload.deleted, `Should upload tombstone for left pane query ${guid}`);
}
checkRecordedEvents([
{ object: "repairResponse",
method: "finished",
value: undefined,
extra: {flowID: request.flowID, numIDs: "4"},
},
]);
await cleanup(server);
});
add_task(async function test_folder_descendants() {
let server = await setup();
let parentFolder = await PlacesUtils.bookmarks.insert({
type: PlacesUtils.bookmarks.TYPE_FOLDER,
parentGuid: PlacesUtils.bookmarks.menuGuid,
title: "Parent folder",
});
let childFolder = await PlacesUtils.bookmarks.insert({
type: PlacesUtils.bookmarks.TYPE_FOLDER,
parentGuid: parentFolder.guid,
title: "Child folder",
});
// This item is in parentFolder and *should not* be uploaded as part of
// the repair even though we explicitly request its parent.
let existingChildBmk = await PlacesUtils.bookmarks.insert({
parentGuid: parentFolder.guid,
title: "Get Firefox",
url: "http://firefox.com",
});
// This item is in parentFolder and *should* be uploaded as part of
// the repair because we explicitly request its ID.
let childSiblingBmk = await PlacesUtils.bookmarks.insert({
parentGuid: parentFolder.guid,
title: "Get Thunderbird",
url: "http://getthunderbird.com",
});
_("Initial sync to upload roots and parent folder");
Service.sync();
let initialSyncIds = [
"menu",
"mobile",
"toolbar",
"unfiled",
parentFolder.guid,
existingChildBmk.guid,
childFolder.guid,
childSiblingBmk.guid,
].sort();
deepEqual(getServerBookmarks(server).keys().sort(), initialSyncIds,
"Should upload roots and partial folder contents on first sync");
_("Insert missing bookmarks locally to request later");
// Note that the fact we insert the bookmarks via PlacesSyncUtils.bookmarks.insert
// means that we are pretending Sync itself wrote them, hence they aren't
// considered "changed" locally so never get uploaded.
let childBmk = await PlacesSyncUtils.bookmarks.insert({
kind: "bookmark",
syncId: Utils.makeGUID(),
parentSyncId: parentFolder.guid,
title: "Get Firefox",
url: "http://getfirefox.com",
});
let grandChildBmk = await PlacesSyncUtils.bookmarks.insert({
kind: "bookmark",
syncId: Utils.makeGUID(),
parentSyncId: childFolder.guid,
title: "Bugzilla",
url: "https://bugzilla.mozilla.org",
});
let grandChildSiblingBmk = await PlacesSyncUtils.bookmarks.insert({
kind: "bookmark",
syncId: Utils.makeGUID(),
parentSyncId: childFolder.guid,
title: "Mozilla",
url: "https://mozilla.org",
});
_("Sync again; server contents shouldn't change");
Service.sync();
deepEqual(getServerBookmarks(server).keys().sort(), initialSyncIds,
"Second sync should not upload missing bookmarks");
// This assumes the parent record on the server is correct, and the server
// is just missing the children. This isn't a correct assumption if the
// parent's `children` array is wrong, or if the parent and children disagree.
_("Request missing bookmarks");
let request = {
request: "upload",
ids: [
// Already on server (but still uploaded as they are explicitly requested)
parentFolder.guid,
childSiblingBmk.guid,
// Explicitly upload these. We should also upload `grandChildBmk`,
// since it's a descendant of `parentFolder` and we requested its
// ancestor.
childBmk.syncId,
grandChildSiblingBmk.syncId],
flowID: Utils.makeGUID(),
}
let responder = new BookmarkRepairResponder();
await responder.repair(request, null);
checkRecordedEvents([
{ object: "repairResponse",
method: "uploading",
value: undefined,
extra: {flowID: request.flowID, numIDs: "5"},
},
]);
_("Sync after requesting repair; should upload missing records");
Service.sync();
deepEqual(getServerBookmarks(server).keys().sort(), [
...initialSyncIds,
childBmk.syncId,
grandChildBmk.syncId,
grandChildSiblingBmk.syncId,
].sort(), "Third sync should upload requested items");
checkRecordedEvents([
{ object: "repairResponse",
method: "finished",
value: undefined,
extra: {flowID: request.flowID, numIDs: "5"},
},
]);
await cleanup(server);
});
// Error handling.
add_task(async function test_aborts_unknown_request() {
let server = await setup();
let request = {
request: "not-upload",
ids: [],
flowID: Utils.makeGUID(),
}
let responder = new BookmarkRepairResponder();
await responder.repair(request, null);
checkRecordedEvents([
{ object: "repairResponse",
method: "aborted",
value: undefined,
extra: { flowID: request.flowID,
reason: "Don't understand request type 'not-upload'",
},
},
]);
await cleanup(server);
});

Просмотреть файл

@ -137,7 +137,6 @@ tags = addons
[test_bookmark_places_query_rewriting.js]
[test_bookmark_record.js]
[test_bookmark_repair_requestor.js]
[test_bookmark_repair_responder.js]
[test_bookmark_smart_bookmarks.js]
[test_bookmark_store.js]
[test_bookmark_decline_undecline.js]

Просмотреть файл

@ -126,48 +126,6 @@ const BookmarkSyncUtils = PlacesSyncUtils.bookmarks = Object.freeze({
);
}),
/**
* Returns an array of `{ syncId, syncable }` tuples for all items in
* `requestedSyncIds`. If any requested ID is a folder, all its descendants
* will be included. Ancestors of non-syncable items are not included; if
* any are missing on the server, the requesting client will need to make
* another repair request.
*
* Sync calls this method to respond to incoming bookmark repair requests
* and upload items that are missing on the server.
*/
fetchSyncIdsForRepair: Task.async(function* (requestedSyncIds) {
let requestedGuids = requestedSyncIds.map(BookmarkSyncUtils.syncIdToGuid);
let db = yield PlacesUtils.promiseDBConnection();
let rows = yield db.executeCached(`
WITH RECURSIVE
syncedItems(id) AS (
SELECT b.id FROM moz_bookmarks b
WHERE b.guid IN ('menu________', 'toolbar_____', 'unfiled_____',
'mobile______')
UNION ALL
SELECT b.id FROM moz_bookmarks b
JOIN syncedItems s ON b.parent = s.id
),
descendants(id) AS (
SELECT b.id FROM moz_bookmarks b
WHERE b.guid IN (${requestedGuids.map(guid => JSON.stringify(guid)).join(",")})
UNION ALL
SELECT b.id FROM moz_bookmarks b
JOIN descendants d ON d.id = b.parent
)
SELECT b.guid, s.id NOT NULL AS syncable
FROM descendants d
JOIN moz_bookmarks b ON b.id = d.id
LEFT JOIN syncedItems s ON s.id = d.id
`);
return rows.map(row => {
let syncId = BookmarkSyncUtils.guidToSyncId(row.getResultByName("guid"));
let syncable = !!row.getResultByName("syncable");
return { syncId, syncable };
});
}),
/**
* Migrates an array of `{ syncId, modified }` tuples from the old JSON-based
* tracker to the new sync change counter. `modified` is when the change was

Просмотреть файл

@ -20,7 +20,7 @@
"blocklist-clients.js": ["AddonBlocklistClient", "GfxBlocklistClient", "OneCRLBlocklistClient", "PluginBlocklistClient"],
"blocklist-updater.js": ["checkVersions", "addTestBlocklistClient"],
"bogus_element_type.jsm": [],
"bookmark_repair.js": ["BookmarkRepairRequestor", "BookmarkRepairResponder"],
"bookmark_repair.js": ["BookmarkRepairRequestor"],
"bookmark_validator.js": ["BookmarkValidator", "BookmarkProblemData"],
"bookmarks.js": ["BookmarksEngine", "PlacesItem", "Bookmark", "BookmarkFolder", "BookmarkQuery", "Livemark", "BookmarkSeparator"],
"bookmarks.jsm": ["PlacesItem", "Bookmark", "Separator", "Livemark", "BookmarkFolder", "DumpBookmarks"],