Bug 1603703 - Part 1: Change mozIStorageAsyncConnection::CreateFuncti…

…on to accept a nsCOMPtr rather than a raw pointer. r=asuth,mak Differential Revision: https://phabricator.services.mozilla.com/D57089 --HG-- extra : moz-landing-system : lando
dothq · Mar 5, 2020 · e6cae21 · e6cae21
1 parent bb063ab
commit e6cae21
Show file tree

Hide file tree

Showing 7 changed files with 74 additions and 89 deletions.
diff --git a/dom/cache/Connection.cpp b/dom/cache/Connection.cpp
@@ -100,7 +100,7 @@ Connection::ExecuteSimpleSQLAsync(const nsACString&,
 NS_IMETHODIMP
 Connection::CreateFunction(const nsACString& aFunctionName,
                            int32_t aNumArguments,
-                           mozIStorageFunction* aFunction) {
+                           nsCOMPtr<mozIStorageFunction> aFunction) {
   // async methods are not supported
   return NS_ERROR_NOT_IMPLEMENTED;
 }

diff --git a/dom/indexedDB/ActorsParent.cpp b/dom/indexedDB/ActorsParent.cpp
@@ -2847,23 +2847,19 @@ nsresult UpgradeSchemaFrom17_0To18_0Helper::DoUpgrade(
   MOZ_ASSERT(!aOrigin.IsEmpty());
 
   // Register the |upgrade_key| function.
-  RefPtr<UpgradeKeyFunction> updateFunction = new UpgradeKeyFunction();
-
   NS_NAMED_LITERAL_CSTRING(upgradeKeyFunctionName, "upgrade_key");
 
-  nsresult rv =
-      aConnection->CreateFunction(upgradeKeyFunctionName, 1, updateFunction);
+  nsresult rv = aConnection->CreateFunction(
+      upgradeKeyFunctionName, 1, MakeAndAddRef<UpgradeKeyFunction>());
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   // Register the |insert_idv| function.
-  RefPtr<InsertIndexDataValuesFunction> insertIDVFunction =
-      new InsertIndexDataValuesFunction();
-
   NS_NAMED_LITERAL_CSTRING(insertIDVFunctionName, "insert_idv");
 
-  rv = aConnection->CreateFunction(insertIDVFunctionName, 4, insertIDVFunction);
+  rv = aConnection->CreateFunction(
+      insertIDVFunctionName, 4, MakeAndAddRef<InsertIndexDataValuesFunction>());
   if (NS_WARN_IF(NS_FAILED(rv))) {
     MOZ_ALWAYS_SUCCEEDS(aConnection->RemoveFunction(upgradeKeyFunctionName));
     return rv;
@@ -3494,7 +3490,7 @@ nsresult UpgradeSchemaFrom19_0To20_0(nsIFile* aFMDirectory,
 
   NS_NAMED_LITERAL_CSTRING(functionName, "upgrade");
 
-  rv = aConnection->CreateFunction(functionName, 2, function);
+  rv = aConnection->CreateFunction(functionName, 2, std::move(function));
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
@@ -3701,12 +3697,10 @@ nsresult UpgradeSchemaFrom20_0To21_0(mozIStorageConnection* aConnection) {
 
   AUTO_PROFILER_LABEL("UpgradeSchemaFrom20_0To21_0", DOM);
 
-  RefPtr<UpgradeIndexDataValuesFunction> function =
-      new UpgradeIndexDataValuesFunction();
-
   NS_NAMED_LITERAL_CSTRING(functionName, "upgrade_idv");
 
-  nsresult rv = aConnection->CreateFunction(functionName, 1, function);
+  nsresult rv = aConnection->CreateFunction(
+      functionName, 1, MakeAndAddRef<UpgradeIndexDataValuesFunction>());
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
@@ -10457,7 +10451,7 @@ nsresult DatabaseConnection::BeginWriteTransaction() {
 
     rv = (*mStorageConnection)
              ->CreateFunction(NS_LITERAL_CSTRING("update_refcount"),
-                              /* aNumArguments */ 2, function);
+                              /* aNumArguments */ 2, do_AddRef(function));
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
@@ -23870,13 +23864,11 @@ nsresult CreateIndexOp::InsertDataFromObjectStore(
       aConnection->GetStorageConnection();
   MOZ_ASSERT(storageConnection);
 
-  RefPtr<UpdateIndexDataValuesFunction> updateFunction =
-      new UpdateIndexDataValuesFunction(this, aConnection);
-
   NS_NAMED_LITERAL_CSTRING(updateFunctionName, "update_index_data_values");
 
-  nsresult rv =
-      storageConnection->CreateFunction(updateFunctionName, 4, updateFunction);
+  nsresult rv = storageConnection->CreateFunction(
+      updateFunctionName, 4,
+      MakeAndAddRef<UpdateIndexDataValuesFunction>(this, aConnection));
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }

diff --git a/netwerk/cache/nsDiskCacheDeviceSQL.cpp b/netwerk/cache/nsDiskCacheDeviceSQL.cpp
@@ -1164,7 +1164,7 @@ nsresult nsOfflineCacheDevice::InitWithSqlite(mozIStorageService* ss) {
   if (!mEvictionFunction) return NS_ERROR_OUT_OF_MEMORY;
 
   rv = mDB->CreateFunction(NS_LITERAL_CSTRING("cache_eviction_observer"), 3,
-                           mEvictionFunction);
+                           do_AddRef(mEvictionFunction));
   NS_ENSURE_SUCCESS(rv, rv);
 
   // create all (most) of our statements up front
@@ -2337,8 +2337,8 @@ nsresult nsOfflineCacheDevice::Evict(
 
   nsresult rv;
 
-  nsCOMPtr<mozIStorageFunction> function1(new OriginMatch(aPattern));
-  rv = mDB->CreateFunction(NS_LITERAL_CSTRING("ORIGIN_MATCH"), 1, function1);
+  rv = mDB->CreateFunction(NS_LITERAL_CSTRING("ORIGIN_MATCH"), 1,
+                           MakeAndAddRef<OriginMatch>(aPattern));
   NS_ENSURE_SUCCESS(rv, rv);
 
   class AutoRemoveFunc {

diff --git a/netwerk/cookie/nsCookieService.cpp b/netwerk/cookie/nsCookieService.cpp
@@ -1161,24 +1161,18 @@ OpenDBResult nsCookieService::TryInitDB(bool aRecreateDB) {
 
         // Compute and populate the values of appId and inBrwoserElement from
         // originAttributes.
-        nsCOMPtr<mozIStorageFunction> setAppId(
-            new SetAppIdFromOriginAttributesSQLFunction());
-        NS_ENSURE_TRUE(setAppId, RESULT_RETRY);
-
         NS_NAMED_LITERAL_CSTRING(setAppIdName, "SET_APP_ID");
 
-        rv = mDefaultDBState->syncConn->CreateFunction(setAppIdName, 1,
-                                                       setAppId);
+        rv = mDefaultDBState->syncConn->CreateFunction(
+            setAppIdName, 1,
+            MakeAndAddRef<SetAppIdFromOriginAttributesSQLFunction>());
         NS_ENSURE_SUCCESS(rv, RESULT_RETRY);
 
-        nsCOMPtr<mozIStorageFunction> setInBrowser(
-            new SetInBrowserFromOriginAttributesSQLFunction());
-        NS_ENSURE_TRUE(setInBrowser, RESULT_RETRY);
-
         NS_NAMED_LITERAL_CSTRING(setInBrowserName, "SET_IN_BROWSER");
 
-        rv = mDefaultDBState->syncConn->CreateFunction(setInBrowserName, 1,
-                                                       setInBrowser);
+        rv = mDefaultDBState->syncConn->CreateFunction(
+            setInBrowserName, 1,
+            MakeAndAddRef<SetInBrowserFromOriginAttributesSQLFunction>());
         NS_ENSURE_SUCCESS(rv, RESULT_RETRY);
 
         rv = mDefaultDBState->syncConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(

diff --git a/storage/mozIStorageAsyncConnection.idl b/storage/mozIStorageAsyncConnection.idl
@@ -5,6 +5,12 @@
 
 #include "nsISupports.idl"
 
+%{C++
+#include "nsCOMPtr.h"
+%}
+
+native nsCOMPtr_mozIStorageFunction(nsCOMPtr<mozIStorageFunction>);
+
 interface mozIStorageAggregateFunction;
 interface mozIStorageCompletionCallback;
 interface mozIStorageFunction;
@@ -265,11 +271,12 @@ interface mozIStorageAsyncConnection : nsISupports {
    *        variable-argument functions.
    * @param aFunction
    *        The instance of mozIStorageFunction, which implements the function
-   *        in question.
+   *        in question. Takes ownership of the passed function pointer, and
+   *        discards it in case of an error.
    */
   void createFunction(in AUTF8String aFunctionName,
                       in long aNumArguments,
-                      in mozIStorageFunction aFunction);
+                      in nsCOMPtr_mozIStorageFunction aFunction);
 
   /**
    * Create a new SQL aggregate function.  If you use your connection on

diff --git a/storage/mozStorageConnection.cpp b/storage/mozStorageConnection.cpp
@@ -2057,7 +2057,7 @@ Connection::CreateTable(const char* aTableName, const char* aTableSchema) {
 NS_IMETHODIMP
 Connection::CreateFunction(const nsACString& aFunctionName,
                            int32_t aNumArguments,
-                           mozIStorageFunction* aFunction) {
+                           nsCOMPtr<mozIStorageFunction> aFunction) {
   if (!connectionReady()) {
     return NS_ERROR_NOT_INITIALIZED;
   }
@@ -2076,8 +2076,12 @@ Connection::CreateFunction(const nsACString& aFunctionName,
       SQLITE_ANY, aFunction, basicFunctionHelper, nullptr, nullptr);
   if (srv != SQLITE_OK) return convertResultCode(srv);
 
-  FunctionInfo info = {aFunction, Connection::FunctionInfo::SIMPLE,
-                       aNumArguments};
+  // TODO (Bug 1620198): we can't use std::move(aFunction) here because
+  // FunctionInfo::function is a nsCOMPtr<nsISupports> to allow for either
+  // mozIStorageFunction or mozIStorageAggregateFunction. When
+  // mozIStorageAggregateFunction is removed, this can be changed.
+  FunctionInfo info = {aFunction.forget().take(),
+                       Connection::FunctionInfo::SIMPLE, aNumArguments};
   mFunctions.Put(aFunctionName, info);
 
   return NS_OK;

diff --git a/toolkit/components/places/SQLFunctions.cpp b/toolkit/components/places/SQLFunctions.cpp
@@ -370,10 +370,9 @@ namespace places {
 
 /* static */
 nsresult MatchAutoCompleteFunction::create(mozIStorageConnection* aDBConn) {
-  RefPtr<MatchAutoCompleteFunction> function = new MatchAutoCompleteFunction();
-
   nsresult rv = aDBConn->CreateFunction(
-      NS_LITERAL_CSTRING("autocomplete_match"), kArgIndexLength, function);
+      NS_LITERAL_CSTRING("autocomplete_match"), kArgIndexLength,
+      MakeAndAddRef<MatchAutoCompleteFunction>());
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
@@ -549,10 +548,9 @@ MatchAutoCompleteFunction::OnFunctionCall(mozIStorageValueArray* aArguments,
 
 /* static */
 nsresult CalculateFrecencyFunction::create(mozIStorageConnection* aDBConn) {
-  RefPtr<CalculateFrecencyFunction> function = new CalculateFrecencyFunction();
-
-  nsresult rv = aDBConn->CreateFunction(
-      NS_LITERAL_CSTRING("calculate_frecency"), -1, function);
+  nsresult rv =
+      aDBConn->CreateFunction(NS_LITERAL_CSTRING("calculate_frecency"), -1,
+                              MakeAndAddRef<CalculateFrecencyFunction>());
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
@@ -760,9 +758,8 @@ CalculateFrecencyFunction::OnFunctionCall(mozIStorageValueArray* aArguments,
 
 /* static */
 nsresult GenerateGUIDFunction::create(mozIStorageConnection* aDBConn) {
-  RefPtr<GenerateGUIDFunction> function = new GenerateGUIDFunction();
-  nsresult rv =
-      aDBConn->CreateFunction(NS_LITERAL_CSTRING("generate_guid"), 0, function);
+  nsresult rv = aDBConn->CreateFunction(NS_LITERAL_CSTRING("generate_guid"), 0,
+                                        MakeAndAddRef<GenerateGUIDFunction>());
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
@@ -786,9 +783,8 @@ GenerateGUIDFunction::OnFunctionCall(mozIStorageValueArray* aArguments,
 
 /* static */
 nsresult IsValidGUIDFunction::create(mozIStorageConnection* aDBConn) {
-  RefPtr<IsValidGUIDFunction> function = new IsValidGUIDFunction();
   return aDBConn->CreateFunction(NS_LITERAL_CSTRING("is_valid_guid"), 1,
-                                 function);
+                                 MakeAndAddRef<IsValidGUIDFunction>());
 }
 
 NS_IMPL_ISUPPORTS(IsValidGUIDFunction, mozIStorageFunction)
@@ -813,9 +809,9 @@ IsValidGUIDFunction::OnFunctionCall(mozIStorageValueArray* aArguments,
 
 /* static */
 nsresult GetUnreversedHostFunction::create(mozIStorageConnection* aDBConn) {
-  RefPtr<GetUnreversedHostFunction> function = new GetUnreversedHostFunction();
-  nsresult rv = aDBConn->CreateFunction(
-      NS_LITERAL_CSTRING("get_unreversed_host"), 1, function);
+  nsresult rv =
+      aDBConn->CreateFunction(NS_LITERAL_CSTRING("get_unreversed_host"), 1,
+                              MakeAndAddRef<GetUnreversedHostFunction>());
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
@@ -851,9 +847,8 @@ GetUnreversedHostFunction::OnFunctionCall(mozIStorageValueArray* aArguments,
 
 /* static */
 nsresult FixupURLFunction::create(mozIStorageConnection* aDBConn) {
-  RefPtr<FixupURLFunction> function = new FixupURLFunction();
-  nsresult rv =
-      aDBConn->CreateFunction(NS_LITERAL_CSTRING("fixup_url"), 1, function);
+  nsresult rv = aDBConn->CreateFunction(NS_LITERAL_CSTRING("fixup_url"), 1,
+                                        MakeAndAddRef<FixupURLFunction>());
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
@@ -894,10 +889,9 @@ FixupURLFunction::OnFunctionCall(mozIStorageValueArray* aArguments,
 
 /* static */
 nsresult FrecencyNotificationFunction::create(mozIStorageConnection* aDBConn) {
-  RefPtr<FrecencyNotificationFunction> function =
-      new FrecencyNotificationFunction();
-  nsresult rv = aDBConn->CreateFunction(NS_LITERAL_CSTRING("notify_frecency"),
-                                        5, function);
+  nsresult rv =
+      aDBConn->CreateFunction(NS_LITERAL_CSTRING("notify_frecency"), 5,
+                              MakeAndAddRef<FrecencyNotificationFunction>());
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
@@ -943,10 +937,9 @@ FrecencyNotificationFunction::OnFunctionCall(mozIStorageValueArray* aArgs,
 
 /* static */
 nsresult StoreLastInsertedIdFunction::create(mozIStorageConnection* aDBConn) {
-  RefPtr<StoreLastInsertedIdFunction> function =
-      new StoreLastInsertedIdFunction();
-  nsresult rv = aDBConn->CreateFunction(
-      NS_LITERAL_CSTRING("store_last_inserted_id"), 2, function);
+  nsresult rv =
+      aDBConn->CreateFunction(NS_LITERAL_CSTRING("store_last_inserted_id"), 2,
+                              MakeAndAddRef<StoreLastInsertedIdFunction>());
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
@@ -993,9 +986,8 @@ StoreLastInsertedIdFunction::OnFunctionCall(mozIStorageValueArray* aArgs,
 
 /* static */
 nsresult GetQueryParamFunction::create(mozIStorageConnection* aDBConn) {
-  RefPtr<GetQueryParamFunction> function = new GetQueryParamFunction();
   return aDBConn->CreateFunction(NS_LITERAL_CSTRING("get_query_param"), 2,
-                                 function);
+                                 MakeAndAddRef<GetQueryParamFunction>());
 }
 
 NS_IMPL_ISUPPORTS(GetQueryParamFunction, mozIStorageFunction)
@@ -1024,8 +1016,8 @@ GetQueryParamFunction::OnFunctionCall(mozIStorageValueArray* aArguments,
 
 /* static */
 nsresult HashFunction::create(mozIStorageConnection* aDBConn) {
-  RefPtr<HashFunction> function = new HashFunction();
-  return aDBConn->CreateFunction(NS_LITERAL_CSTRING("hash"), -1, function);
+  return aDBConn->CreateFunction(NS_LITERAL_CSTRING("hash"), -1,
+                                 MakeAndAddRef<HashFunction>());
 }
 
 NS_IMPL_ISUPPORTS(HashFunction, mozIStorageFunction)
@@ -1064,9 +1056,8 @@ HashFunction::OnFunctionCall(mozIStorageValueArray* aArguments,
 
 /* static */
 nsresult GetPrefixFunction::create(mozIStorageConnection* aDBConn) {
-  RefPtr<GetPrefixFunction> function = new GetPrefixFunction();
-  nsresult rv =
-      aDBConn->CreateFunction(NS_LITERAL_CSTRING("get_prefix"), 1, function);
+  nsresult rv = aDBConn->CreateFunction(NS_LITERAL_CSTRING("get_prefix"), 1,
+                                        MakeAndAddRef<GetPrefixFunction>());
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
@@ -1097,9 +1088,9 @@ GetPrefixFunction::OnFunctionCall(mozIStorageValueArray* aArgs,
 
 /* static */
 nsresult GetHostAndPortFunction::create(mozIStorageConnection* aDBConn) {
-  RefPtr<GetHostAndPortFunction> function = new GetHostAndPortFunction();
-  nsresult rv = aDBConn->CreateFunction(NS_LITERAL_CSTRING("get_host_and_port"),
-                                        1, function);
+  nsresult rv =
+      aDBConn->CreateFunction(NS_LITERAL_CSTRING("get_host_and_port"), 1,
+                              MakeAndAddRef<GetHostAndPortFunction>());
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
@@ -1134,10 +1125,9 @@ GetHostAndPortFunction::OnFunctionCall(mozIStorageValueArray* aArgs,
 /* static */
 nsresult StripPrefixAndUserinfoFunction::create(
     mozIStorageConnection* aDBConn) {
-  RefPtr<StripPrefixAndUserinfoFunction> function =
-      new StripPrefixAndUserinfoFunction();
   nsresult rv = aDBConn->CreateFunction(
-      NS_LITERAL_CSTRING("strip_prefix_and_userinfo"), 1, function);
+      NS_LITERAL_CSTRING("strip_prefix_and_userinfo"), 1,
+      MakeAndAddRef<StripPrefixAndUserinfoFunction>());
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
@@ -1170,10 +1160,9 @@ StripPrefixAndUserinfoFunction::OnFunctionCall(mozIStorageValueArray* aArgs,
 
 /* static */
 nsresult IsFrecencyDecayingFunction::create(mozIStorageConnection* aDBConn) {
-  RefPtr<IsFrecencyDecayingFunction> function =
-      new IsFrecencyDecayingFunction();
-  nsresult rv = aDBConn->CreateFunction(
-      NS_LITERAL_CSTRING("is_frecency_decaying"), 0, function);
+  nsresult rv =
+      aDBConn->CreateFunction(NS_LITERAL_CSTRING("is_frecency_decaying"), 0,
+                              MakeAndAddRef<IsFrecencyDecayingFunction>());
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
@@ -1206,9 +1195,8 @@ IsFrecencyDecayingFunction::OnFunctionCall(mozIStorageValueArray* aArgs,
 
 /* static */
 nsresult SqrtFunction::create(mozIStorageConnection* aDBConn) {
-  RefPtr<SqrtFunction> function = new SqrtFunction();
-  nsresult rv =
-      aDBConn->CreateFunction(NS_LITERAL_CSTRING("sqrt"), 1, function);
+  nsresult rv = aDBConn->CreateFunction(NS_LITERAL_CSTRING("sqrt"), 1,
+                                        MakeAndAddRef<SqrtFunction>());
   NS_ENSURE_SUCCESS(rv, rv);
   return NS_OK;
 }
@@ -1240,9 +1228,9 @@ SqrtFunction::OnFunctionCall(mozIStorageValueArray* aArgs,
 
 /* static */
 nsresult NoteSyncChangeFunction::create(mozIStorageConnection* aDBConn) {
-  RefPtr<NoteSyncChangeFunction> function = new NoteSyncChangeFunction();
-  nsresult rv = aDBConn->CreateFunction(NS_LITERAL_CSTRING("note_sync_change"),
-                                        0, function);
+  nsresult rv =
+      aDBConn->CreateFunction(NS_LITERAL_CSTRING("note_sync_change"), 0,
+                              MakeAndAddRef<NoteSyncChangeFunction>());
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;