Added Layer based clipping to MenuScrollViewContainer::MenuScrollView

ui/views/controls/menu/menu_scroll_view_container.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 "ui/views/controls/menu/menu_scroll_view_container.h"
 
 #include "base/macros.h"
 #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"
 #include "ui/views/bubble/bubble_border.h"
 #include "ui/views/controls/menu/menu_config.h"
 #include "ui/views/controls/menu/menu_controller.h"
 #include "ui/views/controls/menu/menu_item_view.h"
 #include "ui/views/controls/menu/submenu_view.h"
 #include "ui/views/round_rect_painter.h"
 
@@ skipping 107 matching lines @@
 // that ScrollRectToVisible works.
 //
 // NOTE: It is possible to use ScrollView directly (after making it deal with
 // null scrollbars), but clicking on a child of ScrollView forces the window to
 // become active, which we don't want. As we really only need a fraction of
 // what ScrollView does, so we use a one off variant.
 
 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);
   }
 
   void ScrollRectToVisible(const gfx::Rect& rect) override {
     // NOTE: this assumes we only want to scroll in the y direction.
 
     // If the rect is already visible, do not scroll.
     if (GetLocalBounds().Contains(rect))
       return;
 
@@ skipping 170 matching lines @@
     case MENU_ANCHOR_BUBBLE_ABOVE:
       return BubbleBorder::BOTTOM_CENTER;
     case MENU_ANCHOR_BUBBLE_BELOW:
       return BubbleBorder::TOP_CENTER;
     default:
       return BubbleBorder::NONE;
   }
 }
 
 }  // namespace views