Bug 1412718 (part 2) - Improve nsPrefBranch::DeleteBranch(). r=glandium. draft
authorNicholas Nethercote <nnethercote@mozilla.com>
Wed, 01 Nov 2017 08:28:10 +1100
changeset 689668 c6788cbed5c4b932eb5b4119c407aea538cb1799
parent 689667 fa13eb8be2a088b717d834686d60b4eea6b1df4d
child 738373 a06b1439dfad0bc6033b7b168f7d72bc825c8c5f
push id87084
push usernnethercote@mozilla.com
push dateTue, 31 Oct 2017 21:31:54 +0000
reviewersglandium
bugs1412718
milestone58.0a1
Bug 1412718 (part 2) - Improve nsPrefBranch::DeleteBranch(). r=glandium. This patch converts it to use Gecko strings instead of C strings, which makes it much nicer. MozReview-Commit-ID: KtRp3vaXwN5
modules/libpref/Preferences.cpp
--- a/modules/libpref/Preferences.cpp
+++ b/modules/libpref/Preferences.cpp
@@ -2813,41 +2813,36 @@ nsPrefBranch::DeleteBranch(const char* a
 
   MOZ_ASSERT(NS_IsMainThread());
 
   if (!gHashTable) {
     return NS_ERROR_NOT_INITIALIZED;
   }
 
   const PrefName& pref = GetPrefName(aStartingAt);
-  const char* branchName = pref.get();
-  size_t len = strlen(branchName);
-
-  // The following check insures that if the branch name already has a "." at
-  // the end, we don't end up with a "..". This fixes an incompatibility
-  // between nsIPref, which needs the period added, and nsIPrefBranch which
-  // does not. When nsIPref goes away this function should be fixed to never
-  // add the period at all.
-  nsAutoCString branch_dot(branchName);
-  if (len > 1 && branchName[len - 1] != '.') {
-    branch_dot += '.';
-  }
-
-  // Delete a branch. Used for deleting mime types.
-  const char* to_delete = branch_dot.get();
-  MOZ_ASSERT(to_delete);
-  len = strlen(to_delete);
+  nsAutoCString branchName(pref.get());
+
+  // Add a trailing '.' if it doesn't already have one.
+  if (branchName.Length() > 1 &&
+      !StringEndsWith(branchName, NS_LITERAL_CSTRING("."))) {
+    branchName += '.';
+  }
+
+  const nsACString& branchNameNoDot =
+    Substring(branchName, 0, branchName.Length() - 1);
+
   for (auto iter = gHashTable->Iter(); !iter.Done(); iter.Next()) {
     auto entry = static_cast<PrefHashEntry*>(iter.Get());
 
-    // Note: if we're deleting "ldap" then we want to delete "ldap.xxx" and
-    // "ldap" (if such a leaf node exists) but not "ldap_1.xxx".
-    if (PL_strncmp(entry->mKey, to_delete, len) == 0 ||
-        (len - 1 == strlen(entry->mKey) &&
-         PL_strncmp(entry->mKey, to_delete, len - 1) == 0)) {
+    // The first disjunct matches branches: e.g. a branch name "foo.bar."
+    // matches an mKey "foo.bar.baz" (but it won't match "foo.barrel.baz").
+    // The second disjunct matches leaf nodes: e.g. a branch name "foo.bar."
+    // matches an mKey "foo.bar" (by ignoring the trailing '.').
+    nsDependentCString key(entry->mKey);
+    if (StringBeginsWith(key, branchName) || key.Equals(branchNameNoDot)) {
       iter.Remove();
     }
   }
 
   Preferences::HandleDirty();
   return NS_OK;
 }