Scale bounds in aura-mus client-lib and NativeWidgetMus in hdpi mode.

ui/aura/mus/window_tree_client.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/aura/mus/window_tree_client.h"
 
 #include <stddef.h>
 
 #include <string>
 #include <utility>
@@ skipping 116 matching lines @@
   if (type_iter == properties.end())
     return;
 
   // TODO: need to validate type! http://crbug.com/654924.
   ui::mojom::WindowType window_type = static_cast<ui::mojom::WindowType>(
       mojo::ConvertTo<int32_t>(type_iter->second));
   SetWindowType(window, window_type);
 }
 
 // Helper function to get the device_scale_factor() of the display::Display
-// with |display_id|.
+// nearest to |window|.
 float ScaleFactorForDisplay(Window* window) {
-  // TODO(riajiang): Change to use display::GetDisplayWithDisplayId() after
-  // https://codereview.chromium.org/2361283002/ is landed.
   return display::Screen::GetScreen()
       ->GetDisplayNearestWindow(window)
       .device_scale_factor();
 }
 
 }  // namespace
 
 WindowTreeClient::WindowTreeClient(
     service_manager::Connector* connector,
     WindowTreeClientDelegate* delegate,
@@ skipping 487 matching lines @@
 
 void WindowTreeClient::OnWindowMusBoundsChanged(WindowMus* window,
                                                 const gfx::Rect& old_bounds,
                                                 const gfx::Rect& new_bounds) {
   // Changes to bounds of root windows are routed through
   // OnWindowTreeHostBoundsWillChange(). Any bounds that happen here are a side
   // effect of those and can be ignored.
   if (IsRoot(window))
     return;
 
-  ScheduleInFlightBoundsChange(window, old_bounds, new_bounds);
+  ScheduleInFlightBoundsChange(
+      window, gfx::ConvertRectToPixel(
+                  ScaleFactorForDisplay(window->GetWindow()), old_bounds),
+      gfx::ConvertRectToPixel(ScaleFactorForDisplay(window->GetWindow()),
+                              new_bounds));
 }
 
 void WindowTreeClient::OnWindowMusAddChild(WindowMus* parent,
                                            WindowMus* child) {
   // TODO: add checks to ensure this can work.
   const uint32_t change_id = ScheduleInFlightChange(
       base::MakeUnique<CrashInFlightChange>(parent, ChangeType::ADD_CHILD));
   tree_->AddWindow(change_id, parent->server_id(), child->server_id());
 }
 
@@ skipping 256 matching lines @@
       SetWindowVisibleFromServer(window, true);
   }
 
   const gfx::Rect bounds(data->bounds);
   {
     InFlightBoundsChange bounds_change(this, window, bounds);
     InFlightChange* current_change =
         GetOldestInFlightChangeMatching(bounds_change);
     if (current_change)
       current_change->SetRevertValueFrom(bounds_change);
-    else if (window->GetWindow()->bounds() != bounds)
+    else if (gfx::ConvertRectToPixel(ScaleFactorForDisplay(window->GetWindow()),
+                                     window->GetWindow()->bounds()) != bounds)

[sadrul, 2016/12/16 18:14:08]

Some conversions are happening in WindowPortMus, and some here in
WindowTreeClient. Would it be possible to keep all the conversion in one place?

[riajiang, 2016/12/19 23:51:55]

Moved all conversions in WTC. ServerChangeData now takes bounds_in_dip.

       SetWindowBoundsFromServer(window, bounds);
   }
 
   // There is currently no API to bulk set properties, so we iterate over each
   // property individually.
   for (const auto& pair : data->properties) {
     std::unique_ptr<std::vector<uint8_t>> revert_value(
         base::MakeUnique<std::vector<uint8_t>>(pair.second));
     InFlightPropertyChange property_change(window, pair.first,
                                            std::move(revert_value));
@@ skipping 401 matching lines @@
     }
   }
 }
 
 void WindowTreeClient::WmDisplayModified(const display::Display& display) {
   DCHECK(window_manager_delegate_);
   // TODO(sky): this should likely route to WindowTreeHost.
   window_manager_delegate_->OnWmDisplayModified(display);
 }
 
-// TODO(riajiang): Convert between pixel and DIP for window bounds properly.
-// (http://crbug.com/646942)
 void WindowTreeClient::WmSetBounds(uint32_t change_id,
                                    Id window_id,
                                    const gfx::Rect& transit_bounds) {

[sky, 2016/12/15 22:07:32]

Rename to transit_bounds_in_pixels.

[riajiang, 2016/12/19 23:51:55]

Done.

   WindowMus* window = GetWindowByServerId(window_id);
   bool result = false;
   if (window) {
+    float device_scale_factor = ScaleFactorForDisplay(window->GetWindow());
     DCHECK(window_manager_delegate_);
-    gfx::Rect bounds = transit_bounds;
+    gfx::Rect transit_bounds_in_dip =
+        gfx::ConvertRectToDIP(device_scale_factor, transit_bounds);
+    gfx::Rect bounds_in_dip = transit_bounds_in_dip;
     // TODO: this needs to trigger scheduling a bounds change on |window|.
-    result =
-        window_manager_delegate_->OnWmSetBounds(window->GetWindow(), &bounds);
+    result = window_manager_delegate_->OnWmSetBounds(window->GetWindow(),
+                                                     &bounds_in_dip);
     if (result) {
       // If the resulting bounds differ return false. Returning false ensures
       // the client applies the bounds we set below.
-      result = bounds == transit_bounds;
-      window->SetBoundsFromServer(bounds);
+      result = bounds_in_dip == transit_bounds_in_dip;
+      window->SetBoundsFromServer(
+          gfx::ConvertRectToPixel(device_scale_factor, bounds_in_dip));

[sky, 2016/12/15 22:07:32]

I think you should fix 672151 before this, otherwise I'm a bit worried about
rounding errors leading to weird bugs. By that I mean the conversion from pixels
-> dip -> pixels won't always give the same value, leading to confusion about
the actual bounds.

[riajiang, 2016/12/19 23:51:55]

As discussed, ignoring this comment.

     }
   }
   if (window_manager_internal_client_)
     window_manager_internal_client_->WmResponse(change_id, result);
 }
 
 void WindowTreeClient::WmSetProperty(
     uint32_t change_id,
     Id window_id,
     const std::string& name,
@@ skipping 143 matching lines @@
 void WindowTreeClient::ActivateNextWindow() {
   if (window_manager_internal_client_)
     window_manager_internal_client_->ActivateNextWindow();
 }
 
 void WindowTreeClient::SetUnderlaySurfaceOffsetAndExtendedHitArea(
     Window* window,
     const gfx::Vector2d& offset,
     const gfx::Insets& hit_area) {
   if (window_manager_internal_client_) {
-    // TODO(riajiang): Figure out if |offset| needs to be converted.
-    // (http://crbugs.com/646932)
+    float device_scale_factor = ScaleFactorForDisplay(window);
     window_manager_internal_client_->SetUnderlaySurfaceOffsetAndExtendedHitArea(
-        WindowMus::Get(window)->server_id(), offset.x(), offset.y(),
-        gfx::ConvertInsetsToDIP(ScaleFactorForDisplay(window), hit_area));
+        WindowMus::Get(window)->server_id(), offset.x() * device_scale_factor,
+        offset.y() * device_scale_factor,
+        gfx::ConvertInsetsToPixel(device_scale_factor, hit_area));
   }
 }
 
 void WindowTreeClient::RequestClose(Window* window) {
   DCHECK(window);
   if (window_manager_internal_client_)
     window_manager_internal_client_->WmRequestClose(
         WindowMus::Get(window)->server_id());
 }
 
@@ skipping 144 matching lines @@
   return ScheduleInFlightChange(base::MakeUnique<InFlightCaptureChange>(
       this, capture_synchronizer_.get(), window));
 }
 
 uint32_t WindowTreeClient::CreateChangeIdForFocus(WindowMus* window) {
   return ScheduleInFlightChange(base::MakeUnique<InFlightFocusChange>(
       this, focus_synchronizer_.get(), window));
 }
 
 }  // namespace aura