From 2d484049e88157419b77ae3e68f4a8ff598a242d Mon Sep 17 00:00:00 2001
From: Tommy Steimel <[email protected]>
Date: Tue, 17 Jun 2025 12:16:40 -0700
Subject: [PATCH] Track floating glic widgets to handle when they occlude dialogs

The PictureInPictureOcclusionTracker keeps track of picture-in-picture
widgets and important dialogs (e.g. permission dialogs) to ensure that
when a picture-in-picture window occludes an important dialog, we
properly handle it (e.g. by disabling the dialog buttons).

GLIC widgets also float on top of other windows, and so face some of
the same security issues as picture-in-picture windows.

This CL tells the PictureInPictureOcclusionTracker to also track the
GLIC widget (when it's floating) so we can handle when it is occluding
important dialogs.

Bug: 407791462
Change-Id: Ic1e67d193d6f61db0d12a342da867b1e89475ae8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6639092
Reviewed-by: Justin DeWitt <[email protected]>
Reviewed-by: Frank Liberato <[email protected]>
Commit-Queue: Tommy Steimel <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1475149}
---

diff --git a/chrome/browser/glic/BUILD.gn b/chrome/browser/glic/BUILD.gn
index c816509..57bd9f9 100644
--- a/chrome/browser/glic/BUILD.gn
+++ b/chrome/browser/glic/BUILD.gn
@@ -94,6 +94,8 @@
     "glic_metrics.cc",
     "glic_metrics_provider.cc",
     "glic_metrics_provider.h",
+    "glic_occlusion_notifier.cc",
+    "glic_occlusion_notifier.h",
     "glic_pref_names.cc",
     "glic_profile_manager.cc",
     "glic_settings_util.cc",
@@ -150,6 +152,7 @@
     "//chrome/browser/glic/media",
     "//chrome/browser/lifetime:termination_notification",
     "//chrome/browser/media/webrtc",
+    "//chrome/browser/picture_in_picture",
     "//chrome/browser/profiles:profile_util",
     "//chrome/browser/profiles/keep_alive",
     "//chrome/browser/resources/glic:resources",
diff --git a/chrome/browser/glic/glic_keyed_service.cc b/chrome/browser/glic/glic_keyed_service.cc
index 61ec92c..c5cd105 100644
--- a/chrome/browser/glic/glic_keyed_service.cc
+++ b/chrome/browser/glic/glic_keyed_service.cc
@@ -23,6 +23,7 @@
 #include "chrome/browser/glic/glic_enums.h"
 #include "chrome/browser/glic/glic_keyed_service_factory.h"
 #include "chrome/browser/glic/glic_metrics.h"
+#include "chrome/browser/glic/glic_occlusion_notifier.h"
 #include "chrome/browser/glic/glic_pref_names.h"
 #include "chrome/browser/glic/glic_profile_manager.h"
 #include "chrome/browser/glic/host/auth_controller.h"
@@ -105,6 +106,8 @@
       auth_controller_(std::make_unique<AuthController>(profile,
                                                         identity_manager,
                                                         /*use_for_fre=*/false)),
+      occlusion_notifier_(
+          std::make_unique<GlicOcclusionNotifier>(*window_controller_)),
       contextual_cueing_service_(contextual_cueing_service) {
   CHECK(GlicEnabling::IsProfileEligible(Profile::FromBrowserContext(profile)));
   host_->Initialize(window_controller_.get());
diff --git a/chrome/browser/glic/glic_keyed_service.h b/chrome/browser/glic/glic_keyed_service.h
index e6eb733..35002f61 100644
--- a/chrome/browser/glic/glic_keyed_service.h
+++ b/chrome/browser/glic/glic_keyed_service.h
@@ -41,6 +41,7 @@
 class GlicActorController;
 class GlicEnabling;
 class GlicMetrics;
+class GlicOcclusionNotifier;
 class GlicProfileManager;
 class GlicScreenshotCapturer;
 class GlicSharingManagerImpl;
@@ -243,6 +244,7 @@
   std::unique_ptr<AuthController> auth_controller_;
   std::unique_ptr<GlicActorController> actor_controller_;
   std::unique_ptr<base::MemoryPressureListener> memory_pressure_listener_;
+  std::unique_ptr<GlicOcclusionNotifier> occlusion_notifier_;
   base::OnceCallback<void()> preload_callback_;
 
   // Unowned
diff --git a/chrome/browser/glic/glic_occlusion_notifier.cc b/chrome/browser/glic/glic_occlusion_notifier.cc
new file mode 100644
index 0000000..eb044a6
--- /dev/null
+++ b/chrome/browser/glic/glic_occlusion_notifier.cc
@@ -0,0 +1,44 @@
+// Copyright 2025 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/glic/glic_occlusion_notifier.h"
+
+#include "chrome/browser/glic/widget/glic_widget.h"
+#include "chrome/browser/picture_in_picture/picture_in_picture_occlusion_tracker.h"
+#include "chrome/browser/picture_in_picture/picture_in_picture_window_manager.h"
+
+namespace glic {
+
+GlicOcclusionNotifier::GlicOcclusionNotifier(
+    GlicWindowController& window_controller)
+    : window_controller_(window_controller) {
+  window_controller_->AddStateObserver(this);
+}
+
+GlicOcclusionNotifier::~GlicOcclusionNotifier() {
+  window_controller_->RemoveStateObserver(this);
+}
+
+void GlicOcclusionNotifier::PanelStateChanged(
+    const mojom::PanelState& panel_state,
+    Browser*) {
+  PictureInPictureOcclusionTracker* tracker =
+      PictureInPictureWindowManager::GetInstance()->GetOcclusionTracker();
+  if (!tracker) {
+    return;
+  }
+
+  views::Widget* glic_widget = window_controller_->GetGlicWidget();
+  if (!glic_widget) {
+    return;
+  }
+
+  if (panel_state.kind == mojom::PanelState_Kind::kDetached) {
+    tracker->OnPictureInPictureWidgetOpened(glic_widget);
+  } else {
+    tracker->RemovePictureInPictureWidget(glic_widget);
+  }
+}
+
+}  // namespace glic
diff --git a/chrome/browser/glic/glic_occlusion_notifier.h b/chrome/browser/glic/glic_occlusion_notifier.h
new file mode 100644
index 0000000..a0bc7c2
--- /dev/null
+++ b/chrome/browser/glic/glic_occlusion_notifier.h
@@ -0,0 +1,31 @@
+// Copyright 2025 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_GLIC_GLIC_OCCLUSION_NOTIFIER_H_
+#define CHROME_BROWSER_GLIC_GLIC_OCCLUSION_NOTIFIER_H_
+
+#include "chrome/browser/glic/widget/glic_window_controller.h"
+
+namespace glic {
+
+// The GlicOcclusionNotifier notifies the PictureInPictureOcclusionTracker when
+// to track the Glic window for occlusion of important security dialogs.
+class GlicOcclusionNotifier : public GlicWindowController::StateObserver {
+ public:
+  explicit GlicOcclusionNotifier(GlicWindowController& window_controller);
+  GlicOcclusionNotifier(const GlicOcclusionNotifier&) = delete;
+  GlicOcclusionNotifier& operator=(const GlicOcclusionNotifier&) = delete;
+  ~GlicOcclusionNotifier() override;
+
+  // GlicWindowController::StateObserver:
+  void PanelStateChanged(const mojom::PanelState& panel_state,
+                         Browser*) override;
+
+ private:
+  raw_ref<GlicWindowController> window_controller_;
+};
+
+}  // namespace glic
+
+#endif  // CHROME_BROWSER_GLIC_GLIC_OCCLUSION_NOTIFIER_H_
diff --git a/chrome/browser/glic/test_support/interactive_glic_test.h b/chrome/browser/glic/test_support/interactive_glic_test.h
index 1779f35..b84004a3 100644
--- a/chrome/browser/glic/test_support/interactive_glic_test.h
+++ b/chrome/browser/glic/test_support/interactive_glic_test.h
@@ -26,6 +26,8 @@
 #include "chrome/browser/glic/widget/glic_view.h"
 #include "chrome/browser/glic/widget/glic_widget.h"
 #include "chrome/browser/glic/widget/glic_window_controller.h"
+#include "chrome/browser/picture_in_picture/picture_in_picture_occlusion_tracker.h"
+#include "chrome/browser/picture_in_picture/picture_in_picture_window_manager.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_element_identifiers.h"
 #include "chrome/browser/ui/browser_list.h"
@@ -379,6 +381,17 @@
         expected_count, "CheckTabCount");
   }
 
+  auto CheckOcclusionTracked(bool expect_is_tracked) {
+    return Api::CheckResult(
+        [this]() {
+          return base::Contains(PictureInPictureWindowManager::GetInstance()
+                                    ->GetOcclusionTracker()
+                                    ->GetPictureInPictureWidgetsForTesting(),
+                                window_controller().GetGlicWidget());
+        },

diff --git a/chrome/browser/glic/widget/glic_window_controller_interactive_uitest.cc b/chrome/browser/glic/widget/glic_window_controller_interactive_uitest.cc
index 12d0386..45d49e1 100644
--- a/chrome/browser/glic/widget/glic_window_controller_interactive_uitest.cc
+++ b/chrome/browser/glic/widget/glic_window_controller_interactive_uitest.cc
@@ -376,6 +376,12 @@
       WaitForState(test::internal::kGlicAppState, mojom::WebUiState::kReady));
 }
 
+IN_PROC_BROWSER_TEST_F(GlicWindowControllerUiTest,
+                       DetachedWidgetIsTrackedByOcclusionTracker) {
+  RunTestSequence(OpenGlicWindow(GlicWindowMode::kDetached),
+                  CheckOcclusionTracked(true));
+}
+
 IN_PROC_BROWSER_TEST_F(GlicWindowControllerUiTest, TestInitialBounds) {
   // The GlicButton and Tabstrip are not actually shown until a tab is created.
   chrome::AddTabAt(browser(), GURL("about:blank"), 0, true);
@@ -680,6 +686,6 @@
                   InAnyContext(DetachGlicWindow(), MoveWidgetToSecondDisplay(),
                                CheckWidgetMovedToSecondaryDisplay(true)));
 }
-#endif
+#endif  // BUILDFLAG(IS_MAC)
 
 }  // namespace glic
diff --git a/chrome/browser/picture_in_picture/picture_in_picture_occlusion_tracker_unittest.cc b/chrome/browser/picture_in_picture/picture_in_picture_occlusion_tracker_unittest.cc
index b5590f8d..25a5a43 100644
--- a/chrome/browser/picture_in_picture/picture_in_picture_occlusion_tracker_unittest.cc
+++ b/chrome/browser/picture_in_picture/picture_in_picture_occlusion_tracker_unittest.cc
@@ -305,4 +305,41 @@
   observation.Observe(occludable_widget1.get());
 }
 
+TEST_F(PictureInPictureOcclusionTrackerTest,
+       CanStopObservingExistingPictureInPictureWidget) {
+  MockPictureInPictureOcclusionObserver observer;
+  ScopedPictureInPictureOcclusionObservation observation(&observer);
+  std::unique_ptr<views::Widget> picture_in_picture_widget =
+      CreatePictureInPictureWidget();
+
+  // Create a widget to track the occlusion state of, placing it so that it
+  // starts out occluded.
+  std::unique_ptr<views::Widget> occludable_widget =
+      CreateTestWidget(views::Widget::InitParams::CLIENT_OWNS_WIDGET);
+  occludable_widget->Show();
+  occludable_widget->SetBounds({50, 50, 200, 200});
+
+  // Start observing occlusion state. This should immediately tell the observer
+  // that it's occluded.
+  EXPECT_CALL(observer, OnOcclusionStateChanged(true));
+  observation.Observe(occludable_widget.get());
+  testing::Mock::VerifyAndClearExpectations(&observer);
+
+  // Tell the tracker to stop tracking the picture-in-picture widget. This
+  // should inform the observer that it's no longer occluded.
+  EXPECT_CALL(observer, OnOcclusionStateChanged(false));
+  PictureInPictureWindowManager::GetInstance()
+      ->GetOcclusionTracker()
+      ->RemovePictureInPictureWidget(picture_in_picture_widget.get());
+  testing::Mock::VerifyAndClearExpectations(&observer);
+
+  // Tell the tracker to start tracking the picture-in-picture widget again.
+  // This should inform the observer that it's occluded again.
+  EXPECT_CALL(observer, OnOcclusionStateChanged(true));
+  PictureInPictureWindowManager::GetInstance()
+      ->GetOcclusionTracker()
+      ->OnPictureInPictureWidgetOpened(picture_in_picture_widget.get());
+  testing::Mock::VerifyAndClearExpectations(&observer);
+}
+
 }  // namespace