Fail early in sql::MetaTable::Init if setting versions fails

An incorrect version number can cause issues with migrations, including
crashes, so Init should function atomically and only commit its
transaction if all meta table operations succeeded.

We accomplish this by returning success/failure from the two version-
setter methods, which also allows callers to return early from their
own migration code.

Change-Id: I2ed6ad38623ff46a53711e3d1041f8878a546cc1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4228100
Reviewed-by: Austin Sullivan <asully@chromium.org>
Commit-Queue: Andrew Paseltiner <apaseltiner@chromium.org>
Quick-Run: Andrew Paseltiner <apaseltiner@chromium.org>
Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1102399}

sql/meta_table.cc

@@ -130,7 +130,7 @@ bool MetaTable::Init(Database* db, int version, int compatible_version) {
   DCHECK_GT(version, 0);
   DCHECK_GT(compatible_version, 0);
 
-  // Make sure the table is created an populated atomically.
+  // Make sure the table is created and populated atomically.
   sql::Transaction transaction(db_);
   if (!transaction.Begin())
     return false;
@@ -149,8 +149,13 @@ bool MetaTable::Init(Database* db, int version, int compatible_version) {
     // Note: there is no index over the meta table. We currently only have a
     // couple of keys, so it doesn't matter. If we start storing more stuff in
     // there, we should create an index.
-    SetVersionNumber(version);
-    SetCompatibleVersionNumber(compatible_version);
+
+    // If setting either version number fails, return early to avoid likely
+    // crashes or incorrect behavior with respect to migrations.
+    if (!SetVersionNumber(version) ||
+        !SetCompatibleVersionNumber(compatible_version)) {
+      return false;
+    }
   }
   return transaction.Commit();
 }
@@ -159,19 +164,19 @@ void MetaTable::Reset() {
   db_ = nullptr;
 }
 
-void MetaTable::SetVersionNumber(int version) {
+bool MetaTable::SetVersionNumber(int version) {
   DCHECK_GT(version, 0);
-  SetValue(kVersionKey, version);
+  return SetValue(kVersionKey, version);
 }
 
 int MetaTable::GetVersionNumber() {
   int64_t version = 0;
   return GetValue(kVersionKey, &version) ? version : 0;
 }
 
-void MetaTable::SetCompatibleVersionNumber(int version) {
+bool MetaTable::SetCompatibleVersionNumber(int version) {
   DCHECK_GT(version, 0);
-  SetValue(kCompatibleVersionKey, version);
+  return SetValue(kCompatibleVersionKey, version);
 }
 
 int MetaTable::GetCompatibleVersionNumber() {

sql/meta_table.h

@@ -77,6 +77,8 @@ class COMPONENT_EXPORT(SQL) MetaTable {
   // Versions must be greater than 0 to distinguish missing versions (see
   // GetVersionNumber()). If there was no meta table (proxy for a fresh
   // database), mmap status is set to `kMmapSuccess`.
+  // TODO(apaseltiner): Mark this [[nodiscard]] once all callers have been
+  // updated.
   bool Init(Database* db, int version, int compatible_version);
 
   // Resets this MetaTable object, making another call to Init() possible.
@@ -87,7 +89,7 @@ class COMPONENT_EXPORT(SQL) MetaTable {
   // previously set version number.
   //
   // See also Get/SetCompatibleVersionNumber().
-  void SetVersionNumber(int version);
+  bool SetVersionNumber(int version);
   int GetVersionNumber();
 
   // The compatible version number is the lowest current version embedded in
@@ -106,7 +108,7 @@ class COMPONENT_EXPORT(SQL) MetaTable {
   //
   // The compatible version number will be 0 if there is no previously set
   // compatible version number.
-  void SetCompatibleVersionNumber(int version);
+  bool SetCompatibleVersionNumber(int version);
   int GetCompatibleVersionNumber();
 
   // Set the given arbitrary key with the given data. Returns true on success.