Fix form validation bubble positioning at hidpi.

chrome/browser/ui/views/validation_message_bubble_view.cc

 // Copyright (c) 2013 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 "chrome/browser/ui/views/validation_message_bubble_view.h"
 
 #include "chrome/grit/theme_resources.h"
 #include "content/public/browser/render_widget_host.h"
-#include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/web_contents.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/widget/widget.h"
 
 namespace {
 
 const int kWindowMinWidth = 64;
 const int kWindowMaxWidth = 256;
 const int kIconTextMargin = 8;
 const int kTextVerticalMargin = 4;
 
 }  // namespace
 
 ValidationMessageBubbleView::ValidationMessageBubbleView(
     content::WebContents* web_contents,
     const gfx::Rect& anchor_in_root_view,
     const base::string16& main_text,
     const base::string16& sub_text) {
   content::RenderWidgetHostView* rwhv = web_contents->GetRenderWidgetHostView();
   set_parent_window(rwhv->GetNativeView());
 
   set_can_activate(false);
   set_arrow(views::BubbleBorder::TOP_LEFT);
-  const gfx::Rect anchor_in_screen =
-      anchor_in_root_view + rwhv->GetViewBounds().origin().OffsetFromOrigin();
-  SetAnchorRect(anchor_in_screen);
+  SetAnchorRect(RootViewToScreenRect(anchor_in_root_view, rwhv));
 
   ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance();
   views::ImageView* icon = new views::ImageView();
   icon->SetImage(*bundle.GetImageSkiaNamed(IDR_INPUT_ALERT));
   icon->SizeToPreferredSize();
   AddChildView(icon);
 
   views::Label* label = new views::Label(
       main_text, bundle.GetFontList(ui::ResourceBundle::MediumFont));
   label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
@@ skipping 27 matching lines @@
   }
 
   size_ = gfx::Size(text_start_x + label_width, content_bottom);
 
   views::BubbleDialogDelegateView::CreateBubble(this)->ShowInactive();
 }
 
 ValidationMessageBubbleView::~ValidationMessageBubbleView() {
 }
 
+gfx::Rect ValidationMessageBubbleView::RootViewToScreenRect(
+    const gfx::Rect& rect_in_root_view,
+    const content::RenderWidgetHostView* rwhv) const {
+  const float scale = rwhv->GetDeviceScaleFactor();
+  return gfx::ScaleToEnclosingRect(rect_in_root_view, 1 / scale) +

[msw, 2016/11/10 18:59:14]

Is there any chance this is zero? Consider adding a CHECK?

[Bret, 2016/11/11 23:51:27]

I don't think we could reach this method if scale was zero. We would crash on
browser startup.

+         rwhv->GetViewBounds().origin().OffsetFromOrigin();

[sadrul, 2016/11/10 20:44:41]

Can you use aura::client::ScreenPositionClient instead? That should work, and
that would also mean you won't need to add to the public API of content.

[Bret, 2016/11/11 23:51:27]

I can't get access to aura-specific methods on RenderWidgetHostView due to
dependency restrictions (otherwise I could add GetDeviceScaleFactor to
RenderWidgetHostViewAura without touching the public API), which means I can't
get a Window to get a ScreenPositionClient instance. And re-plumbing
ValidationMessageBubble is tricky because the interface is used across all
platforms.

Regardless of that, it's a different kind of conversion anyway: between
different window spaces. I hacked the dependencies to work just to test and it
gives the wrong answer. This needs a Blink->browser conversion.

[sadrul, 2016/11/12 02:30:44]

You have the RenderWidgetHostView here. You can get the aura::Window by calling
GetNativeView() on it. From there, it should be possible to use
ScreenPositionClient (see, e.g.
RenderWidgetHostViewAura::ConvertRectToScreen()).
Can you explain this a bit more? What is 'root view' here? RWHVAura does some
conversions on the rects blink sends to be in screen/browser space (e.g. see
uses of ConvertRectToScreen()/ConvertRectFromScreen()). How is this different?

[Bret, 2016/11/12 22:04:36]

Okay that plumbing does work. But as far as I can tell this whole family of
methods that delegate to ScreenPositionClient give the wrong answer, they're
doing the same thing the unpatched code did (i.e. not taking the device scale
factor into account).

I'm not very familiar with this area but I think it's because it's mostly
converting mouse events. When a UI event happens it captures the mouse position,
which is in the operating system's coordinate system. So all it has to do is
find which window it was relative to. In this case this rectangle is the bounds
of a Blink UI element, which is a different coordinate space even though until
recently they were coincidentally identical.

+}
+
 gfx::Size ValidationMessageBubbleView::GetPreferredSize() const {
   return size_;
 }
 
 int ValidationMessageBubbleView::GetDialogButtons() const {
   return ui::DIALOG_BUTTON_NONE;
 }
 
 void ValidationMessageBubbleView::SetPositionRelativeToAnchor(
     content::RenderWidgetHost* widget_host,
     const gfx::Rect& anchor_in_root_view) {
-  SetAnchorRect(anchor_in_root_view +
-      widget_host->GetView()->GetViewBounds().origin().OffsetFromOrigin());
+  SetAnchorRect(
+      RootViewToScreenRect(anchor_in_root_view, widget_host->GetView()));
 }
 
 void ValidationMessageBubbleView::CloseValidationMessage() {
   GetWidget()->Close();
 }