Address some bubble clipping issues

ui/views/view.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.
 
 #define _USE_MATH_DEFINES // For VC++ to get M_PI. This has to be first.
 
 #include "ui/views/view.h"
 
 #include <algorithm>
 #include <cmath>
@@ skipping 790 matching lines @@
   bool paint_relative_to_parent = !layer();
 
   // TODO(danakj): Rework clip and transform recorder usage here to use
   // std::optional once we can do so.
   ui::ClipRecorder clip_recorder(parent_context);
   if (paint_relative_to_parent) {
     // Set the clip rect to the bounds of this View. Note that the X (or left)
     // position we pass to ClipRect takes into consideration whether or not the
     // View uses a right-to-left layout so that we paint the View in its
     // mirrored position if need be.
-    gfx::Rect clip_rect_in_parent = bounds();
-    clip_rect_in_parent.Inset(clip_insets_);
-    if (parent_)
-      clip_rect_in_parent.set_x(
-          parent_->GetMirroredXForRect(clip_rect_in_parent));
-    clip_recorder.ClipRect(clip_rect_in_parent);
+    gfx::Path clip_path_in_parent = clip_path_;
+    if (clip_path_in_parent.isEmpty())
+      clip_path_in_parent.addRect(gfx::RectToSkRect(GetLocalBounds()));
+    clip_path_in_parent.offset(GetMirroredX(), y());
+    clip_recorder.ClipPathWithAntiAliasing(clip_path_in_parent);

[tapted, 2016/03/22 07:38:04]

I don't have a sense for how computationally expensive it is to use this versus
a boring old rectangle. I guess somewhere between "a bit" and "a lot". But do
you think it's worth only using path-based clipping when the |clip_path_| is
non-empty?  (e.g., it's probably fine for bubbles and SearchResultPageView to
use path-based clipping, but doing it for the general case might be
unnecessarily wasteful -- cc::ClipDisplayItem does seem like it has a lot of
custom codepaths for rectangles. In fact, there's even custom code in
ClipDisplayItem for *rounded* rectangles, but ClipRecorder::ClipRect always
passes an empty vector for the `SkRRect` argument.  Using
cc::clip*Path*DisplayItem would miss out on all that

[Evan Stade, 2016/03/22 17:59:58]

I was hoping it would be smart, i.e. if the path is a rectangle there's no AA to
be done. But I guess it's no more LOC to explicitly use ClipRect in the unset
case.

   }
 
   ui::TransformRecorder transform_recorder(context);
   if (paint_relative_to_parent) {
     // Translate the graphics such that 0,0 corresponds to where
     // this View is located relative to its parent.
     gfx::Transform transform_from_parent;
     gfx::Vector2d offset_from_parent = GetMirroredPosition().OffsetFromOrigin();
     transform_from_parent.Translate(offset_from_parent.x(),
                                     offset_from_parent.y());
@@ skipping 1597 matching lines @@
   // Message the RootView to do the drag and drop. That way if we're removed
   // the RootView can detect it and avoid calling us back.
   gfx::Point widget_location(event.location());
   ConvertPointToWidget(this, &widget_location);
   widget->RunShellDrag(this, data, widget_location, drag_operations, source);
   // WARNING: we may have been deleted.
   return true;
 }
 
 }  // namespace views