Bug 1915665 - [2/2] Simplify the Chromium-inherited API r=win-reviewers,handyman

The original Chromium listener code has a few infelicities in its API.

* The current session ID is always available, so the `Nothing()` branch
  is impossible. Remove the `Maybe` from the interface.

* If session-change messages for other sessions are received, discard
  them early rather than propagating them.

* Use the whole DWORD value supplied by `GUID_SESSION_DISPLAY_STATUS`,
  rather than just the first byte. (This has no effect here, since
  `MONITOR_DISPLAY_STATE` is an enum with very few values, but might if
  someone blindly copies this code with minimal adjustments to be used
  with another `PBT_POWERSETTINGCHANGE` value.)

Differential Revision: https://phabricator.services.mozilla.com/D220749

widget/windows/WinEventObserver.cpp

@@ -4,7 +4,9 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+#include <corecrt_memcpy_s.h>
 #include <windows.h>
+#include <winternl.h>
 #include <winuser.h>
 #include <wtsapi32.h>
 
@@ -14,7 +16,6 @@
 #include "mozilla/Assertions.h"
 #include "mozilla/ClearOnShutdown.h"
 #include "mozilla/Logging.h"
-#include "mozilla/Maybe.h"
 #include "nsWindowDbg.h"
 #include "nsdefs.h"
 #include "nsXULAppAPI.h"
@@ -94,20 +95,38 @@ namespace evtwin_details {
 
 static void OnSessionChange(WPARAM wParam, LPARAM lParam) {
   if (wParam == WTS_SESSION_LOCK || wParam == WTS_SESSION_UNLOCK) {
-    Maybe<bool> isCurrentSession;
-    DWORD currentSessionId = 0;
-    if (!::ProcessIdToSessionId(::GetCurrentProcessId(), &currentSessionId)) {
-      isCurrentSession = Nothing();
-    } else {
-      OBS_LOG(
-          "WinEventWindow OnSessionChange() wParam %zu lParam "
-          "%" PRIdLPTR " currentSessionId %lu",
+    DWORD currentSessionId;
+    BOOL const rv =
+        ::ProcessIdToSessionId(::GetCurrentProcessId(), &currentSessionId);
+    if (!rv) {
+      // A process should always have the relevant access privileges for itself,
+      // but the above call could still fail if, e.g., someone's playing games
+      // with function imports. If so, just assert and/or skip out.
+      //
+      // Should this turn out to somehow be a real concern, we could do
+      // ```
+      //    DWORD const currentSessionId =
+      //       ::NtCurrentTeb()->ProcessEnvironmentBlock->SessionId;
+      // ```
+      // instead, which is actually documented (albeit abjured against).
+      MOZ_ASSERT(false, "::ProcessIdToSessionId() failed");
+      return;
+    }
+
+    OBS_LOG("WinEventWindow OnSessionChange(): wParam=%zu lParam=%" PRIdLPTR
+            " currentSessionId=%lu",
             wParam, lParam, currentSessionId);
 
-      isCurrentSession = Some(static_cast<DWORD>(lParam) == currentSessionId);
+    // Ignore lock/unlock messages for other sessions -- which Windows actually
+    // _does_ send in some scenarios; see review of Chromium changeset 1929489:
+    //
+    // https://chromium-review.googlesource.com/c/chromium/src/+/1929489
+    if (currentSessionId != (DWORD)lParam) {
+      return;
     }
+
     if (auto* wwot = WinWindowOcclusionTracker::Get()) {
-      wwot->OnSessionChange(wParam, isCurrentSession);
+      wwot->OnSessionChange(wParam);
     }
   }
 }
@@ -115,14 +134,22 @@ static void OnSessionChange(WPARAM wParam, LPARAM lParam) {
 static void OnPowerBroadcast(WPARAM wParam, LPARAM lParam) {
   if (wParam == PBT_POWERSETTINGCHANGE) {
     POWERBROADCAST_SETTING* setting = (POWERBROADCAST_SETTING*)lParam;
+    MOZ_ASSERT(setting);
 
-    if (setting &&
-        ::IsEqualGUID(setting->PowerSetting, GUID_SESSION_DISPLAY_STATUS) &&
+    if (::IsEqualGUID(setting->PowerSetting, GUID_SESSION_DISPLAY_STATUS) &&
         setting->DataLength == sizeof(DWORD)) {
-      bool displayOn = PowerMonitorOff !=
-                       static_cast<MONITOR_DISPLAY_STATE>(setting->Data[0]);
+      MONITOR_DISPLAY_STATE state{};
+      errno_t const err =
+          ::memcpy_s(&state, sizeof(state), setting->Data, setting->DataLength);
+      if (err) {
+        MOZ_ASSERT(false, "bad data in POWERBROADCAST_SETTING in lParam");
+        return;
+      }
 
-      OBS_LOG("WinEventWindow OnPowerBroadcast() displayOn %d", displayOn);
+      bool const displayOn = MONITOR_DISPLAY_STATE::PowerMonitorOff != state;
+
+      OBS_LOG("WinEventWindow OnPowerBroadcast(): displayOn=%d",
+              int(displayOn ? 1 : 0));
 
       if (auto* wwot = WinWindowOcclusionTracker::Get()) {
         wwot->OnDisplayStateChanged(displayOn);
@@ -154,7 +181,7 @@ LRESULT CALLBACK WinEventWindow::WndProc(HWND hwnd, UINT msg, WPARAM wParam,
     } break;
   }
 
-  LRESULT ret = ::DefWindowProcW(hwnd, msg, wParam, lParam);
+  LRESULT const ret = ::DefWindowProcW(hwnd, msg, wParam, lParam);
   eventLogger.SetResult(ret, false);
   return ret;
 }

widget/windows/WinWindowOcclusionTracker.cpp

@@ -703,18 +703,13 @@ void WinWindowOcclusionTracker::UpdateOcclusionState(
   }
 }
 
-void WinWindowOcclusionTracker::OnSessionChange(WPARAM aStatusCode,
-                                                Maybe<bool> aIsCurrentSession) {
+void WinWindowOcclusionTracker::OnSessionChange(WPARAM aStatusCode) {
   MOZ_ASSERT(NS_IsMainThread());
   if (!StaticPrefs::
           widget_windows_window_occlusion_tracking_session_lock_enabled()) {
     return;
   }
 
-  if (aIsCurrentSession.isNothing() || !*aIsCurrentSession) {
-    return;
-  }
-
   if (aStatusCode == WTS_SESSION_UNLOCK) {
     LOG(LogLevel::Info,
         "WinWindowOcclusionTracker::OnSessionChange() WTS_SESSION_UNLOCK");

widget/windows/WinWindowOcclusionTracker.h

@@ -274,7 +274,7 @@ class WinWindowOcclusionTracker final {
  public:
   // This is called with session changed notifications. If the screen is locked
   // by the current session, it marks app windows as occluded.
-  void OnSessionChange(WPARAM aStatusCode, Maybe<bool> aIsCurrentSession);
+  void OnSessionChange(WPARAM aStatusCode);
 
   // This is called when the display is put to sleep. If the display is sleeping
   // it marks app windows as occluded.