--- a/browser/components/places/tests/unit/test_clearHistory_shutdown.js
+++ b/browser/components/places/tests/unit/test_clearHistory_shutdown.js
@@ -14,17 +14,16 @@ const URIS = [
"http://b.example2.com/",
"http://c.example3.com/"
];
const TOPIC_CONNECTION_CLOSED = "places-connection-closed";
var EXPECTED_NOTIFICATIONS = [
"places-shutdown",
- "places-will-close-connection",
"places-expiration-finished",
"places-connection-closed"
];
const UNEXPECTED_NOTIFICATIONS = [
"xpcom-shutdown"
];
--- a/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_placesTitleNoUpdate.js
+++ b/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_placesTitleNoUpdate.js
@@ -1,72 +1,45 @@
/* 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/. */
// Test to make sure that the visited page titles do not get updated inside the
// private browsing mode.
"use strict";
-Cu.import("resource://gre/modules/Services.jsm");
-Cu.import("resource://gre/modules/PlacesUtils.jsm");
-
add_task(async function test() {
const TEST_URL = "http://mochi.test:8888/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_placesTitleNoUpdate.html"
- const TEST_URI = Services.io.newURI(TEST_URL);
const TITLE_1 = "Title 1";
const TITLE_2 = "Title 2";
- function waitForTitleChanged() {
- return new Promise(resolve => {
- let historyObserver = {
- onTitleChanged(uri, pageTitle) {
- PlacesUtils.history.removeObserver(historyObserver, false);
- resolve({uri, pageTitle});
- },
- onBeginUpdateBatch() {},
- onEndUpdateBatch() {},
- onVisit() {},
- onDeleteURI() {},
- onClearHistory() {},
- onPageChanged() {},
- onDeleteVisits() {},
- QueryInterface: XPCOMUtils.generateQI([Ci.nsINavHistoryObserver])
- };
-
- PlacesUtils.history.addObserver(historyObserver);
- });
- }
+ await PlacesUtils.history.clear();
- await PlacesTestUtils.clearHistory();
-
- let tabToClose = gBrowser.selectedTab = BrowserTestUtils.addTab(gBrowser, TEST_URL);
- await waitForTitleChanged();
- is(PlacesUtils.history.getPageTitle(TEST_URI), TITLE_1, "The title matches the orignal title after first visit");
+ let promiseTitleChanged = PlacesTestUtils.waitForNotification(
+ "onTitleChanged", (uri, title) => uri.spec == TEST_URL, "history");
+ let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_URL);
+ registerCleanupFunction(async () => {
+ await BrowserTestUtils.removeTab(tab);
+ });
+ info("Wait for a title change notification.");
+ await promiseTitleChanged;
+ is((await PlacesUtils.history.fetch(TEST_URL)).title, TITLE_1,
+ "The title matches the orignal title after first visit");
- let place = {
- uri: TEST_URI,
- title: TITLE_2,
- visits: [{
- visitDate: Date.now() * 1000,
- transitionType: Ci.nsINavHistoryService.TRANSITION_LINK
- }]
- };
- PlacesUtils.asyncHistory.updatePlaces(place, {
- handleError: () => ok(false, "Unexpected error in adding visit."),
- handleResult() { },
- handleCompletion() {}
- });
-
- await waitForTitleChanged();
- is(PlacesUtils.history.getPageTitle(TEST_URI), TITLE_2, "The title matches the updated title after updating visit");
+ promiseTitleChanged = PlacesTestUtils.waitForNotification(
+ "onTitleChanged", (uri, title) => uri.spec == TEST_URL, "history");
+ await PlacesTestUtils.addVisits({ uri: TEST_URL, title: TITLE_2 });
+ info("Wait for a title change notification.");
+ await promiseTitleChanged;
+ is((await PlacesUtils.history.fetch(TEST_URL)).title, TITLE_2,
+ "The title matches the orignal title after updating visit");
let privateWin = await BrowserTestUtils.openNewBrowserWindow({private: true});
- await BrowserTestUtils.browserLoaded(privateWin.gBrowser.addTab(TEST_URL).linkedBrowser);
-
- is(PlacesUtils.history.getPageTitle(TEST_URI), TITLE_2, "The title remains the same after visiting in private window");
- await PlacesTestUtils.clearHistory();
-
- // Cleanup
- BrowserTestUtils.closeWindow(privateWin);
- gBrowser.removeTab(tabToClose);
+ registerCleanupFunction(async () => {
+ await BrowserTestUtils.closeWindow(privateWin);
+ });
+ await BrowserTestUtils.openNewForegroundTab(privateWin.gBrowser, TEST_URL);
+ // Wait long enough to be sure history didn't set a title.
+ await new Promise(resolve => setTimeout(resolve, 1000));
+ is((await PlacesUtils.history.fetch(TEST_URL)).title, TITLE_2,
+ "The title remains the same after visiting in private window");
});
--- a/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_placestitle.js
+++ b/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_placestitle.js
@@ -13,83 +13,53 @@ add_task(async function test() {
function cleanup() {
// delete all cookies
cm.removeAll();
// delete all history items
return PlacesTestUtils.clearHistory();
}
await cleanup();
-
- let deferredFirst = PromiseUtils.defer();
- let deferredSecond = PromiseUtils.defer();
- let deferredThird = PromiseUtils.defer();
-
- let testNumber = 0;
- let historyObserver = {
- onTitleChanged(aURI, aPageTitle) {
- if (aURI.spec != TEST_URL)
- return;
- switch (++testNumber) {
- case 1:
- // The first time that the page is loaded
- deferredFirst.resolve(aPageTitle);
- break;
- case 2:
- // The second time that the page is loaded
- deferredSecond.resolve(aPageTitle);
- break;
- case 3:
- // After clean up
- deferredThird.resolve(aPageTitle);
- break;
- default:
- // Checks that opening the page in a private window should not fire a
- // title change.
- ok(false, "Title changed. Unexpected pass: " + testNumber);
- }
- },
-
- onBeginUpdateBatch() {},
- onEndUpdateBatch() {},
- onVisit() {},
- onDeleteURI() {},
- onClearHistory() {},
- onPageChanged() {},
- onDeleteVisits() {},
- QueryInterface: XPCOMUtils.generateQI([Ci.nsINavHistoryObserver])
- };
- PlacesUtils.history.addObserver(historyObserver);
-
+ registerCleanupFunction(cleanup);
let win = await BrowserTestUtils.openNewBrowserWindow();
- win.gBrowser.selectedTab = win.gBrowser.addTab(TEST_URL);
- let aPageTitle = await deferredFirst.promise;
- // The first time that the page is loaded
- is(aPageTitle, "No Cookie",
+ registerCleanupFunction(async () => {
+ await BrowserTestUtils.closeWindow(win);
+ });
+
+ let promiseTitleChanged = PlacesTestUtils.waitForNotification(
+ "onTitleChanged", (uri, title) => uri.spec == TEST_URL, "history");
+ await BrowserTestUtils.openNewForegroundTab(win.gBrowser, TEST_URL);
+ await promiseTitleChanged;
+ is((await PlacesUtils.history.fetch(TEST_URL)).title, "No Cookie",
"The page should be loaded without any cookie for the first time");
- win.gBrowser.selectedTab = win.gBrowser.addTab(TEST_URL);
- aPageTitle = await deferredSecond.promise;
- // The second time that the page is loaded
- is(aPageTitle, "Cookie",
+ promiseTitleChanged = PlacesTestUtils.waitForNotification(
+ "onTitleChanged", (uri, title) => uri.spec == TEST_URL, "history");
+ await BrowserTestUtils.openNewForegroundTab(win.gBrowser, TEST_URL);
+ await promiseTitleChanged;
+ is((await PlacesUtils.history.fetch(TEST_URL)).title, "Cookie",
"The page should be loaded with a cookie for the second time");
await cleanup();
- win.gBrowser.selectedTab = win.gBrowser.addTab(TEST_URL);
- aPageTitle = await deferredThird.promise;
- // After clean up
- is(aPageTitle, "No Cookie",
+ promiseTitleChanged = PlacesTestUtils.waitForNotification(
+ "onTitleChanged", (uri, title) => uri.spec == TEST_URL, "history");
+ await BrowserTestUtils.openNewForegroundTab(win.gBrowser, TEST_URL);
+ await promiseTitleChanged;
+ is((await PlacesUtils.history.fetch(TEST_URL)).title, "No Cookie",
"The page should be loaded without any cookie again");
+ // Reopen the page in a private browser window, it should not notify a title
+ // change.
let win2 = await BrowserTestUtils.openNewBrowserWindow({private: true});
-
- let private_tab = win2.gBrowser.addTab(TEST_URL);
- win2.gBrowser.selectedTab = private_tab;
- await BrowserTestUtils.browserLoaded(private_tab.linkedBrowser);
+ registerCleanupFunction(async () => {
+ let promisePBExit = TestUtils.topicObserved("last-pb-context-exited");
+ await BrowserTestUtils.closeWindow(win2);
+ await promisePBExit;
+ });
- // Cleanup
- await cleanup();
- PlacesUtils.history.removeObserver(historyObserver);
- await BrowserTestUtils.closeWindow(win);
- await BrowserTestUtils.closeWindow(win2);
+ await BrowserTestUtils.openNewForegroundTab(win2.gBrowser, TEST_URL);
+ // Wait long enough to be sure history didn't set a title.
+ await new Promise(resolve => setTimeout(resolve, 1000));
+ is((await PlacesUtils.history.fetch(TEST_URL)).title, "No Cookie",
+ "The title remains the same after visiting in private window");
});
--- a/browser/components/privatebrowsing/test/browser/head.js
+++ b/browser/components/privatebrowsing/test/browser/head.js
@@ -1,14 +1,19 @@
/* Any copyright is dedicated to the Public Domain.
* http://creativecommons.org/publicdomain/zero/1.0/ */
var {PromiseUtils} = Cu.import("resource://gre/modules/PromiseUtils.jsm", {});
+Cu.import("resource://gre/modules/Services.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils",
+ "resource://gre/modules/PlacesUtils.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "PlacesTestUtils",
"resource://testing-common/PlacesTestUtils.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "TestUtils",
+ "resource://testing-common/TestUtils.jsm");
function whenNewWindowLoaded(aOptions, aCallback) {
let win = OpenBrowserWindow(aOptions);
let focused = SimpleTest.promiseFocus(win);
let startupFinished = TestUtils.topicObserved("browser-delayed-startup-finished",
subject => subject == win).then(() => win);
Promise.all([focused, startupFinished])
.then(results => executeSoon(() => aCallback(results[1])));
--- a/toolkit/components/asyncshutdown/AsyncShutdown.jsm
+++ b/toolkit/components/asyncshutdown/AsyncShutdown.jsm
@@ -998,17 +998,16 @@ Barrier.prototype = Object.freeze({
// Ideally, phases should be registered from the component that decides
// when they start/stop. For compatibility with existing startup/shutdown
// mechanisms, we register a few phases here.
// Parent process
if (!isContent) {
this.AsyncShutdown.profileChangeTeardown = getPhase("profile-change-teardown");
this.AsyncShutdown.profileBeforeChange = getPhase("profile-before-change");
- this.AsyncShutdown.placesClosingInternalConnection = getPhase("places-will-close-connection");
this.AsyncShutdown.sendTelemetry = getPhase("profile-before-change-telemetry");
}
// Notifications that fire in the parent and content process, but should
// only have phases in the parent process.
if (!isContent) {
this.AsyncShutdown.quitApplicationGranted = getPhase("quit-application-granted");
}
--- a/toolkit/components/places/Database.cpp
+++ b/toolkit/components/places/Database.cpp
@@ -451,18 +451,27 @@ Database::GetStatement(const nsACString&
// already.
MOZ_ASSERT(mMainConn);
return mAsyncThreadStatements.GetCachedStatement(aQuery);
}
already_AddRefed<nsIAsyncShutdownClient>
Database::GetClientsShutdown()
{
- MOZ_ASSERT(mClientsShutdown);
- return mClientsShutdown->GetClient();
+ if (mClientsShutdown)
+ return mClientsShutdown->GetClient();
+ return nullptr;
+}
+
+already_AddRefed<nsIAsyncShutdownClient>
+Database::GetConnectionShutdown()
+{
+ if (mConnectionShutdown)
+ return mConnectionShutdown->GetClient();
+ return nullptr;
}
// static
already_AddRefed<Database>
Database::GetDatabase()
{
if (PlacesShutdownBlocker::IsStarted()) {
return nullptr;
--- a/toolkit/components/places/Database.h
+++ b/toolkit/components/places/Database.h
@@ -27,20 +27,16 @@
// initial shutdown work and notifies TOPIC_PLACES_SHUTDOWN to all listeners.
// Any shutdown work that requires the Places APIs should happen here.
#define TOPIC_PROFILE_CHANGE_TEARDOWN "profile-change-teardown"
// Fired when Places is shutting down. Any code should stop accessing Places
// APIs after this notification. If you need to listen for Places shutdown
// you should only use this notification, next ones are intended only for
// internal Places use.
#define TOPIC_PLACES_SHUTDOWN "places-shutdown"
-// For Internal use only. Fired when connection is about to be closed, only
-// cleanup tasks should run at this stage, nothing should be added to the
-// database, nor APIs should be called.
-#define TOPIC_PLACES_WILL_CLOSE_CONNECTION "places-will-close-connection"
// Fired when the connection has gone, nothing will work from now on.
#define TOPIC_PLACES_CONNECTION_CLOSED "places-connection-closed"
// Simulate profile-before-change. This topic may only be used by
// calling `observe` directly on the database. Used for testing only.
#define TOPIC_SIMULATE_PLACES_SHUTDOWN "test-simulate-places-shutdown"
class nsIRunnable;
@@ -82,16 +78,21 @@ public:
nsresult Init();
/**
* The AsyncShutdown client used by clients of this API to be informed of shutdown.
*/
already_AddRefed<nsIAsyncShutdownClient> GetClientsShutdown();
/**
+ * The AsyncShutdown client used by clients of this API to be informed of connection shutdown.
+ */
+ already_AddRefed<nsIAsyncShutdownClient> GetConnectionShutdown();
+
+ /**
* Getter to use when instantiating the class.
*
* @return Singleton instance of this class.
*/
static already_AddRefed<Database> GetDatabase();
/**
* Actually initialized the connection on first need.
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -2137,17 +2137,18 @@ XPCOMUtils.defineLazyGetter(this, "bundl
function setupDbForShutdown(conn, name) {
try {
let state = "0. Not started.";
let promiseClosed = new Promise((resolve, reject) => {
// The service initiates shutdown.
// Before it can safely close its connection, we need to make sure
// that we have closed the high-level connection.
try {
- AsyncShutdown.placesClosingInternalConnection.addBlocker(`${name} closing as part of Places shutdown`,
+ PlacesUtils.history.connectionShutdownClient.jsclient.addBlocker(
+ `${name} closing as part of Places shutdown`,
async function() {
state = "1. Service has initiated shutdown";
// At this stage, all external clients have finished using the
// database. We just need to close the high-level connection.
await conn.close();
state = "2. Closed Sqlite.jsm connection.";
--- a/toolkit/components/places/Shutdown.cpp
+++ b/toolkit/components/places/Shutdown.cpp
@@ -17,16 +17,29 @@ PlacesShutdownBlocker::PlacesShutdownBlo
, mCounter(sCounter++)
{
MOZ_ASSERT(NS_IsMainThread());
// During tests, we can end up with the Database singleton being resurrected.
// Make sure that each instance of DatabaseShutdown has a unique name.
if (mCounter > 1) {
mName.AppendInt(mCounter);
}
+ // Create a barrier that will be exposed to clients through GetClient(), so
+ // they can block Places shutdown.
+ nsCOMPtr<nsIAsyncShutdownService> asyncShutdown = services::GetAsyncShutdown();
+ MOZ_ASSERT(asyncShutdown);
+ if (asyncShutdown) {
+ nsCOMPtr<nsIAsyncShutdownBarrier> barrier;
+ nsresult rv = asyncShutdown->MakeBarrier(mName, getter_AddRefs(barrier));
+ MOZ_ALWAYS_SUCCEEDS(rv);
+ if (NS_SUCCEEDED(rv) && barrier) {
+ mBarrier = new nsMainThreadPtrHolder<nsIAsyncShutdownBarrier>(
+ "PlacesShutdownBlocker::mBarrier", barrier);
+ }
+ }
}
// nsIAsyncShutdownBlocker
NS_IMETHODIMP
PlacesShutdownBlocker::GetName(nsAString& aName)
{
aName = mName;
return NS_OK;
@@ -66,60 +79,29 @@ PlacesShutdownBlocker::GetState(nsIPrope
if (NS_WARN_IF(NS_FAILED(rv))) return rv;
rv = static_cast<nsIWritablePropertyBag2*>(*_state)->SetPropertyAsInterface(
NS_LITERAL_STRING("Barrier"), barrier);
if (NS_WARN_IF(NS_FAILED(rv))) return rv;
return NS_OK;
}
-// nsIAsyncShutdownBlocker
-NS_IMETHODIMP
-PlacesShutdownBlocker::BlockShutdown(nsIAsyncShutdownClient* aParentClient)
-{
- MOZ_ASSERT(false, "should always be overridden");
- return NS_ERROR_NOT_IMPLEMENTED;
-}
-
-NS_IMPL_ISUPPORTS(
- PlacesShutdownBlocker,
- nsIAsyncShutdownBlocker
-)
-
-////////////////////////////////////////////////////////////////////////////////
-
-ClientsShutdownBlocker::ClientsShutdownBlocker()
- : PlacesShutdownBlocker(NS_LITERAL_STRING("Places Clients shutdown"))
-{
- MOZ_ASSERT(NS_IsMainThread());
- // Create a barrier that will be exposed to clients through GetClient(), so
- // they can block Places shutdown.
- nsCOMPtr<nsIAsyncShutdownService> asyncShutdown = services::GetAsyncShutdown();
- MOZ_ASSERT(asyncShutdown);
- if (asyncShutdown) {
- nsCOMPtr<nsIAsyncShutdownBarrier> barrier;
- MOZ_ALWAYS_SUCCEEDS(asyncShutdown->MakeBarrier(mName, getter_AddRefs(barrier)));
- mBarrier = new nsMainThreadPtrHolder<nsIAsyncShutdownBarrier>(
- "ClientsShutdownBlocker::mBarrier", barrier);
- }
-}
-
already_AddRefed<nsIAsyncShutdownClient>
-ClientsShutdownBlocker::GetClient()
+PlacesShutdownBlocker::GetClient()
{
nsCOMPtr<nsIAsyncShutdownClient> client;
if (mBarrier) {
MOZ_ALWAYS_SUCCEEDS(mBarrier->GetClient(getter_AddRefs(client)));
}
return client.forget();
}
// nsIAsyncShutdownBlocker
NS_IMETHODIMP
-ClientsShutdownBlocker::BlockShutdown(nsIAsyncShutdownClient* aParentClient)
+PlacesShutdownBlocker::BlockShutdown(nsIAsyncShutdownClient* aParentClient)
{
MOZ_ASSERT(NS_IsMainThread());
mParentClient = new nsMainThreadPtrHolder<nsIAsyncShutdownClient>(
"ClientsShutdownBlocker::mParentClient", aParentClient);
mState = RECEIVED_BLOCK_SHUTDOWN;
if (NS_WARN_IF(!mBarrier)) {
return NS_ERROR_NOT_AVAILABLE;
@@ -129,75 +111,89 @@ ClientsShutdownBlocker::BlockShutdown(ns
MOZ_ALWAYS_SUCCEEDS(mBarrier->Wait(this));
mState = CALLED_WAIT_CLIENTS;
return NS_OK;
}
// nsIAsyncShutdownCompletionCallback
NS_IMETHODIMP
+PlacesShutdownBlocker::Done()
+{
+ MOZ_ASSERT(false, "Should always be overridden");
+ return NS_OK;
+}
+
+NS_IMPL_ISUPPORTS(
+ PlacesShutdownBlocker,
+ nsIAsyncShutdownBlocker,
+ nsIAsyncShutdownCompletionCallback
+)
+
+////////////////////////////////////////////////////////////////////////////////
+
+ClientsShutdownBlocker::ClientsShutdownBlocker()
+ : PlacesShutdownBlocker(NS_LITERAL_STRING("Places Clients shutdown"))
+{
+ // Do nothing.
+}
+
+// nsIAsyncShutdownCompletionCallback
+NS_IMETHODIMP
ClientsShutdownBlocker::Done()
{
// At this point all the clients are done, we can stop blocking the shutdown
// phase.
mState = RECEIVED_DONE;
// mParentClient is nullptr in tests.
if (mParentClient) {
nsresult rv = mParentClient->RemoveBlocker(this);
if (NS_WARN_IF(NS_FAILED(rv))) return rv;
mParentClient = nullptr;
}
mBarrier = nullptr;
return NS_OK;
}
-NS_IMPL_ISUPPORTS_INHERITED(
+NS_IMPL_ISUPPORTS_INHERITED0(
ClientsShutdownBlocker,
- PlacesShutdownBlocker,
- nsIAsyncShutdownCompletionCallback
+ PlacesShutdownBlocker
)
////////////////////////////////////////////////////////////////////////////////
ConnectionShutdownBlocker::ConnectionShutdownBlocker(Database* aDatabase)
: PlacesShutdownBlocker(NS_LITERAL_STRING("Places Connection shutdown"))
, mDatabase(aDatabase)
{
// Do nothing.
}
-// nsIAsyncShutdownBlocker
+// nsIAsyncShutdownCompletionCallback
NS_IMETHODIMP
-ConnectionShutdownBlocker::BlockShutdown(nsIAsyncShutdownClient* aParentClient)
+ConnectionShutdownBlocker::Done()
{
- MOZ_ASSERT(NS_IsMainThread());
- mParentClient = new nsMainThreadPtrHolder<nsIAsyncShutdownClient>(
- "ConnectionShutdownBlocker::mParentClient", aParentClient);
- mState = RECEIVED_BLOCK_SHUTDOWN;
+ // At this point all the clients are done, we can stop blocking the shutdown
+ // phase.
+ mState = RECEIVED_DONE;
+
// Annotate that Database shutdown started.
sIsStarted = true;
- // Fire internal database closing notification.
- nsCOMPtr<nsIObserverService> os = services::GetObserverService();
- MOZ_ASSERT(os);
- if (os) {
- Unused << os->NotifyObservers(nullptr, TOPIC_PLACES_WILL_CLOSE_CONNECTION, nullptr);
- }
- mState = NOTIFIED_OBSERVERS_PLACES_WILL_CLOSE_CONNECTION;
-
// At this stage, any use of this database is forbidden. Get rid of
// `gDatabase`. Note, however, that the database could be
// resurrected. This can happen in particular during tests.
MOZ_ASSERT(Database::gDatabase == nullptr || Database::gDatabase == mDatabase);
Database::gDatabase = nullptr;
// Database::Shutdown will invoke Complete once the connection is closed.
mDatabase->Shutdown();
mState = CALLED_STORAGESHUTDOWN;
+ mBarrier = nullptr;
return NS_OK;
}
// mozIStorageCompletionCallback
NS_IMETHODIMP
ConnectionShutdownBlocker::Complete(nsresult, nsISupports*)
{
MOZ_ASSERT(NS_IsMainThread());
--- a/toolkit/components/places/Shutdown.h
+++ b/toolkit/components/places/Shutdown.h
@@ -38,38 +38,38 @@ class Database;
* its `Done` method is invoked, and it stops blocking the shutdown phase, so
* that it can continue.
*
* PHASE 3 (Connection shutdown)
* ConnectionBlocker is registered as a profile-before-change blocker in
* Database::Init (see Database::mConnectionShutdown).
* When profile-before-change is observer by async shutdown, it calls
* ConnectionShutdownBlocker::BlockShutdown.
- * This is the last chance for any Places internal work, like privacy cleanups,
- * before the connection is closed. This a places-will-close-connection
- * notification is sent to legacy clients that must complete any operation in
- * the same tick, since we won't wait for them.
* Then the control is passed to Database::Shutdown, that executes some sanity
* checks, clears cached statements and proceeds with asyncClose.
* Once the connection is definitely closed, Database will call back
* ConnectionBlocker::Complete. At this point a final
* places-connection-closed notification is sent, for testing purposes.
*/
/**
* A base AsyncShutdown blocker in charge of shutting down Places.
*/
class PlacesShutdownBlocker : public nsIAsyncShutdownBlocker
+ , public nsIAsyncShutdownCompletionCallback
{
public:
NS_DECL_THREADSAFE_ISUPPORTS
NS_DECL_NSIASYNCSHUTDOWNBLOCKER
+ NS_DECL_NSIASYNCSHUTDOWNCOMPLETIONCALLBACK
explicit PlacesShutdownBlocker(const nsString& aName);
+ already_AddRefed<nsIAsyncShutdownClient> GetClient();
+
/**
* `true` if we have not started shutdown, i.e. if
* `BlockShutdown()` hasn't been called yet, false otherwise.
*/
static bool IsStarted() {
return sIsStarted;
}
@@ -121,45 +121,40 @@ protected:
virtual ~PlacesShutdownBlocker() {}
};
/**
* Blocker also used to wait for clients, through an owned barrier.
*/
class ClientsShutdownBlocker final : public PlacesShutdownBlocker
- , public nsIAsyncShutdownCompletionCallback
{
public:
NS_DECL_ISUPPORTS_INHERITED
- NS_DECL_NSIASYNCSHUTDOWNCOMPLETIONCALLBACK
explicit ClientsShutdownBlocker();
- NS_IMETHOD BlockShutdown(nsIAsyncShutdownClient* aParentClient) override;
-
- already_AddRefed<nsIAsyncShutdownClient> GetClient();
-
+ NS_IMETHOD Done() override;
private:
~ClientsShutdownBlocker() {}
};
/**
* Blocker used to wait when closing the database connection.
*/
class ConnectionShutdownBlocker final : public PlacesShutdownBlocker
, public mozIStorageCompletionCallback
{
public:
NS_DECL_ISUPPORTS_INHERITED
NS_DECL_MOZISTORAGECOMPLETIONCALLBACK
- NS_IMETHOD BlockShutdown(nsIAsyncShutdownClient* aParentClient) override;
+ explicit ConnectionShutdownBlocker(mozilla::places::Database* aDatabase);
- explicit ConnectionShutdownBlocker(mozilla::places::Database* aDatabase);
+ NS_IMETHOD Done() override;
private:
~ConnectionShutdownBlocker() {}
// The owning database.
// The cycle is broken in method Complete(), once the connection
// has been closed by mozStorage.
RefPtr<mozilla::places::Database> mDatabase;
--- a/toolkit/components/places/nsNavHistory.cpp
+++ b/toolkit/components/places/nsNavHistory.cpp
@@ -2900,20 +2900,33 @@ nsNavHistory::GetDBConnection(mozIStorag
return NS_OK;
}
NS_IMETHODIMP
nsNavHistory::GetShutdownClient(nsIAsyncShutdownClient **_shutdownClient)
{
NS_ENSURE_ARG_POINTER(_shutdownClient);
- RefPtr<nsIAsyncShutdownClient> client = mDB->GetClientsShutdown();
- MOZ_ASSERT(client);
+ nsCOMPtr<nsIAsyncShutdownClient> client = mDB->GetClientsShutdown();
+ if (!client) {
+ return NS_ERROR_UNEXPECTED;
+ }
client.forget(_shutdownClient);
-
+ return NS_OK;
+}
+
+NS_IMETHODIMP
+nsNavHistory::GetConnectionShutdownClient(nsIAsyncShutdownClient **_shutdownClient)
+{
+ NS_ENSURE_ARG_POINTER(_shutdownClient);
+ nsCOMPtr<nsIAsyncShutdownClient> client = mDB->GetConnectionShutdown();
+ if (!client) {
+ return NS_ERROR_UNEXPECTED;
+ }
+ client.forget(_shutdownClient);
return NS_OK;
}
NS_IMETHODIMP
nsNavHistory::AsyncExecuteLegacyQueries(nsINavHistoryQuery** aQueries,
uint32_t aQueryCount,
nsINavHistoryQueryOptions* aOptions,
mozIStorageStatementCallback* aCallback,
--- a/toolkit/components/places/nsPIPlacesDatabase.idl
+++ b/toolkit/components/places/nsPIPlacesDatabase.idl
@@ -42,11 +42,19 @@ interface nsPIPlacesDatabase : nsISuppor
[array, size_is(aQueryCount)] in nsINavHistoryQuery aQueries,
in unsigned long aQueryCount,
in nsINavHistoryQueryOptions aOptions,
in mozIStorageStatementCallback aCallback);
/**
* Hook for clients who need to perform actions during/by the end of
* the shutdown of the database.
+ * May be null if it's too late to get one.
*/
readonly attribute nsIAsyncShutdownClient shutdownClient;
+
+ /**
+ * Hook for internal clients who need to perform actions just before the
+ * connection gets closed.
+ * May be null if it's too late to get one.
+ */
+ readonly attribute nsIAsyncShutdownClient connectionShutdownClient;
};
--- a/toolkit/components/places/nsPlacesExpiration.js
+++ b/toolkit/components/places/nsPlacesExpiration.js
@@ -27,17 +27,16 @@ Cu.import("resource://gre/modules/XPCOMU
Cu.import("resource://gre/modules/Services.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils",
"resource://gre/modules/PlacesUtils.jsm");
// Constants
// Last expiration step should run before the final sync.
-const TOPIC_SHUTDOWN = "places-will-close-connection";
const TOPIC_PREF_CHANGED = "nsPref:changed";
const TOPIC_DEBUG_START_EXPIRATION = "places-debug-start-expiration";
const TOPIC_EXPIRATION_FINISHED = "places-expiration-finished";
const TOPIC_IDLE_BEGIN = "idle";
const TOPIC_IDLE_END = "active";
const TOPIC_IDLE_DAILY = "idle-daily";
const TOPIC_TESTING_MODE = "testing-mode";
const TOPIC_TEST_INTERVAL_CHANGED = "test-interval-changed";
@@ -428,47 +427,47 @@ function nsPlacesExpiration() {
// Observe our preferences branch for changes.
this._prefBranch.addObserver("", this, true);
// Create our expiration timer.
this._newTimer();
}, Cu.reportError);
// Register topic observers.
- Services.obs.addObserver(this, TOPIC_SHUTDOWN, true);
Services.obs.addObserver(this, TOPIC_DEBUG_START_EXPIRATION, true);
Services.obs.addObserver(this, TOPIC_IDLE_DAILY, true);
+
+ // Block shutdown.
+ let shutdownClient = PlacesUtils.history.connectionShutdownClient.jsclient;
+ shutdownClient.addBlocker("Places Expiration: shutdown", () => {
+ if (this._shuttingDown) {
+ return;
+ }
+ this._shuttingDown = true;
+ this.expireOnIdle = false;
+ if (this._timer) {
+ this._timer.cancel();
+ this._timer = null;
+ }
+ // If the database is dirty, we want to expire some entries, to speed up
+ // the expiration process.
+ if (this.status == STATUS.DIRTY) {
+ this._expireWithActionAndLimit(ACTION.SHUTDOWN_DIRTY, LIMIT.LARGE);
+ }
+ this._finalizeInternalStatements();
+ });
}
nsPlacesExpiration.prototype = {
-
- // nsIObserver
-
observe: function PEX_observe(aSubject, aTopic, aData) {
if (this._shuttingDown) {
return;
}
- if (aTopic == TOPIC_SHUTDOWN) {
- this._shuttingDown = true;
- this.expireOnIdle = false;
-
- if (this._timer) {
- this._timer.cancel();
- this._timer = null;
- }
-
- // If the database is dirty, we want to expire some entries, to speed up
- // the expiration process.
- if (this.status == STATUS.DIRTY) {
- this._expireWithActionAndLimit(ACTION.SHUTDOWN_DIRTY, LIMIT.LARGE);
- }
-
- this._finalizeInternalStatements();
- } else if (aTopic == TOPIC_PREF_CHANGED) {
+ if (aTopic == TOPIC_PREF_CHANGED) {
this._loadPrefsPromise = this._loadPrefs().then(() => {
if (aData == PREF_INTERVAL_SECONDS) {
// Renew the timer with the new interval value.
this._newTimer();
}
}, Cu.reportError);
} else if (aTopic == TOPIC_DEBUG_START_EXPIRATION) {
// The passed-in limit is the maximum number of visits to expire when
--- a/toolkit/components/places/tests/bookmarks/test_async_observers.js
+++ b/toolkit/components/places/tests/bookmarks/test_async_observers.js
@@ -86,33 +86,25 @@ add_task(async function test_remove_page
add_task(async function shutdown() {
// Check that async observers don't try to create async statements after
// shutdown. That would cause assertions, since the async thread is gone
// already. Note that in such a case the notifications are not fired, so we
// cannot test for them.
// Put an history notification that triggers AsyncGetBookmarksForURI between
// asyncClose() and the actual connection closing. Enqueuing a main-thread
- // event just after places-will-close-connection should ensure it runs before
+ // event as a shutdown blocker should ensure it runs before
// places-connection-closed.
// Notice this code is not using helpers cause it depends on a very specific
// order, a change in the helpers code could make this test useless.
- await new Promise(resolve => {
- Services.obs.addObserver(function onNotification() {
- Services.obs.removeObserver(onNotification, "places-will-close-connection");
- do_check_true(true, "Observed fake places shutdown");
- Services.tm.dispatchToMainThread(() => {
+ let shutdownClient = PlacesUtils.history.shutdownClient.jsclient;
+ shutdownClient.addBlocker("Places Expiration: shutdown",
+ function() {
+ Services.tm.mainThread.dispatch(() => {
// WARNING: this is very bad, never use out of testing code.
PlacesUtils.bookmarks.QueryInterface(Ci.nsINavHistoryObserver)
.onPageChanged(NetUtil.newURI("http://book.ma.rk/"),
Ci.nsINavHistoryObserver.ATTRIBUTE_FAVICON,
"test", "test");
- resolve(promiseTopicObserved("places-connection-closed"));
- });
- }, "places-will-close-connection");
- shutdownPlaces();
- });
+ }, Ci.nsIThread.DISPATCH_NORMAL);
+ });
});
-
-
-
-
--- a/toolkit/components/places/tests/expiration/head_expiration.js
+++ b/toolkit/components/places/tests/expiration/head_expiration.js
@@ -16,24 +16,16 @@ Cu.import("resource://gre/modules/Servic
/* import-globals-from ../head_common.js */
let commonFile = do_get_file("../head_common.js", false);
let uri = Services.io.newFileURI(commonFile);
Services.scriptloader.loadSubScript(uri.spec, this);
}
// Put any other stuff relative to this test folder below.
-
-// Simulates an expiration at shutdown.
-function shutdownExpiration() {
- let expire = Cc["@mozilla.org/places/expiration;1"].getService(Ci.nsIObserver);
- expire.observe(null, "places-will-close-connection", null);
-}
-
-
/**
* Causes expiration component to start, otherwise it would wait for the first
* history notification.
*/
function force_expiration_start() {
Cc["@mozilla.org/places/expiration;1"]
.getService(Ci.nsIObserver)
.observe(null, "testing-mode", null);