Better handling border color style decoration

Source/core/paint/ObjectPainter.cpp

Index: Source/core/paint/ObjectPainter.cpp
diff --git a/Source/core/paint/ObjectPainter.cpp b/Source/core/paint/ObjectPainter.cpp
index 976976753229652d44e93f25d7aa2640f0b76bff..54c8edbf162e124f89029a346f988ca90bdd190a 100644
--- a/Source/core/paint/ObjectPainter.cpp
+++ b/Source/core/paint/ObjectPainter.cpp
@@ -134,14 +134,8 @@ void ObjectPainter::drawLineForBoxSide(GraphicsContext* graphicsContext, int x1,
             style, adjacentWidth1, adjacentWidth2, antialias);
         break;
     case INSET:
-        // FIXME: Maybe we should lighten the colors on one side like Firefox.
-        // https://bugs.webkit.org/show_bug.cgi?id=58608
-        if (side == BSTop || side == BSLeft)
-            color = color.dark();
-        // fall through
     case OUTSET:
-        if (style == OUTSET && (side == BSBottom || side == BSRight))
-            color = color.dark();
+        calculateBorderStyleColor(style, side, color);
         // fall through
     case SOLID:
         drawSolidBoxSide(graphicsContext, x1, y1, x2, y2, side, color, adjacentWidth1, adjacentWidth2, antialias);
@@ -345,4 +339,30 @@ void ObjectPainter::drawSolidBoxSide(GraphicsContext* graphicsContext, int x1, i
     graphicsContext->fillPolygon(4, quad, color, antialias);
 }
 
+void ObjectPainter::calculateBorderStyleColor(const EBorderStyle& style, const BoxSide& side, Color& color)

[mstensho (USE GERRIT), 2014/11/27 09:33:29]

Should probably be called modifyBorderColorForStyleIfNeeded() or something like
that.

+{
+    const RGBA32 baseDarkColor = 0xFF202020;
+    const RGBA32 baseLightColor = 0xFFEBEBEB;
+
+    if ((side == BSTop || side == BSLeft)) {

[mstensho (USE GERRIT), 2014/11/27 09:33:29]

Extraneous pair of parentheses.

+        if (style == INSET) {
+            if (differenceSquared(color, Color::black) > differenceSquared(baseDarkColor, Color::black))
+                color = color.dark();
+        } else if (style == OUTSET) {
+            if (differenceSquared(color, Color::white) > differenceSquared(baseLightColor, Color::white))
+                color = color.light();
+        }
+    }
+
+    if ((side == BSBottom || side == BSRight)) {

[mstensho (USE GERRIT), 2014/11/27 09:33:29]

Should be "else if", or actually just "else" (I mean, are there more than those
four sides?) Also extraneous pair of parentheses.

But it strikes me that this block is very similar to the block above.

How about simplifying everything to:

enum Operation { Darken, Lighten };
Operation operation = (side == BSTop || side == BSLeft) == (style == INSET) ?
Darken : Lighten;
if (operation == Darken) {
    if (differenceSquared(color, Color::black) >
differenceSquared(baseDarkColor, Color::black))
        color = color.dark();
} else {
    if (differenceSquared(color, Color::white) >
differenceSquared(baseLightColor, Color::white))
        color = color.light();
}

+        if (style == INSET) {
+            if (differenceSquared(color, Color::white) > differenceSquared(baseLightColor, Color::white))
+                color = color.light();
+        } else if (style == OUTSET) {
+            if (differenceSquared(color, Color::black) > differenceSquared(baseDarkColor, Color::black))
+                color = color.dark();
+        }
+    }
+}
+
 } // namespace blink