sheriff: Revert "Use unoccluded desktop region to calculate root window occlusion."

This reverts commit f34c7c2b.

Reason for revert:
Likely culprit for frequent test flakes on win7 dbg bot.
Sample failing build: https://ci.chromium.org/p/chromium/builders/ci/Win7%20Tests%20%28dbg%29%281%29
Sample failure stack:
[1744:6088:1218/071720.166:FATAL:native_window_occlusion_tracker_win.cc(420)] Check failed: FALSE. A root window did not get its occlusion state set
Backtrace:
	base::debug::CollectStackTrace [0x6F2A256C+60] (o:\base\debug\stack_trace_win.cc:284)
	base::debug::StackTrace::StackTrace [0x6EFAA18E+78] (o:\base\debug\stack_trace.cc:206)
	base::debug::StackTrace::StackTrace [0x6EFAA102+34] (o:\base\debug\stack_trace.cc:203)
	logging::LogMessage::~LogMessage [0x6EFE9AC0+144] (o:\base\logging.cc:628)
	aura::NativeWindowOcclusionTrackerWin::WindowOcclusionCalculator::ComputeNativeWindowOcclusionStatus [0x647E0ADC+1660] (o:\ui\aura\native_window_occlusion_tracker_win.cc:418)
	base::internal::FunctorTraits<void (aura::NativeWindowOcclusionTrackerWin::WindowOcclusionCalculator::*)() __attribute__((thiscall)),void>::Invoke<void (aura::NativeWindowOcclusionTrackerWin::WindowOcclusionCalculator::*)() __attribute__((thiscall)),aura: [0x647F372C+28] (o:\base\bind_internal.h:498)
	base::internal::InvokeHelper<0,void>::MakeItSo<void (aura::NativeWindowOcclusionTrackerWin::WindowOcclusionCalculator::*)() __attribute__((thiscall)),aura::NativeWindowOcclusionTrackerWin::WindowOcclusionCalculator *> [0x647F369F+79] (o:\base\bind_internal.h:598)
	base::internal::Invoker<base::internal::BindState<void (aura::NativeWindowOcclusionTrackerWin::WindowOcclusionCalculator::*)() __attribute__((thiscall)),base::internal::UnretainedWrapper<aura::NativeWindowOcclusionTrackerWin::WindowOcclusionCalculator> >, [0x647F35F5+85] (o:\base\bind_internal.h:671)
	base::internal::Invoker<base::internal::BindState<void (aura::NativeWindowOcclusionTrackerWin::WindowOcclusionCalculator::*)() __attribute__((thiscall)),base::internal::UnretainedWrapper<aura::NativeWindowOcclusionTrackerWin::WindowOcclusionCalculator> >, [0x647F3584+84] (o:\base\bind_internal.h:640)
	base::OnceCallback<void ()>::Run [0x6EF62360+80] (o:\base\callback.h:99)
	base::OneShotTimer::RunUserTask [0x6F1F8DB9+265] (o:\base\timer\timer.cc:265)
	base::internal::TimerBase::RunScheduledTask [0x6F1F88D5+293] (o:\base\timer\timer.cc:227)
	base::internal::BaseTimerTaskInternal::Run [0x6F1F875C+60] (o:\base\timer\timer.cc:50)
	base::internal::FunctorTraits<void (base::internal::BaseTimerTaskInternal::*)() __attribute__((thiscall)),void>::Invoke<void (base::internal::BaseTimerTaskInternal::*)() __attribute__((thiscall)),base::internal::BaseTimerTaskInternal *> [0x6F1F9CAC+28] (o:\base\bind_internal.h:498)
	base::internal::InvokeHelper<0,void>::MakeItSo<void (base::internal::BaseTimerTaskInternal::*)() __attribute__((thiscall)),base::internal::BaseTimerTaskInternal *> [0x6F1F9C0F+79] (o:\base\bind_internal.h:598)
	base::internal::Invoker<base::internal::BindState<void (base::internal::BaseTimerTaskInternal::*)() __attribute__((thiscall)),base::internal::OwnedWrapper<base::internal::BaseTimerTaskInternal> >,void ()>::RunImpl<void (base::internal::BaseTimerTaskIntern [0x6F1F9B65+85] (o:\base\bind_internal.h:671)
	base::internal::Invoker<base::internal::BindState<void (base::internal::BaseTimerTaskInternal::*)() __attribute__((thiscall)),base::internal::OwnedWrapper<base::internal::BaseTimerTaskInternal> >,void ()>::RunOnce [0x6F1F9AF4+84] (o:\base\bind_internal.h:640)
	base::OnceCallback<void ()>::Run [0x6EF62360+80] (o:\base\callback.h:99)
	base::TaskAnnotator::RunTask [0x6F116BF7+1447] (o:\base\task\common\task_annotator.cc:144)
	base::internal::TaskTracker::RunSkipOnShutdown [0x6F1A354D+61] (o:\base\task\thread_pool\task_tracker.cc:778)
	base::internal::TaskTracker::RunTaskWithShutdownBehavior [0x6F1A2C1C+108] (o:\base\task\thread_pool\task_tracker.cc:796)
	base::internal::TaskTracker::RunTask [0x6F1A2289+2521] (o:\base\task\thread_pool\task_tracker.cc:639)
	base::internal::TaskTracker::RunAndPopNextTask [0x6F1A0FA6+598] (o:\base\task\thread_pool\task_tracker.cc:512)
	base::internal::WorkerThread::RunWorker [0x6F1C47CE+2270] (o:\base\task\thread_pool\worker_thread.cc:321)
	base::internal::WorkerThread::RunSharedCOMWorker [0x6F1C3E92+34] (o:\base\task\thread_pool\worker_thread.cc:261)
	base::internal::WorkerThread::ThreadMain [0x6F1C3C3A+250] (o:\base\task\thread_pool\worker_thread.cc:213)
	base::`anonymous namespace'::ThreadFunc [0x6F2CCF4D+301] (o:\base\threading\platform_thread_win.cc:112)
	BaseThreadInitThunk [0x7740343D+18]
	RtlInitializeExceptionChain [0x77919802+99]
	RtlInitializeExceptionChain [0x779197D5+54]
Task trace:
Backtrace:
	aura::NativeWindowOcclusionTrackerWin::WindowOcclusionCalculator::ScheduleOcclusionCalculationIfNeeded [0x647DF4F2+290] (o:\ui\aura\native_window_occlusion_tracker_win.cc:433)
	aura::NativeWindowOcclusionTrackerWin::WindowOcclusionCalculator::ProcessEventHookCallback [0x647DFBB4+324] (o:\ui\aura\native_window_occlusion_tracker_win.cc:600)

Original change's description:
> Use unoccluded desktop region to calculate root window occlusion.
> 
> Bug: 1029842
> Change-Id: I285c8dec077575ce246c476645e2ff4a3294124c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1951775
> Commit-Queue: David Bienvenu <davidbienvenu@chromium.org>
> Reviewed-by: François Doray <fdoray@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#725750}

TBR=chrisha@chromium.org,fdoray@chromium.org,davidbienvenu@chromium.org

Change-Id: I3de865e1e2bfbc0178f117ff5756394b52f1de88
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1029842
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1974052Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#725993}

ui/aura/native_window_occlusion_tracker_win.cc

@@ -263,7 +263,8 @@ NativeWindowOcclusionTrackerWin::WindowOcclusionCalculator::
 void NativeWindowOcclusionTrackerWin::WindowOcclusionCalculator::
     EnableOcclusionTrackingForWindow(HWND hwnd) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  root_window_hwnds_occlusion_state_[hwnd] = Window::OcclusionState::UNKNOWN;
+  NativeWindowOcclusionState default_state;
+  root_window_hwnds_occlusion_state_[hwnd] = default_state;
   if (global_event_hooks_.empty())
     RegisterEventHooks();
 
@@ -354,26 +355,38 @@ void NativeWindowOcclusionTrackerWin::WindowOcclusionCalculator::
       SkIRect::MakeLTRB(screen_left, screen_top,
                         screen_left + GetSystemMetrics(SM_CXVIRTUALSCREEN),
                         screen_top + GetSystemMetrics(SM_CYVIRTUALSCREEN)));
-  num_root_windows_with_unknown_occlusion_state_ = 0;
 
   for (auto& root_window_pair : root_window_hwnds_occlusion_state_) {
+    root_window_pair.second.unoccluded_region.setEmpty();
     HWND hwnd = root_window_pair.first;
 
     // IsIconic() checks for a minimized window. Immediately set the state of
     // minimized windows to HIDDEN.
     if (IsIconic(hwnd)) {
-      root_window_pair.second = Window::OcclusionState::HIDDEN;
+      root_window_pair.second.occlusion_state = Window::OcclusionState::HIDDEN;
     } else if (IsWindowOnCurrentVirtualDesktop(hwnd) == false) {
       // If window is not on the current virtual desktop, immediately
       // set the state of the window to OCCLUDED.
-      root_window_pair.second = Window::OcclusionState::OCCLUDED;
+      root_window_pair.second.occlusion_state =
+          Window::OcclusionState::OCCLUDED;
       // Don't unregister event hooks when not on current desktop. There's no
       // notification when that changes, so we can't reregister event hooks.
       should_unregister_event_hooks = false;
     } else {
-      root_window_pair.second = Window::OcclusionState::UNKNOWN;
+      root_window_pair.second.occlusion_state = Window::OcclusionState::UNKNOWN;
+      RECT window_rect;
+      if (GetWindowRect(hwnd, &window_rect) != 0) {
+        root_window_pair.second.unoccluded_region =
+            SkRegion(SkIRect::MakeLTRB(window_rect.left, window_rect.top,
+                                       window_rect.right, window_rect.bottom));
+        // Clip the unoccluded region by the screen dimensions, to handle the
+        // case of the app window being partly off screen.
+        root_window_pair.second.unoccluded_region.op(screen_region,
+                                                     SkRegion::kIntersect_Op);
+      }
+      // If call to GetWindowRect fails, window will be treated as occluded,
+      // because unoccluded_region will be empty.
       should_unregister_event_hooks = false;
-      num_root_windows_with_unknown_occlusion_state_++;
     }
   }
   // Unregister event hooks if all native windows are minimized.
@@ -381,7 +394,6 @@ void NativeWindowOcclusionTrackerWin::WindowOcclusionCalculator::
     UnregisterEventHooks();
   } else {
     base::flat_set<DWORD> current_pids_with_visible_windows;
-    unoccluded_desktop_region_ = screen_region;
     // Calculate unoccluded region if there is a non-minimized native window.
     // Also compute |current_pids_with_visible_windows| as we enumerate
     // the windows.
@@ -415,15 +427,25 @@ void NativeWindowOcclusionTrackerWin::WindowOcclusionCalculator::
   }
   // Determine new occlusion status and post a task to the browser ui
   // thread to update the window occlusion state on the root windows.
+  base::flat_map<HWND, Window::OcclusionState> window_occlusion_states;
+
   for (auto& root_window_pair : root_window_hwnds_occlusion_state_) {
-    if (root_window_pair.second == Window::OcclusionState::UNKNOWN)
-      DCHECK(FALSE) << "A root window did not get its occlusion state set";
+    Window::OcclusionState new_state;
+    if (root_window_pair.second.occlusion_state !=
+        Window::OcclusionState::UNKNOWN) {
+      new_state = root_window_pair.second.occlusion_state;
+    } else {
+      new_state = root_window_pair.second.unoccluded_region.isEmpty()
+                      ? Window::OcclusionState::OCCLUDED
+                      : Window::OcclusionState::VISIBLE;
+    }
+    window_occlusion_states[root_window_pair.first] = new_state;
+    root_window_pair.second.occlusion_state = new_state;
   }
   ui_thread_task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(&NativeWindowOcclusionTrackerWin::UpdateOcclusionState,
-                     base::Unretained(g_tracker),
-                     root_window_hwnds_occlusion_state_));
+                     base::Unretained(g_tracker), window_occlusion_states));
 }
 
 void NativeWindowOcclusionTrackerWin::WindowOcclusionCalculator::
@@ -502,56 +524,46 @@ bool NativeWindowOcclusionTrackerWin::WindowOcclusionCalculator::
         base::flat_set<DWORD>* current_pids_with_visible_windows) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  SkRegion curr_unoccluded_destkop = unoccluded_desktop_region_;
   gfx::Rect window_rect;
-  bool window_is_occluding =
-      WindowCanOccludeOtherWindowsOnCurrentVirtualDesktop(hwnd, &window_rect);
-  if (window_is_occluding) {
-    // Hook this window's process with EVENT_OBJECT_LOCATION_CHANGE, if we are
-    // not already doing so.
-    DWORD pid;
-    GetWindowThreadProcessId(hwnd, &pid);
-    current_pids_with_visible_windows->insert(pid);
-    if (!base::Contains(process_event_hooks_, pid))
-      RegisterEventHookForProcess(pid);
-
-    // If no more root windows to consider, return true so we can continue
-    // looking for windows we haven't hooked.
-    if (num_root_windows_with_unknown_occlusion_state_ == 0)
-      return true;
-
-    SkRegion window_region(SkIRect::MakeLTRB(window_rect.x(), window_rect.y(),
-                                             window_rect.right(),
-                                             window_rect.bottom()));
-    unoccluded_desktop_region_.op(window_region, SkRegion::kDifference_Op);
-  } else if (num_root_windows_with_unknown_occlusion_state_ == 0) {
-    // This window can't occlude other windows, but we've determined the
-    // occlusion state of all root windows, so we can return.
-    return true;
-  }
-
-  // Check if |hwnd| is a root window; if so, we're done figuring out
+  // Check if |hwnd| is a root_window; if so, we're done figuring out
   // if it's occluded because we've seen all the windows "over" it.
+  // TODO(davidbienvenu): Explore checking if occlusion state has been
+  // computed for all |root_window_hwnds_occlusion_state_|, and if so, skipping
+  // further oclcusion calculations. However, we still want to keep computing
+  // |current_pids_with_visible_windows_|, so this function always returns true.
   auto it = root_window_hwnds_occlusion_state_.find(hwnd);
-  if (it == root_window_hwnds_occlusion_state_.end() ||
-      it->second != Window::OcclusionState::UNKNOWN) {
-    return true;
+  if (it != root_window_hwnds_occlusion_state_.end() &&
+      it->second.occlusion_state != Window::OcclusionState::HIDDEN) {
+    it->second.occlusion_state = it->second.unoccluded_region.isEmpty()
+                                     ? Window::OcclusionState::OCCLUDED
+                                     : Window::OcclusionState::VISIBLE;
   }
-  // On Win7, default theme makes root windows have complex regions by
-  // default. But we can still check if their bounding rect is occluded.
-  if (!window_is_occluding) {
-    RECT rect;
-    if (::GetWindowRect(hwnd, &rect) != 0) {
-      SkRegion window_region(
-          SkIRect::MakeLTRB(rect.left, rect.top, rect.right, rect.bottom));
-
-      curr_unoccluded_destkop.op(window_region, SkRegion::kDifference_Op);
+
+  if (!WindowCanOccludeOtherWindowsOnCurrentVirtualDesktop(hwnd, &window_rect))
+    return true;
+  // We are interested in this window, but are not currently hooking it with
+  // EVENT_OBJECT_LOCATION_CHANGE, so we need to hook it. We check
+  // this by seeing if its PID is in |process_event_hooks_|.
+  DWORD pid;
+  GetWindowThreadProcessId(hwnd, &pid);
+  current_pids_with_visible_windows->insert(pid);
+  if (!base::Contains(process_event_hooks_, pid))
+    RegisterEventHookForProcess(pid);
+
+  SkRegion window_region(SkIRect::MakeLTRB(window_rect.x(), window_rect.y(),
+                                           window_rect.right(),
+                                           window_rect.bottom()));
+
+  for (auto& root_window_pair : root_window_hwnds_occlusion_state_) {
+    if (root_window_pair.second.occlusion_state !=
+        Window::OcclusionState::UNKNOWN)
+      continue;
+    if (!root_window_pair.second.unoccluded_region.op(
+            window_region, SkRegion::kDifference_Op)) {
+      root_window_pair.second.occlusion_state =
+          Window::OcclusionState::OCCLUDED;
     }
   }
-  it->second = (unoccluded_desktop_region_ == curr_unoccluded_destkop)
-                   ? Window::OcclusionState::OCCLUDED
-                   : Window::OcclusionState::VISIBLE;
-  num_root_windows_with_unknown_occlusion_state_--;
   return true;
 }
 

ui/aura/native_window_occlusion_tracker_win.h

@@ -78,6 +78,17 @@ class AURA_EXPORT NativeWindowOcclusionTrackerWin : public WindowObserver {
     friend class NativeWindowOcclusionTrackerTest;
     friend class test::AuraTestHelper;
 
+    struct NativeWindowOcclusionState {
+      // The region of the native window that is not occluded by other windows.
+      SkRegion unoccluded_region;
+
+      // The current occlusion state of the native window. Default to UNKNOWN
+      // because we do not know the state starting out. More information on
+      // these states can be found in aura::Window.
+      aura::Window::OcclusionState occlusion_state =
+          aura::Window::OcclusionState::UNKNOWN;
+    };
+
     // Registers event hooks, if not registered.
     void MaybeRegisterEventHooks();
 
@@ -173,7 +184,7 @@ class AURA_EXPORT NativeWindowOcclusionTrackerWin : public WindowObserver {
 
     // Map of root app window hwnds and their occlusion state. This contains
     // both visible and hidden windows.
-    base::flat_map<HWND, Window::OcclusionState>
+    base::flat_map<HWND, NativeWindowOcclusionState>
         root_window_hwnds_occlusion_state_;
 
     // Values returned by SetWinEventHook are stored so that hooks can be
@@ -196,19 +207,6 @@ class AURA_EXPORT NativeWindowOcclusionTrackerWin : public WindowObserver {
     // calculating window occlusion.
     bool window_is_moving_ = false;
 
-    // Used to determine if a root window is occluded. As we iterate through the
-    // hwnds in z-order, we subtract each opaque window's rect from
-    // |unoccluded_desktop_region_|. When we get to a root window, we subtract
-    // it from |unoccluded_desktop_region_|, and if |unoccluded_desktop_region_|
-    // doesn't change, the root window was already occluded.
-    SkRegion unoccluded_desktop_region_;
-
-    // Keeps track of how many root windows we need to compute the occlusion
-    // state of in a call to ComputeNativeWindowOcclusionStatus. Once we've
-    // determined the state of all root windows, we can stop subtracting
-    // windows from |unoccluded_desktop_region_|.
-    int num_root_windows_with_unknown_occlusion_state_;
-
     // Only used on Win10+.
     Microsoft::WRL::ComPtr<IVirtualDesktopManager> virtual_desktop_manager_;