[Backport] CVE-2021-30510: Race in Aura

Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/2868317:
views: handle deletion when toggling fullscreen

This differs from the first in so far as needing to add more early
outs in the windows side if destroyed. This was caught by the asan
bot.

Toggling fullscreen means the bounds change. There are some
code paths that may delete the Widget when the bounds changes.
This patch ensures the right thing happens if the Widget is
deleted when this happens.

BUG=1197436
TEST=DesktopWidgetTest.DestroyInSetFullscreen

(cherry picked from commit 60fe7a686c0620855c28a60721f668a99e409ee4)

Change-Id: I8ce8f2045878b6f6de530f58e386149189900498
Reviewed-by: Thomas Anderson <[email protected]>
Commit-Queue: Scott Violet <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#877640}
Auto-Submit: Scott Violet <[email protected]>
Commit-Queue: Thomas Anderson <[email protected]>
Cr-Commit-Position: refs/branch-heads/4430@{#1383}
Cr-Branched-From: e5ce7dc4f7518237b3d9bb93cccca35d25216cbe-refs/heads/master@{#857950}
Reviewed-by: Allan Sandfeld Jensen <[email protected]>

Author: Scott Violet

chromium/ui/views/widget/desktop_aura/desktop_window_tree_host_platform.cc

@@ -583,7 +583,10 @@ void DesktopWindowTreeHostPlatform::SetFullscreen(bool fullscreen) {
   if (IsFullscreen() == fullscreen)
     return;
 
+  auto weak_ptr = GetWeakPtr();
   platform_window()->ToggleFullscreen();
+  if (!weak_ptr)
+    return;
 
   // The state must change synchronously to let media react on fullscreen
   // changes.

chromium/ui/views/widget/desktop_aura/desktop_window_tree_host_win.cc

@@ -463,7 +463,10 @@ void DesktopWindowTreeHostWin::FrameTypeChanged() {
 }
 
 void DesktopWindowTreeHostWin::SetFullscreen(bool fullscreen) {
+  auto weak_ptr = GetWeakPtr();
   message_handler_->SetFullscreen(fullscreen);
+  if (!weak_ptr)
+    return;
   // TODO(sky): workaround for ScopedFullscreenVisibility showing window
   // directly. Instead of this should listen for visibility changes and then
   // update window.

chromium/ui/views/widget/widget.cc

@@ -724,7 +724,10 @@ void Widget::SetFullscreen(bool fullscreen) {
   if (IsFullscreen() == fullscreen)
     return;
 
+  auto weak_ptr = GetWeakPtr();
   native_widget_->SetFullscreen(fullscreen);
+  if (!weak_ptr)
+    return;
 
   if (non_client_view_)
     non_client_view_->InvalidateLayout();

chromium/ui/views/widget/widget_unittest.cc

@@ -3084,6 +3084,41 @@ TEST_F(DesktopWidgetTest, FullscreenStatePropagated_DesktopWidget) {
             IsNativeWindowVisible(top_level_widget->GetNativeWindow()));
 }
 
+// Used to delete the widget when the supplied bounds changes.
+class DestroyingWidgetBoundsObserver : public WidgetObserver {
+ public:
+  explicit DestroyingWidgetBoundsObserver(std::unique_ptr<Widget> widget)
+      : widget_(std::move(widget)) {
+    widget_->AddObserver(this);
+  }
+
+  // There are no assertions here as not all platforms call
+  // OnWidgetBoundsChanged() when going fullscreen.
+  ~DestroyingWidgetBoundsObserver() override = default;
+
+  // WidgetObserver:
+  void OnWidgetBoundsChanged(Widget* widget,
+                             const gfx::Rect& new_bounds) override {
+    widget_->RemoveObserver(this);
+    widget_.reset();
+  }
+
+ private:
+  std::unique_ptr<Widget> widget_;
+};
+
+// Deletes a Widget when the bounds change as part of toggling fullscreen.
+// This is a regression test for https://crbug.com/1197436 .
+TEST_F(DesktopWidgetTest, DeleteInSetFullscreen) {
+  std::unique_ptr<Widget> widget = std::make_unique<Widget>();
+  Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_WINDOW);
+  params.ownership = Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
+  widget->Init(std::move(params));
+  Widget* w = widget.get();
+  DestroyingWidgetBoundsObserver destroyer(std::move(widget));
+  w->SetFullscreen(true);
+}
+
 namespace {
 
 class FullscreenAwareFrame : public views::NonClientFrameView {

chromium/ui/views/win/fullscreen_handler.cc

@@ -70,6 +70,7 @@ void FullscreenHandler::SetFullscreenImpl(bool fullscreen) {
 
   fullscreen_ = fullscreen;
 
+  auto ref = weak_ptr_factory_.GetWeakPtr();
   if (fullscreen_) {
     // Set new window style and size.
     SetWindowLong(hwnd_, GWL_STYLE,
@@ -102,6 +103,8 @@ void FullscreenHandler::SetFullscreenImpl(bool fullscreen) {
                  new_rect.height(),
                  SWP_NOZORDER | SWP_NOACTIVATE | SWP_FRAMECHANGED);
   }
+  if (!ref)
+    return;
 
   MarkFullscreen(fullscreen);
 }

chromium/ui/views/win/fullscreen_handler.h

@@ -11,6 +11,7 @@
 #include <map>
 
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 
 namespace gfx {
 class Rect;
@@ -54,6 +55,8 @@ class FullscreenHandler {
   // Used to mark a window as fullscreen.
   Microsoft::WRL::ComPtr<ITaskbarList2> task_bar_list_;
 
+  base::WeakPtrFactory<FullscreenHandler> weak_ptr_factory_{this};
+
   DISALLOW_COPY_AND_ASSIGN(FullscreenHandler);
 };
 

chromium/ui/views/win/hwnd_message_handler.cc

@@ -895,7 +895,10 @@ void HWNDMessageHandler::SetWindowIcons(const gfx::ImageSkia& window_icon,
 
 void HWNDMessageHandler::SetFullscreen(bool fullscreen) {
   background_fullscreen_hack_ = false;
+  auto ref = msg_handler_weak_factory_.GetWeakPtr();
   fullscreen_handler()->SetFullscreen(fullscreen);
+  if (!ref)
+    return;
 
   // Add the fullscreen window to the fullscreen window map which is used to
   // handle window activations.
@@ -1392,8 +1395,10 @@ void HWNDMessageHandler::ClientAreaSizeChanged() {
   // Ignore size changes due to fullscreen windows losing activation.
   if (background_fullscreen_hack_ && !sent_window_size_changing_)
     return;
-  gfx::Size s = GetClientAreaBounds().size();
-  delegate_->HandleClientSizeChanged(s);
+  auto ref = msg_handler_weak_factory_.GetWeakPtr();
+  delegate_->HandleClientSizeChanged(GetClientAreaBounds().size());
+  if (!ref)
+    return;
 
   current_window_size_message_++;
   sent_window_size_changing_ = false;
@@ -2918,8 +2923,11 @@ void HWNDMessageHandler::OnWindowPosChanging(WINDOWPOS* window_pos) {
 void HWNDMessageHandler::OnWindowPosChanged(WINDOWPOS* window_pos) {
   TRACE_EVENT0("ui", "HWNDMessageHandler::OnWindowPosChanged");
 
+  base::WeakPtr<HWNDMessageHandler> ref(msg_handler_weak_factory_.GetWeakPtr());
   if (DidClientAreaSizeChange(window_pos))
     ClientAreaSizeChanged();
+  if (!ref)
+    return;
   if (window_pos->flags & SWP_FRAMECHANGED)
     SetDwmFrameExtension(DwmFrameState::kOn);
   if (window_pos->flags & SWP_SHOWWINDOW) {