Bug 1437516 - Remove nsINavHistoryService/nsINavBookmarksService::runInBatchMode. r=mak

MozReview-Commit-ID: D1H3xdjpMAx

--HG--
extra : rebase_source : 3ad1d72758c20bf0b4964d212565c720654984e0
This commit is contained in:
Mark Banner 2018-03-13 16:19:33 +00:00
Родитель 44322b0b23
Коммит 5d0c2063f4
15 изменённых файлов: 59 добавлений и 385 удалений

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

@ -2,7 +2,7 @@
http://creativecommons.org/publicdomain/zero/1.0/ */
/**
* Tests that Library handles correctly batch deletes.
* Tests that Library correctly handles deletes.
*/
const TEST_URL = "http://www.batch.delete.me/";
@ -19,27 +19,26 @@ add_task(async function test_setup() {
});
});
add_task(async function test_create_and_batch_remove_bookmarks() {
let testURI = makeURI(TEST_URL);
PlacesUtils.history.runInBatchMode({
runBatched(aUserData) {
// Create a folder in unserted and populate it with bookmarks.
let folder = PlacesUtils.bookmarks.createFolder(
PlacesUtils.unfiledBookmarksFolderId, "deleteme",
PlacesUtils.bookmarks.DEFAULT_INDEX
);
PlacesUtils.bookmarks.createFolder(
PlacesUtils.unfiledBookmarksFolderId, "keepme",
PlacesUtils.bookmarks.DEFAULT_INDEX
);
for (let i = 0; i < 10; i++) {
PlacesUtils.bookmarks.insertBookmark(folder,
testURI,
PlacesUtils.bookmarks.DEFAULT_INDEX,
"bm" + i);
}
}
}, null);
add_task(async function test_create_and_remove_bookmarks() {
let bmChildren = [];
for (let i = 0; i < 10; i++) {
bmChildren.push({
title: `bm${i}`,
url: TEST_URL,
});
}
await PlacesUtils.bookmarks.insertTree({
guid: PlacesUtils.bookmarks.unfiledGuid,
children: [{
title: "deleteme",
type: PlacesUtils.bookmarks.TYPE_FOLDER,
children: bmChildren
}, {
title: "keepme",
type: PlacesUtils.bookmarks.TYPE_FOLDER,
}],
});
// Select and open the left pane "History" query.
let PO = gLibrary.PlacesOrganizer;
@ -47,10 +46,10 @@ add_task(async function test_create_and_batch_remove_bookmarks() {
Assert.notEqual(PO._places.selectedNode, null, "Selected unsorted bookmarks");
let unsortedNode = PlacesUtils.asContainer(PO._places.selectedNode);
unsortedNode.containerOpen = true;
Assert.equal(unsortedNode.childCount, 2, "Unsorted node has 2 children");
let folderNode = unsortedNode.getChild(0);
Assert.equal(folderNode.title, "deleteme", "Folder found in unsorted bookmarks");
// Check delete command is available.
PO._places.selectNode(folderNode);
Assert.equal(PO._places.selectedNode.title, "deleteme", "Folder node selected");
@ -58,17 +57,17 @@ add_task(async function test_create_and_batch_remove_bookmarks() {
"Delete command is enabled");
let promiseItemRemovedNotification = PlacesTestUtils.waitForNotification(
"onItemRemoved", (itemId, parentId, index, type, uri, guid) => guid == folderNode.bookmarkGuid);
// Execute the delete command and check bookmark has been removed.
PO._places.controller.doCommand("cmd_delete");
await promiseItemRemovedNotification;
Assert.ok(!(await PlacesUtils.bookmarks.fetch({url: testURI})),
Assert.ok(!(await PlacesUtils.bookmarks.fetch({url: TEST_URL})),
"Bookmark has been correctly removed");
// Test live update.
Assert.equal(unsortedNode.childCount, 1, "Unsorted node has 1 child");
Assert.equal(PO._places.selectedNode.title, "keepme", "Folder node selected");
unsortedNode.containerOpen = false;
});
add_task(async function test_ensure_correct_selection_and_functionality() {
@ -81,11 +80,13 @@ add_task(async function test_ensure_correct_selection_and_functionality() {
ContentTree.view.selectNode(ContentTree.view.result.root.getChild(0));
Assert.equal(ContentTree.view.selectedNode.title, "keepme",
"Found folder in content pane");
// Test live update.
PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
makeURI(TEST_URL),
PlacesUtils.bookmarks.DEFAULT_INDEX,
"bm");
await PlacesUtils.bookmarks.insert({
parentGuid: PlacesUtils.bookmarks.unfiledGuid,
title: "bm",
url: TEST_URL,
});
Assert.equal(ContentTree.view.result.root.childCount, 2,
"Right pane was correctly updated");
});

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

@ -242,63 +242,6 @@ add_task(async function test_tracking() {
}
});
add_task(async function test_batch_tracking() {
_("Test tracker does the correct thing during and after a places 'batch'");
await startTracking();
PlacesUtils.bookmarks.runInBatchMode({
runBatched() {
PlacesUtils.bookmarks.createFolder(
PlacesUtils.bookmarks.bookmarksMenuFolder,
"Test Folder", PlacesUtils.bookmarks.DEFAULT_INDEX);
// We should be tracking the new folder and its parent (and need to jump
// through blocking hoops...)
promiseSpinningly(verifyTrackedCount(2));
// But not have bumped the score.
Assert.equal(tracker.score, 0);
}
}, null);
// Out of batch mode - tracker should be the same, but score should be up.
await verifyTrackedCount(2);
Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE);
await cleanup();
});
add_task(async function test_nested_batch_tracking() {
_("Test tracker does the correct thing if a places 'batch' is nested");
await startTracking();
PlacesUtils.bookmarks.runInBatchMode({
runBatched() {
PlacesUtils.bookmarks.runInBatchMode({
runBatched() {
PlacesUtils.bookmarks.createFolder(
PlacesUtils.bookmarks.bookmarksMenuFolder,
"Test Folder", PlacesUtils.bookmarks.DEFAULT_INDEX);
// We should be tracking the new folder and its parent (and need to jump
// through blocking hoops...)
promiseSpinningly(verifyTrackedCount(2));
// But not have bumped the score.
Assert.equal(tracker.score, 0);
}
}, null);
_("inner batch complete.");
// should still not have a score as the outer batch is pending.
promiseSpinningly(verifyTrackedCount(2));
Assert.equal(tracker.score, 0);
}
}, null);
// Out of both batches - tracker should be the same, but score should be up.
await verifyTrackedCount(2);
Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE);
await cleanup();
});
add_task(async function test_tracker_sql_batching() {
_("Test tracker does the correct thing when it is forced to batch SQL queries");

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

@ -491,19 +491,4 @@ interface nsINavBookmarksService : nsISupports
*/
void getObservers([optional] out unsigned long count,
[retval, array, size_is(count)] out nsINavBookmarkObserver observers);
/**
* Runs the passed callback inside of a database transaction.
* Use this when a lot of things are about to change, for example
* adding or deleting a large number of bookmark items. Calls can
* be nested. Observers are notified when batches begin and end, via
* nsINavBookmarkObserver.onBeginUpdateBatch/onEndUpdateBatch.
*
* @param aCallback
* nsINavHistoryBatchCallback interface to call.
* @param aUserData
* Opaque parameter passed to nsINavBookmarksBatchCallback
*/
void runInBatchMode(in nsINavHistoryBatchCallback aCallback,
in nsISupports aUserData);
};

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

@ -1392,19 +1392,6 @@ interface nsINavHistoryService : nsISupports
void getObservers([optional] out unsigned long count,
[retval, array, size_is(count)] out nsINavHistoryObserver observers);
/**
* Runs the passed callback in batch mode. Use this when a lot of things
* are about to change. Calls can be nested, observers will only be
* notified when all batches begin/end.
*
* @param aCallback
* nsINavHistoryBatchCallback interface to call.
* @param aUserData
* Opaque parameter passed to nsINavBookmarksBatchCallback
*/
void runInBatchMode(in nsINavHistoryBatchCallback aCallback,
in nsISupports aClosure);
/**
* True if history is disabled. currently,
* history is disabled if the places.history.enabled pref is false.
@ -1433,11 +1420,3 @@ interface nsINavHistoryService : nsISupports
*/
unsigned long long hashURL(in ACString aSpec, [optional] in ACString aMode);
};
/**
* @see runInBatchMode of nsINavHistoryService/nsINavBookmarksService
*/
[scriptable, function, uuid(5a5a9154-95ac-4e3d-90df-558816297407)]
interface nsINavHistoryBatchCallback : nsISupports {
void runBatched(in nsISupports aUserData);
};

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

@ -163,7 +163,6 @@ nsNavBookmarks::nsNavBookmarks()
, mToolbarRoot(0)
, mMobileRoot(0)
, mCanNotify(false)
, mBatching(false)
{
NS_ASSERTION(!gBookmarksService,
"Attempting to create two instances of the service!");
@ -1943,27 +1942,6 @@ nsNavBookmarks::GetBookmarksForURI(nsIURI* aURI,
NS_IMETHODIMP
nsNavBookmarks::RunInBatchMode(nsINavHistoryBatchCallback* aCallback,
nsISupports* aUserData) {
AUTO_PROFILER_LABEL("nsNavBookmarks::RunInBatchMode", OTHER);
NS_ENSURE_ARG(aCallback);
mBatching = true;
// Just forward the request to history. History service must exist for
// bookmarks to work and we are observing it, thus batch notifications will be
// forwarded to bookmarks observers.
nsNavHistory* history = nsNavHistory::GetHistoryService();
NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY);
nsresult rv = history->RunInBatchMode(aCallback, aUserData);
NS_ENSURE_SUCCESS(rv, rv);
return NS_OK;
}
NS_IMETHODIMP
nsNavBookmarks::AddObserver(nsINavBookmarkObserver* aObserver,
bool aOwnsWeak)
@ -2097,10 +2075,6 @@ nsNavBookmarks::OnBeginUpdateBatch()
NS_IMETHODIMP
nsNavBookmarks::OnEndUpdateBatch()
{
if (mBatching) {
mBatching = false;
}
NOTIFY_OBSERVERS(mCanNotify, mObservers,
nsINavBookmarkObserver, OnEndUpdateBatch());
return NS_OK;

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

@ -382,10 +382,6 @@ private:
// Used to enable and disable the observer notifications.
bool mCanNotify;
// Tracks whether we are in batch mode.
// Note: this is only tracking bookmarks batches, not history ones.
bool mBatching;
};
#endif // nsNavBookmarks_h_

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

@ -2482,17 +2482,6 @@ nsNavHistory::EndUpdateBatch()
return NS_OK;
}
NS_IMETHODIMP
nsNavHistory::RunInBatchMode(nsINavHistoryBatchCallback* aCallback,
nsISupports* aUserData)
{
NS_ASSERTION(NS_IsMainThread(), "This can only be called on the main thread");
NS_ENSURE_ARG(aCallback);
UpdateBatchScoper batch(*this);
return aCallback->RunBatched(aUserData);
}
NS_IMETHODIMP
nsNavHistory::GetHistoryDisabled(bool *_retval)
{

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

@ -101,25 +101,6 @@ add_task(function setup() {
PlacesUtils.bookmarks.addObserver(gBookmarkSkipObserver);
});
add_task(async function batch() {
let promise = Promise.all([
gBookmarksObserver.setup([
{ name: "onBeginUpdateBatch",
args: [] },
{ name: "onEndUpdateBatch",
args: [] },
]),
gBookmarkSkipObserver.setup([
"onBeginUpdateBatch", "onEndUpdateBatch"
])]);
PlacesUtils.bookmarks.runInBatchMode({
runBatched() {
// Nothing.
}
}, null);
await promise;
});
add_task(async function onItemAdded_bookmark() {
const title = "Bookmark 1";
let uri = Services.io.newURI("http://1.mozilla.org/");

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

@ -95,28 +95,7 @@ add_task(async function test_onlyBookmarked() {
info("begin live-update test");
compareArrayToResult(testData, root);
info("end live-update test");
/*
// we are actually not updating during a batch.
// see bug 432706 for details.
// Here's a batch update
var updateBatch = {
runBatched: function (aUserData) {
liveUpdateTestData[0].uri = "http://bookmarked3.com";
liveUpdateTestData[1].uri = "http://bookmarked-elsewhere3.com";
populateDB(liveUpdateTestData);
testData.push(liveUpdateTestData[0]);
testData.push(liveUpdateTestData[1]);
}
};
PlacesUtils.history.runInBatchMode(updateBatch, null);
// re-query and test
do_print("begin batched test");
compareArrayToResult(testData, root);
do_print("end batched test");
*/
// Close the container when finished
root.containerOpen = false;
});

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

@ -1,53 +0,0 @@
/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
/* vim:set ts=2 sw=2 sts=2 et: */
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
var hs = Cc["@mozilla.org/browser/nav-history-service;1"].
getService(Ci.nsINavHistoryService);
/**
* The callback object for runInBatchMode.
*
* @param aService
* Takes a reference to the history service or the bookmark service.
* This determines which service should be called when calling the second
* runInBatchMode the second time.
*/
function callback(aService) {
this.callCount = 0;
this.service = aService;
}
callback.prototype = {
// nsINavHistoryBatchCallback
runBatched(aUserData) {
this.callCount++;
if (this.callCount == 1) {
// We want to call run in batched once more.
this.service.runInBatchMode(this, null);
return;
}
Assert.equal(this.callCount, 2);
do_test_finished();
},
// nsISupports
QueryInterface: XPCOMUtils.generateQI([Ci.nsINavHistoryBatchCallback])
};
function run_test() {
// checking the history service
do_test_pending();
hs.runInBatchMode(new callback(hs), null);
// checking the bookmark service
do_test_pending();
PlacesUtils.bookmarks.runInBatchMode(new callback(PlacesUtils.bookmarks), null);
}

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

@ -148,14 +148,10 @@ async function task_setCountRank(aURI, aCount, aRank, aSearch, aBookmark) {
* Decay the adaptive entries by sending the daily idle topic.
*/
function doAdaptiveDecay() {
PlacesUtils.history.runInBatchMode({
runBatched() {
for (let i = 0; i < 10; i++) {
PlacesUtils.history.QueryInterface(Ci.nsIObserver)
.observe(null, "idle-daily", null);
}
}
}, this);
for (let i = 0; i < 10; i++) {
PlacesUtils.history.QueryInterface(Ci.nsIObserver)
.observe(null, "idle-daily", null);
}
}
var uri1 = uri("http://site.tld/1");

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

@ -1,54 +0,0 @@
// This is testing the frankenstein situation Sync forces Places into.
// Sync does runInBatchMode() and before the callback returns the Places async
// APIs are used (either by Sync itself, or by any other code in the system)
// As seen in bug 1197856 and bug 1190131.
ChromeUtils.import("resource://gre/modules/PlacesUtils.jsm");
// This function "waits" for a promise to resolve by spinning a nested event
// loop.
function waitForPromise(promise) {
let tm = Cc["@mozilla.org/thread-manager;1"].getService();
let finalResult, finalException;
promise.then(result => {
finalResult = result;
}, err => {
finalException = err;
});
// Keep waiting until our callback is triggered (unless the app is quitting).
tm.spinEventLoopUntil(() => finalResult || finalException);
if (finalException) {
throw finalException;
}
return finalResult;
}
add_test(function() {
let testCompleted = false;
PlacesUtils.bookmarks.runInBatchMode({
runBatched() {
// create a bookmark.
let info = { parentGuid: PlacesUtils.bookmarks.unfiledGuid,
type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
url: "http://example.com/" };
let insertPromise = PlacesUtils.bookmarks.insert(info);
let bookmark = waitForPromise(insertPromise);
// Check we got a bookmark (bookmark creation failed completely in
// bug 1190131)
equal(bookmark.url, info.url);
// Check the promiseItemGuid and promiseItemId helpers - failure in these
// was the underlying reason for the failure.
let id = waitForPromise(PlacesUtils.promiseItemId(bookmark.guid));
let guid = waitForPromise(PlacesUtils.promiseItemGuid(id));
equal(guid, bookmark.guid, "id and guid round-tripped correctly");
testCompleted = true;
}
}, null);
// make sure we tested what we think we tested.
ok(testCompleted);
run_next_test();
});

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

@ -123,21 +123,6 @@ add_test(function check_history_query() {
PlacesUtils.history.clear().then(() => {
Assert.equal(root.uri, resultObserver.invalidatedContainer.uri);
// nsINavHistoryResultObserver.batching
Assert.ok(!resultObserver.inBatchMode);
PlacesUtils.history.runInBatchMode({
runBatched(aUserData) {
Assert.ok(resultObserver.inBatchMode);
}
}, null);
Assert.ok(!resultObserver.inBatchMode);
PlacesUtils.bookmarks.runInBatchMode({
runBatched(aUserData) {
Assert.ok(resultObserver.inBatchMode);
}
}, null);
Assert.ok(!resultObserver.inBatchMode);
root.containerOpen = false;
Assert.equal(resultObserver.closedContainer, resultObserver.openedContainer);
result.removeObserver(resultObserver);
@ -208,21 +193,6 @@ add_task(async function check_bookmarks_query() {
Assert.equal(resultObserver.sortingMode, options.SORT_BY_TITLE_ASCENDING);
Assert.equal(resultObserver.invalidatedContainer, result.root);
// nsINavHistoryResultObserver.batching
Assert.ok(!resultObserver.inBatchMode);
PlacesUtils.history.runInBatchMode({
runBatched(aUserData) {
Assert.ok(resultObserver.inBatchMode);
}
}, null);
Assert.ok(!resultObserver.inBatchMode);
PlacesUtils.bookmarks.runInBatchMode({
runBatched(aUserData) {
Assert.ok(resultObserver.inBatchMode);
}
}, null);
Assert.ok(!resultObserver.inBatchMode);
root.containerOpen = false;
Assert.equal(resultObserver.closedContainer, resultObserver.openedContainer);
result.removeObserver(resultObserver);
@ -241,21 +211,6 @@ add_test(function check_mixed_query() {
Assert.notEqual(resultObserver.openedContainer, null);
// nsINavHistoryResultObserver.batching
Assert.ok(!resultObserver.inBatchMode);
PlacesUtils.history.runInBatchMode({
runBatched(aUserData) {
Assert.ok(resultObserver.inBatchMode);
}
}, null);
Assert.ok(!resultObserver.inBatchMode);
PlacesUtils.bookmarks.runInBatchMode({
runBatched(aUserData) {
Assert.ok(resultObserver.inBatchMode);
}
}, null);
Assert.ok(!resultObserver.inBatchMode);
root.containerOpen = false;
Assert.equal(resultObserver.closedContainer, resultObserver.openedContainer);
result.removeObserver(resultObserver);

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

@ -26,7 +26,10 @@ var mDBConn = hs.QueryInterface(Ci.nsPIPlacesDatabase).DBConnection;
// Helpers
var defaultBookmarksMaxId = 0;
function cleanDatabase() {
async function cleanDatabase() {
// First clear any bookmarks the "proper way" to ensure caches like GuidHelper
// are properly cleared.
await PlacesUtils.bookmarks.eraseEverything();
mDBConn.executeSimpleSQL("DELETE FROM moz_places");
mDBConn.executeSimpleSQL("DELETE FROM moz_historyvisits");
mDBConn.executeSimpleSQL("DELETE FROM moz_anno_attributes");
@ -703,23 +706,25 @@ tests.push({
async setup() {
const NUM_BOOKMARKS = 20;
bs.runInBatchMode({
runBatched(aUserData) {
// Add bookmarks to two folders to better perturbate the table.
for (let i = 0; i < NUM_BOOKMARKS; i++) {
bs.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
NetUtil.newURI("http://example.com/"),
bs.DEFAULT_INDEX, "testbookmark", null,
PlacesUtils.bookmarks.SOURCES.SYNC);
}
for (let i = 0; i < NUM_BOOKMARKS; i++) {
bs.insertBookmark(PlacesUtils.toolbarFolderId,
NetUtil.newURI("http://example.com/"),
bs.DEFAULT_INDEX, "testbookmark", null,
PlacesUtils.bookmarks.SOURCES.SYNC);
}
}
}, null);
let children = [];
for (let i = 0; i < NUM_BOOKMARKS; i++) {
children.push({
title: "testbookmark",
url: "http://example.com",
});
}
// Add bookmarks to two folders to better perturbate the table.
await PlacesUtils.bookmarks.insertTree({
guid: PlacesUtils.bookmarks.unfiledGuid,
children,
source: PlacesUtils.bookmarks.SOURCES.SYNC,
});
await PlacesUtils.bookmarks.insertTree({
guid: PlacesUtils.bookmarks.toolbarGuid,
children,
source: PlacesUtils.bookmarks.SOURCES.SYNC,
});
function randomize_positions(aParent, aResultArray) {
let stmt = mDBConn.createStatement(
@ -1687,7 +1692,7 @@ add_task(async function test_preventive_maintenance() {
await test.check();
cleanDatabase();
await cleanDatabase();
}
// Sanity check: all roots should be intact

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

@ -28,7 +28,6 @@ skip-if = (os == 'win' && ccov) # Bug 1423667
# Bug 821781: test fails intermittently on Linux
skip-if = os == "linux"
[test_402799.js]
[test_405497.js]
[test_408221.js]
[test_412132.js]
[test_413784.js]
@ -52,7 +51,6 @@ skip-if = os == "linux"
[test_adaptive_bug527311.js]
[test_annotations.js]
[test_asyncExecuteLegacyQueries.js]
[test_async_in_batchmode.js]
[test_async_transactions.js]
skip-if = (os == "win" && os_version == "5.1") # Bug 1158887
[test_bookmarks_json.js]