Bug 1638256 - Initialize mTelemetryFilename in the Connection constru…

…ctor. r=asuth Differential Revision: https://phabricator.services.mozilla.com/D181037
dothq · Jun 24, 2023 · 81f54fd · 81f54fd
1 parent cbd07d9
commit 81f54fd
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 49 deletions.
diff --git a/storage/mozStorageConnection.cpp b/storage/mozStorageConnection.cpp
@@ -4,6 +4,7 @@
  * 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/. */
 
+#include "nsError.h"
 #include "nsThreadUtils.h"
 #include "nsIFile.h"
 #include "nsIFileURL.h"
@@ -37,10 +38,13 @@
 #include "FileSystemModule.h"
 #include "mozStorageHelper.h"
 
+#include "mozilla/Assertions.h"
 #include "mozilla/Logging.h"
 #include "mozilla/Printf.h"
 #include "mozilla/ProfilerLabels.h"
+#include "mozilla/RefPtr.h"
 #include "nsProxyRelease.h"
+#include "nsStringFwd.h"
 #include "nsURLHelper.h"
 
 #define MIN_AVAILABLE_BYTES_PER_CHUNKED_GROWTH 524288000  // 500 MiB
@@ -572,7 +576,8 @@ class AsyncVacuumEvent final : public Runnable {
 
 Connection::Connection(Service* aService, int aFlags,
                        ConnectionOperation aSupportedOperations,
-                       bool aInterruptible, bool aIgnoreLockingMode)
+                       const nsCString& aTelemetryFilename, bool aInterruptible,
+                       bool aIgnoreLockingMode)
     : sharedAsyncExecutionMutex("Connection::sharedAsyncExecutionMutex"),
       sharedDBMutex("Connection::sharedDBMutex"),
       eventTargetOpenedOn(WrapNotNull(GetCurrentSerialEventTarget())),
@@ -594,6 +599,9 @@ Connection::Connection(Service* aService, int aFlags,
   MOZ_ASSERT(!mIgnoreLockingMode || mFlags & SQLITE_OPEN_READONLY,
              "Can't ignore locking for a non-readonly connection!");
   mStorageService->registerConnection(this);
+  MOZ_ASSERT(!aTelemetryFilename.IsEmpty(),
+             "A telemetry filename should have been passed-in.");
+  mTelemetryFilename.Assign(aTelemetryFilename);
 }
 
 Connection::~Connection() {
@@ -823,8 +831,6 @@ nsresult Connection::initialize(const nsACString& aStorageKey,
       mName.IsEmpty() ? nsAutoCString(":memory:"_ns)
                       : "file:"_ns + mName + "?mode=memory&cache=shared"_ns;
 
-  mTelemetryFilename.AssignLiteral(":memory:");
-
   int srv =
       ::sqlite3_open_v2(path.get(), &mDBConn, mFlags, GetBaseVFSName(true));
   if (srv != SQLITE_OK) {
@@ -860,7 +866,6 @@ nsresult Connection::initialize(nsIFile* aDatabaseFile) {
   // Do not set mFileURL here since this is database does not have an associated
   // URL.
   mDatabaseFile = aDatabaseFile;
-  aDatabaseFile->GetNativeLeafName(mTelemetryFilename);
 
   nsAutoString path;
   nsresult rv = aDatabaseFile->GetPath(path);
@@ -909,8 +914,7 @@ nsresult Connection::initialize(nsIFile* aDatabaseFile) {
   return NS_OK;
 }
 
-nsresult Connection::initialize(nsIFileURL* aFileURL,
-                                const nsACString& aTelemetryFilename) {
+nsresult Connection::initialize(nsIFileURL* aFileURL) {
   NS_ASSERTION(aFileURL, "Passed null file URL!");
   NS_ASSERTION(!connectionReady(),
                "Initialize called on already opened database!");
@@ -924,12 +928,6 @@ nsresult Connection::initialize(nsIFileURL* aFileURL,
   mFileURL = aFileURL;
   mDatabaseFile = databaseFile;
 
-  if (!aTelemetryFilename.IsEmpty()) {
-    mTelemetryFilename = aTelemetryFilename;
-  } else {
-    databaseFile->GetNativeLeafName(mTelemetryFilename);
-  }
-
   nsAutoCString spec;
   rv = aFileURL->GetSpec(spec);
   NS_ENSURE_SUCCESS(rv, rv);
@@ -990,9 +988,6 @@ nsresult Connection::initializeInternal() {
              "SQLITE_DBCONFIG_ENABLE_FTS3_TOKENIZER should be enabled");
 #endif
 
-  MOZ_ASSERT(!mTelemetryFilename.IsEmpty(),
-             "A telemetry filename should have been set by now.");
-
   // Properly wrap the database handle's mutex.
   sharedDBMutex.initWithMutex(sqlite3_db_mutex(mDBConn));
 
@@ -1714,7 +1709,7 @@ Connection::AsyncClone(bool aReadOnly,
   // The cloned connection will still implement the synchronous API, but throw
   // if any synchronous methods are called on the main thread.
   RefPtr<Connection> clone =
-      new Connection(mStorageService, flags, ASYNCHRONOUS);
+      new Connection(mStorageService, flags, ASYNCHRONOUS, mTelemetryFilename);
 
   RefPtr<AsyncInitializeClone> initEvent =
       new AsyncInitializeClone(this, clone, aReadOnly, aCallback);
@@ -1733,7 +1728,7 @@ nsresult Connection::initializeClone(Connection* aClone, bool aReadOnly) {
   if (!mStorageKey.IsEmpty()) {
     rv = aClone->initialize(mStorageKey, mName);
   } else if (mFileURL) {
-    rv = aClone->initialize(mFileURL, mTelemetryFilename);
+    rv = aClone->initialize(mFileURL);
   } else {
     rv = aClone->initialize(mDatabaseFile);
   }
@@ -1878,8 +1873,9 @@ Connection::Clone(bool aReadOnly, mozIStorageConnection** _connection) {
     flags = (~SQLITE_OPEN_CREATE & flags);
   }
 
-  RefPtr<Connection> clone = new Connection(
-      mStorageService, flags, mSupportedOperations, mInterruptible);
+  RefPtr<Connection> clone =
+      new Connection(mStorageService, flags, mSupportedOperations,
+                     mTelemetryFilename, mInterruptible);
 
   rv = initializeClone(clone, aReadOnly);
   if (NS_FAILED(rv)) {
@@ -2057,8 +2053,9 @@ Connection::GetSchemaVersion(int32_t* _version) {
 
   *_version = 0;
   bool hasResult;
-  if (NS_SUCCEEDED(stmt->ExecuteStep(&hasResult)) && hasResult)
+  if (NS_SUCCEEDED(stmt->ExecuteStep(&hasResult)) && hasResult) {
     *_version = stmt->AsInt32(0);
+  }
 
   return NS_OK;
 }

diff --git a/storage/mozStorageConnection.h b/storage/mozStorageConnection.h
@@ -32,8 +32,7 @@ class nsIEventTarget;
 class nsISerialEventTarget;
 class nsIThread;
 
-namespace mozilla {
-namespace storage {
+namespace mozilla::storage {
 
 class Connection final : public mozIStorageConnection,
                          public nsIInterfaceRequestor {
@@ -85,7 +84,8 @@ class Connection final : public mozIStorageConnection,
    */
   Connection(Service* aService, int aFlags,
              ConnectionOperation aSupportedOperations,
-             bool aInterruptible = false, bool aIgnoreLockingMode = false);
+             const nsCString& aTelemetryFilename, bool aInterruptible = false,
+             bool aIgnoreLockingMode = false);
 
   /**
    * Creates the connection to an in-memory database.
@@ -108,8 +108,7 @@ class Connection final : public mozIStorageConnection,
    *        The nsIFileURL of the location of the database to open, or create if
    * it does not exist.
    */
-  nsresult initialize(nsIFileURL* aFileURL,
-                      const nsACString& aTelemetryFilename);
+  nsresult initialize(nsIFileURL* aFileURL);
 
   /**
    * Same as initialize, but to be used on the async thread.
@@ -186,7 +185,7 @@ class Connection final : public mozIStorageConnection,
   /**
    * Closes the SQLite database, and warns about any non-finalized statements.
    */
-  nsresult internalClose(sqlite3* aDBConn);
+  nsresult internalClose(sqlite3* aNativeconnection);
 
   /**
    * Shuts down the passed-in async thread.
@@ -547,8 +546,7 @@ class CallbackComplete final : public Runnable {
   RefPtr<mozIStorageCompletionCallback> mCallback;
 };
 
-}  // namespace storage
-}  // namespace mozilla
+}  // namespace mozilla::storage
 
 /**
  * Casting Connection to nsISupports is ambiguous.

diff --git a/storage/mozStorageService.cpp b/storage/mozStorageService.cpp
@@ -7,7 +7,8 @@
 #include "mozilla/Attributes.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/SpinEventLoopUntil.h"
-
+#include "nsIFile.h"
+#include "nsIFileURL.h"
 #include "mozStorageService.h"
 #include "mozStorageConnection.h"
 #include "nsComponentManagerUtils.h"
@@ -35,8 +36,7 @@
 
 using mozilla::intl::Collator;
 
-namespace mozilla {
-namespace storage {
+namespace mozilla::storage {
 
 ////////////////////////////////////////////////////////////////////////////////
 //// Memory Reporting
@@ -467,9 +467,8 @@ Service::OpenSpecialDatabase(const nsACString& aStorageKey,
     flags |= SQLITE_OPEN_URI;
   }
 
-  RefPtr<Connection> msc =
-      new Connection(this, flags, Connection::SYNCHRONOUS, interruptible);
-
+  RefPtr<Connection> msc = new Connection(this, flags, Connection::SYNCHRONOUS,
+                                          ":memory:"_ns, interruptible);
   const nsresult rv = msc->initialize(aStorageKey, aName);
   NS_ENSURE_SUCCESS(rv, rv);
 
@@ -588,8 +587,15 @@ Service::OpenAsyncDatabase(nsIVariant* aDatabaseStore, uint32_t aOpenFlags,
   }
 
   // Create connection on this thread, but initialize it on its helper thread.
+  nsAutoCString telemetryFilename;
+  if (!storageFile) {
+    telemetryFilename.AssignLiteral("memory");
+  } else {
+    rv = storageFile->GetNativeLeafName(telemetryFilename);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
   RefPtr<Connection> msc =
-      new Connection(this, flags, Connection::ASYNCHRONOUS,
+      new Connection(this, flags, Connection::ASYNCHRONOUS, telemetryFilename,
                      /* interruptible */ true, ignoreLockingMode);
   nsCOMPtr<nsIEventTarget> target = msc->getAsyncExecutionTarget();
   MOZ_ASSERT(target,
@@ -612,10 +618,12 @@ Service::OpenDatabase(nsIFile* aDatabaseFile, uint32_t aConnectionFlags,
   // reasons.
   const int flags =
       SQLITE_OPEN_READWRITE | SQLITE_OPEN_SHAREDCACHE | SQLITE_OPEN_CREATE;
-  RefPtr<Connection> msc =
-      new Connection(this, flags, Connection::SYNCHRONOUS, interruptible);
-
-  const nsresult rv = msc->initialize(aDatabaseFile);
+  nsAutoCString telemetryFilename;
+  nsresult rv = aDatabaseFile->GetNativeLeafName(telemetryFilename);
+  NS_ENSURE_SUCCESS(rv, rv);
+  RefPtr<Connection> msc = new Connection(this, flags, Connection::SYNCHRONOUS,
+                                          telemetryFilename, interruptible);
+  rv = msc->initialize(aDatabaseFile);
   NS_ENSURE_SUCCESS(rv, rv);
 
   msc.forget(_connection);
@@ -634,10 +642,12 @@ Service::OpenUnsharedDatabase(nsIFile* aDatabaseFile, uint32_t aConnectionFlags,
   // reasons.
   const int flags =
       SQLITE_OPEN_READWRITE | SQLITE_OPEN_PRIVATECACHE | SQLITE_OPEN_CREATE;
-  RefPtr<Connection> msc =
-      new Connection(this, flags, Connection::SYNCHRONOUS, interruptible);
-
-  const nsresult rv = msc->initialize(aDatabaseFile);
+  nsAutoCString telemetryFilename;
+  nsresult rv = aDatabaseFile->GetNativeLeafName(telemetryFilename);
+  NS_ENSURE_SUCCESS(rv, rv);
+  RefPtr<Connection> msc = new Connection(this, flags, Connection::SYNCHRONOUS,
+                                          telemetryFilename, interruptible);
+  rv = msc->initialize(aDatabaseFile);
   NS_ENSURE_SUCCESS(rv, rv);
 
   msc.forget(_connection);
@@ -658,10 +668,20 @@ Service::OpenDatabaseWithFileURL(nsIFileURL* aFileURL,
   // reasons.
   const int flags = SQLITE_OPEN_READWRITE | SQLITE_OPEN_SHAREDCACHE |
                     SQLITE_OPEN_CREATE | SQLITE_OPEN_URI;
-  RefPtr<Connection> msc =
-      new Connection(this, flags, Connection::SYNCHRONOUS, interruptible);
-
-  const nsresult rv = msc->initialize(aFileURL, aTelemetryFilename);
+  nsresult rv;
+  nsAutoCString telemetryFilename;
+  if (!aTelemetryFilename.IsEmpty()) {
+    telemetryFilename = aTelemetryFilename;
+  } else {
+    nsCOMPtr<nsIFile> databaseFile;
+    rv = aFileURL->GetFile(getter_AddRefs(databaseFile));
+    NS_ENSURE_SUCCESS(rv, rv);
+    rv = databaseFile->GetNativeLeafName(telemetryFilename);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+  RefPtr<Connection> msc = new Connection(this, flags, Connection::SYNCHRONOUS,
+                                          telemetryFilename, interruptible);
+  rv = msc->initialize(aFileURL);
   NS_ENSURE_SUCCESS(rv, rv);
 
   msc.forget(_connection);
@@ -758,5 +778,4 @@ Service::Observe(nsISupports*, const char* aTopic, const char16_t*) {
   return NS_OK;
 }
 
-}  // namespace storage
-}  // namespace mozilla
+}  // namespace mozilla::storage