Updated DropdownBarHost to use Layer clipping instead of OnPaint() clipping.

chrome/browser/ui/views/dropdown_bar_host.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 "chrome/browser/ui/views/dropdown_bar_host.h"
 
 #include <algorithm>
 
 #include "chrome/browser/ui/view_ids.h"
 #include "chrome/browser/ui/views/dropdown_bar_host_delegate.h"
 #include "chrome/browser/ui/views/dropdown_bar_view.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "ui/events/keycodes/keyboard_codes.h"
 #include "ui/gfx/animation/slide_animation.h"
 #include "ui/gfx/scrollbar_size.h"
 #include "ui/views/focus/external_focus_tracker.h"
 #include "ui/views/focus/view_storage.h"
 #include "ui/views/widget/widget.h"
 
 // static
 bool DropdownBarHost::disable_animations_during_testing_ = false;
 
 ////////////////////////////////////////////////////////////////////////////////
 // DropdownBarHost, public:
 
 DropdownBarHost::DropdownBarHost(BrowserView* browser_view)
     : browser_view_(browser_view),
+      clip_view_(new views::View()),
       view_(NULL),
       delegate_(NULL),
-      animation_offset_(0),
       focus_manager_(NULL),
       esc_accel_target_registered_(false),
       is_visible_(false) {
+  // The |clip_view_| must paint to a layer so that it can clip descendent Views
+  // which also paint to a Layer.
+  clip_view_->SetPaintToLayer(true);
+  clip_view_->SetFillsBoundsOpaquely(false);
+  clip_view_->layer()->SetMasksToBounds(true);
 }
 
 void DropdownBarHost::Init(views::View* host_view,
                            views::View* view,
                            DropdownBarHostDelegate* delegate) {
   DCHECK(view);
   DCHECK(delegate);
 
   view_ = view;
   delegate_ = delegate;
 
   // Initialize the host.
   host_.reset(new views::Widget);
   views::Widget::InitParams params(views::Widget::InitParams::TYPE_CONTROL);
   params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
   params.parent = browser_view_->GetWidget()->GetNativeView();
   params.opacity = views::Widget::InitParams::TRANSLUCENT_WINDOW;
   host_->Init(params);
-  host_->SetContentsView(view_);
+  host_->SetContentsView(clip_view_);
+  clip_view_->AddChildView(view_);
 
   SetHostViewNative(host_view);
 
   // Start listening to focus changes, so we can register and unregister our
   // own handler for Escape.
   focus_manager_ = host_->GetFocusManager();
   if (focus_manager_) {
     focus_manager_->AddFocusChangeListener(this);
   } else {
     // In some cases (see bug http://crbug.com/17056) it seems we may not have
     // a focus manager.  Please reopen the bug if you hit this.
     NOTREACHED();
   }
 
-  // Start the process of animating the opening of the widget.
   animation_.reset(new gfx::SlideAnimation(this));
+  // Update the widget and |view_| bounds to the hidden state.
+  AnimationProgressed(animation_.get());
 }
 
 DropdownBarHost::~DropdownBarHost() {
   focus_manager_->RemoveFocusChangeListener(this);
   focus_tracker_.reset(NULL);
 }
 
 void DropdownBarHost::Show(bool animate) {
   // Stores the currently focused view, and tracks focus changes so that we can
   // restore focus when the dropdown widget is closed.
   focus_tracker_.reset(new views::ExternalFocusTracker(view_, focus_manager_));
 
+  gfx::Rect dlg_rect = GetDialogPosition(gfx::Rect());
+  SetDialogPosition(dlg_rect);

[Peter Kasting, 2015/11/24 22:39:43]

Nit: Just: SetDialogPosition(GetDialogPosition(gfx::Rect()));

[bruthig, 2015/11/25 16:55:06]

Done.

+
+  host_->Show();
+
   bool was_visible = is_visible_;
   is_visible_ = true;
   if (!animate || disable_animations_during_testing_) {
     animation_->Reset(1);
     AnimationProgressed(animation_.get());
   } else if (!was_visible) {
     // Don't re-start the animation.
     animation_->Reset();
     animation_->Show();
   }
@@ skipping 34 matching lines @@
 }
 
 void DropdownBarHost::StopAnimation() {
   animation_->End();
 }
 
 bool DropdownBarHost::IsVisible() const {
   return is_visible_;
 }
 
+void DropdownBarHost::SetDialogPosition(const gfx::Rect& new_pos) {
+  view_->SetSize(new_pos.size());
+
+  if (new_pos.IsEmpty())
+    return;

[Peter Kasting, 2015/11/24 22:39:43]

Should this hide the host if it's currently visible?

[bruthig, 2015/11/25 16:55:06]

No, not from what I can tell anyway.  From what I can tell the host()->Show()
call below shouldn't be necessary either so I've removed that.  It should be
more consistent now.

[Peter Kasting, 2015/11/25 23:16:54]

I don't really know what this code does so I can't speak to it either way.

You might rewrite this function for brevity as

  view_->SetSize(new_pos.size());
  if (!new_pos.IsEmpty())
    host()->SetBounds(new_pos);

But it's minor so feel free not to if you don't want.

+
+  if (!host()->IsVisible())
+    host()->Show();
+  host()->SetBounds(new_pos);
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // DropdownBarHost, views::FocusChangeListener implementation:
 void DropdownBarHost::OnWillChangeFocus(views::View* focused_before,
                                         views::View* focused_now) {
   // First we need to determine if one or both of the views passed in are child
   // views of our view.
   bool our_view_before = focused_before && view_->Contains(focused_before);
   bool our_view_now = focused_now && view_->Contains(focused_now);
 
   // When both our_view_before and our_view_now are false, it means focus is
@@ skipping 15 matching lines @@
 void DropdownBarHost::OnDidChangeFocus(views::View* focused_before,
                                        views::View* focused_now) {
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // DropdownBarHost, gfx::AnimationDelegate implementation:
 
 void DropdownBarHost::AnimationProgressed(const gfx::Animation* animation) {
   // First, we calculate how many pixels to slide the widget.
   gfx::Size pref_size = view_->GetPreferredSize();
-  animation_offset_ = static_cast<int>((1.0 - animation_->GetCurrentValue()) *
-                                       pref_size.height());
+  int view_offset = static_cast<int>((animation_->GetCurrentValue() - 1.0) *
+                                     pref_size.height());
 
-  // This call makes sure it appears in the right location, the size and shape
-  // is correct and that it slides in the right direction.
+  // This call makes sure |view_| appears in the right location, the size and
+  // shape is correct and that it slides in the right direction.
   gfx::Rect dlg_rect = GetDialogPosition(gfx::Rect());

[Peter Kasting, 2015/11/24 22:39:43]

This temp is now unused.

[bruthig, 2015/11/25 16:55:06]

Done.

-  SetDialogPosition(dlg_rect);
-
-  // Let the view know if we are animating, and at which offset to draw the
-  // edges.
-  delegate_->SetAnimationOffset(animation_offset_);
-  view_->SchedulePaint();
+  view_->SetPosition(gfx::Point(0, view_offset));
 }
 
 void DropdownBarHost::AnimationEnded(const gfx::Animation* animation) {
-  // Place the dropdown widget in its fully opened state.
-  animation_offset_ = 0;
-
   if (!animation_->IsShowing()) {
     // Animation has finished closing.
     host_->Hide();
     is_visible_ = false;
     OnVisibilityChanged();
   } else {
     // Animation has finished opening.
   }
 }
 
@@ skipping 19 matching lines @@
       escape, ui::AcceleratorManager::kNormalPriority, this);
   esc_accel_target_registered_ = true;
 }
 
 void DropdownBarHost::UnregisterAccelerators() {
   DCHECK(esc_accel_target_registered_);
   ui::Accelerator escape(ui::VKEY_ESCAPE, ui::EF_NONE);
   focus_manager_->UnregisterAccelerator(escape, this);
   esc_accel_target_registered_ = false;
 }