Added Layer based clipping to MenuScrollViewContainer::MenuScrollView

ui/views/controls/menu/menu_scroll_view_container.cc

Index: ui/views/controls/menu/menu_scroll_view_container.cc
diff --git a/ui/views/controls/menu/menu_scroll_view_container.cc b/ui/views/controls/menu/menu_scroll_view_container.cc
index 381bef70f6f8147990c13a0c7d0235500fb10d34..5e2b9f28f84d976b6be9bbd5dcfd76e0886094e2 100644
--- a/ui/views/controls/menu/menu_scroll_view_container.cc
+++ b/ui/views/controls/menu/menu_scroll_view_container.cc
@@ -8,6 +8,7 @@
 #include "third_party/skia/include/core/SkPaint.h"
 #include "third_party/skia/include/core/SkPath.h"
 #include "ui/accessibility/ax_view_state.h"
+#include "ui/compositor/layer.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/views/border.h"
@@ -135,6 +136,9 @@ class MenuScrollButton : public View {
 class MenuScrollViewContainer::MenuScrollView : public View {
  public:
   explicit MenuScrollView(View* child) {
+    SetPaintToLayer(true);

[varkha, 2016/03/22 14:30:01]

I wonder if this should be an optional behavior triggered by the caller. This
way we don't have to worry about perf with an expense of having to worry about
each individual scenario. This way app menu that knows that it has ink-rich
items would trigger this mask layer creation but other MenuScrollViews would not
need to.
However what you have here is certainly safer.

[bruthig, 2016/03/22 21:45:29]

I will consider this once I have the other issues sorted out.

[bruthig, 2016/04/12 19:09:51]

It is not straightforward for the owner of the menu to configure the
MenuScrollView.  However, I'm not sure making it configurable is a win,
especially if we plan to add ripples to all menu items.

+    layer()->SetMasksToBounds(true);
+    layer()->SetFillsBoundsOpaquely(false);

[Evan Stade, 2016/03/18 19:07:02]

is this correct/necessary?

[bruthig, 2016/03/18 19:26:43]

Yeah it's necessary.  Without it the child menu items paint but never 'erase'
themselves when the View is scrolled.  So you end up with severe ghosting. 
Alternatively I could omit this and explicitly set a solid background color.

[Evan Stade, 2016/03/18 19:34:40]

that seems better as opaquely filling bounds allows for perf optimizations.
Should be easy to get the right color from the native theme.

[bruthig, 2016/03/22 21:45:29]

So I've played around with this for 2 days now and when I replace the
SetFillsBoundsOpaquely(false) with a solid background (see patch set 2) there
are issues with drawing menus on ChromeOS with a resolution of 683x384.  See
screenshot here:
https://drive.google.com/a/google.com/file/d/0B9S3QZqpjnG7Sm1WN2hOUzVNMTg/vie...

For some reason there is a very thin black line near the bottom and on the right
side that should not be there.  The one on the right side is very difficult to
see. Has anyone ever seen this before?  I wasn't sure if this was a
manifestation of https://bugs.chromium.org/p/chromium/issues/detail?id=133477
and/or a limitation of Skia.

[bruthig, 2016/04/12 19:09:51]

It has turned in to a bit of a rabbit hole trying to fill the bounds opaquely. 
I'm going to get this in as-is for now and track that work with
http://crbug.com/601135.

     AddChildView(child);
   }