Bug 1215885 - Adding a shutdown phase placesClosingInternalConnection. r=froydnj
authorDavid Rajchenbach-Teller <dteller@mozilla.com>
Wed, 21 Oct 2015 12:30:30 +0200
changeset 304896 ab1666bd0cc7ac3931bcbe11481ab9ecf7c720d0
parent 304895 6735d6ed0ad17b9a3c4adcda9425aba4c98ab91d
child 304897 396ed517522291ed0af520e2f117c98a04217502
push id1001
push userraliiev@mozilla.com
push dateMon, 18 Jan 2016 19:06:03 +0000
treeherdermozilla-release@8b89261f3ac4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1215885
milestone44.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1215885 - Adding a shutdown phase placesClosingInternalConnection. r=froydnj
toolkit/components/asyncshutdown/AsyncShutdown.jsm
toolkit/components/places/PlacesUtils.jsm
--- a/toolkit/components/asyncshutdown/AsyncShutdown.jsm
+++ b/toolkit/components/asyncshutdown/AsyncShutdown.jsm
@@ -976,15 +976,16 @@ Barrier.prototype = Object.freeze({
 
 // List of well-known phases
 // 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.
 
 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-change2");
 this.AsyncShutdown.webWorkersShutdown = getPhase("web-workers-shutdown");
 this.AsyncShutdown.xpcomThreadsShutdown = getPhase("xpcom-threads-shutdown");
 
 this.AsyncShutdown.Barrier = Barrier;
 
 Object.freeze(this.AsyncShutdown);
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -2036,98 +2036,77 @@ XPCOMUtils.defineLazyGetter(PlacesUtils,
 
 XPCOMUtils.defineLazyGetter(this, "bundle", function() {
   const PLACES_STRING_BUNDLE_URI = "chrome://places/locale/places.properties";
   return Cc["@mozilla.org/intl/stringbundle;1"].
          getService(Ci.nsIStringBundleService).
          createBundle(PLACES_STRING_BUNDLE_URI);
 });
 
+/**
+ * Setup internal databases for closing properly during shutdown.
+ *
+ * 1. Places initiates shutdown.
+ * 2. Before places can move to the step where it closes the low-level connection,
+ *   we need to make sure that we have closed `conn`.
+ * 3. Before we can close `conn`, we need to make sure that all external clients
+ *   have stopped using `conn`.
+ * 4. Before we can close Sqlite, we need to close `conn`.
+ */
+function setupDbForShutdown(conn, name) {
+  try {
+    let state = "0. Not started.";
+    let promiseClosed = new Promise(resolve => {
+      // The service initiates shutdown.
+      // Before it can safely close its connection, we need to make sure
+      // that we have closed the high-level connection.
+      AsyncShutdown.placesClosingInternalConnection.addBlocker(`${name} closing as part of Places shutdown`,
+        Task.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.
+          yield conn.close();
+          state = "2. Closed Sqlite.jsm connection.";
+
+          resolve();
+        }),
+        () => state
+      );
+    });
+
+    // Make sure that Sqlite.jsm doesn't close until we are done
+    // with the high-level connection.
+    Sqlite.shutdown.addBlocker(`${name} must be closed before Sqlite.jsm`,
+      () => promiseClosed,
+      () => state
+    );
+  } catch(ex) {
+    // It's too late to block shutdown, just close the connection.
+    conn.close();
+    throw ex;
+  }
+}
+
 XPCOMUtils.defineLazyGetter(this, "gAsyncDBConnPromised",
-  () => new Promise((resolve) => {
-    Sqlite.cloneStorageConnection({
-      connection: PlacesUtils.history.DBConnection,
-      readOnly:   true
-    }).then(conn => {
-      try {
-        let state = "0. Not started.";
-        let promiseClosed = new Promise(resolve => {
-          // The service initiates shutdown.
-          // Before it can safely close its connection, we need to make sure
-          // that we have closed the high-level connection.
-          AsyncShutdown.placesClosingInternalConnection.addBlocker("PlacesUtils read-only connection closing",
-            Task.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.
-              yield conn.close();
-              state = "2. Closed Sqlite.jsm connection.";
-
-              resolve();
-            }),
-            () => state
-          );
-        });
-
-        // Make sure that Sqlite.jsm doesn't close until we are done
-        // with the high-level connection.
-        Sqlite.shutdown.addBlocker("PlacesUtils read-only connection closing",
-          () => promiseClosed,
-          () => state
-        );
-      } catch(ex) {
-        // It's too late to block shutdown, just close the connection.
-        conn.close();
-        throw ex;
-      }
-      resolve(conn);
-    });
+  () => Sqlite.cloneStorageConnection({
+    connection: PlacesUtils.history.DBConnection,
+    readOnly:   true
+  }).then(conn => {
+      setupDbForShutdown(conn, "PlacesUtils read-only connection");
+      return conn;
   })
 );
 
 XPCOMUtils.defineLazyGetter(this, "gAsyncDBWrapperPromised",
-  () => new Promise((resolve) => {
-    Sqlite.wrapStorageConnection({
+  () => Sqlite.wrapStorageConnection({
       connection: PlacesUtils.history.DBConnection,
-    }).then(conn => {
-      try {
-        let state = "0. Not started.";
-        let promiseClosed = new Promise(resolve => {
-          // The service initiates shutdown.
-          // Before it can safely close its connection, we need to make sure
-          // that we have closed the high-level connection.
-          AsyncShutdown.placesClosingInternalConnection.addBlocker("PlacesUtils wrapped connection closing",
-            Task.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.
-              yield conn.close();
-              state = "2. Closed Sqlite.jsm connection.";
-
-              resolve();
-            }),
-            () => state
-          );
-        });
-
-        // Make sure that Sqlite.jsm doesn't close until we are done
-        // with the high-level connection.
-        Sqlite.shutdown.addBlocker("PlacesUtils wrapped connection closing",
-          () => promiseClosed,
-          () => state
-        );
-      } catch(ex) {
-        // It's too late to block shutdown, just close the connection.
-        conn.close();
-        throw ex;
-      }
-      resolve(conn);
-    });
+  }).then(conn => {
+    setupDbForShutdown(conn, "PlacesUtils wrapped connection");
+    return conn;
   })
 );
 
 /**
  * Keywords management API.
  * Sooner or later these keywords will merge with search keywords, this is an
  * interim API that should then be replaced by a unified one.
  * Keywords are associated with URLs and can have POST data.