Title: [212154] trunk/Source/WebCore
Revision
212154
Author
ander...@apple.com
Date
2017-02-10 15:09:45 -0800 (Fri, 10 Feb 2017)

Log Message

Add a DragImage class that wraps a DragImageRef
https://bugs.webkit.org/show_bug.cgi?id=168131

Reviewed by Beth Dakin.

This allows us to get rid of the explicit deleteDragImage calls and will make additional cleanup of the
various drag code paths possible. No functionality change.

* dom/DataTransfer.cpp:
(WebCore::DataTransfer::updateDragImage):
* page/DragController.cpp:
(WebCore::DragController::startDrag):
(WebCore::DragController::doImageDrag):
(WebCore::DragController::doSystemDrag):
* page/DragController.h:
* platform/DragImage.cpp:
(WebCore::DragImage::DragImage):
(WebCore::DragImage::operator=):
(WebCore::DragImage::~DragImage):
* platform/DragImage.h:
* platform/Pasteboard.h:
* platform/StaticPasteboard.h:
* platform/mac/PasteboardMac.mm:
(WebCore::Pasteboard::setDragImage):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (212153 => 212154)


--- trunk/Source/WebCore/ChangeLog	2017-02-10 23:02:36 UTC (rev 212153)
+++ trunk/Source/WebCore/ChangeLog	2017-02-10 23:09:45 UTC (rev 212154)
@@ -1,3 +1,30 @@
+2017-02-10  Anders Carlsson  <ander...@apple.com>
+
+        Add a DragImage class that wraps a DragImageRef
+        https://bugs.webkit.org/show_bug.cgi?id=168131
+
+        Reviewed by Beth Dakin.
+
+        This allows us to get rid of the explicit deleteDragImage calls and will make additional cleanup of the
+        various drag code paths possible. No functionality change.
+
+        * dom/DataTransfer.cpp:
+        (WebCore::DataTransfer::updateDragImage):
+        * page/DragController.cpp:
+        (WebCore::DragController::startDrag):
+        (WebCore::DragController::doImageDrag):
+        (WebCore::DragController::doSystemDrag):
+        * page/DragController.h:
+        * platform/DragImage.cpp:
+        (WebCore::DragImage::DragImage):
+        (WebCore::DragImage::operator=):
+        (WebCore::DragImage::~DragImage):
+        * platform/DragImage.h:
+        * platform/Pasteboard.h:
+        * platform/StaticPasteboard.h:
+        * platform/mac/PasteboardMac.mm:
+        (WebCore::Pasteboard::setDragImage):
+
 2017-02-10  Simon Fraser  <simon.fra...@apple.com>
 
         Make sure the "inwindow" flag propagates to TiledBackings for masks and reflections

Modified: trunk/Source/WebCore/dom/DataTransfer.cpp (212153 => 212154)


--- trunk/Source/WebCore/dom/DataTransfer.cpp	2017-02-10 23:02:36 UTC (rev 212153)
+++ trunk/Source/WebCore/dom/DataTransfer.cpp	2017-02-10 23:09:45 UTC (rev 212154)
@@ -283,11 +283,11 @@
         return;
 
     IntPoint computedHotSpot;
-    DragImageRef computedImage = createDragImage(computedHotSpot);
+    auto computedImage = DragImage { createDragImage(computedHotSpot) };
     if (!computedImage)
         return;
 
-    m_pasteboard->setDragImage(computedImage, computedHotSpot);
+    m_pasteboard->setDragImage(WTFMove(computedImage), computedHotSpot);
 }
 
 #if !PLATFORM(MAC)

Modified: trunk/Source/WebCore/page/DragController.cpp (212153 => 212154)


--- trunk/Source/WebCore/page/DragController.cpp	2017-02-10 23:02:36 UTC (rev 212153)
+++ trunk/Source/WebCore/page/DragController.cpp	2017-02-10 23:09:45 UTC (rev 212154)
@@ -772,11 +772,13 @@
     else
         sourceContainsHitNode = state.source->containsIncludingShadowDOM(hitTestResult.innerNode());
 
-    if (!sourceContainsHitNode)
+    if (!sourceContainsHitNode) {
         // The original node being dragged isn't under the drag origin anymore... maybe it was
         // hidden or moved out from under the cursor. Regardless, we don't want to start a drag on
         // something that's not actually under the drag origin.
         return false;
+    }
+
     URL linkURL = hitTestResult.absoluteLinkURL();
     URL imageURL = hitTestResult.absoluteImageURL();
 #if ENABLE(ATTACHMENT_ELEMENT)
@@ -789,7 +791,7 @@
     m_draggingImageURL = URL();
     m_sourceDragOperation = srcOp;
 
-    DragImageRef dragImage = nullptr;
+    DragImage dragImage;
     IntPoint dragLoc(0, 0);
     IntPoint dragImageOffset(0, 0);
 
@@ -797,7 +799,7 @@
 
     DataTransfer& dataTransfer = *state.dataTransfer;
     if (state.type == DragSourceActionDHTML)
-        dragImage = dataTransfer.createDragImage(dragImageOffset);
+        dragImage = DragImage { dataTransfer.createDragImage(dragImageOffset) };
     if (state.type == DragSourceActionSelection || !imageURL.isEmpty() || !linkURL.isEmpty())
         // Selection, image, and link drags receive a default set of allowed drag operations that
         // follows from:
@@ -850,7 +852,7 @@
         }
         m_client.willPerformDragSourceAction(DragSourceActionSelection, dragOrigin, dataTransfer);
         if (!dragImage) {
-            dragImage = dissolveDragImageToFraction(createDragImageForSelection(src), DragImageAlpha);
+            dragImage = DragImage { dissolveDragImageToFraction(createDragImageForSelection(src), DragImageAlpha) };
             dragLoc = dragLocForSelectionDrag(src);
             m_dragOffset = IntPoint(dragOrigin.x() - dragLoc.x(), dragOrigin.y() - dragLoc.y());
         }
@@ -858,7 +860,7 @@
         if (!dragImage)
             return false;
 
-        doSystemDrag(dragImage, dragLoc, dragOrigin, dragImageBounds, dataTransfer, src, false);
+        doSystemDrag(WTFMove(dragImage), dragLoc, dragOrigin, dragImageBounds, dataTransfer, src, false);
     } else if (!src.document()->securityOrigin().canDisplay(linkURL)) {
         src.document()->addConsoleMessage(MessageSource::Security, MessageLevel::Error, "Not allowed to drag local resource: " + linkURL.stringCenterEllipsizedToLength());
         startedDrag = false;
@@ -881,7 +883,7 @@
             doImageDrag(element, dragOrigin, hitTestResult.imageRect(), dataTransfer, src, m_dragOffset);
         } else {
             // DHTML defined drag image
-            doSystemDrag(dragImage, dragLoc, dragOrigin, { }, dataTransfer, src, false);
+            doSystemDrag(WTFMove(dragImage), dragLoc, dragOrigin, { }, dataTransfer, src, false);
         }
     } else if (!linkURL.isEmpty() && (m_dragSourceAction & DragSourceActionLink)) {
         if (!dataTransfer.pasteboard().hasData()) {
@@ -910,14 +912,14 @@
 
         m_client.willPerformDragSourceAction(DragSourceActionLink, dragOrigin, dataTransfer);
         if (!dragImage) {
-            dragImage = createDragImageForLink(linkURL, hitTestResult.textContent(), src.settings().fontRenderingMode());
-            IntSize size = dragImageSize(dragImage);
+            dragImage = DragImage { createDragImageForLink(linkURL, hitTestResult.textContent(), src.settings().fontRenderingMode()) };
+            IntSize size = dragImageSize(dragImage.get());
             m_dragOffset = IntPoint(-size.width() / 2, -LinkDragBorderInset);
             dragLoc = IntPoint(mouseDraggedPoint.x() + m_dragOffset.x(), mouseDraggedPoint.y() + m_dragOffset.y());
             // Later code expects the drag image to be scaled by device's scale factor.
-            dragImage = scaleDragImage(dragImage, FloatSize(m_page.deviceScaleFactor(), m_page.deviceScaleFactor()));
+            dragImage = DragImage { scaleDragImage(dragImage.get(), FloatSize(m_page.deviceScaleFactor(), m_page.deviceScaleFactor())) };
         }
-        doSystemDrag(dragImage, dragLoc, mouseDraggedPoint, { }, dataTransfer, src, true);
+        doSystemDrag(WTFMove(dragImage), dragLoc, mouseDraggedPoint, { }, dataTransfer, src, true);
 #if ENABLE(ATTACHMENT_ELEMENT)
     } else if (!attachmentURL.isEmpty() && (m_dragSourceAction & DragSourceActionAttachment)) {
         if (!dataTransfer.pasteboard().hasData()) {
@@ -929,17 +931,17 @@
         m_client.willPerformDragSourceAction(DragSourceActionAttachment, dragOrigin, dataTransfer);
         
         if (!dragImage) {
-            dragImage = dissolveDragImageToFraction(createDragImageForSelection(src), DragImageAlpha);
+            dragImage = DragImage { dissolveDragImageToFraction(createDragImageForSelection(src), DragImageAlpha) };
             dragLoc = dragLocForSelectionDrag(src);
             m_dragOffset = IntPoint(dragOrigin.x() - dragLoc.x(), dragOrigin.y() - dragLoc.y());
         }
-        doSystemDrag(dragImage, dragLoc, dragOrigin, { }, dataTransfer, src, false);
+        doSystemDrag(WTFMove(dragImage), dragLoc, dragOrigin, { }, dataTransfer, src, false);
 #endif
     } else if (state.type == DragSourceActionDHTML) {
         if (dragImage) {
             ASSERT(m_dragSourceAction & DragSourceActionDHTML);
             m_client.willPerformDragSourceAction(DragSourceActionDHTML, dragOrigin, dataTransfer);
-            doSystemDrag(dragImage, dragLoc, dragOrigin, { }, dataTransfer, src, false);
+            doSystemDrag(WTFMove(dragImage), dragLoc, dragOrigin, { }, dataTransfer, src, false);
         } else
             startedDrag = false;
     } else {
@@ -948,8 +950,6 @@
         startedDrag = false;
     }
 
-    if (dragImage)
-        deleteDragImage(dragImage);
     return startedDrag;
 }
 
@@ -956,7 +956,7 @@
 void DragController::doImageDrag(Element& element, const IntPoint& dragOrigin, const IntRect& layoutRect, DataTransfer& dataTransfer, Frame& frame, IntPoint& dragImageOffset)
 {
     IntPoint mouseDownPoint = dragOrigin;
-    DragImageRef dragImage = nullptr;
+    DragImage dragImage;
     IntPoint scaledOrigin;
 
     if (!element.renderer())
@@ -966,13 +966,13 @@
 
     Image* image = getImage(element);
     if (image && image->size().height() * image->size().width() <= MaxOriginalImageArea
-        && (dragImage = createDragImageFromImage(image, element.renderer() ? orientationDescription : ImageOrientationDescription()))) {
+        && (dragImage = DragImage { createDragImageFromImage(image, element.renderer() ? orientationDescription : ImageOrientationDescription()) })) {
 
-        dragImage = fitDragImageToMaxSize(dragImage, layoutRect.size(), maxDragImageSize());
-        IntSize fittedSize = dragImageSize(dragImage);
+        dragImage = DragImage { fitDragImageToMaxSize(dragImage.get(), layoutRect.size(), maxDragImageSize()) };
+        IntSize fittedSize = dragImageSize(dragImage.get());
 
-        dragImage = scaleDragImage(dragImage, FloatSize(m_page.deviceScaleFactor(), m_page.deviceScaleFactor()));
-        dragImage = dissolveDragImageToFraction(dragImage, DragImageAlpha);
+        dragImage = DragImage { scaleDragImage(dragImage.get(), FloatSize(m_page.deviceScaleFactor(), m_page.deviceScaleFactor())) };
+        dragImage = DragImage { dissolveDragImageToFraction(dragImage.get(), DragImageAlpha) };
 
         // Properly orient the drag image and orient it differently if it's smaller than the original.
         float scale = fittedSize.width() / (float)layoutRect.width();
@@ -986,9 +986,9 @@
         scaledOrigin = IntPoint((int)(dx + 0.5), (int)(dy + 0.5));
     } else {
         if (CachedImage* cachedImage = getCachedImage(element)) {
-            dragImage = createDragImageIconForCachedImageFilename(cachedImage->response().suggestedFilename());
+            dragImage = DragImage { createDragImageIconForCachedImageFilename(cachedImage->response().suggestedFilename()) };
             if (dragImage)
-                scaledOrigin = IntPoint(DragIconRightInset - dragImageSize(dragImage).width(), DragIconBottomInset);
+                scaledOrigin = IntPoint(DragIconRightInset - dragImageSize(dragImage.get()).width(), DragIconBottomInset);
         }
     }
 
@@ -996,12 +996,10 @@
         return;
 
     dragImageOffset = mouseDownPoint + scaledOrigin;
-    doSystemDrag(dragImage, dragImageOffset, dragOrigin, element.boundsInRootViewSpace(), dataTransfer, frame, false);
-
-    deleteDragImage(dragImage);
+    doSystemDrag(WTFMove(dragImage), dragImageOffset, dragOrigin, element.boundsInRootViewSpace(), dataTransfer, frame, false);
 }
 
-void DragController::doSystemDrag(DragImageRef image, const IntPoint& dragLoc, const IntPoint& eventPos, const IntRect& dragImageBounds, DataTransfer& dataTransfer, Frame& frame, bool forLink)
+void DragController::doSystemDrag(DragImage image, const IntPoint& dragLoc, const IntPoint& eventPos, const IntRect& dragImageBounds, DataTransfer& dataTransfer, Frame& frame, bool forLink)
 {
     FloatPoint dragImageAnchor = { 0.5, 0.5 };
     if (!dragImageBounds.isEmpty()) {
@@ -1014,7 +1012,7 @@
     // Protect this frame and view, as a load may occur mid drag and attempt to unload this frame
     Ref<MainFrame> frameProtector(m_page.mainFrame());
     RefPtr<FrameView> viewProtector = frameProtector->view();
-    m_client.startDrag(image, viewProtector->rootViewToContents(frame.view()->contentsToRootView(dragLoc)), viewProtector->rootViewToContents(frame.view()->contentsToRootView(eventPos)), dragImageAnchor, dataTransfer, frameProtector.get(), forLink);
+    m_client.startDrag(image.get(), viewProtector->rootViewToContents(frame.view()->contentsToRootView(dragLoc)), viewProtector->rootViewToContents(frame.view()->contentsToRootView(eventPos)), dragImageAnchor, dataTransfer, frameProtector.get(), forLink);
     // DragClient::startDrag can cause our Page to dispear, deallocating |this|.
     if (!frameProtector->page())
         return;

Modified: trunk/Source/WebCore/page/DragController.h (212153 => 212154)


--- trunk/Source/WebCore/page/DragController.h	2017-02-10 23:02:36 UTC (rev 212153)
+++ trunk/Source/WebCore/page/DragController.h	2017-02-10 23:09:45 UTC (rev 212154)
@@ -112,7 +112,7 @@
         void mouseMovedIntoDocument(Document*);
 
         void doImageDrag(Element&, const IntPoint&, const IntRect&, DataTransfer&, Frame&, IntPoint&);
-        void doSystemDrag(DragImageRef, const IntPoint&, const IntPoint&, const IntRect& dragImageBounds, DataTransfer&, Frame&, bool forLink);
+        void doSystemDrag(DragImage, const IntPoint&, const IntPoint&, const IntRect& dragImageBounds, DataTransfer&, Frame&, bool forLink);
         void cleanupAfterSystemDrag();
         void declareAndWriteDragImage(DataTransfer&, Element&, const URL&, const String& label);
 #if ENABLE(ATTACHMENT_ELEMENT)

Modified: trunk/Source/WebCore/platform/DragImage.cpp (212153 => 212154)


--- trunk/Source/WebCore/platform/DragImage.cpp	2017-02-10 23:02:36 UTC (rev 212153)
+++ trunk/Source/WebCore/platform/DragImage.cpp	2017-02-10 23:09:45 UTC (rev 212154)
@@ -217,5 +217,36 @@
 }
 #endif
 
+DragImage::DragImage()
+    : m_dragImageRef { nullptr }
+{
+}
+
+DragImage::DragImage(DragImageRef dragImageRef)
+    : m_dragImageRef { dragImageRef }
+{
+}
+
+DragImage::DragImage(DragImage&& other)
+    : m_dragImageRef { std::exchange(other.m_dragImageRef, nullptr) }
+{
+}
+
+DragImage& DragImage::operator=(DragImage&& other)
+{
+    if (m_dragImageRef)
+        deleteDragImage(m_dragImageRef);
+
+    m_dragImageRef = std::exchange(other.m_dragImageRef, nullptr);
+
+    return *this;
+}
+
+DragImage::~DragImage()
+{
+    if (m_dragImageRef)
+        deleteDragImage(m_dragImageRef);
+}
+
 } // namespace WebCore
 

Modified: trunk/Source/WebCore/platform/DragImage.h (212153 => 212154)


--- trunk/Source/WebCore/platform/DragImage.h	2017-02-10 23:02:36 UTC (rev 212153)
+++ trunk/Source/WebCore/platform/DragImage.h	2017-02-10 23:09:45 UTC (rev 212154)
@@ -90,4 +90,20 @@
 DragImageRef createDragImageForLink(URL&, const String& label, FontRenderingMode);
 void deleteDragImage(DragImageRef);
 
+class DragImage final {
+public:
+    DragImage();
+    explicit DragImage(DragImageRef);
+    DragImage(DragImage&&);
+    ~DragImage();
+
+    DragImage& operator=(DragImage&&);
+
+    explicit operator bool() const { return !!m_dragImageRef; }
+    DragImageRef get() const { return m_dragImageRef; }
+
+private:
+    DragImageRef m_dragImageRef;
+};
+
 }

Modified: trunk/Source/WebCore/platform/Pasteboard.h (212153 => 212154)


--- trunk/Source/WebCore/platform/Pasteboard.h	2017-02-10 23:02:36 UTC (rev 212153)
+++ trunk/Source/WebCore/platform/Pasteboard.h	2017-02-10 23:09:45 UTC (rev 212154)
@@ -23,8 +23,7 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#ifndef Pasteboard_h
-#define Pasteboard_h
+#pragma once
 
 #include "DragImage.h"
 #include "URL.h"
@@ -182,7 +181,7 @@
     WEBCORE_EXPORT static std::unique_ptr<Pasteboard> createForDragAndDrop();
     WEBCORE_EXPORT static std::unique_ptr<Pasteboard> createForDragAndDrop(const DragData&);
 
-    virtual void setDragImage(DragImageRef, const IntPoint& hotSpot);
+    virtual void setDragImage(DragImage, const IntPoint& hotSpot);
 #endif
 
 #if PLATFORM(WIN)
@@ -262,5 +261,3 @@
 #endif
 
 } // namespace WebCore
-
-#endif // Pasteboard_h

Modified: trunk/Source/WebCore/platform/StaticPasteboard.h (212153 => 212154)


--- trunk/Source/WebCore/platform/StaticPasteboard.h	2017-02-10 23:02:36 UTC (rev 212153)
+++ trunk/Source/WebCore/platform/StaticPasteboard.h	2017-02-10 23:09:45 UTC (rev 212154)
@@ -62,7 +62,7 @@
     void writePasteboard(const Pasteboard&) final { }
 
 #if ENABLE(DRAG_SUPPORT)
-    void setDragImage(DragImageRef, const IntPoint&) final { }
+    void setDragImage(DragImage, const IntPoint&) final { }
 #endif
 
 private:

Modified: trunk/Source/WebCore/platform/efl/PasteboardEfl.cpp (212153 => 212154)


--- trunk/Source/WebCore/platform/efl/PasteboardEfl.cpp	2017-02-10 23:02:36 UTC (rev 212153)
+++ trunk/Source/WebCore/platform/efl/PasteboardEfl.cpp	2017-02-10 23:09:45 UTC (rev 212154)
@@ -119,7 +119,7 @@
 }
 
 #if ENABLE(DRAG_SUPPORT)
-void Pasteboard::setDragImage(DragImageRef, const IntPoint&)
+void Pasteboard::setDragImage(DragImage, const IntPoint&)
 {
     notImplemented();
 }

Modified: trunk/Source/WebCore/platform/gtk/PasteboardGtk.cpp (212153 => 212154)


--- trunk/Source/WebCore/platform/gtk/PasteboardGtk.cpp	2017-02-10 23:02:36 UTC (rev 212153)
+++ trunk/Source/WebCore/platform/gtk/PasteboardGtk.cpp	2017-02-10 23:09:45 UTC (rev 212154)
@@ -271,7 +271,7 @@
 }
 
 #if ENABLE(DRAG_SUPPORT)
-void Pasteboard::setDragImage(DragImageRef, const IntPoint&)
+void Pasteboard::setDragImage(DragImage, const IntPoint&)
 {
 }
 #endif

Modified: trunk/Source/WebCore/platform/mac/PasteboardMac.mm (212153 => 212154)


--- trunk/Source/WebCore/platform/mac/PasteboardMac.mm	2017-02-10 23:02:36 UTC (rev 212153)
+++ trunk/Source/WebCore/platform/mac/PasteboardMac.mm	2017-02-10 23:09:45 UTC (rev 212154)
@@ -663,7 +663,7 @@
 }
 
 #if ENABLE(DRAG_SUPPORT)
-void Pasteboard::setDragImage(DragImageRef image, const IntPoint& location)
+void Pasteboard::setDragImage(DragImage image, const IntPoint& location)
 {
     // Don't allow setting the drag image if someone kept a pasteboard and is trying to set the image too late.
     if (m_changeCount != platformStrategies()->pasteboardStrategy()->changeCount(m_pasteboardName))
@@ -671,7 +671,7 @@
 
     // Dashboard wants to be able to set the drag image during dragging, but Cocoa does not allow this.
     // Instead we must drop down to the CoreGraphics API.
-    wkSetDragImage(image.get(), location);
+    wkSetDragImage(image.get().get(), location);
 
     // Hack: We must post an event to wake up the NSDragManager, which is sitting in a nextEvent call
     // up the stack from us because the CoreFoundation drag manager does not use the run loop by itself.
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to