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
- trunk/Source/WebCore/ChangeLog
- trunk/Source/WebCore/dom/DataTransfer.cpp
- trunk/Source/WebCore/page/DragController.cpp
- trunk/Source/WebCore/page/DragController.h
- trunk/Source/WebCore/platform/DragImage.cpp
- trunk/Source/WebCore/platform/DragImage.h
- trunk/Source/WebCore/platform/Pasteboard.h
- trunk/Source/WebCore/platform/StaticPasteboard.h
- trunk/Source/WebCore/platform/efl/PasteboardEfl.cpp
- trunk/Source/WebCore/platform/gtk/PasteboardGtk.cpp
- trunk/Source/WebCore/platform/mac/PasteboardMac.mm
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