Make LocalFrame::nodeImage() to take an image for the element with :-webkit-drag pseudo class

third_party/WebKit/Source/core/frame/LocalFrame.cpp

Index: third_party/WebKit/Source/core/frame/LocalFrame.cpp
diff --git a/third_party/WebKit/Source/core/frame/LocalFrame.cpp b/third_party/WebKit/Source/core/frame/LocalFrame.cpp
index 32a773882d0a5c5cac2d54dd3236a728cc8f112f..18d1bcc565456eedf88c2d74707c1d69141ef906 100644
--- a/third_party/WebKit/Source/core/frame/LocalFrame.cpp
+++ b/third_party/WebKit/Source/core/frame/LocalFrame.cpp
@@ -103,14 +103,13 @@ namespace {
 class DragImageBuilder {
     STACK_ALLOCATED();
 public:
-    DragImageBuilder(const LocalFrame* localFrame, const FloatRect& bounds, Node* draggedNode, float opacity = 1)
+    DragImageBuilder(const LocalFrame* localFrame, const FloatRect& bounds, LayoutObject* layoutObject, float opacity = 1)
         : m_localFrame(localFrame)
-        , m_draggedNode(draggedNode)
+        , m_draggedLayoutObject(layoutObject)
         , m_bounds(bounds)
         , m_opacity(opacity)
     {
-        if (m_draggedNode && m_draggedNode->layoutObject())
-            m_draggedNode->layoutObject()->updateDragState(true);
+        DCHECK(!m_draggedLayoutObject || m_draggedLayoutObject->isDragging());
         // TODO(oshima): Remove this when all platforms are migrated to use-zoom-for-dsf.
         float deviceScaleFactor = m_localFrame->host()->deviceScaleFactorDeprecated();
         m_bounds.setWidth(m_bounds.width() * deviceScaleFactor);
@@ -123,12 +122,18 @@ public:
         context().getPaintController().createAndAppend<BeginTransformDisplayItem>(*m_pictureBuilder, transform);
     }
 
+    ~DragImageBuilder()
+    {
+        if (!m_draggedLayoutObject)
+            return;
+        DCHECK(m_draggedLayoutObject->isDragging());
+        m_draggedLayoutObject->updateDragState(false);
+    }
+
     GraphicsContext& context() { return m_pictureBuilder->context(); }
 
     std::unique_ptr<DragImage> createImage()
     {
-        if (m_draggedNode && m_draggedNode->layoutObject())
-            m_draggedNode->layoutObject()->updateDragState(false);
         context().getPaintController().endItem<EndTransformDisplayItem>(*m_pictureBuilder);
         // TODO(fmalita): endRecording() should return a non-const SKP.
         sk_sp<SkPicture> recording(const_cast<SkPicture*>(m_pictureBuilder->endRecording().leakRef()));
@@ -136,8 +141,8 @@ public:
             SkISize::Make(m_bounds.width(), m_bounds.height()), nullptr, nullptr));
         RefPtr<Image> image = StaticBitmapImage::create(skImage.release());
         RespectImageOrientationEnum imageOrientation = DoNotRespectImageOrientation;
-        if (m_draggedNode && m_draggedNode->layoutObject())
-            imageOrientation = LayoutObject::shouldRespectImageOrientation(m_draggedNode->layoutObject());
+        if (m_draggedLayoutObject)
+            imageOrientation = LayoutObject::shouldRespectImageOrientation(m_draggedLayoutObject);
 
         float screenDeviceScaleFactor = m_localFrame->page()->chromeClient().screenInfo().deviceScaleFactor;
 
@@ -146,9 +151,9 @@ public:
 
 private:
     Member<const LocalFrame> m_localFrame;
-    Member<Node> m_draggedNode;
+    LayoutObject* const m_draggedLayoutObject;

[pdr., 2016/06/23 20:29:13]

Why "LayoutObject* const" instead of "const LayoutObject*"?

[yosin_UTC9, 2016/06/24 06:02:57]

|m_layoutObject| is read-only member and |LayoutObject::updateDragState()| is
non-const.

     FloatRect m_bounds;
-    float m_opacity;
+    float const m_opacity;
     std::unique_ptr<SkPictureBuilder> m_pictureBuilder;
 };
 
@@ -598,11 +603,21 @@ double LocalFrame::devicePixelRatio() const
 
 std::unique_ptr<DragImage> LocalFrame::nodeImage(Node& node)
 {
+    // TODO(yosin): We should handle pseudo-class ":drag" as similar as

[pdr., 2016/06/23 20:29:13]

I was a little confused by ":drag", I think we use ":-webkit-drag"

[yosin_UTC9, 2016/06/24 06:02:57]

Done.

+    // ":hover" and ":active", rather than using flag in |LayoutObject|, to
+    // avoid update layout tree here.
     m_view->updateAllLifecyclePhasesExceptPaint();
     LayoutObject* layoutObject = node.layoutObject();
     if (!layoutObject)
         return nullptr;
 
+    // Update layout object for |node| with pseudo-class ":drag".
+    layoutObject->updateDragState(true);

[pdr., 2016/06/23 20:29:13]

Previously, calling updateDragState was the responsibility of DragImageBuilder
but now it's split in two places. Can these be unified (either all in this
function or all in DragImageBuilder)? This will make it harder to forget to call
updateDragState.

One option would be to just update the return statement:
auto image = dragImageBuilder.createImage();
if (layoutObject)
    layoutObject->updateDragState(false);
return image;

[yosin_UTC9, 2016/06/24 06:02:57]

I introduce |NodeImageBuilder| class and make its ctor/dtor manage
|updateDragState|.

+    m_view->updateAllLifecyclePhasesExceptPaint();
+    // |node| with pseudo-class ":drag" may blow away layout object.
+    layoutObject = node.layoutObject();
+    if (!layoutObject)
+        return nullptr;
     // Paint starting at the nearest stacking context, clipped to the object itself.
     // This will also paint the contents behind the object if the object contains transparency
     // and there are other elements in the same stacking context which stacked below.
@@ -611,7 +626,7 @@ std::unique_ptr<DragImage> LocalFrame::nodeImage(Node& node)
         layer = layer->stackingNode()->ancestorStackingContextNode()->layer();
     IntRect absoluteBoundingBox = layoutObject->absoluteBoundingBoxRectIncludingDescendants();
     FloatRect boundingBox = layer->layoutObject()->absoluteToLocalQuad(FloatQuad(absoluteBoundingBox), UseTransforms).boundingBox();
-    DragImageBuilder dragImageBuilder(this, boundingBox, &node);
+    DragImageBuilder dragImageBuilder(this, boundingBox, layoutObject);
     {
         PaintLayerPaintingInfo paintingInfo(layer, LayoutRect(boundingBox), GlobalPaintFlattenCompositingLayers, LayoutSize());
         PaintLayerFlags flags = PaintLayerHaveTransparency | PaintLayerAppliedTransform | PaintLayerUncachedClipRects;