From 458cb99966914ffe4c4f175dc8f8b1a9e50aa425 Mon Sep 17 00:00:00 2001
From: Mikita Kuchyn <kuchyn@google.com>
Date: Tue, 09 Sep 2025 04:13:50 -0700
Subject: [PATCH] Fix a clickjacking out-of-screen Autofill popup issue

Autofill popups could appear on the page off-screen horizontally. This
CL restricts popup-generating elements to appear at least 100px
horizontally on-screen to generate the popup.

Previous attempt to upload this ( crrev.com/c/6779373 ) got reverted (
crrev.com/c/6918682 ) since ResizeTestScreen function failed Linux UBSan
builder test.

List of changes from crrev.com/c/6779373 :
* Changed screen class from display::test::TestScreen to
display::ScreenBase (popup_view_views_unittest.cc:338-339) to fix Linux
UBSan builder test.
* Had to migrate from display::Screen::GetScreen() to
display::Screen::Get() (crrev.com/c/6860363)
* Added a comment over kMinHorizontalOverlapForPopup to mention
clickjacking as a goal of having this constant (popup_view_utils.h:23).

Bug: 430555440
Change-Id: If17de61a9d50688bbd556ce84fbc03b5026b70b0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6923585
Auto-Submit: Mikita Kuchyn <kuchyn@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Reviewed-by: Jan Keitel <jkeitel@google.com>
Commit-Queue: Dominic Battré <battre@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1512982}
---

diff --git a/chrome/browser/ui/views/autofill/popup/popup_base_view.cc b/chrome/browser/ui/views/autofill/popup/popup_base_view.cc
index dc0e053f..cada3d7 100644
--- a/chrome/browser/ui/views/autofill/popup/popup_base_view.cc
+++ b/chrome/browser/ui/views/autofill/popup/popup_base_view.cc
@@ -33,6 +33,7 @@
 #include "ui/base/metadata/metadata_impl_macros.h"
 #include "ui/color/color_id.h"
 #include "ui/color/color_provider.h"
+#include "ui/display/display.h"
 #include "ui/display/screen.h"
 #include "ui/events/base_event_utils.h"
 #include "ui/gfx/color_palette.h"
@@ -453,7 +454,7 @@
 
 gfx::Rect PopupBaseView::GetOptimalPositionAndPlaceArrowOnPopup(
     const gfx::Rect& element_bounds,
-    const gfx::Rect& max_bounds_for_popup,
+    const gfx::Rect& visible_content_area_bounds,
     const gfx::Size& preferred_size,
     base::span<const views::BubbleArrowSide> preferred_popup_sides) {
   views::BubbleBorder* border = static_cast<views::BubbleBorder*>(
@@ -469,7 +470,7 @@
 
   // Deduce the arrow and the position.
   views::BubbleBorder::Arrow arrow = GetOptimalPopupPlacement(
-      /*content_area_bounds=*/max_bounds_for_popup,
+      /*visible_content_area_bounds=*/visible_content_area_bounds,
       /*element_bounds=*/element_bounds,
       /*popup_preferred_size=*/preferred_size,
       /*right_to_left=*/delegate_->GetElementTextDirection() ==
@@ -492,7 +493,7 @@
     border->set_arrow(arrow);
     border->AddArrowToBubbleCornerAndPointTowardsAnchor(
         element_bounds, popup_bounds,
-        max_bounds_for_popup.y() - kMaxPopupWebContentsTopYOverflow);
+        visible_content_area_bounds.y() - kMaxPopupWebContentsTopYOverflow);
   }
 
   return popup_bounds;
@@ -508,18 +509,21 @@
       PopupMayExceedContentAreaBounds(GetWebContents()) ? top_window_bounds
                                                         : content_area_bounds;
 
+  // Intersect with the current monitor's work area to avoid showing popups
+  // outside the screen.
+  gfx::Rect visible_content_area_bounds =
+      IntersectWithDisplayBounds(max_bounds_for_popup);
+
   gfx::Rect element_bounds = gfx::ToEnclosingRect(delegate_->element_bounds());
 
   // An element is never outside the content area if it is contained by the
-  // `content_area_bounds`. This also applies if the element is empty,
+  // `visible_content_area_bounds`. This also applies if the element is empty,
   // which means that either the height or the width is 0. An element can be
   // empty in case the popup is anchored to a caret, which has a 0 width.
-  // TODO(crbug.com/430555440) - We want the element also to intersect with
-  // the screen bounds.
-  if (!content_area_bounds.Contains(element_bounds)) {
-    // If the element exceeds the content area, ensure that the popup is still
-    // visually attached to the input element.
-    element_bounds.Intersect(content_area_bounds);
+  if (!visible_content_area_bounds.Contains(element_bounds)) {
+    // If the element exceeds the visible content area, ensure that the popup
+    // is still visually attached to the input element.
+    element_bounds.Intersect(visible_content_area_bounds);
     if (element_bounds.IsEmpty()) {
       HideController(SuggestionHidingReason::kElementOutsideOfContentArea);
       return false;
@@ -531,17 +535,21 @@
   element_bounds.Inset(
       gfx::Insets::VH(/*vertical=*/-kElementBorderPadding, /*horizontal=*/0));
 
-  // At least one row of the popup should be shown in the bounds of the content
-  // area so that the user notices the presence of the popup.
+  // At least one row of the popup should be shown in the bounds of the
+  // visible content area so that the user notices the presence of the popup.
   int item_height =
       children().size() > 0 ? children()[0]->GetPreferredSize().height() : 0;
-  if (!CanShowDropdownHere(item_height, max_bounds_for_popup, element_bounds)) {
+  // That function checks whether the popup element has enough place to render
+  // either to the top of the focused element or to the bottom of the focused
+  // element.
+  if (!CanShowDropdownHere(item_height, visible_content_area_bounds,
+                           element_bounds)) {
     HideController(SuggestionHidingReason::kInsufficientSpace);
     return false;
   }
 
   gfx::Rect popup_bounds = GetOptimalPositionAndPlaceArrowOnPopup(
-      element_bounds, max_bounds_for_popup, preferred_size,
+      element_bounds, visible_content_area_bounds, preferred_size,
       kDefaultPreferredPopupSides);
 
   if (BoundsOverlapWithPictureInPictureWindow(popup_bounds)) {
diff --git a/chrome/browser/ui/views/autofill/popup/popup_view_utils.cc b/chrome/browser/ui/views/autofill/popup/popup_view_utils.cc
index 512aeca8..4775789 100644
--- a/chrome/browser/ui/views/autofill/popup/popup_view_utils.cc
+++ b/chrome/browser/ui/views/autofill/popup/popup_view_utils.cc
@@ -5,6 +5,7 @@
 #include "chrome/browser/ui/views/autofill/popup/popup_view_utils.h"
 
 #include <algorithm>
+#include <optional>
 
 #include "chrome/browser/picture_in_picture/picture_in_picture_window_manager.h"
 #include "chrome/browser/platform_util.h"
@@ -19,6 +20,7 @@
 #include "components/autofill/core/common/autofill_features.h"
 #include "content/public/browser/web_contents.h"
 #include "extensions/common/constants.h"
+#include "ui/display/screen.h"
 #include "ui/views/widget/widget.h"
 
 using views::BubbleBorder;
@@ -43,16 +45,16 @@
 // to be at least 3x wider than the arrow for esthetic reasons.
 // This must only be called for vertical arrows (pointing up or down).
 bool IsElementSufficientlyVisibleForAVerticalArrow(
-    const gfx::Rect& content_area_bounds,
+    const gfx::Rect& visible_content_area_bounds,
     const gfx::Rect& element_bounds,
     views::BubbleArrowSide side) {
   CHECK(IsVerticalArrowSide(side));
 
   int visible_width =
-      std::clamp(element_bounds.right(), content_area_bounds.x(),
-                 content_area_bounds.right()) -
-      std::clamp(element_bounds.x(), content_area_bounds.x(),
-                 content_area_bounds.right());
+      std::clamp(element_bounds.right(), visible_content_area_bounds.x(),
+                 visible_content_area_bounds.right()) -
+      std::clamp(element_bounds.x(), visible_content_area_bounds.x(),
+                 visible_content_area_bounds.right());
 
   return visible_width > 3 * BubbleBorder::kVisibleArrowRadius;
 }
@@ -81,17 +83,17 @@
 
 // Returns the size of popup placed on the |side| of the |element_bounds| once
 // the popup is expanded to its |popup_preferred_size| or the maximum size
-// available on the |content_area_bounds|.
-gfx::Size GetExpandedPopupSize(const gfx::Rect& content_area_bounds,
+// available on the |visible_content_area_bounds|.
+gfx::Size GetExpandedPopupSize(const gfx::Rect& visible_content_area_bounds,
                                const gfx::Rect& element_bounds,
                                const gfx::Size& popup_preferred_size,
                                int scrollbar_width,
                                views::BubbleArrowSide side) {
   // Get the maximum available space for the popup
   int available_height = GetAvailableVerticalSpaceOnSideOfElement(
-      content_area_bounds, element_bounds, side);
+      visible_content_area_bounds, element_bounds, side);
   int available_width = GetAvailableHorizontalSpaceOnSideOfElement(
-      content_area_bounds, element_bounds, side);
+      visible_content_area_bounds, element_bounds, side);
 
   int height = std::min(available_height, popup_preferred_size.height());
   int width = std::min(
@@ -116,17 +118,19 @@
 }  // namespace
 
 void CalculatePopupYAndHeight(int popup_preferred_height,
-                              const gfx::Rect& content_area_bounds,
+                              const gfx::Rect& visible_content_area_bounds,
                               const gfx::Rect& element_bounds,
                               gfx::Rect* popup_bounds) {
-  int top_growth_end = std::clamp(element_bounds.y(), content_area_bounds.y(),
-                                  content_area_bounds.bottom());

diff --git a/chrome/browser/ui/views/autofill/popup/popup_view_views_unittest.cc b/chrome/browser/ui/views/autofill/popup/popup_view_views_unittest.cc
index 6d77c43f..ccda77c 100644
--- a/chrome/browser/ui/views/autofill/popup/popup_view_views_unittest.cc
+++ b/chrome/browser/ui/views/autofill/popup/popup_view_views_unittest.cc
@@ -56,17 +56,23 @@
 #include "ui/accessibility/ax_node_data.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/compositor/canvas_painter.h"
+#include "ui/display/display.h"
+#include "ui/display/screen.h"
+#include "ui/display/screen_base.h"
+#include "ui/display/test/test_screen.h"
 #include "ui/events/base_event_utils.h"
 #include "ui/events/keycodes/keyboard_codes.h"
 #include "ui/events/test/event_generator.h"
 #include "ui/gfx/geometry/point.h"
 #include "ui/gfx/geometry/point_f.h"
 #include "ui/gfx/geometry/rect.h"
+#include "ui/gfx/geometry/size.h"
 #include "ui/gfx/geometry/vector2d.h"
 #include "ui/views/accessibility/ax_update_notifier.h"
 #include "ui/views/accessibility/view_accessibility.h"
 #include "ui/views/bubble/bubble_border.h"
 #include "ui/views/bubble/bubble_border_arrow_utils.h"
+#include "ui/views/controls/webview/webview.h"
 #include "ui/views/test/ax_event_counter.h"
 #include "ui/views/view_class_properties.h"
 #include "ui/views/widget/widget.h"
@@ -175,7 +181,18 @@
     profile_ = std::make_unique<TestingProfile>();
     web_contents_ = content::WebContentsTester::CreateTestWebContents(
         profile_.get(), nullptr);
-    web_contents_->Resize({0, 0, 1024, 768});
+
+    // Create and configure the hosting widget for the WebContents. We need it
+    // since on Mac platforms WebContents delegate windowing management to its
+    // container, and we need to resize WebContents for some of the tests.
+    web_contents_widget_ =
+        CreateTestWidget(views::Widget::InitParams::CLIENT_OWNS_WIDGET);
+    auto* web_view = web_contents_widget_->SetContentsView(
+        std::make_unique<views::WebView>(profile_.get()));
+    web_view->SetWebContents(web_contents_.get());
+    ResizeWebContents({0, 0, 1000, 1000});
+    web_contents_widget_->Show();
+
     // Make sure the element is inside the web contents area.
     autofill_popup_controller_.set_element_bounds(
         autofill_popup_controller_.element_bounds() +
@@ -196,6 +213,12 @@
     view_ = nullptr;
     generator_.reset();
     widget_.reset();
+
+    // Destroy the WebContents hosting widget.
+    web_contents_widget_.reset();
+    web_contents_.reset();  // Destroy WebContents after its hosting widget's
+                            // WebView is gone.
+    profile_.reset();
     ChromeViewsTestBase::TearDown();
   }
 
@@ -210,13 +233,18 @@
           std::nullopt) {
     view_ = nullptr;
     generator_.reset();
+    widget_.reset();
 
-    widget_ = CreateTestWidget(
+    views::Widget::InitParams params =
         widget_params
             ? std::move(*widget_params)
             : CreateParamsForTestWidget(
                   views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET,
-                  views::Widget::InitParams::Type::TYPE_POPUP));
+                  views::Widget::InitParams::Type::TYPE_POPUP);
+    params.parent = web_contents_widget_->GetNativeView();
+    params.context = web_contents_widget_->GetNativeWindow();
+
+    widget_ = CreateTestWidget(std::move(params));
     generator_ = std::make_unique<ui::test::EventGenerator>(
         GetRootWindow(widget_.get()));
     view_ = new TestPopupViewViews(controller().GetWeakPtr(),
@@ -306,6 +334,19 @@
                             non_shift_modifier_pressed);
   }
 
+  void ResizeTestScreen(int new_width, int new_height) {
+    display::ScreenBase* test_screen =
+        static_cast<display::ScreenBase*>(display::Screen::Get());
+    display::Display primary_display = test_screen->GetPrimaryDisplay();
+    primary_display.set_bounds(gfx::Rect(0, 0, new_width, new_height));
+    primary_display.set_work_area(gfx::Rect(0, 0, new_width, new_height));
+    test_screen->display_list().UpdateDisplay(primary_display);
+  }
+
+  void ResizeWebContents(const gfx::Rect& bounds) {
+    web_contents_widget().SetBounds(bounds);
+  }
+
  protected:
   views::View& GetRowViewAt(size_t index) {
     return *std::visit([](views::View* view) { return view; },
@@ -325,6 +366,7 @@
   TestPopupViewViews& view() { return *view_; }
   views::Widget& widget() { return *widget_; }
   content::WebContents& web_contents() { return *web_contents_; }
+  views::Widget& web_contents_widget() { return *web_contents_widget_; }
 
   std::pair<std::unique_ptr<NiceMock<MockAutofillPopupController>>,
             PopupViewViews*>
@@ -349,6 +391,7 @@
   std::unique_ptr<TestingProfile> profile_;
   std::unique_ptr<content::WebContents> web_contents_;
   std::unique_ptr<views::Widget> widget_;
+  std::unique_ptr<views::Widget> web_contents_widget_;
   std::unique_ptr<ui::test::EventGenerator> generator_;
   raw_ptr<TestPopupViewViews> view_;
   NiceMock<MockAutofillPopupController> autofill_popup_controller_;
@@ -411,7 +454,7 @@
             node_data.GetString16Attribute(ax::mojom::StringAttribute::kName));
 }
 
-TEST_F(PopupViewViewsTest, CanShowDropdownInBounds) {
+TEST_F(PopupViewViewsTest, CanShowDropdownInBoundsVertically) {
   CreateAndShowView({SuggestionType::kAutocompleteEntry,
                      SuggestionType::kSeparator,
                      SuggestionType::kManageAddress});
@@ -419,17 +462,20 @@
   const int kSingleItemPopupHeight = view().GetPreferredSize().height();
   const int kElementY = 10;
   const int kElementHeight = 15;
+
   controller().set_element_bounds({10, kElementY, 100, kElementHeight});
 
-  EXPECT_FALSE(test_api(view()).CanShowDropdownInBounds({0, 0, 100, 35}));
+  // The width is 120px to make it a bit bigger then
+  // kMinHorizontalOverlapForPopup.
+  EXPECT_FALSE(test_api(view()).CanShowDropdownInBounds({0, 0, 120, 35}));
 
   // Test a smaller than the popup height (-10px) available space.
   EXPECT_FALSE(test_api(view()).CanShowDropdownInBounds(
-      {0, 0, 100, kElementY + kElementHeight + kSingleItemPopupHeight - 10}));
+      {0, 0, 120, kElementY + kElementHeight + kSingleItemPopupHeight - 10}));
 
   // Test a larger than the popup height (+10px) available space.
   EXPECT_TRUE(test_api(view()).CanShowDropdownInBounds(
-      {0, 0, 100, kElementY + kElementHeight + kSingleItemPopupHeight + 10}));
+      {0, 0, 120, kElementY + kElementHeight + kSingleItemPopupHeight + 10}));
 
   view().Hide();
 
@@ -439,11 +485,339 @@
       {SuggestionType::kAutocompleteEntry, SuggestionType::kAutocompleteEntry,
        SuggestionType::kAutocompleteEntry, SuggestionType::kSeparator,
        SuggestionType::kManageAddress});
-  EXPECT_FALSE(test_api(view()).CanShowDropdownInBounds({0, 0, 100, 35}));
+  EXPECT_FALSE(test_api(view()).CanShowDropdownInBounds({0, 0, 120, 35}));
   EXPECT_FALSE(test_api(view()).CanShowDropdownInBounds(
-      {0, 0, 100, kElementY + kElementHeight + kSingleItemPopupHeight - 10}));
+      {0, 0, 120, kElementY + kElementHeight + kSingleItemPopupHeight - 10}));
   EXPECT_TRUE(test_api(view()).CanShowDropdownInBounds(
-      {0, 0, 100, kElementY + kElementHeight + kSingleItemPopupHeight + 10}));
+      {0, 0, 120, kElementY + kElementHeight + kSingleItemPopupHeight + 10}));
+}
+
+namespace {
+
+constexpr gfx::Size kScreenSize = {1000, 1000};
+// Defines element size that has less width then kMinHorizontalOverlapForPopup.
+constexpr gfx::SizeF kNarrowerThanMargin(kMinHorizontalOverlapForPopup - 10,
+                                         35);
+// Defines element size that has more width then kMinHorizontalOverlapForPopup.
+constexpr gfx::SizeF kWiderThanMargin(kMinHorizontalOverlapForPopup + 10, 35);
+
+gfx::Rect GetLeftHalfScreenBounds(const gfx::Size& screen_size) {
+  return {0, 0, screen_size.width() / 2, screen_size.height()};
+}
+
+gfx::Rect GetRightHalfScreenBounds(const gfx::Size& screen_size) {
+  return {screen_size.width() / 2, 0, screen_size.width() / 2,
+          screen_size.height()};
+}
+
+}  // namespace
+
+// -----------------------------------------------------------------------------
+// Tests for CanShowDropdownInBounds to correctly handle horizontal overlap
+// with the display bounds. We have 'Left Edge Tests' and 'Right Edge Tests'.
+// -----------------------------------------------------------------------------
+// -----------------------------------------------------------------------------
+// Left Edge Tests
+// -----------------------------------------------------------------------------
+// Hides narrow popup if element is fully left of screen.
+TEST_F(PopupViewViewsTest, HidesNarrowPopup_ElementFullyLeftOfScreen) {
+  CreateAndShowView({SuggestionType::kAutocompleteEntry,
+                     SuggestionType::kSeparator,
+                     SuggestionType::kManageAddress});
+  const gfx::Rect web_contents_bounds = GetLeftHalfScreenBounds(kScreenSize);
+  // Element is 10px to the left of the visible area.
+  controller().set_element_bounds(
+      {/*origin=*/{-kNarrowerThanMargin.width() - 10,
+                   static_cast<float>(web_contents_bounds.y())},
+       /*size=*/kNarrowerThanMargin});
+
+  bool dropdown_shown =
+      test_api(view()).CanShowDropdownInBounds(web_contents_bounds);
+
+  EXPECT_FALSE(dropdown_shown);
+}
+
+// Hides narrow popup if element is partially left of screen.
+TEST_F(PopupViewViewsTest, HidesNarrowPopup_ElementPartiallyLeftOfScreen) {
+  CreateAndShowView({SuggestionType::kAutocompleteEntry,
+                     SuggestionType::kSeparator,
+                     SuggestionType::kManageAddress});
+  const gfx::Rect web_contents_bounds = GetLeftHalfScreenBounds(kScreenSize);
+  // Element starts 5px to the left of the visible area.
+  controller().set_element_bounds(
+      {/*origin=*/{static_cast<float>(web_contents_bounds.x() - 5),
+                   static_cast<float>(web_contents_bounds.y())},
+       /*size=*/kNarrowerThanMargin});
+
+  bool dropdown_shown =
+      test_api(view()).CanShowDropdownInBounds(web_contents_bounds);
+
+  EXPECT_FALSE(dropdown_shown);
+}
+
+// Shows narrow popup if element is at the left edge of the screen.
+TEST_F(PopupViewViewsTest, ShowsNarrowPopup_ElementAtLeftEdge) {
+  CreateAndShowView({SuggestionType::kAutocompleteEntry,
+                     SuggestionType::kSeparator,
+                     SuggestionType::kManageAddress});
+  const gfx::Rect web_contents_bounds = GetLeftHalfScreenBounds(kScreenSize);
+  // Element starts at the left edge of the visible area.
+  controller().set_element_bounds(
+      {/*origin=*/{static_cast<float>(web_contents_bounds.x()),
+                   static_cast<float>(web_contents_bounds.y())},
+       /*size=*/kNarrowerThanMargin});
+
+  bool dropdown_shown =
+      test_api(view()).CanShowDropdownInBounds(web_contents_bounds);
+
+  EXPECT_TRUE(dropdown_shown);
+}
+
+// Shows narrow popup if element is fully on-screen.
+TEST_F(PopupViewViewsTest, ShowsNarrowPopup_ElementFullyOnScreen) {
+  CreateAndShowView({SuggestionType::kAutocompleteEntry,
+                     SuggestionType::kSeparator,
+                     SuggestionType::kManageAddress});
+  const gfx::Rect web_contents_bounds = GetLeftHalfScreenBounds(kScreenSize);
+  // Element starts at the left edge of the visible area.
+  controller().set_element_bounds(
+      {/*origin=*/{static_cast<float>(web_contents_bounds.x() + 50),
+                   static_cast<float>(web_contents_bounds.y())},
+       /*size=*/kNarrowerThanMargin});
+
+  bool dropdown_shown =
+      test_api(view()).CanShowDropdownInBounds(web_contents_bounds);
+
+  EXPECT_TRUE(dropdown_shown);
+}
+
+// Hides wide popup if element is fully left of screen.
+TEST_F(PopupViewViewsTest, HidesWidePopup_ElementFullyLeftOfScreen) {
+  CreateAndShowView({SuggestionType::kAutocompleteEntry,
+                     SuggestionType::kSeparator,
+                     SuggestionType::kManageAddress});
+  const gfx::Rect web_contents_bounds = GetLeftHalfScreenBounds(kScreenSize);
+  // Element is 10px to the left of the visible area.
+  controller().set_element_bounds(
+      {/*origin=*/{-kWiderThanMargin.width() - 10,
+                   static_cast<float>(web_contents_bounds.y())},
+       /*size=*/kWiderThanMargin});
+
+  bool dropdown_shown =
+      test_api(view()).CanShowDropdownInBounds(web_contents_bounds);
+
+  EXPECT_FALSE(dropdown_shown);
+}
+
+// Hides wide popup if element has insufficient overlap on the left edge.
+TEST_F(PopupViewViewsTest, HidesWidePopup_InsufficientLeftOverlap) {
+  CreateAndShowView({SuggestionType::kAutocompleteEntry,
+                     SuggestionType::kSeparator,
+                     SuggestionType::kManageAddress});
+  const gfx::Rect web_contents_bounds = GetLeftHalfScreenBounds(kScreenSize);
+  // Element's right edge is 1px short of the required left overlap.
+  controller().set_element_bounds(
+      {/*origin=*/{static_cast<float>(web_contents_bounds.x() -
+                                      kWiderThanMargin.width() +
+                                      kMinHorizontalOverlapForPopup - 1),
+                   static_cast<float>(web_contents_bounds.y())},
+       /*size=*/kWiderThanMargin});
+
+  bool dropdown_shown =
+      test_api(view()).CanShowDropdownInBounds(web_contents_bounds);
+
+  EXPECT_FALSE(dropdown_shown);
... (truncated)