Fix image map focus ring painting

third_party/WebKit/Source/core/paint/ImagePainter.cpp

Index: third_party/WebKit/Source/core/paint/ImagePainter.cpp
diff --git a/third_party/WebKit/Source/core/paint/ImagePainter.cpp b/third_party/WebKit/Source/core/paint/ImagePainter.cpp
index bca60d96915fc1305dfc6033787175a331f6214f..15e2b5cd2dee8a9922c5e7ea393611ffb367026e 100644
--- a/third_party/WebKit/Source/core/paint/ImagePainter.cpp
+++ b/third_party/WebKit/Source/core/paint/ImagePainter.cpp
@@ -33,8 +33,6 @@ void ImagePainter::paint(const PaintInfo& paintInfo, const LayoutPoint& paintOff
 
 void ImagePainter::paintAreaElementFocusRing(const PaintInfo& paintInfo, const LayoutPoint& paintOffset)
 {
-    // TODO(wangxianzhu): In other places, we just paint focus ring if outline style is auto.
-    // We should also do that here to keep consistency.
     Document& document = m_layoutImage.document();
 
     if (paintInfo.isPrinting() || !document.frame()->selection().isFocusedAndActive())
@@ -51,26 +49,35 @@ void ImagePainter::paintAreaElementFocusRing(const PaintInfo& paintInfo, const L
     // Even if the theme handles focus ring drawing for entire elements, it won't do it for
     // an area within an image, so we don't call LayoutTheme::themeDrawsFocusRing here.
 
-    Path path = areaElement.computePath(&m_layoutImage);
-    if (path.isEmpty())
-        return;
-
     const ComputedStyle& areaElementStyle = *areaElement.ensureComputedStyle();
     int outlineWidth = areaElementStyle.outlineWidth();
     if (!outlineWidth)
         return;
 
+    Path path = areaElement.getPath(m_layoutImage.size());

[pdr., 2016/04/26 01:29:35]

I think this needs to be areaElement.getPath(&m_layoutImage);

[Xianzhu, 2016/04/26 17:42:27]

Done.

I remember I had changed this before uploading. Don't know how I made this wrong
here :(

+    if (path.isEmpty())
+        return;
+
+    AffineTransform zoomTransform;

[pdr., 2016/04/26 01:29:35]

Why is it correct to zoom here? There are almost no calls to effectiveZoom in
core/paint/ so it feels like we may have this in the wrong place.

Does contentBoxRect (used below for focusRect) have zoom applied? If so, maybe
the whole path should always have zoom applied like it was before, and then you
could remove the inverse zoom in HitTestResult::imageAreaForImage.

[Xianzhu, 2016/04/26 17:42:27]

Thanks for the finding. I didn't carefully think about zoom when I applied the
webkit patch. Now I found the problem of the webkit patch: it applies zoom
inconsistently for default shape and non-default shape, and didn't test default
shape with zoom (which is problematic). A non-default shape doesn't have zoom
applied in HTMLAreaElement::getPath(), so need to apply zoom here, but a default
shape will have zoom applied twice.

+    zoomTransform.scale(m_layoutImage.styleRef().effectiveZoom());
+    path.transform(zoomTransform);
+
+    LayoutPoint adjustedPaintOffset = paintOffset;
+    adjustedPaintOffset.moveBy(m_layoutImage.location());
+    path.translate(FloatSize(adjustedPaintOffset.x(), adjustedPaintOffset.y()));
+
     if (LayoutObjectDrawingRecorder::useCachedDrawingIfPossible(paintInfo.context, m_layoutImage, DisplayItem::ImageAreaFocusRing))
         return;
 
-    IntRect focusRect = m_layoutImage.absoluteContentBox();
+    LayoutRect focusRect = m_layoutImage.contentBoxRect();
+    focusRect.moveBy(adjustedPaintOffset);
     LayoutObjectDrawingRecorder drawingRecorder(paintInfo.context, m_layoutImage, DisplayItem::ImageAreaFocusRing, focusRect);
 
     // FIXME: Clip path instead of context when Skia pathops is ready.
     // https://crbug.com/251206
 
     paintInfo.context.save();
-    paintInfo.context.clip(focusRect);
+    paintInfo.context.clip(pixelSnappedIntRect(focusRect));
     paintInfo.context.drawFocusRing(path, outlineWidth,
         areaElementStyle.outlineOffset(),
         m_layoutImage.resolveColor(areaElementStyle, CSSPropertyOutlineColor));