Fix image map focus ring painting

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

 // Copyright 2014 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 "core/paint/ImagePainter.h"
 
 #include "core/dom/Document.h"
 #include "core/dom/Element.h"
 #include "core/editing/FrameSelection.h"
 #include "core/frame/LocalFrame.h"
@@ skipping 15 matching lines @@
 void ImagePainter::paint(const PaintInfo& paintInfo, const LayoutPoint& paintOffset)
 {
     m_layoutImage.LayoutReplaced::paint(paintInfo, paintOffset);
 
     if (paintInfo.phase == PaintPhaseOutline)
         paintAreaElementFocusRing(paintInfo, paintOffset);
 }
 
 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())
         return;
 
     Element* focusedElement = document.focusedElement();
     if (!isHTMLAreaElement(focusedElement))
         return;
 
     HTMLAreaElement& areaElement = toHTMLAreaElement(*focusedElement);
     if (areaElement.imageElement() != m_layoutImage.node())
         return;
 
     // 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));
     paintInfo.context.restore();
 }
 
 void ImagePainter::paintReplaced(const PaintInfo& paintInfo, const LayoutPoint& paintOffset)
 {
     LayoutUnit cWidth = m_layoutImage.contentWidth();
     LayoutUnit cHeight = m_layoutImage.contentHeight();
@@ skipping 52 matching lines @@
 
     TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("devtools.timeline"), "PaintImage", "data", InspectorPaintImageEvent::data(m_layoutImage));
 
     InterpolationQuality previousInterpolationQuality = context.imageInterpolationQuality();
     context.setImageInterpolationQuality(interpolationQuality);
     context.drawImage(image.get(), alignedRect, SkXfermode::kSrcOver_Mode, LayoutObject::shouldRespectImageOrientation(&m_layoutImage));
     context.setImageInterpolationQuality(previousInterpolationQuality);
 }
 
 } // namespace blink