Do not regenerate local-surface-id on DSF change in the client

When the device-scale-factor changes, the change is currently handled
in the window server. The server re-generate the local surface id and
notifies to the client through OnWindowBoundsChanged. When this
mojo call arrives to the client, it's propagated to
TopLevelAllocator::OnDeviceScaleFactorChange, and it re-generate again
the local surface id, and sends back to the window server through
SetWindowBounds(). This causes some troubles.

This means that TopLevelAllocator does not have to generate the local
surface id on DSF change since it's already handled in the server.
This CL skips this regeneration.

Bug: 942647
Test: the new test case in aura_unittests
Change-Id: I86f9d8952082f1b2e7bb9ab74e38f556783447f7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1540069Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Jun Mukai <mukai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#644613}

ui/aura/mus/mus_lsi_allocator.cc

@@ -188,8 +188,13 @@ void TopLevelAllocator::InvalidateLocalSurfaceId() {
 }
 
 void TopLevelAllocator::OnDeviceScaleFactorChanged() {
-  AllocateLocalSurfaceId();
-  NotifyServerOfLocalSurfaceId();
+  // TopLevelAllocator does not have to allocate the new local surface id on
+  // device scale factor change. When the DSF changes, the window server first
+  // catches it, regenerates the local surface id, and then notifies the bounds
+  // with the new local surface id to the client. Therefore it should simply
+  // accept the new local surface id from the server. See the test case
+  // WindowTreeClientTest.SetBoundsFromServerDoesntCallWindowBoundsChanged and
+  // also https://crbug.com/942647.
 }
 
 void TopLevelAllocator::OnDidChangeBounds(const gfx::Size& size_in_pixels,
@@ -218,12 +223,7 @@ void TopLevelAllocator::DidGenerateLocalSurfaceIdAllocation(
   compositor->SetScaleAndSize(window_tree_host->device_scale_factor(),
                               window_tree_host->GetBoundsInPixels().size(),
                               local_surface_id_allocation_);
-  NotifyServerOfLocalSurfaceId();
-}
-
-void TopLevelAllocator::NotifyServerOfLocalSurfaceId() {
-  WindowTreeHostMus* host =
-      static_cast<WindowTreeHostMus*>(GetWindow()->GetHost());
+  WindowTreeHostMus* host = WindowTreeHostMus::ForWindow(GetWindow());
   window_tree_client()->OnWindowTreeHostBoundsWillChange(host,
                                                          host->bounds_in_dip());
 }

ui/aura/mus/mus_lsi_allocator.h

@@ -140,8 +140,6 @@ class AURA_EXPORT TopLevelAllocator : public MusLsiAllocator,
   const viz::LocalSurfaceIdAllocation& GetLocalSurfaceIdAllocation() override;
 
  private:
-  void NotifyServerOfLocalSurfaceId();
-
   // ui::CompositorObserver:
   void OnCompositingShuttingDown(ui::Compositor* compositor) override;
   void DidGenerateLocalSurfaceIdAllocation(

ui/aura/mus/window_tree_client_unittest.cc

@@ -2419,6 +2419,40 @@ TEST_F(WindowTreeClientTest, OnWindowDeletedDoesntNotifyServer) {
   EXPECT_FALSE(window_tree()->has_change());
 }
 
+TEST_F(WindowTreeClientTest, SetBoundsFromServerDoesntCallWindowBoundsChanged) {
+  WindowTreeHostMus window_tree_host(
+      CreateInitParamsForTopLevel(window_tree_client_impl()));
+  Window* top_level = window_tree_host.window();
+  window_tree_host.InitHost();
+
+  const gfx::Rect bounds(10, 20, 150, 200);
+  ws::mojom::WindowDataPtr data = ws::mojom::WindowData::New();
+  data->window_id = server_id(top_level);
+  data->bounds = bounds;
+  const int64_t display_id = 10;
+  uint32_t change_id;
+  ASSERT_TRUE(window_tree()->GetAndRemoveFirstChangeOfType(
+      WindowTreeChangeType::NEW_TOP_LEVEL, &change_id));
+  const viz::LocalSurfaceIdAllocation lsia =
+      GenerateLocalSurfaceIdForNewTopLevel();
+  window_tree_client()->OnTopLevelCreated(change_id, std::move(data),
+                                          display_id, true, lsia);
+  EXPECT_EQ(lsia, top_level->GetLocalSurfaceIdAllocation());
+
+  window_tree()->AckAllChanges();
+  test_screen()->SetDeviceScaleFactor(2.0f);
+  // Generates a new local surface id from the server.
+  const viz::LocalSurfaceIdAllocation lsia2 =
+      GenerateLocalSurfaceIdForNewTopLevel();
+  window_tree_client()->OnWindowBoundsChanged(server_id(top_level), bounds,
+                                              lsia2);
+  EXPECT_EQ(0u,
+            window_tree()->GetChangeCountForType(WindowTreeChangeType::BOUNDS));
+  // The local surface id is updated from lsia2, so it won't match with either.
+  EXPECT_NE(lsia, top_level->GetLocalSurfaceIdAllocation());
+  EXPECT_EQ(lsia2, top_level->GetLocalSurfaceIdAllocation());
+}
+
 TEST_F(WindowTreeClientTestHighDPI, SetBounds) {
   const gfx::Rect new_bounds(gfx::Rect(0, 0, 100, 100));
   ASSERT_NE(new_bounds, root_window()->bounds());