[MD] replace infobar bottom solid line separator with a shadow.

chrome/browser/ui/views/infobars/infobar_container_view.cc

Index: chrome/browser/ui/views/infobars/infobar_container_view.cc
diff --git a/chrome/browser/ui/views/infobars/infobar_container_view.cc b/chrome/browser/ui/views/infobars/infobar_container_view.cc
index d2f1fab0d7df556be99a9f4b86078b168a3a4756..19e3e734b2a0e101514f2e188c50fa939ff96d8f 100644
--- a/chrome/browser/ui/views/infobars/infobar_container_view.cc
+++ b/chrome/browser/ui/views/infobars/infobar_container_view.cc
@@ -4,20 +4,73 @@
 
 #include "chrome/browser/ui/views/infobars/infobar_container_view.h"
 
+#include "chrome/browser/ui/infobar_container_delegate.h"
 #include "chrome/browser/ui/view_ids.h"
 #include "chrome/browser/ui/views/infobars/infobar_view.h"
 #include "chrome/grit/generated_resources.h"
 #include "ui/accessibility/ax_view_state.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/base/material_design/material_design_controller.h"
+#include "ui/gfx/canvas.h"
+#include "ui/gfx/skia_util.h"
 #include "ui/views/view_targeter.h"
 
+namespace {
+
+// The content shadow is drawn in two stages. A darker, shorter shadow is
+// blended with a taller, lighter shadow. The heights are in dp.
+const int kSmallShadowHeight = 1;
+const int kLargeShadowHeight = 3;
+const SkAlpha kSmallShadowAlpha = 0x33;
+const SkAlpha kLargeShadowAlpha = 0x1A;
+
+class ContentShadow : public views::View {
+ public:
+  ContentShadow() {

[Peter Kasting, 2016/03/16 03:10:27]

Nit: I tend to find out-of-line definitions of methods even for file-scope
classes more readable, but the style guide allows both, so up to you.

[Evan Stade, 2016/03/16 19:29:51]

Acknowledged.

+    SetPaintToLayer(true);
+    layer()->SetFillsBoundsOpaquely(false);
+  }
+  ~ContentShadow() override {}
+
+  // views::View:
+  void OnPaint(gfx::Canvas* canvas) override {

[Peter Kasting, 2016/03/16 03:10:27]

Nit: Could be private

[Evan Stade, 2016/03/16 19:29:52]

I guess I should make it protected. I prefer to keep the same visibility as what
the parent class defines, rather than defining to private, because to me private
suggests that it will only be called from within this class (which is false).

+    // The first shader (small shadow) blurs from 0 to kSmallShadowHeight.
+    SkPaint paint;
+    skia::RefPtr<SkShader> shader = gfx::CreateGradientShader(

[Peter Kasting, 2016/03/16 03:10:27]

I assume we want a gradient shader because at scales 2x and above it will look
better than using a solid color for this one?

[Evan Stade, 2016/03/16 19:29:51]

right, that's why I decided against drawing a solid color. It's possible this
could be simplified to a single gradient with extra points in it, but this is
more of a verbatim copy of Sebastien's shadow definitions (which is useful I
guess in case he wants to tweak some value it should be easy to tweak here as
well).

Because we're drawing to a layer paints are pretty uncommon (unless you're
resizing the browser I guess?).

+        0, kSmallShadowHeight, SkColorSetA(SK_ColorBLACK, kSmallShadowAlpha),
+        SkColorSetA(SK_ColorBLACK, 0));

[Peter Kasting, 2016/03/16 03:10:27]

Nit: Could use SK_AlphaTRANSPARENT in place of 0, that might read more clearly
(or not) (2 places)

[Evan Stade, 2016/03/16 19:29:51]

will give it a go.

+    paint.setShader(shader.get());
+    gfx::Rect small_shadow_bounds = GetLocalBounds();
+    small_shadow_bounds.set_height(kSmallShadowHeight);
+    canvas->DrawRect(small_shadow_bounds, paint);
+
+    // The second shader (large shadow) is solid from 0 to kSmallShadowHeight
+    // (blending with the first shader) and then blurs from kSmallShadowHeight
+    // to kLargeShadowHeight.
+    shader =
+        gfx::CreateGradientShader(kSmallShadowHeight, height(),
+                                  SkColorSetA(SK_ColorBLACK, kLargeShadowAlpha),
+                                  SkColorSetA(SK_ColorBLACK, 0));
+    paint.setShader(shader.get());
+    canvas->DrawRect(GetLocalBounds(), paint);
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ContentShadow);
+};
+
+}  // namespace
+
 // static
 const char InfoBarContainerView::kViewClassName[] = "InfoBarContainerView";
 
 InfoBarContainerView::InfoBarContainerView(Delegate* delegate)
-    : infobars::InfoBarContainer(delegate) {
+    : infobars::InfoBarContainer(delegate), content_shadow_(nullptr) {
   set_id(VIEW_ID_INFO_BAR_CONTAINER);
-  SetEventTargeter(make_scoped_ptr(new views::ViewTargeter(this)));
+  if (ui::MaterialDesignController::IsModeMaterial()) {
+    content_shadow_ = new ContentShadow();
+    AddChildView(content_shadow_);
+  }
 }
 
 InfoBarContainerView::~InfoBarContainerView() {
@@ -26,7 +79,14 @@ InfoBarContainerView::~InfoBarContainerView() {
 
 gfx::Size InfoBarContainerView::GetPreferredSize() const {
   int total_height;
-  GetVerticalOverlap(&total_height);
+  int overlap = GetVerticalOverlap(&total_height);

[Peter Kasting, 2016/03/16 03:10:27]

Nit: Can inline into next line

[Evan Stade, 2016/03/16 19:29:51]

perhaps this works but it's not clear to me that it works (due to order of
operations):

total_height -= overlap - GetVerticalOverlap(&total_height);

[Peter Kasting, 2016/03/17 03:30:01]

Oh, never mind, I didn't even see that the call was affecting |total_height|. 
Yeah, it would work correctly, but leave them separate.

+  total_height -= overlap;
+
+  // No need to reserve space for the bottom bar's separator; the shadow is good
+  // enough.
+  if (ui::MaterialDesignController::IsModeMaterial())
+    total_height -= InfoBarContainerDelegate::kSeparatorLineHeight;
+
   gfx::Size size(0, total_height);
   for (int i = 0; i < child_count(); ++i)
     size.SetToMax(gfx::Size(child_at(i)->GetPreferredSize().width(), 0));
@@ -38,15 +98,27 @@ const char* InfoBarContainerView::GetClassName() const {
 }
 
 void InfoBarContainerView::Layout() {
-  int top = GetVerticalOverlap(NULL);
+  int top = 0;
 
   for (int i = 0; i < child_count(); ++i) {
+    if (child_at(i) == content_shadow_)
+      continue;
+
     InfoBarView* child = static_cast<InfoBarView*>(child_at(i));
     top -= child->arrow_height();
     int child_height = child->total_height();
+
+    // Trim off the bottom bar's separator; the shadow is good enough.
+    if (ui::MaterialDesignController::IsModeMaterial() &&
+        i == child_count() - 2) {

[Peter Kasting, 2016/03/16 03:10:27]

Nit: It might be good for a comment to explain why this works, since it looks a
bit cryptic/fragile.

[Evan Stade, 2016/03/16 19:29:51]

will add

+      child_height -= InfoBarContainerDelegate::kSeparatorLineHeight;
+    }
     child->SetBounds(0, top, width(), child_height);
     top += child_height;
   }
+
+  if (ui::MaterialDesignController::IsModeMaterial())
+    content_shadow_->SetBounds(0, top, width(), kLargeShadowHeight);
 }
 
 void InfoBarContainerView::GetAccessibleState(ui::AXViewState* state) {
@@ -65,12 +137,3 @@ void InfoBarContainerView::PlatformSpecificRemoveInfoBar(
     infobars::InfoBar* infobar) {
   RemoveChildView(static_cast<InfoBarView*>(infobar));
 }
-
-bool InfoBarContainerView::DoesIntersectRect(const View* target,
-                                             const gfx::Rect& rect) const {
-  DCHECK_EQ(this, target);
-  // Only events that intersect the portion below the arrow are interesting.
-  gfx::Rect non_arrow_bounds = GetLocalBounds();
-  non_arrow_bounds.Inset(0, GetVerticalOverlap(nullptr), 0, 0);
-  return rect.Intersects(non_arrow_bounds);
-}