From 399b9db4d1f3dcd34554ba54cf2c54400ad4053a Mon Sep 17 00:00:00 2001
From: Rouslan Solomakhin <[email protected]>
Date: Fri, 25 Oct 2024 13:08:17 +0000
Subject: [PATCH] Close PaymentRequest UI when picture-in-picture occludes it.

Before this patch, picture-in-picture video could be displayed on top of
the PaymentRequest UI (including SPC) which could confuse users.

This patch adds a picture-in-picture occlusion observer to the desktop
PaymentRequest UI, SPC authentication UI, and SPC no-credentials UI.
When a UI is occluded, it is immediately disabled and a task to close
the UI is posted. Posting the task is necessary because the occlusion
could happen when initially showing the UI, when closing the UI is not
supported.

After this patch, PaymentRequest desktop UI is hidden when a
picture-in-picture video occludes it.

Bug: 40940854, 297885233, 40280009
Change-Id: I4430e573227d746a9a246f727b48798e4590198d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5955634
Commit-Queue: Rouslan Solomakhin <[email protected]>
Reviewed-by: Nick Navarro <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1373855}
---

diff --git a/chrome/browser/ui/views/payments/payment_request_dialog_view.cc b/chrome/browser/ui/views/payments/payment_request_dialog_view.cc
index 819cc3aa..25cee96 100644
--- a/chrome/browser/ui/views/payments/payment_request_dialog_view.cc
+++ b/chrome/browser/ui/views/payments/payment_request_dialog_view.cc
@@ -111,6 +111,7 @@
   views::Widget* widget = constrained_window::ShowWebModalDialogViews(
       this, request_->web_contents());
   extensions::SecurityDialogTracker::GetInstance()->AddSecurityDialog(widget);
+  occlusion_observation_.Observe(widget);
 }
 
 void PaymentRequestDialogView::CloseDialog() {
@@ -578,6 +579,15 @@
   }
 }
 
+void PaymentRequestDialogView::OnOcclusionStateChanged(bool occluded) {
+  if (occluded) {
+    SetEnabled(false);
+    base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
+        FROM_HERE, base::BindOnce(&PaymentRequestDialogView::CloseDialog,
+                                  weak_ptr_factory_.GetWeakPtr()));
+  }
+}
+
 void PaymentRequestDialogView::ResizeDialogWindow() {
   if (GetWidget() && request_->web_contents()) {
     constrained_window::UpdateWebContentsModalDialogPosition(
diff --git a/chrome/browser/ui/views/payments/payment_request_dialog_view.h b/chrome/browser/ui/views/payments/payment_request_dialog_view.h
index 304c964..8fcd4fa 100644
--- a/chrome/browser/ui/views/payments/payment_request_dialog_view.h
+++ b/chrome/browser/ui/views/payments/payment_request_dialog_view.h
@@ -11,6 +11,8 @@
 #include "base/functional/callback_forward.h"
 #include "base/memory/raw_ptr.h"
 #include "base/memory/weak_ptr.h"
+#include "chrome/browser/picture_in_picture/picture_in_picture_occlusion_observer.h"
+#include "chrome/browser/picture_in_picture/scoped_picture_in_picture_occlusion_observation.h"
 #include "chrome/browser/ui/views/payments/view_stack.h"
 #include "components/payments/content/initialization_task.h"
 #include "components/payments/content/payment_request_dialog.h"
@@ -48,7 +50,8 @@
 class PaymentRequestDialogView : public views::DialogDelegateView,
                                  public PaymentRequestDialog,
                                  public PaymentRequestSpec::Observer,
-                                 public InitializationTask::Observer {
+                                 public InitializationTask::Observer,
+                                 public PictureInPictureOcclusionObserver {
   METADATA_HEADER(PaymentRequestDialogView, views::DialogDelegateView)
 
  public:
@@ -204,6 +207,9 @@
   void ViewHierarchyChanged(
       const views::ViewHierarchyChangedDetails& details) override;
 
+  // PictureInPictureOcclusionObserver
+  void OnOcclusionStateChanged(bool occluded) override;
+
   // The PaymentRequest object that initiated this dialog.
   base::WeakPtr<PaymentRequest> request_;
   ControllerMap controller_map_;
@@ -231,6 +237,8 @@
   // handler window.
   int payment_handler_window_height_ = 0;
 
+  ScopedPictureInPictureOcclusionObservation occlusion_observation_{this};
+
   base::WeakPtrFactory<PaymentRequestDialogView> weak_ptr_factory_{this};
 };
 
diff --git a/chrome/browser/ui/views/payments/payment_request_picture_in_picture_occlusion_browsertest.cc b/chrome/browser/ui/views/payments/payment_request_picture_in_picture_occlusion_browsertest.cc
new file mode 100644
index 0000000..4c31b040
--- /dev/null
+++ b/chrome/browser/ui/views/payments/payment_request_picture_in_picture_occlusion_browsertest.cc
@@ -0,0 +1,40 @@
+// Copyright 2024 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <string>
+
+#include "chrome/browser/picture_in_picture/picture_in_picture_occlusion_observer.h"
+#include "chrome/browser/ui/views/payments/payment_request_browsertest_base.h"
+#include "content/public/test/browser_test.h"
+#include "content/public/test/browser_test_utils.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace payments {
+namespace {
+
+using PaymentRequestPictureInPictureOcclusionTest =
+    PaymentRequestBrowserTestBase;
+
+IN_PROC_BROWSER_TEST_F(PaymentRequestPictureInPictureOcclusionTest,
+                       OcclusionClosesDialog) {
+  std::string a_method_name;
+  InstallPaymentApp("a.com", "/payment_request_success_responder.js",
+                    &a_method_name);
+  std::string b_method_name;
+  InstallPaymentApp("b.com", "/payment_request_success_responder.js",
+                    &b_method_name);
+  NavigateTo("/payment_request_no_shipping_test.html");
+  InvokePaymentRequestUIWithJs(content::JsReplace(
+      "buyWithMethods([{supportedMethods:$1}, {supportedMethods:$2}]);",
+      a_method_name, b_method_name));
+  ResetEventWaiter(DialogEvent::DIALOG_CLOSED);
+
+  static_cast<PictureInPictureOcclusionObserver*>(dialog_view())
+      ->OnOcclusionStateChanged(/*occluded=*/true);
+
+  ASSERT_TRUE(WaitForObservedEvent());
+}
+
+}  // namespace
+}  // namespace payments
diff --git a/chrome/browser/ui/views/payments/secure_payment_confirmation_dialog_view.cc b/chrome/browser/ui/views/payments/secure_payment_confirmation_dialog_view.cc
index b5f26d249..1dad9bb 100644
--- a/chrome/browser/ui/views/payments/secure_payment_confirmation_dialog_view.cc
+++ b/chrome/browser/ui/views/payments/secure_payment_confirmation_dialog_view.cc
@@ -5,6 +5,7 @@
 #include "chrome/browser/ui/views/payments/secure_payment_confirmation_dialog_view.h"
 
 #include "base/metrics/histogram_functions.h"
+#include "base/task/single_thread_task_runner.h"
 #include "chrome/app/vector_icons/vector_icons.h"
 #include "chrome/browser/ui/views/chrome_layout_provider.h"
 #include "chrome/browser/ui/views/extensions/security_dialog_tracker.h"
@@ -142,6 +143,7 @@
   views::Widget* widget =
       constrained_window::ShowWebModalDialogViews(this, web_contents);
   extensions::SecurityDialogTracker::GetInstance()->AddSecurityDialog(widget);
+  occlusion_observation_.Observe(widget);
 
   // The progress bar doesn't exist until after ShowWebModalDialogViews, so we
   // have to update it here in case it starts visible.
@@ -513,6 +515,17 @@
   return row;
 }
 
+void SecurePaymentConfirmationDialogView::OnOcclusionStateChanged(
+    bool occluded) {
+  if (occluded) {
+    SetEnabled(false);
+    base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
+        FROM_HERE,
+        base::BindOnce(&SecurePaymentConfirmationDialogView::HideDialog,
+                       weak_ptr_factory_.GetWeakPtr()));
+  }
+}
+
 BEGIN_METADATA(SecurePaymentConfirmationDialogView)
 END_METADATA
 
diff --git a/chrome/browser/ui/views/payments/secure_payment_confirmation_dialog_view.h b/chrome/browser/ui/views/payments/secure_payment_confirmation_dialog_view.h
index a60e4065..2447c53 100644
--- a/chrome/browser/ui/views/payments/secure_payment_confirmation_dialog_view.h
+++ b/chrome/browser/ui/views/payments/secure_payment_confirmation_dialog_view.h
@@ -7,6 +7,8 @@
 
 #include "base/memory/raw_ptr.h"
 #include "base/memory/weak_ptr.h"
+#include "chrome/browser/picture_in_picture/picture_in_picture_occlusion_observer.h"
+#include "chrome/browser/picture_in_picture/scoped_picture_in_picture_occlusion_observation.h"
 #include "components/payments/content/secure_payment_confirmation_view.h"
 #include "ui/base/metadata/metadata_header_macros.h"
 #include "ui/views/controls/button/button.h"
@@ -24,7 +26,8 @@
 // the SecurePaymentConfirmationController.
 class SecurePaymentConfirmationDialogView
     : public SecurePaymentConfirmationView,
-      public views::DialogDelegateView {
+      public views::DialogDelegateView,
+      public PictureInPictureOcclusionObserver {

diff --git a/chrome/browser/ui/views/payments/payment_request_picture_in_picture_occlusion_browsertest.cc b/chrome/browser/ui/views/payments/payment_request_picture_in_picture_occlusion_browsertest.cc
new file mode 100644
index 0000000..4c31b040
--- /dev/null
+++ b/chrome/browser/ui/views/payments/payment_request_picture_in_picture_occlusion_browsertest.cc
@@ -0,0 +1,40 @@
+// Copyright 2024 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <string>
+
+#include "chrome/browser/picture_in_picture/picture_in_picture_occlusion_observer.h"
+#include "chrome/browser/ui/views/payments/payment_request_browsertest_base.h"
+#include "content/public/test/browser_test.h"
+#include "content/public/test/browser_test_utils.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace payments {
+namespace {
+
+using PaymentRequestPictureInPictureOcclusionTest =
+    PaymentRequestBrowserTestBase;
+
+IN_PROC_BROWSER_TEST_F(PaymentRequestPictureInPictureOcclusionTest,
+                       OcclusionClosesDialog) {
+  std::string a_method_name;
+  InstallPaymentApp("a.com", "/payment_request_success_responder.js",
+                    &a_method_name);
+  std::string b_method_name;
+  InstallPaymentApp("b.com", "/payment_request_success_responder.js",
+                    &b_method_name);
+  NavigateTo("/payment_request_no_shipping_test.html");
+  InvokePaymentRequestUIWithJs(content::JsReplace(
+      "buyWithMethods([{supportedMethods:$1}, {supportedMethods:$2}]);",
+      a_method_name, b_method_name));
+  ResetEventWaiter(DialogEvent::DIALOG_CLOSED);
+
+  static_cast<PictureInPictureOcclusionObserver*>(dialog_view())
+      ->OnOcclusionStateChanged(/*occluded=*/true);
+
+  ASSERT_TRUE(WaitForObservedEvent());
+}
+
+}  // namespace
+}  // namespace payments
diff --git a/chrome/browser/ui/views/payments/secure_payment_confirmation_dialog_view_browsertest.cc b/chrome/browser/ui/views/payments/secure_payment_confirmation_dialog_view_browsertest.cc
index abad5de..e3d5f023 100644
--- a/chrome/browser/ui/views/payments/secure_payment_confirmation_dialog_view_browsertest.cc
+++ b/chrome/browser/ui/views/payments/secure_payment_confirmation_dialog_view_browsertest.cc
@@ -5,12 +5,16 @@
 #include "chrome/browser/ui/views/payments/secure_payment_confirmation_dialog_view.h"
 
 #include <optional>
+#include <utility>
 
+#include "base/functional/callback.h"
+#include "base/run_loop.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
 #include "cc/test/pixel_comparator.h"
 #include "cc/test/pixel_test_utils.h"
 #include "chrome/app/vector_icons/vector_icons.h"
+#include "chrome/browser/picture_in_picture/picture_in_picture_occlusion_observer.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/test/test_browser_dialog.h"
@@ -310,7 +314,14 @@
   }
 
   // SecurePaymentConfirmationDialogView::ObserverForTest:
-  void OnDialogClosed() override { dialog_closed_ = true; }
+  void OnDialogClosed() override {
+    dialog_closed_ = true;
+    if (dialog_closed_callback_) {
+      std::move(dialog_closed_callback_).Run();
+    }
+  }
+
+  // SecurePaymentConfirmationDialogView::ObserverForTest:
   void OnConfirmButtonPressed() override { confirm_pressed_ = true; }
   void OnCancelButtonPressed() override { cancel_pressed_ = true; }
   void OnOptOutClicked() override { opt_out_clicked_ = true; }
@@ -328,6 +339,7 @@
   bool opt_out_clicked_ = false;
 
   base::HistogramTester histogram_tester_;
+  base::OnceClosure dialog_closed_callback_;
 
   base::WeakPtrFactory<SecurePaymentConfirmationDialogViewTest>
       weak_ptr_factory_{this};
@@ -624,6 +636,22 @@
   EXPECT_TRUE(opt_out_clicked_);
 }
 
+// Occlusion by picture-in-picture video should dismiss the SPC authentication
+// dialog.
+IN_PROC_BROWSER_TEST_F(SecurePaymentConfirmationDialogViewTest,
+                       PictureInPictureOcclusionClosesTheDialog) {
+  CreateModel();
+  InvokeSecurePaymentConfirmationUI();
+  base::RunLoop run_loop;
+  dialog_closed_callback_ = run_loop.QuitClosure();
+
+  static_cast<PictureInPictureOcclusionObserver*>(test_delegate_->dialog_view())
+      ->OnOcclusionStateChanged(/*occluded=*/true);
+
+  run_loop.Run();
+  EXPECT_TRUE(dialog_closed_);
+}
+
 // A variant of SecurePaymentConfirmationDialogViewTest that enables the network
 // and issuer rows feature, and verifies their contents.
 class SecurePaymentConfirmationDialogViewNetworkAndIssuerIconsTest
diff --git a/chrome/browser/ui/views/payments/secure_payment_confirmation_no_creds_dialog_view_browsertest.cc b/chrome/browser/ui/views/payments/secure_payment_confirmation_no_creds_dialog_view_browsertest.cc
index cdf280b..b8febbb 100644
--- a/chrome/browser/ui/views/payments/secure_payment_confirmation_no_creds_dialog_view_browsertest.cc
+++ b/chrome/browser/ui/views/payments/secure_payment_confirmation_no_creds_dialog_view_browsertest.cc
@@ -4,8 +4,13 @@
 
 #include "chrome/browser/ui/views/payments/secure_payment_confirmation_no_creds_dialog_view.h"
 
+#include <utility>
+
+#include "base/functional/callback.h"
 #include "base/functional/callback_helpers.h"
+#include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/picture_in_picture/picture_in_picture_occlusion_observer.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/test/test_browser_dialog.h"
 #include "components/payments/content/secure_payment_confirmation_no_creds_model.h"
@@ -76,7 +81,14 @@
   }
 
   // SecurePaymentConfirmationNoCredsDialogView::ObserverForTest
-  void OnDialogClosed() override { dialog_closed_ = true; }
+  void OnDialogClosed() override {
+    dialog_closed_ = true;
+    if (dialog_closed_callback_) {
+      std::move(dialog_closed_callback_).Run();
+    }
+  }
+
+  // SecurePaymentConfirmationNoCredsDialogView::ObserverForTest
   void OnOptOutClicked() override { opt_out_clicked_ = true; }
 
  protected:
@@ -86,6 +98,8 @@
 
   bool dialog_closed_ = false;
   bool opt_out_clicked_ = false;
+
+  base::OnceClosure dialog_closed_callback_;
 };
 
 IN_PROC_BROWSER_TEST_F(SecurePaymentConfirmationNoCredsDialogViewTest,
@@ -159,6 +173,21 @@
   EXPECT_TRUE(opt_out_clicked_);
 }
 
+// Occlusion by picture-in-picture video should dismiss the SPC no-credentials
+// dialog.
+IN_PROC_BROWSER_TEST_F(SecurePaymentConfirmationNoCredsDialogViewTest,
+                       PictureInPictureOcclusionClosesTheDialog) {
+  CreateAndShowDialog(u"merchant.example", /*show_opt_out=*/true);
+  base::RunLoop run_loop;
+  dialog_closed_callback_ = run_loop.QuitClosure();
+
+  static_cast<PictureInPictureOcclusionObserver*>(dialog_view_.get())
+      ->OnOcclusionStateChanged(/*occluded=*/true);
+
+  run_loop.Run();
+  EXPECT_TRUE(dialog_closed_);
+}
+
 class SecurePaymentConfirmationNoCredsDialogViewWithInlineNetworkAndIssuerTest
     : public SecurePaymentConfirmationNoCredsDialogViewTest {
  public:
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn
index ea09666c..644b8da 100644
--- a/chrome/test/BUILD.gn
+++ b/chrome/test/BUILD.gn
@@ -4501,6 +4501,7 @@
         "../browser/ui/views/payments/payment_request_no_update_with_browsertest.cc",
         "../browser/ui/views/payments/payment_request_payment_app_browsertest.cc",
         "../browser/ui/views/payments/payment_request_payment_response_browsertest.cc",
+        "../browser/ui/views/payments/payment_request_picture_in_picture_occlusion_browsertest.cc",
         "../browser/ui/views/payments/payment_request_retry_browsertest.cc",
         "../browser/ui/views/payments/payment_request_sheet_controller_browsertest.cc",
         "../browser/ui/views/payments/payment_request_shipping_address_instance_browsertest.cc",