aura: Take transformation into account when computing window bounds in root-window/screen.

ui/aura/window.cc

 // Copyright (c) 2012 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/window.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 365 matching lines @@
     if (window->layer_)
       return window->layer_->IsDrawn();
   }
   return false;
 }
 
 gfx::Rect Window::GetBoundsInRootWindow() const {
   // TODO(beng): There may be a better way to handle this, and the existing code
   //             is likely wrong anyway in a multi-display world, but this will
   //             do for now.
-  if (!GetRootWindow())
+  const Window* root = GetRootWindow();
+  if (!root)
     return bounds();
-  gfx::Point origin = bounds().origin();
-  ConvertPointToTarget(parent_, GetRootWindow(), &origin);
-  return gfx::Rect(origin, bounds().size());
+  gfx::Transform transform;
+  if (!layer()->GetTargetTransformRelativeTo(root->layer(), &transform))
+    return gfx::Rect();
+  gfx::RectF rect(bounds().size());
+  transform.TransformRect(&rect);

[oshima, 2014/02/20 19:39:35]

While I understand the motivation and the current impl is also not accurate, I
wonder if this can cause problem in the place where we are using the value of
GetBounsdInRootWindow to later compute & set the new bounds to the same window.
(which will change the value of GetBoundsInRootWindow again)

[sadrul, 2014/02/20 19:59:59]

If there are such cases, perhaps we should find and fix them? (I expected more
tests to fail if that were the case, but the lack such failures makes me think
perhaps there are not many places that do this)

[oshima, 2014/02/20 20:18:51]

I think the reason is that such scenario is rare (and probably not important)
and not covered by tests.

I'm not against this change BTW. I was just wondering if there is better way to
deal with transform.

For example, the above case probably should use

GetBoundsInRootWindowNoTransform()

to compute the bounds. Or the ideal form of the following code

window->GetBoundsInRootWindow().Contains(poitn_in_root)

may be something like

window->ContainsPointInRoot(point_in_root);

[sadrul, 2014/02/20 21:53:48]

Indeed. The motivation from this patch is from here:
https://codereview.chromium.org/167323004/#msg2 For the change in that patch, I
considered adding Window::GetTransformedBoundsInRootWindow(), but I decided
against adding yet another Get*Bounds*() function in the API so there's less
chance of someone using the wrong function for some quick fix. Instead, I think
it makes more sense to have GetBoundsInRootWindow/InScreen() take the transform
into account, and if some code expects the non-transformed bounds, then find
some alternate solution for those cases (like the one you mention above). This
may very well mean adding GetBoundsInRootWindowNoTransform() if it's needed in a
lot of places, but hopefully we can avoid that.

+  return gfx::ToEnclosingRect(rect);
 }
 
 gfx::Rect Window::GetBoundsInScreen() const {
   gfx::Rect bounds(GetBoundsInRootWindow());
   const Window* root = GetRootWindow();
   if (root) {
     aura::client::ScreenPositionClient* screen_position_client =
         aura::client::GetScreenPositionClient(root);
     if (screen_position_client) {
       gfx::Point origin = bounds.origin();
@@ skipping 1037 matching lines @@
       return window;
     if (offset)
       *offset += window->bounds().OffsetFromOrigin();
   }
   if (offset)
     *offset = gfx::Vector2d();
   return NULL;
 }
 
 }  // namespace aura