Change ARP property Installed to 0 when registrationType is InProgress.

Being registered in ARP and "installed" were always separate concepts, and some things like fEligibleForCleanup were looking at the wrong thing. This also allows the BA to tell the difference.

12 files changed, 69 insertions, 55 deletions

diff --git a/src/burn/engine/core.cpp b/src/burn/engine/core.cpp
index 9d5364a4..ca4d607b 100644
--- a/src/burn/engine/core.cpp
+++ b/src/burn/engine/core.cpp
@@ -315,26 +315,19 @@ extern "C" HRESULT CoreDetect(
     DetectReset(&pEngineState->registration, &pEngineState->packages);
     PlanReset(&pEngineState->plan, &pEngineState->containers, &pEngineState->packages, &pEngineState->layoutPayloads);
 
-    // Detect if bundle installed state has changed since start up. This
-    // only happens if Apply() changed the state of bundle (installed or
-    // uninstalled). In that case, Detect() can be used here to reset
-    // the installed state.
+    // Detect if bundle installed state has changed since start up.
+    // This only happens if Apply() changed the state of bundle (installed, in progress, or uninstalled).
+    // In that case, Detect() can be used here to reset the installed state.
+    // Of course, there's also cases outside of this bundle's control,
+    // like other processes messing with its registration.
     hr = RegistrationDetectInstalled(&pEngineState->registration);
     ExitOnFailure(hr, "Failed to detect bundle install state.");
 
-    if (pEngineState->registration.fInstalled)
-    {
-        hr = VariableSetNumeric(&pEngineState->variables, BURN_BUNDLE_INSTALLED, 1, TRUE);
-        ExitOnFailure(hr, "Failed to set the bundle installed built-in variable.");
-    }
-    else
-    {
-        hr = VariableSetString(&pEngineState->variables, BURN_BUNDLE_INSTALLED, NULL, TRUE, FALSE);
-        ExitOnFailure(hr, "Failed to unset the bundle installed built-in variable.");
-    }
+    hr = RegistrationSetDynamicVariables(&pEngineState->registration, &pEngineState->variables);
+    ExitOnFailure(hr, "Failed to reset the dynamic registration variables during detect.");
 
     fDetectBegan = TRUE;
-    hr = UserExperienceOnDetectBegin(&pEngineState->userExperience, pEngineState->registration.fCached, pEngineState->registration.fInstalled, pEngineState->packages.cPackages);
+    hr = UserExperienceOnDetectBegin(&pEngineState->userExperience, pEngineState->registration.fCached, pEngineState->registration.detectedRegistrationType, pEngineState->packages.cPackages);
     ExitOnRootFailure(hr, "UX aborted detect begin.");
 
     pEngineState->userExperience.hwndDetect = hwndParent;
@@ -444,7 +437,7 @@ LExit:
 
     pEngineState->userExperience.hwndDetect = NULL;
 
-    LogId(REPORT_STANDARD, MSG_DETECT_COMPLETE, hr, !fDetectBegan ? "(failed)" : LoggingBoolToString(pEngineState->registration.fInstalled), !fDetectBegan ? "(failed)" : LoggingBoolToString(pEngineState->registration.fCached), FAILED(hr) ? "(failed)" : LoggingBoolToString(pEngineState->registration.fEligibleForCleanup));
+    LogId(REPORT_STANDARD, MSG_DETECT_COMPLETE, hr, !fDetectBegan ? "(failed)" : LoggingRegistrationTypeToString(pEngineState->registration.detectedRegistrationType), !fDetectBegan ? "(failed)" : LoggingBoolToString(pEngineState->registration.fCached), FAILED(hr) ? "(failed)" : LoggingBoolToString(pEngineState->registration.fEligibleForCleanup));
 
     return hr;
 }
@@ -489,7 +482,7 @@ extern "C" HRESULT CorePlan(
     pEngineState->plan.wzBundleId = pEngineState->registration.sczId;
     pEngineState->plan.wzBundleProviderKey = pEngineState->registration.sczId;
     pEngineState->plan.fDisableRollback = pEngineState->fDisableRollback || BOOTSTRAPPER_ACTION_UNSAFE_UNINSTALL == pEngineState->plan.action;
-    pEngineState->plan.fBundleAlreadyRegistered = pEngineState->registration.fInstalled;
+    pEngineState->plan.fPlanPackageCacheRollback = BOOTSTRAPPER_REGISTRATION_TYPE_NONE == pEngineState->registration.detectedRegistrationType;
 
     hr = PlanSetVariables(action, &pEngineState->variables);
     ExitOnFailure(hr, "Failed to update action.");
diff --git a/src/burn/engine/detect.cpp b/src/burn/engine/detect.cpp
index 3bd39784..f7a030ff 100644
--- a/src/burn/engine/detect.cpp
+++ b/src/burn/engine/detect.cpp
@@ -165,7 +165,7 @@ extern "C" HRESULT DetectReportRelatedBundles(
 {
     HRESULT hr = S_OK;
     BOOTSTRAPPER_REQUEST_STATE uninstallRequestState = BOOTSTRAPPER_REQUEST_STATE_NONE;
-    *pfEligibleForCleanup = pRegistration->fInstalled || pRegistration->fCached;
+    *pfEligibleForCleanup = BOOTSTRAPPER_REGISTRATION_TYPE_NONE != pRegistration->detectedRegistrationType || pRegistration->fCached;
 
     for (DWORD iRelatedBundle = 0; iRelatedBundle < pRegistration->relatedBundles.cRelatedBundles; ++iRelatedBundle)
     {
diff --git a/src/burn/engine/engine.mc b/src/burn/engine/engine.mc
index 4425af12..6a826365 100644
--- a/src/burn/engine/engine.mc
+++ b/src/burn/engine/engine.mc
@@ -335,7 +335,7 @@ MessageId=199
 Severity=Success
 SymbolicName=MSG_DETECT_COMPLETE
 Language=English
-Detect complete, result: 0x%1!x!, installed: %2!hs!, cached: %3!hs!, eligible for cleanup: %4!hs!
+Detect complete, result: 0x%1!x!, registration state: %2!hs!, cached: %3!hs!, eligible for cleanup: %4!hs!
 .
 
 MessageId=200
diff --git a/src/burn/engine/plan.cpp b/src/burn/engine/plan.cpp
index 49dd83f4..c9337df5 100644
--- a/src/burn/engine/plan.cpp
+++ b/src/burn/engine/plan.cpp
@@ -1692,7 +1692,7 @@ extern "C" HRESULT PlanExecuteCacheSyncAndRollback(
     HRESULT hr = S_OK;
     BURN_EXECUTE_ACTION* pAction = NULL;
 
-    if (!pPlan->fBundleAlreadyRegistered)
+    if (pPlan->fPlanPackageCacheRollback)
     {
         hr = PlanAppendRollbackAction(pPlan, &pAction);
         ExitOnFailure(hr, "Failed to append rollback action.");
@@ -2241,7 +2241,7 @@ static HRESULT AddCachePackageHelper(
     pCacheAction->type = BURN_CACHE_ACTION_TYPE_CHECKPOINT;
     pCacheAction->checkpoint.dwId = dwCheckpoint;
 
-    if (!pPlan->fBundleAlreadyRegistered)
+    if (pPlan->fPlanPackageCacheRollback)
     {
         // Create a package cache rollback action *before* the checkpoint.
         hr = AppendRollbackCacheAction(pPlan, &pCacheAction);
diff --git a/src/burn/engine/plan.h b/src/burn/engine/plan.h
index c0936970..63bcd3ce 100644
--- a/src/burn/engine/plan.h
+++ b/src/burn/engine/plan.h
@@ -249,7 +249,7 @@ typedef struct _BURN_PLAN
     BOOL fDisableRollback;
     BOOL fAffectedMachineState;
     LPWSTR sczLayoutDirectory;
-    BOOL fBundleAlreadyRegistered;
+    BOOL fPlanPackageCacheRollback;
 
     DWORD64 qwCacheSizeTotal;
 
diff --git a/src/burn/engine/registration.cpp b/src/burn/engine/registration.cpp
index a1b1b607..fcefe5e0 100644
--- a/src/burn/engine/registration.cpp
+++ b/src/burn/engine/registration.cpp
@@ -69,6 +69,7 @@ static HRESULT UpdateResumeMode(
     __in BURN_REGISTRATION* pRegistration,
     __in HKEY hkRegistration,
     __in BURN_RESUME_MODE resumeMode,
+    __in BOOTSTRAPPER_REGISTRATION_TYPE registrationType,
     __in BOOL fRestartInitiated
     );
 static HRESULT ParseRelatedCodes(
@@ -444,11 +445,8 @@ extern "C" HRESULT RegistrationSetVariables(
     HRESULT hr = S_OK;
     LPWSTR scz = NULL;
 
-    if (pRegistration->fInstalled)
-    {
-        hr = VariableSetNumeric(pVariables, BURN_BUNDLE_INSTALLED, 1, TRUE);
-        ExitOnFailure(hr, "Failed to set the bundle installed built-in variable.");
-    }
+    hr = RegistrationSetDynamicVariables(pRegistration, pVariables);
+    ExitOnFailure(hr, "Failed to set the dynamic registration variables.");
 
     // Ensure the registration bundle name is updated.
     hr = GetBundleInProgressName(pRegistration, pVariables, &scz);
@@ -469,12 +467,32 @@ extern "C" HRESULT RegistrationSetVariables(
     hr = VariableSetVersion(pVariables, BURN_BUNDLE_VERSION, pRegistration->pVersion, TRUE);
     ExitOnFailure(hr, "Failed to overwrite the bundle version built-in variable.");
 
+LExit:
+    ReleaseStr(scz);
+
+    return hr;
+}
+
+/*******************************************************************
+ RegistrationSetDynamicVariables - Initializes bundle variables that
+     map to registration entities that can change during execution.
+
+*******************************************************************/
+extern "C" HRESULT RegistrationSetDynamicVariables(
+    __in BURN_REGISTRATION* pRegistration,
+    __in BURN_VARIABLES* pVariables
+    )
+{
+    HRESULT hr = S_OK;
+    LONGLONG llInstalled = BOOTSTRAPPER_REGISTRATION_TYPE_FULL == pRegistration->detectedRegistrationType ? 1 : 0;
+
+    hr = VariableSetNumeric(pVariables, BURN_BUNDLE_INSTALLED, llInstalled, TRUE);
+    ExitOnFailure(hr, "Failed to set the bundle installed built-in variable.");
+
     hr = VariableSetNumeric(pVariables, BURN_REBOOT_PENDING, IsWuRebootPending() || IsRegistryRebootPending(), TRUE);
     ExitOnFailure(hr, "Failed to overwrite the bundle reboot-pending built-in variable.");
 
 LExit:
-    ReleaseStr(scz);
-
     return hr;
 }
 
@@ -487,12 +505,15 @@ extern "C" HRESULT RegistrationDetectInstalled(
     DWORD dwInstalled = 0;
 
     pRegistration->fCached = FileExistsEx(pRegistration->sczCacheExecutablePath, NULL);
+    pRegistration->detectedRegistrationType = BOOTSTRAPPER_REGISTRATION_TYPE_NONE;
 
     // open registration key
     hr = RegOpen(pRegistration->hkRoot, pRegistration->sczRegistrationKey, KEY_QUERY_VALUE, &hkRegistration);
     if (SUCCEEDED(hr))
     {
         hr = RegReadNumber(hkRegistration, REGISTRY_BUNDLE_INSTALLED, &dwInstalled);
+
+        pRegistration->detectedRegistrationType = (1 == dwInstalled) ? BOOTSTRAPPER_REGISTRATION_TYPE_FULL : BOOTSTRAPPER_REGISTRATION_TYPE_INPROGRESS;
     }
 
     // Not finding the key or value is okay.
@@ -501,8 +522,6 @@ extern "C" HRESULT RegistrationDetectInstalled(
         hr = S_OK;
     }
 
-    pRegistration->fInstalled = (1 == dwInstalled);
-
     ReleaseRegKey(hkRegistration);
     return hr;
 }
@@ -833,7 +852,7 @@ extern "C" HRESULT RegistrationSessionBegin(
     }
 
     // update resume mode
-    hr = UpdateResumeMode(pRegistration, hkRegistration, BURN_RESUME_MODE_ACTIVE, FALSE);
+    hr = UpdateResumeMode(pRegistration, hkRegistration, BURN_RESUME_MODE_ACTIVE, registrationType, FALSE);
     ExitOnFailure(hr, "Failed to update resume mode.");
 
 LExit:
@@ -864,7 +883,7 @@ extern "C" HRESULT RegistrationSessionResume(
     ExitOnFailure(hr, "Failed to open registration key.");
 
     // update resume mode
-    hr = UpdateResumeMode(pRegistration, hkRegistration, BURN_RESUME_MODE_ACTIVE, FALSE);
+    hr = UpdateResumeMode(pRegistration, hkRegistration, BURN_RESUME_MODE_ACTIVE, registrationType, FALSE);
     ExitOnFailure(hr, "Failed to update resume mode.");
 
     // update display name
@@ -934,7 +953,7 @@ extern "C" HRESULT RegistrationSessionEnd(
     }
 
     // Update resume mode.
-    hr = UpdateResumeMode(pRegistration, hkRegistration, resumeMode, BOOTSTRAPPER_APPLY_RESTART_INITIATED == restart);
+    hr = UpdateResumeMode(pRegistration, hkRegistration, resumeMode, registrationType, BOOTSTRAPPER_APPLY_RESTART_INITIATED == restart);
     ExitOnFailure(hr, "Failed to update resume mode.");
 
 LExit:
@@ -1271,6 +1290,7 @@ static HRESULT UpdateResumeMode(
     __in BURN_REGISTRATION* pRegistration,
     __in HKEY hkRegistration,
     __in BURN_RESUME_MODE resumeMode,
+    __in BOOTSTRAPPER_REGISTRATION_TYPE registrationType,
     __in BOOL fRestartInitiated
     )
 {
@@ -1289,16 +1309,9 @@ static HRESULT UpdateResumeMode(
         hr = RegWriteNumber(hkRegistration, L"Resume", (DWORD)resumeMode);
         ExitOnFailure(hr, "Failed to write Resume value.");
 
-        // Write the Installed value *only* when the mode is ARP. This will tell us
-        // that the bundle considers itself "installed" on the machine. Note that we
-        // never change the value to "0" after that. The bundle will be considered
-        // "uninstalled" when all of the registration is removed.
-        if (BURN_RESUME_MODE_ARP == resumeMode)
-        {
-            // Write Installed value.
-            hr = RegWriteNumber(hkRegistration, REGISTRY_BUNDLE_INSTALLED, 1);
-            ExitOnFailure(hr, "Failed to write Installed value.");
-        }
+        // Write Installed value.
+        hr = RegWriteNumber(hkRegistration, REGISTRY_BUNDLE_INSTALLED, BOOTSTRAPPER_REGISTRATION_TYPE_FULL == registrationType ? 1 : 0);
+        ExitOnFailure(hr, "Failed to write Installed value.");
     }
 
     // If the engine is active write the run key so we resume if there is an unexpected
diff --git a/src/burn/engine/registration.h b/src/burn/engine/registration.h
index 64191828..bfaab1f1 100644
--- a/src/burn/engine/registration.h
+++ b/src/burn/engine/registration.h
@@ -93,7 +93,7 @@ typedef struct _BURN_REGISTRATION
     BOOL fRegisterArp;
     BOOL fDisableResume;
     BOOL fCached;
-    BOOL fInstalled;
+    BOOTSTRAPPER_REGISTRATION_TYPE detectedRegistrationType;
     LPWSTR sczId;
     LPWSTR sczTag;
 
@@ -171,6 +171,10 @@ HRESULT RegistrationSetVariables(
     __in BURN_REGISTRATION* pRegistration,
     __in BURN_VARIABLES* pVariables
     );
+HRESULT RegistrationSetDynamicVariables(
+    __in BURN_REGISTRATION* pRegistration,
+    __in BURN_VARIABLES* pVariables
+    );
 HRESULT RegistrationDetectInstalled(
     __in BURN_REGISTRATION* pRegistration
     );
diff --git a/src/burn/engine/userexperience.cpp b/src/burn/engine/userexperience.cpp
index a2f33f80..f299772b 100644
--- a/src/burn/engine/userexperience.cpp
+++ b/src/burn/engine/userexperience.cpp
@@ -104,7 +104,7 @@ extern "C" HRESULT UserExperienceLoad(
     args.pCommand = pCommand;
     args.pfnBootstrapperEngineProc = EngineForApplicationProc;
     args.pvBootstrapperEngineProcContext = pEngineContext;
-    args.qwEngineAPIVersion = MAKEQWORDVERSION(2022, 2, 22, 0);
+    args.qwEngineAPIVersion = MAKEQWORDVERSION(2022, 3, 4, 0);
 
     results.cbSize = sizeof(BOOTSTRAPPER_CREATE_RESULTS);
 
@@ -988,7 +988,7 @@ LExit:
 EXTERN_C BAAPI UserExperienceOnDetectBegin(
     __in BURN_USER_EXPERIENCE* pUserExperience,
     __in BOOL fCached,
-    __in BOOL fInstalled,
+    __in BOOTSTRAPPER_REGISTRATION_TYPE registrationType,
     __in DWORD cPackages
     )
 {
@@ -998,7 +998,7 @@ EXTERN_C BAAPI UserExperienceOnDetectBegin(
 
     args.cbSize = sizeof(args);
     args.cPackages = cPackages;
-    args.fInstalled = fInstalled;
+    args.registrationType = registrationType;
     args.fCached = fCached;
 
     results.cbSize = sizeof(results);
diff --git a/src/burn/engine/userexperience.h b/src/burn/engine/userexperience.h
index 8106d7f7..37fa5174 100644
--- a/src/burn/engine/userexperience.h
+++ b/src/burn/engine/userexperience.h
@@ -245,7 +245,7 @@ BAAPI UserExperienceOnCommitMsiTransactionComplete(
 BAAPI UserExperienceOnDetectBegin(
     __in BURN_USER_EXPERIENCE* pUserExperience,
     __in BOOL fCached,
-    __in BOOL fInstalled,
+    __in BOOTSTRAPPER_REGISTRATION_TYPE registrationType,
     __in DWORD cPackages
     );
 BAAPI UserExperienceOnDetectCompatibleMsiPackage(
diff --git a/src/burn/engine/variable.cpp b/src/burn/engine/variable.cpp
index b3dcdb7d..5d92590c 100644
--- a/src/burn/engine/variable.cpp
+++ b/src/burn/engine/variable.cpp
@@ -248,6 +248,7 @@ extern "C" HRESULT VariableInitialize(
 #endif
         {L"ProgramFiles6432Folder", InitializeVariable6432Folder, CSIDL_PROGRAM_FILES},
         {L"ProgramMenuFolder", InitializeVariableCsidlFolder, CSIDL_PROGRAMS},
+        {L"RebootPending", InitializeVariableNumeric, 0},
         {L"SendToFolder", InitializeVariableCsidlFolder, CSIDL_SENDTO},
         {L"ServicePackLevel", InitializeVariableVersionNT, OS_INFO_VARIABLE_ServicePackLevel},
         {L"StartMenuFolder", InitializeVariableCsidlFolder, CSIDL_STARTMENU},
@@ -1571,6 +1572,9 @@ static HRESULT SetVariableValue(
     // Insert element if not found.
     if (S_FALSE == hr)
     {
+        // Not possible from external callers so just assert.
+        AssertSz(SET_VARIABLE_OVERRIDE_BUILTIN != setBuiltin, "Intent to set missing built-in variable.");
+
         hr = InsertVariable(pVariables, wzVariable, iVariable);
         ExitOnFailure(hr, "Failed to insert variable '%ls'.", wzVariable);
     }
diff --git a/src/burn/test/BurnUnitTest/PlanTest.cpp b/src/burn/test/BurnUnitTest/PlanTest.cpp
index ba28713f..685d2ca9 100644
--- a/src/burn/test/BurnUnitTest/PlanTest.cpp
+++ b/src/burn/test/BurnUnitTest/PlanTest.cpp
@@ -792,7 +792,7 @@ namespace Bootstrapper
             InitializeEngineStateForCorePlan(wzSingleMsiManifestFileName, pEngineState);
             DetectPackagesAsAbsent(pEngineState);
 
-            pEngineState->registration.fInstalled = TRUE;
+            pEngineState->registration.detectedRegistrationType = BOOTSTRAPPER_REGISTRATION_TYPE_FULL;
 
             hr = CorePlan(pEngineState, BOOTSTRAPPER_ACTION_MODIFY);
             NativeAssert::Succeeded(hr, "CorePlan failed");
@@ -1457,7 +1457,7 @@ namespace Bootstrapper
         {
             PlanTestDetect(pEngineState);
 
-            pEngineState->registration.fInstalled = TRUE;
+            pEngineState->registration.detectedRegistrationType = BOOTSTRAPPER_REGISTRATION_TYPE_FULL;
 
             for (DWORD i = 0; i < pEngineState->packages.cPackages; ++i)
             {
@@ -1486,7 +1486,7 @@ namespace Bootstrapper
         {
             PlanTestDetect(pEngineState);
 
-            pEngineState->registration.fInstalled = TRUE;
+            pEngineState->registration.detectedRegistrationType = BOOTSTRAPPER_REGISTRATION_TYPE_FULL;
 
             for (DWORD i = 0; i < pEngineState->packages.cPackages; ++i)
             {
diff --git a/src/burn/test/BurnUnitTest/RegistrationTest.cpp b/src/burn/test/BurnUnitTest/RegistrationTest.cpp
index 6a10961d..86ea86b0 100644
--- a/src/burn/test/BurnUnitTest/RegistrationTest.cpp
+++ b/src/burn/test/BurnUnitTest/RegistrationTest.cpp
@@ -218,7 +218,7 @@ namespace Bootstrapper
 
                 // verify that registration was updated
                 this->ValidateUninstallKeyResume(Int32(BURN_RESUME_MODE_ARP));
-                this->ValidateUninstallKeyInstalled(1);
+                this->ValidateUninstallKeyInstalled(0);
                 this->ValidateRunOnceKeyString(TEST_BUNDLE_ID, nullptr);
 
                 //
@@ -231,7 +231,7 @@ namespace Bootstrapper
 
                 // verify that registration was updated
                 this->ValidateUninstallKeyResume(Int32(BURN_RESUME_MODE_ACTIVE));
-                this->ValidateUninstallKeyInstalled(1);
+                this->ValidateUninstallKeyInstalled(0);
                 this->ValidateRunOnceKeyEntry(cacheExePath);
 
                 // delete registration
@@ -337,7 +337,7 @@ namespace Bootstrapper
 
                 // verify that registration variables were updated
                 this->ValidateUninstallKeyDisplayName(L"Product1");
-                registration.fInstalled = TRUE;
+                registration.detectedRegistrationType = BOOTSTRAPPER_REGISTRATION_TYPE_FULL;
 
                 hr = RegistrationSetVariables(&registration, &variables);
                 TestThrowOnFailure(hr, L"Failed to set registration variables.");