Ensure autofill popup border is visible

chrome/browser/ui/views/autofill/autofill_popup_base_view.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 "chrome/browser/ui/views/autofill/autofill_popup_base_view.h"
 
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
@@ skipping 43 matching lines @@
     params.delegate = this;
     params.parent = parent_widget_->GetNativeView();
     widget->Init(params);
 
     // No animation for popup appearance (too distracting).
     widget->SetVisibilityAnimationTransition(views::Widget::ANIMATE_HIDE);
 
     show_time_ = base::Time::Now();
   }
 
-  // TODO(crbug.com/676164): Show different border color when focused/unfocused
-  SetBorder(views::CreateSolidBorder(
-      kPopupBorderThickness,
-      GetNativeTheme()->GetSystemColor(
-          ui::NativeTheme::kColorId_UnfocusedBorderColor)));
-
   DoUpdateBoundsAndRedrawPopup();
   GetWidget()->Show();
 
   // Showing the widget can change native focus (which would result in an
   // immediate hiding of the popup). Only start observing after shown.
   if (initialize_widget)
     views::WidgetFocusManager::GetInstance()->AddFocusChangeListener(this);
 }
 
 void AutofillPopupBaseView::DoHide() {
@@ skipping 20 matching lines @@
 }
 
 void AutofillPopupBaseView::RemoveObserver() {
   parent_widget_->GetFocusManager()->UnregisterAccelerators(this);
   parent_widget_->RemoveObserver(this);
   views::WidgetFocusManager::GetInstance()->RemoveFocusChangeListener(this);
 }
 
 void AutofillPopupBaseView::DoUpdateBoundsAndRedrawPopup() {
   GetWidget()->SetBounds(delegate_->popup_bounds());
+
+  // We update the border whenever bounds are updated. Due to rounding when
+  // calculating the actual view size (bounds()) from the requested view size
+  // (delegate_.popup_bounds()), we may end up with extra space at the bottom
+  // and right of the view. We fill this with extra border pixels.

[Peter Kasting, 2017/02/16 00:30:26]

Rob, can you comment here?

It seems like the issue is that Screen::ScreenToDIPRectInWindow() and
Screen::DIPToScreenRectInWindow() both use ScaleToEnclosingRect().  The
Widget::SetBounds() call above uses the latter to convert the provided DIP
bounds to px bounds, but I guess we've (somehow) run through the former to
compute bounds() by the time we query it below(), leading to error accumulation.

This seems like a budget somewhere in the Widget level of things; calling
SetBounds() and then requesting bounds() should always give you back the same
rect.  Am I wrong?

+  int extra_border_thickness_x =
+      bounds().width() - delegate_->popup_bounds().width();
+  int extra_border_thickness_y =
+      bounds().height() - delegate_->popup_bounds().height();
+
+  // TODO(crbug.com/676164): Show different border color when focused/unfocused
+  SetBorder(views::CreateSolidSidedBorder(
+      kPopupBorderThickness, kPopupBorderThickness,
+      kPopupBorderThickness + extra_border_thickness_y,
+      kPopupBorderThickness + extra_border_thickness_x,
+      GetNativeTheme()->GetSystemColor(
+          ui::NativeTheme::kColorId_UnfocusedBorderColor)));
+
   SchedulePaint();
 }
 
+gfx::Rect AutofillPopupBaseView::GetPopupBounds() const {
+  return delegate_->popup_bounds();
+}
+
 void AutofillPopupBaseView::OnNativeFocusChanged(gfx::NativeView focused_now) {
   if (GetWidget() && GetWidget()->GetNativeView() != focused_now)
     HideController();
 }
 
 void AutofillPopupBaseView::OnMouseCaptureLost() {
   ClearSelection();
 }
 
 bool AutofillPopupBaseView::OnMouseDragged(const ui::MouseEvent& event) {
@@ skipping 112 matching lines @@
 void AutofillPopupBaseView::HideController() {
   if (delegate_)
     delegate_->Hide();
 }
 
 gfx::NativeView AutofillPopupBaseView::container_view() {
   return delegate_->container_view();
 }
 
 }  // namespace autofill