Move test for video size update to OverlayWindowViewsTest

With the aspect ratio maintained on all the platforms at the native
widget level
PictureInPictureWindowControllerBrowserTest.SurfaceIdChangeDoesNotMoveWindow
was not realistic. It called SetBounds() on the window directly the way
it never happens in real life due to the aspect ratio constraints. The
video aspect ratio was 1.5 in this case, but the first SetBounds() call
forced it to 1.0.

Moving the test to OverlayWindowViewsTest makes it simpler and more
stable without losing important coverage. Note that we don't need an
actual video playing in the PiP window to verify the behavior we're
interested in here.

Bug: 1146047
Change-Id: If5ba686350d5823501b9ae05e486f8076771b3ee
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2611261Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: Wojciech Dzierżanowski <wdzierzanowski@opera.com>
Cr-Commit-Position: refs/heads/master@{#841963}

chrome/browser/picture_in_picture/picture_in_picture_window_controller_browsertest.cc

@@ -55,7 +55,6 @@
 #include "ui/gfx/codec/png_codec.h"
 #include "ui/views/controls/button/image_button.h"
 #include "ui/views/view_observer.h"
-#include "ui/views/widget/widget_observer.h"
 
 #if BUILDFLAG(IS_CHROMEOS_ASH)
 #include "ash/public/cpp/accelerators.h"
@@ -226,31 +225,6 @@ class PictureInPictureWindowControllerBrowserTest
       ASSERT_EQ(IsOverlayWindowControlVisible(control), expected_visible);
   }
 
-  class WidgetBoundsChangeWaiter final : public views::WidgetObserver {
-   public:
-    explicit WidgetBoundsChangeWaiter(views::Widget* widget)
-        : widget_(widget), initial_bounds_(widget->GetWindowBoundsInScreen()) {
-      widget_->AddObserver(this);
-    }
-
-    ~WidgetBoundsChangeWaiter() final { widget_->RemoveObserver(this); }
-
-    void OnWidgetBoundsChanged(views::Widget* widget, const gfx::Rect&) final {
-      run_loop_.Quit();
-    }
-
-    void Wait() {
-      if (widget_->GetWindowBoundsInScreen() != initial_bounds_)
-        return;
-      run_loop_.Run();
-    }
-
-   private:
-    views::Widget* widget_;
-    gfx::Rect initial_bounds_;
-    base::RunLoop run_loop_;
-  };
-
   void MoveMouseOverOverlayWindow() {
     auto* const window = GetOverlayWindow();
     gfx::Point p(window->GetBounds().x(), window->GetBounds().y());
@@ -1328,66 +1302,6 @@ IN_PROC_BROWSER_TEST_F(PictureInPictureWindowControllerBrowserTest,
   EXPECT_FALSE(window_controller()->GetWindowForTesting()->IsVisible());
 }
 
-// Tests that when a new surface id is sent to the Picture-in-Picture window, it
-// doesn't move back to its default position.
-// TODO(crbug.com/1146047): Test is flaky on Linux.
-// TODO(crbug.com/1052397): Revisit once build flag switch of lacros-chrome is
-// complete.
-#if defined(OS_LINUX) || BUILDFLAG(IS_CHROMEOS_LACROS)
-#define MAYBE_SurfaceIdChangeDoesNotMoveWindow DISABLED_SurfaceIdChangeDoesNotMoveWindow
-#else
-#define MAYBE_SurfaceIdChangeDoesNotMoveWindow SurfaceIdChangeDoesNotMoveWindow
-#endif
-IN_PROC_BROWSER_TEST_F(PictureInPictureWindowControllerBrowserTest,
-                       MAYBE_SurfaceIdChangeDoesNotMoveWindow) {
-  LoadTabAndEnterPictureInPicture(
-      browser(), base::FilePath(kPictureInPictureWindowSizePage));
-
-  content::WebContents* active_web_contents =
-      browser()->tab_strip_model()->GetActiveWebContents();
-
-  ASSERT_NE(GetOverlayWindow(), nullptr);
-  ASSERT_TRUE(GetOverlayWindow()->IsVisible());
-
-  const auto max_size = GetOverlayWindow()->GetMaximumSize();
-  const int side_length = std::min(max_size.width(), max_size.height());
-
-  // Move and resize the window to the top left corner and wait for ack.
-  {
-    WidgetBoundsChangeWaiter waiter(GetOverlayWindow());
-
-    GetOverlayWindow()->SetBounds(gfx::Rect(0, 0, side_length, side_length));
-
-    waiter.Wait();
-  }
-
-  // Wait for signal that the window was resized.
-  base::string16 expected_title = base::ASCIIToUTF16("resized");
-  EXPECT_EQ(expected_title,
-            content::TitleWatcher(active_web_contents, expected_title)
-                .WaitAndGetTitle());
-
-  // Simulate a new surface layer and a change in aspect ratio then wait for
-  // ack.
-  {
-    WidgetBoundsChangeWaiter waiter(GetOverlayWindow());
-
-    GetOverlayWindow()->SetSurfaceId(viz::SurfaceId(
-        viz::FrameSinkId(1, 1),
-        viz::LocalSurfaceId(9, base::UnguessableToken::Create())));
-    GetOverlayWindow()->UpdateVideoSize(
-        gfx::Size(side_length / 2, side_length / 4));
-
-    waiter.Wait();
-  }
-
-  // The position should be closer to the (0, 0) than the default one (bottom
-  // right corner). This will be reflected by checking that the position is
-  // below (100, 100).
-  EXPECT_LT(GetOverlayWindow()->GetBounds().x(), 100);
-  EXPECT_LT(GetOverlayWindow()->GetBounds().y(), 100);
-}
-
 // Tests that the Picture-in-Picture state is properly updated when the window
 // is closed at a system level.
 IN_PROC_BROWSER_TEST_F(PictureInPictureWindowControllerBrowserTest,

chrome/browser/ui/views/overlay/overlay_window_views_unittest.cc

@@ -314,3 +314,23 @@ TEST_F(OverlayWindowViewsTest, PreviousTrackButtonAddedWhenControlsHidden) {
       origin_before_layout);
   EXPECT_FALSE(overlay_window().IsLayoutPendingForTesting());
 }
+
+TEST_F(OverlayWindowViewsTest, UpdateVideoSizeDoesNotMoveWindow) {
+  // Enter PiP.
+  overlay_window().UpdateVideoSize({300, 200});
+  overlay_window().ShowInactive();
+
+  // Resize the window and move it toward the top-left corner of the work area.
+  // In production, resizing preserves the aspect ratio if possible, so we
+  // preserve it here too.
+  overlay_window().SetBounds({100, 100, 450, 300});
+
+  // Simulate a new surface layer and a change in the aspect ratio.
+  overlay_window().UpdateVideoSize({400, 200});
+
+  // The window should not move.
+  // The window size will be adjusted according to the new aspect ratio, and
+  // clamped to 500x250 to fit within the maximum size for the work area of
+  // 1000x1000.
+  EXPECT_EQ(gfx::Rect(100, 100, 500, 250), overlay_window().GetBounds());
+}