Apply review comments

src/app/FailSafeContext.cpp

@@ -84,7 +84,7 @@ void FailSafeContext::ScheduleFailSafeCleanup(FabricIndex fabricIndex, bool addN
     PlatformMgr().ScheduleWork(HandleDisarmFailSafe, reinterpret_cast<intptr_t>(this));
 }
 
-CHIP_ERROR FailSafeContext::ArmFailSafe(FabricIndex accessingFabricIndex, uint16_t expiryLengthSeconds)
+CHIP_ERROR FailSafeContext::ArmFailSafe(FabricIndex accessingFabricIndex, System::Clock::Seconds16 expiryLengthSeconds)
 {
     CHIP_ERROR err           = CHIP_NO_ERROR;
     bool cancelTimersIfError = false;

src/app/FailSafeContext.h

@@ -26,6 +26,7 @@
 #include <lib/core/CHIPError.h>
 #include <lib/core/DataModelTypes.h>
 #include <platform/internal/CHIPDeviceLayerInternal.h>
+#include <system/SystemClock.h>
 
 namespace chip {
 namespace app {
@@ -41,7 +42,7 @@ class FailSafeContext
      *  when the fail-safe timer is currently armed, the currently-running fail-safe timer will
      *  first be cancelled, then the fail-safe timer will be re-armed.
      */
-    CHIP_ERROR ArmFailSafe(FabricIndex accessingFabricIndex, uint16_t expiryLengthSeconds);
+    CHIP_ERROR ArmFailSafe(FabricIndex accessingFabricIndex, System::Clock::Seconds16 expiryLengthSeconds);
 
     /**
      * @brief Cleanly disarm failsafe timer, such as on CommissioningComplete

src/app/clusters/general-commissioning-server/general-commissioning-server.cpp

@@ -196,7 +196,9 @@ bool emberAfGeneralCommissioningClusterArmFailSafeCallback(app::CommandHandler *
         }
         else
         {
-            CheckSuccess(failSafeContext.ArmFailSafe(accessingFabricIndex, commandData.expiryLengthSeconds), Failure);
+            CheckSuccess(
+                failSafeContext.ArmFailSafe(accessingFabricIndex, System::Clock::Seconds16(commandData.expiryLengthSeconds)),
+                Failure);
             Breadcrumb::Set(commandPath.mEndpointId, commandData.breadcrumb);
             response.errorCode = CommissioningError::kOk;
             commandObj->AddResponse(commandPath, response);

src/app/server/CommissioningWindowManager.cpp

@@ -170,7 +170,7 @@ void CommissioningWindowManager::OnSessionEstablished(const SessionHandle & sess
     }
     else
     {
-        err = failSafeContext.ArmFailSafe(kUndefinedFabricId, 60);
+        err = failSafeContext.ArmFailSafe(kUndefinedFabricId, System::Clock::Seconds16(60));
         if (err != CHIP_NO_ERROR)
         {
             ChipLogError(AppServer, "Error arming failsafe on PASE session establishment completion");

src/app/tests/TestFailSafeContext.cpp

@@ -60,7 +60,7 @@ static void TestFailSafeContext_ArmFailSafe(nlTestSuite * inSuite, void * inCont
 
     chip::app::FailSafeContext failSafeContext;
 
-    err = failSafeContext.ArmFailSafe(kTestAccessingFabricIndex1, 1);
+    err = failSafeContext.ArmFailSafe(kTestAccessingFabricIndex1, System::Clock::Seconds16(1));
     NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
     NL_TEST_ASSERT(inSuite, failSafeContext.IsFailSafeArmed() == true);
     NL_TEST_ASSERT(inSuite, failSafeContext.GetFabricIndex() == kTestAccessingFabricIndex1);
@@ -77,7 +77,7 @@ static void TestFailSafeContext_NocCommandInvoked(nlTestSuite * inSuite, void *
 
     chip::app::FailSafeContext failSafeContext;
 
-    err = failSafeContext.ArmFailSafe(kTestAccessingFabricIndex1, 1);
+    err = failSafeContext.ArmFailSafe(kTestAccessingFabricIndex1, System::Clock::Seconds16(1));
     NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
     NL_TEST_ASSERT(inSuite, failSafeContext.GetFabricIndex() == kTestAccessingFabricIndex1);
 

src/credentials/FabricTable.cpp

@@ -1848,7 +1848,7 @@ CHIP_ERROR FabricTable::CommitPendingFabricData()
         {
             if (mStateFlags.Has(StateFlags::kAbortCommitForTest))
             {
-                // Clear state so that shutdown doesn't attemp clean-up
+                // Clear state so that shutdown doesn't attempt clean-up
                 mStateFlags.ClearAll();
                 mFabricIndexWithPendingState = kUndefinedFabricIndex;
                 mPendingFabric.Reset();

src/credentials/tests/TestFabricTable.cpp

@@ -1885,8 +1885,9 @@ void TestCommitMarker(nlTestSuite * inSuite, void * inContext)
             fabricTable.SetForceAbortCommitForTest(true);
             NL_TEST_ASSERT(inSuite, fabricTable.CommitPendingFabricData() == CHIP_ERROR_INTERNAL);
 
-            // Check that there are more keys now, partially committed.
-            NL_TEST_ASSERT(inSuite, storage.GetNumKeys() > numStorageKeysAfterFirstAdd);
+            // Check that there are more keys now, partially committed: at least a Commit Marker (+1)
+            // and some more keys from the aborted process.
+            NL_TEST_ASSERT(inSuite, storage.GetNumKeys() > (numStorageKeysAfterFirstAdd + 1));
         }
     }
 
@@ -1919,7 +1920,7 @@ void TestCommitMarker(nlTestSuite * inSuite, void * inContext)
         // Make sure that all other pending storage got deleted
         NL_TEST_ASSERT(inSuite, storage.GetNumKeys() == numStorageKeysAfterFirstAdd);
 
-        // Verify we canm only see 1 fabric with the iterator
+        // Verify we can only see 1 fabric with the iterator
         {
             size_t numFabricsIterated = 0;
             bool saw1                 = false;

src/platform/OpenThread/GenericNetworkCommissioningThreadDriver.cpp

@@ -86,15 +86,9 @@ CHIP_ERROR GenericThreadDriver::RevertConfiguration()
     // If no backup could be found, it means that the network configuration has not been modified
     // since the fail-safe was armed, so return with no error.
     ReturnErrorCodeIf(error == CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND, CHIP_NO_ERROR);
+    ReturnErrorOnFailure(error);
 
-    if (error == CHIP_NO_ERROR)
-    {
-        ChipLogError(NetworkProvisioning, "Found Thread configuration backup: reverting configuration");
-    }
-    else
-    {
-        ReturnErrorOnFailure(error);
-    }
+    ChipLogError(NetworkProvisioning, "Found Thread configuration backup: reverting configuration");
 
     // Not all KVS implementations support zero-length values, so handle a special value representing an empty dataset.
     ByteSpan dataset(datasetBytes, datasetLength);