Skip to content

Commit

Permalink
Revert of Correctly update the popup window position (patchset #6 id:…
Browse files Browse the repository at this point in the history 
…160001 of https://codereview.chromium.org/2583873002/ )

Reason for revert:
Breaking M57
crbug/698627

Original issue's description:
> Correctly update the popup window position
>
> It was updating the child window instead.
> This CL hooks up the bounds change to the desktop widget correctly.
>
> BUG=612270
> TEST=fixed test harness to do the correct check.
>
> Review-Url: https://codereview.chromium.org/2583873002
> Cr-Commit-Position: refs/heads/master@{#442741}
> Committed: https://chromium.googlesource.com/chromium/src/+/0aafa24f1ad6b4331bdb65db12ffc1842ba69b1d

TBR=sadrul@chromium.org,bsep@chromium.org,oshima@chromium.org
BUG=612270, 698627

Review-Url: https://codereview.chromium.org/2728293004
Cr-Commit-Position: refs/heads/master@{#455231}
(cherry picked from commit 2ba9b1f)

Review-Url: https://codereview.chromium.org/2732383002 .
Cr-Commit-Position: refs/branch-heads/2987@{#791}
Cr-Branched-From: ad51088-refs/heads/master@{#444943}
  • Loading branch information
@stevenjb
stevenjb committed Mar 7, 2017
1 parent e567e7a commit 750f7aa
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 69 deletions.
25 changes: 15 additions & 10 deletions content/browser/renderer_host/render_widget_host_view_aura.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -581,10 +581,6 @@ void RenderWidgetHostViewAura::Hide() {
#endif
}

aura::Window* RenderWidgetHostViewAura::GetToplevelWindow() {
return window_->GetToplevelWindow();
}

void RenderWidgetHostViewAura::SetSize(const gfx::Size& size) {
// For a SetSize operation, we don't care what coordinate system the origin
// of the window is in, it's only important to make sure that the origin
Expand All @@ -593,12 +589,21 @@ void RenderWidgetHostViewAura::SetSize(const gfx::Size& size) {
}

void RenderWidgetHostViewAura::SetBounds(const gfx::Rect& rect) {
display::Display display =
popup_parent_host_view_
? display::Screen::GetScreen()->GetDisplayNearestWindow(
popup_parent_host_view_->window_)
: display::Screen::GetScreen()->GetDisplayMatching(rect);
GetToplevelWindow()->SetBoundsInScreen(rect, display);
gfx::Point relative_origin(rect.origin());

// RenderWidgetHostViewAura::SetBounds() takes screen coordinates, but
// Window::SetBounds() takes parent coordinates, so do the conversion here.
aura::Window* root = window_->GetRootWindow();
if (root) {
aura::client::ScreenPositionClient* screen_position_client =
aura::client::GetScreenPositionClient(root);
if (screen_position_client) {
screen_position_client->ConvertPointFromScreen(window_->parent(),
&relative_origin);
}
}

InternalSetBounds(gfx::Rect(relative_origin, rect.size()));
}

gfx::Vector2dF RenderWidgetHostViewAura::GetLastScrollOffset() const {
Expand Down
3 changes: 0 additions & 3 deletions content/browser/renderer_host/render_widget_host_view_aura.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,9 +332,6 @@ class CONTENT_EXPORT RenderWidgetHostViewAura
return delegated_frame_host_.get();
}

// Returns the top level window that is hosting the renderwidget.
virtual aura::Window* GetToplevelWindow();

private:
friend class DelegatedFrameHostClientAura;
friend class InputMethodAuraTestBase;
Expand Down
27 changes: 4 additions & 23 deletions content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -430,10 +430,6 @@ class FakeRenderWidgetHostViewAura : public RenderWidgetHostViewAura {
return event_handler()->pointer_state();
}

// In this unit test, |window_| is directly added to the root and is
// toplevel.
aura::Window* GetToplevelWindow() override { return window(); }

gfx::Size last_frame_size_;
std::unique_ptr<cc::CopyOutputRequest> last_copy_request_;
FakeWindowEventDispatcher* dispatcher_;
Expand Down Expand Up @@ -521,20 +517,6 @@ const WebInputEvent* GetInputEventFromMessage(const IPC::Message& message) {
return reinterpret_cast<const WebInputEvent*>(data);
}

class MockRenderWidgetHostViewAura : public RenderWidgetHostViewAura {
public:
MockRenderWidgetHostViewAura(RenderWidgetHost* host, bool is_guest_view_hack)
: RenderWidgetHostViewAura(host, is_guest_view_hack) {}

~MockRenderWidgetHostViewAura() override {}

protected:
aura::Window* GetToplevelWindow() override { return window(); }

private:
DISALLOW_COPY_AND_ASSIGN(MockRenderWidgetHostViewAura);
};

} // namespace

class RenderWidgetHostViewAuraTest : public testing::Test {
Expand All @@ -560,8 +542,6 @@ class RenderWidgetHostViewAuraTest : public testing::Test {
ImageTransportFactory::GetInstance()->GetContextFactory(),
ImageTransportFactory::GetInstance()->GetContextFactoryPrivate());
new wm::DefaultActivationClient(aura_test_helper_->root_window());
aura::client::SetScreenPositionClient(aura_test_helper_->root_window(),
&screen_position_client_);

browser_context_.reset(new TestBrowserContext);
process_host_ = new MockRenderProcessHost(browser_context_.get());
Expand All @@ -575,7 +555,7 @@ class RenderWidgetHostViewAuraTest : public testing::Test {
process_host_, routing_id, false);
delegates_.back()->set_widget_host(parent_host_);
parent_view_ =
new MockRenderWidgetHostViewAura(parent_host_, is_guest_view_hack_);
new RenderWidgetHostViewAura(parent_host_, is_guest_view_hack_);
parent_view_->InitAsChild(nullptr);
aura::client::ParentWindowWithContext(parent_view_->GetNativeView(),
aura_test_helper_->root_window(),
Expand Down Expand Up @@ -733,7 +713,6 @@ class RenderWidgetHostViewAuraTest : public testing::Test {
std::unique_ptr<aura::test::AuraTestHelper> aura_test_helper_;
std::unique_ptr<BrowserContext> browser_context_;
std::vector<std::unique_ptr<MockRenderWidgetHostDelegate>> delegates_;
wm::DefaultScreenPositionClient screen_position_client_;
MockRenderProcessHost* process_host_;

// Tests should set these to nullptr if they've already triggered their
Expand Down Expand Up @@ -1023,9 +1002,11 @@ TEST_F(RenderWidgetHostViewAuraTest, FocusFullscreen) {
// Checks that a popup is positioned correctly relative to its parent using
// screen coordinates.
TEST_F(RenderWidgetHostViewAuraTest, PositionChildPopup) {
wm::DefaultScreenPositionClient screen_position_client;

aura::Window* window = parent_view_->GetNativeView();
aura::Window* root = window->GetRootWindow();
aura::client::SetScreenPositionClient(root, &screen_position_client);

parent_view_->SetBounds(gfx::Rect(10, 10, 800, 600));
gfx::Rect bounds_in_screen = parent_view_->GetViewBounds();
Expand Down Expand Up @@ -4219,7 +4200,7 @@ class RenderWidgetHostViewAuraWithViewHarnessTest
// the RWHVA as the view.
delete contents()->GetRenderViewHost()->GetWidget()->GetView();
// This instance is destroyed in the TearDown method below.
view_ = new MockRenderWidgetHostViewAura(
view_ = new RenderWidgetHostViewAura(
contents()->GetRenderViewHost()->GetWidget(), false);
}

Expand Down
7 changes: 2 additions & 5 deletions ui/aura/window.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,11 +313,8 @@ void Window::SetBoundsInScreen(const gfx::Rect& new_bounds_in_screen,
if (root) {
aura::client::ScreenPositionClient* screen_position_client =
aura::client::GetScreenPositionClient(root);
if (screen_position_client) {
screen_position_client->SetBounds(this, new_bounds_in_screen,
dst_display);
return;
}
screen_position_client->SetBounds(this, new_bounds_in_screen, dst_display);
return;
}
SetBounds(new_bounds_in_screen);
}
Expand Down
19 changes: 0 additions & 19 deletions ui/views/widget/desktop_aura/desktop_native_widget_aura_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -614,25 +614,6 @@ TEST_F(WidgetTest, WindowMouseModalityTest) {
top_level_widget.CloseNow();
}

TEST_F(WidgetTest, SetScreenBoundsOnNativeWindow) {
Widget widget;
Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_WINDOW);
params.native_widget =
CreatePlatformDesktopNativeWidgetImpl(params, &widget, nullptr);
params.ownership = Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
params.bounds = gfx::Rect(10, 10, 100, 100);
widget.Init(params);
widget.Show();
aura::Window* window = widget.GetNativeWindow();
display::Screen* screen = display::Screen::GetScreen();

const gfx::Rect new_screen_bounds(50, 50, 150, 150);
window->SetBoundsInScreen(new_screen_bounds,
screen->GetPrimaryDisplay());
EXPECT_EQ(new_screen_bounds, widget.GetWindowBoundsInScreen());
widget.CloseNow();
}

#if defined(OS_WIN)
// Tests whether we can activate the top level widget when a modal dialog is
// active.
Expand Down
11 changes: 2 additions & 9 deletions ui/views/widget/desktop_aura/desktop_screen_position_client.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,10 @@ void DesktopScreenPositionClient::SetBounds(aura::Window* window,
// TODO(jam): Use the 3rd parameter, |display|.
aura::Window* root = window->GetRootWindow();

// This method assumes that |window| does not have an associated
// DesktopNativeWidgetAura.
internal::NativeWidgetPrivate* desktop_native_widget =
DesktopNativeWidgetAura::ForWindow(root);
// The screen bounds request to the content_window_ should be interpreted as
// a widget bounds change.
if (desktop_native_widget &&
desktop_native_widget->GetNativeView() == window) {
desktop_native_widget->GetWidget()->SetBounds(bounds);
return;
}
// The following logic assumes that |window| does not have an associated
// DesktopNativeWidgetAura.
DCHECK(!desktop_native_widget ||
desktop_native_widget->GetNativeView() != window);

Expand Down

0 comments on commit 750f7aa

Please sign in to comment.