Update bool WindowManager::OnWmSetBounds() to match with its desirable behavior.

services/ui/public/cpp/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 "services/ui/public/cpp/window_tree_client.h"
 
 #include <stddef.h>
 
 #include <string>
 #include <utility>
@@ skipping 1298 matching lines @@
 
 void WindowTreeClient::WmDisplayModified(const display::Display& display) {
   DCHECK(window_manager_delegate_);
   window_manager_delegate_->OnWmDisplayModified(display);
 }
 
 void WindowTreeClient::WmSetBounds(uint32_t change_id,
                                        Id window_id,
                                        const gfx::Rect& transit_bounds) {
   Window* window = GetWindowByServerId(window_id);
-  bool result = false;
   if (window) {
     DCHECK(window_manager_delegate_);
     gfx::Rect transit_bounds_in_dip = gfx::ConvertRectToDIP(
         ScaleFactorForDisplay(window->display_id()), transit_bounds);
-    gfx::Rect bounds = transit_bounds_in_dip;
-    result = window_manager_delegate_->OnWmSetBounds(window, &bounds);
-    if (result) {
-      // If the resulting bounds differ return false. Returning false ensures
-      // the client applies the bounds we set below.
-      result = bounds == transit_bounds_in_dip;
-      window->SetBounds(bounds);
-    }
+    window->SetBounds(transit_bounds_in_dip);

[mfomitchev, 2016/12/06 20:55:20]

Don't we still need to call 
window_manager_delegate_->OnWmSetBounds(window, &bounds)
here?
And then NOT set the bounds on the window?

[thanhph, 2016/12/06 21:41:04]

The window bound is already set here (window->SetBounds(transit_bounds_in_dip))
so I removed the window_manager_delegate_->OnWmSetBounds(window, &bounds)

[mfomitchev, 2016/12/06 21:57:11]

window_manager_delegate_ (which is ash::mus::WindowManager in this case) may not
set the bounds that WindowTreeClient is trying to set though, which is likely
the reason this bug exists. It is important to let WindowManager handle this
though, and so we can't get rid of this call. Let's chat more in person
tomorrow. We can probably land this refactoring anyway, but it doesn't seem like
it fixes the crash.

[thanhph, 2016/12/07 20:48:12]

Done, thanks! I added this back in the new cl.

   }
   if (window_manager_internal_client_)
-    window_manager_internal_client_->WmResponse(change_id, result);
+    window_manager_internal_client_->WmResponse(change_id, false);

[Fady Samuel, 2016/12/06 20:50:07]

I'm confused about this? Shouldn't the second argument be *true* not false?

[mfomitchev, 2016/12/06 20:57:44]

I think false is right here - we want the client to always take the bounds from
the server - see
https://cs.chromium.org/chromium/src/services/ui/ws/window_tree.cc?q=WmRespon...

[thanhph, 2016/12/06 21:41:04]

Thanks for the reference Mikhail. Let me know if you have concerns.

 }
 
 void WindowTreeClient::WmSetProperty(
     uint32_t change_id,
     Id window_id,
     const std::string& name,
     const base::Optional<std::vector<uint8_t>>& transit_data) {
   Window* window = GetWindowByServerId(window_id);
   bool result = false;
   if (window) {
@@ skipping 132 matching lines @@
     // TODO(riajiang): Figure out if |offset| needs to be converted.
     // (http://crbugs.com/646932)
     window_manager_internal_client_->SetUnderlaySurfaceOffsetAndExtendedHitArea(
         server_id(window), offset.x(), offset.y(),
         gfx::ConvertInsetsToDIP(ScaleFactorForDisplay(window->display_id()),
                                 hit_area));
   }
 }
 
 }  // namespace ui