Fix issue where window bounds were being passed with wrong coordinates.

content/browser/renderer_host/render_widget_host_view_aura_unittest.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 "content/browser/renderer_host/render_widget_host_view_aura.h"
 
 #include "base/basictypes.h"
 #include "base/message_loop/message_loop.h"
 #include "base/strings/utf_string_conversions.h"
 #include "content/browser/renderer_host/render_widget_host_delegate.h"
 #include "content/browser/renderer_host/render_widget_host_impl.h"
 #include "content/common/input_messages.h"
 #include "content/common/view_messages.h"
 #include "content/public/browser/render_widget_host_view.h"
 #include "content/public/test/mock_render_process_host.h"
 #include "content/public/test/test_browser_context.h"
 #include "ipc/ipc_test_sink.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/aura/client/aura_constants.h"
+#include "ui/aura/client/screen_position_client.h"
 #include "ui/aura/env.h"
 #include "ui/aura/layout_manager.h"
 #include "ui/aura/root_window.h"
 #include "ui/aura/test/aura_test_helper.h"
 #include "ui/aura/test/test_cursor_client.h"
 #include "ui/aura/test/test_screen.h"
 #include "ui/aura/test/test_window_delegate.h"
 #include "ui/aura/window.h"
 #include "ui/aura/window_observer.h"
 #include "ui/base/events/event.h"
 #include "ui/base/events/event_utils.h"
 #include "ui/base/ui_base_types.h"
 
 namespace content {
 namespace {
+
+
+// Simple screen position client to test coordinate system conversion.
+class TestScreenPositionClient
+    : public aura::client::ScreenPositionClient {
+ public:
+  TestScreenPositionClient() {}
+  virtual ~TestScreenPositionClient() {}
+
+  // aura::client::ScreenPositionClient overrides:
+  virtual void ConvertPointToScreen(const aura::Window* window,
+      gfx::Point* point) OVERRIDE {
+    point->Offset(-1, -1);
+  }
+
+  virtual void ConvertPointFromScreen(const aura::Window* window,
+      gfx::Point* point) OVERRIDE {
+    point->Offset(1, 1);
+  }
+
+  virtual void ConvertHostPointToScreen(aura::RootWindow* window,
+      gfx::Point* point) OVERRIDE {
+    ConvertPointToScreen(window, point);
+  }
+
+  virtual void SetBounds(aura::Window* window,
+      const gfx::Rect& bounds,
+      const gfx::Display& display) OVERRIDE {
+  }
+};
+
 class MockRenderWidgetHostDelegate : public RenderWidgetHostDelegate {
  public:
   MockRenderWidgetHostDelegate() {}
   virtual ~MockRenderWidgetHostDelegate() {}
 };
 
 // Simple observer that keeps track of changes to a window for tests.
 class TestWindowObserver : public aura::WindowObserver {
  public:
   explicit TestWindowObserver(aura::Window* window_to_observe)
@@ skipping 137 matching lines @@
             window->GetProperty(aura::client::kShowStateKey));
 
   // Check that we requested and received the focus.
   EXPECT_TRUE(window->HasFocus());
 
   // Check that we'll also say it's okay to activate the window when there's an
   // ActivationClient defined.
   EXPECT_TRUE(view_->ShouldActivate());
 }
 
+
+// Checks that a popup is positioned correctly relative to its parent using
+// screen coordinates.
+TEST_F(RenderWidgetHostViewAuraTest, PositionChildPopup) {
+  scoped_ptr<TestScreenPositionClient> screen_position_client;
+  screen_position_client.reset(new TestScreenPositionClient());

[sky, 2013/07/26 15:37:59]

Any reason to use a scoped_ptr here rather than on the stack?

+
+  aura::Window* window = parent_view_->GetNativeView();
+  aura::RootWindow* root = window->GetRootWindow();
+  aura::client::SetScreenPositionClient(root, screen_position_client.get());
+
+  parent_view_->SetBounds(gfx::Rect(10, 10, 800, 600));
+  gfx::Rect bounds_in_screen = parent_view_->GetViewBounds();
+  int horiz = bounds_in_screen.width() / 4;
+  int vert = bounds_in_screen.height() / 4;
+  bounds_in_screen.Inset(horiz, vert);
+
+  // Verify that when the popup is initialized for the first time, it correctly
+  // treats the input bounds as screen coordinates.
+  view_->InitAsPopup(parent_view_, bounds_in_screen);
+  gfx::Rect final_bounds_in_screen = view_->GetViewBounds();
+  EXPECT_EQ(final_bounds_in_screen, bounds_in_screen);

[sky, 2013/07/26 15:37:59]

nit: if you compare strings (rect has a ToString() method) on failure you get
something readable.

+
+  // Verify that directly setting the bounds via SetBounds() treats the input
+  // as screen coordinates.
+  bounds_in_screen = gfx::Rect(60, 60, 100, 100);
+  view_->SetBounds(bounds_in_screen);
+  final_bounds_in_screen = view_->GetViewBounds();
+  EXPECT_EQ(final_bounds_in_screen, bounds_in_screen);
+
+  // Verify that setting the size does not alter the origin.
+  gfx::Point original_origin = window->bounds().origin();
+  view_->SetSize(gfx::Size(120, 120));
+  gfx::Point new_origin = window->bounds().origin();
+  EXPECT_EQ(original_origin, new_origin);
+
+  aura::client::SetScreenPositionClient(root, NULL);
+
+  widget_host_ = NULL;
+  view_ = NULL;
+}
+
 // Checks that a fullscreen view is destroyed when it loses the focus.
 TEST_F(RenderWidgetHostViewAuraTest, DestroyFullscreenOnBlur) {
   view_->InitAsFullscreen(parent_view_);
   aura::Window* window = view_->GetNativeView();
   ASSERT_TRUE(window != NULL);
   ASSERT_TRUE(window->HasFocus());
 
   // After we create and focus another window, the RWHVA's window should be
   // destroyed.
   TestWindowObserver observer(window);
@@ skipping 376 matching lines @@
     EXPECT_EQ(ViewMsg_Resize::ID, msg->type());
     ViewMsg_Resize::Param params;
     ViewMsg_Resize::Read(msg, &params);
     EXPECT_EQ("0,0 1600x1200",
               gfx::Rect(params.a.screen_info.availableRect).ToString());
     EXPECT_EQ("1600x1200", params.a.new_size.ToString());
   }
 }
 
 }  // namespace content