Update LocationBarView Background

ui/views/background.cc

Index: ui/views/background.cc
diff --git a/ui/views/background.cc b/ui/views/background.cc
index 62cdc3b4cc613058f53f083cccaa7cfde8e808de..ac28ca8abb99c53ce1f00edfd7091bb0c86c32bc 100644
--- a/ui/views/background.cc
+++ b/ui/views/background.cc
@@ -7,6 +7,8 @@
 #include "base/logging.h"
 #include "skia/ext/skia_utils_win.h"
 #include "third_party/skia/include/core/SkPaint.h"
+#include "third_party/skia/include/core/SkPath.h"
+#include "third_party/skia/include/pathops/SkPathOps.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/color_utils.h"
 #include "ui/views/painter.h"
@@ -32,6 +34,65 @@ class SolidBackground : public Background {
   DISALLOW_COPY_AND_ASSIGN(SolidBackground);
 };
 
+// SolidScaledBackground renders a solid background color, with a one pixel
+// border. This accounts for the scaling of the canvas, so that the border is
+// one pixel regardless of display scaling. Optionally the border can have
+// rounded corners.
+class SolidScaledBackground : public Background {
+ public:
+  explicit SolidScaledBackground(SkColor background, SkColor border, bool round)

[Peter Kasting, 2015/10/19 21:09:44]

Nit: No explicit (only used for 1-arg constructors)

I would name the third arg (and member) "round_corners" or "round_rect" rather
than "round", as "round" sounds like a mathematical operation.

[jonross, 2015/10/20 18:54:53]

Done.

+      : border_color_(border), round_(round) {
+    SetNativeControlColor(background);
+  }
+
+  void Paint(gfx::Canvas* canvas, View* view) const override {
+    gfx::RectF border_rect_f(view->GetContentsBounds());

[Peter Kasting, 2015/10/19 21:09:44]

Nit: Declare |border_rect_f| and |kOffset| right above the conditional that uses
them.

[jonross, 2015/10/20 18:54:54]

Done.

+    const float scale = canvas->SaveAndUnscale();
+    const float kOffset = scale > 1.0f ? 1.5f : 0.5f;

[Peter Kasting, 2015/10/19 21:09:44]

I don't think this gets the right result for scales other than 1x and 2x, e.g.
1.5x.  My understanding from email was that the problem is that Sebastien wants
the omnibox sized based on scaling the interior up and outdenting the border,
rather than based on scaling the outer size up and indenting.  To implement that
you'd need to do one of the following:

(1) Inset the |border_rect_f| by 1 px on each side before scaling, then scale,
then outset by 0.5 px.
(2) Do something like what you're doing here, but compute |kOffset| as "scale -
0.5f" (and name it |inset| or something since it's not a pure constant).

[jonross, 2015/10/20 18:54:54]

I spoke with him, he wants the 200% inset by one pixel. So that the outermost
pixel of the view is transparent.

[Peter Kasting, 2015/10/20 23:04:05]

Yes, and my suggestions should accomplish that, but they will also work in modes
like 140% and 150%, whereas this code will break badly outside 1x and 2x.

[jonross, 2015/10/21 14:52:55]

Done.

+    border_rect_f.Scale(scale);
+
+    SkPaint stroke_paint;
+    stroke_paint.setStyle(SkPaint::Style::kStroke_Style);

[Peter Kasting, 2015/10/19 21:09:44]

Nit: No need for "Style::"

[jonross, 2015/10/20 18:54:53]

Done.

+    stroke_paint.setStrokeWidth(1);
+    stroke_paint.setColor(border_color_);
+    stroke_paint.setAntiAlias(true);
+
+    SkPaint fill_paint;
+    fill_paint.setStyle(SkPaint::kFill_Style);
+    fill_paint.setColor(get_color());
+    fill_paint.setAntiAlias(true);
+
+    SkPath path;
+    if (round_) {
+      border_rect_f.Inset(kOffset, kOffset);
+      const SkScalar kCornerRadius = SkDoubleToScalar(2.5f * scale);
+      path.addRoundRect(gfx::RectFToSkRect(border_rect_f), kCornerRadius,
+                        kCornerRadius);
+    } else {
+      border_rect_f.Inset(-kOffset, kOffset);

[Peter Kasting, 2015/10/19 21:09:44]

I don't think this is right for non-1x.  It seems like in this case you want the
x inset to always be -0.5, I think.  Probably the two will look equivalent
assuming the rect goes to the window bounds, but that would be technically more
correct.

[jonross, 2015/10/20 18:54:54]

Done.

+      path.addRect(gfx::RectFToSkRect(border_rect_f));
+    }
+
+    SkPath stroke_path;
+    stroke_paint.getFillPath(path, &stroke_path);
+
+    SkPath fill_path;
+    Op(path, stroke_path, kDifference_SkPathOp, &fill_path);
+    canvas->sk_canvas()->drawPath(fill_path, fill_paint);
+    canvas->sk_canvas()->drawPath(path, stroke_paint);

[Peter Kasting, 2015/10/19 21:09:44]

Nit: I think it might be slightly more performant to do

  fill_paint.setColor(border_color_);
  canvas->sk_canvas()->drawPath(stroke_path, fill_paint);

This allows you to further simplify things to just use one paint:

  SkPaint paint;
  paint.setAntiAlias(true);
  paint.setStyle(SkPaint::kStroke_Style);
  paint.setStrokeWidth(1);
  SkPath stroke_path;
  paint.getFillPath(path, &stroke_path);

  SkPath fill_path;
  Op(path, stroke_path, kDifference_SkPathOp, &fill_path);
  paint.setStyle(SkPaint::kFill_style);
  paint.setColor(get_color());
  canvas->sk_canvas()->drawPath(fill_path, paint);

  paint.setColor(border_color_);
  canvas->sk_canvas()->drawPath(stroke_path, paint);

[jonross, 2015/10/20 18:54:53]

Done.

+    canvas->Restore();
+  }
+
+ private:
+  // Color for the one pixel border.
+  SkColor border_color_;
+
+  // If true the rect will be a rounded rect.

[Peter Kasting, 2015/10/19 21:09:44]

Nit: Maybe "If true the border and interior corners will be rounded."

[Evan Stade, 2015/10/19 23:21:30]

I don't really care what this comment says because it's clear to me either way
but I think it's usually "roundrect" not "rounded rect".

[jonross, 2015/10/20 18:54:54]

Done.

+  bool round_;
+
+  DISALLOW_COPY_AND_ASSIGN(SolidScaledBackground);
+};
+
 class BackgroundPainter : public Background {
  public:
   BackgroundPainter(bool owns_painter, Painter* painter)
@@ -90,6 +151,12 @@ Background* Background::CreateSolidBackground(SkColor color) {
   return new SolidBackground(color);
 }
 
+Background* Background::CreateSolidScaledBackground(SkColor background,
+                                                    SkColor border,
+                                                    bool round) {
+  return new SolidScaledBackground(background, border, round);
+}
+
 // static
 Background* Background::CreateStandardPanelBackground() {
   // TODO(beng): Should be in NativeTheme.