Skip to content

Commit

Permalink
Refactor some image drag and drop codepaths
Browse files Browse the repository at this point in the history 
https://bugs.webkit.org/show_bug.cgi?id=245058
rdar://98992054

Reviewed by Ryosuke Niwa.

Refactor some codepaths related to dragging and dropping images.

* Source/WebCore/dom/DataTransfer.cpp:
(WebCore::DataTransfer::types const):
(WebCore::DataTransfer::filesFromPasteboardAndItemList const):
* Source/WebCore/dom/DataTransfer.h:
(WebCore::DataTransfer::allowsFileAccess const):
* Source/WebCore/page/DragController.cpp:
(WebCore::DragController::dragEntered):
(WebCore::DragController::dragExited):
(WebCore::DragController::dragUpdated):
(WebCore::DragController::performDragOperation):
(WebCore::DragController::dragEnteredOrUpdated):
(WebCore::DragController::disallowFileAccessIfNeeded):
* Source/WebCore/page/DragController.h:
* Source/WebCore/page/DragState.h:
* Source/WebCore/page/EventHandler.cpp:
(WebCore::EventHandler::canDropCurrentlyDraggedImageAsFile const):
(WebCore::EventHandler::handleDrag):
* Source/WebCore/page/EventHandler.h:
* Source/WebCore/page/SecurityOrigin.cpp:
(WebCore::SecurityOrigin::canReceiveDragData const):
* Source/WebCore/platform/DragData.cpp:
(WebCore::DragData::disallowFileAccess):
* Source/WebCore/platform/DragData.h:
* Source/WebCore/platform/cocoa/DragDataCocoa.mm:
(WebCore::DragData::containsFiles const):
(WebCore::DragData::numberOfFiles const):
(WebCore::DragData::asFilenames const):
(WebCore::DragData::containsCompatibleContent const):
(WebCore::DragData::containsPromise const):
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::performDragControllerAction):
* Source/WebKit/WebProcess/WebPage/WebPage.h:
* Source/WebKitLegacy/mac/WebView/WebView.mm:
(-[WebView _enteredDataInteraction:client:global:operation:]):
(-[WebView _updatedDataInteraction:client:global:operation:]):
(-[WebView _exitedDataInteraction:client:global:operation:]):
(-[WebView _tryToPerformDataInteraction:client:global:operation:]):
(-[WebView draggingEntered:]):
(-[WebView draggingUpdated:]):
(-[WebView draggingExited:]):
(-[WebView performDragOperation:]):
* Source/WebKitLegacy/win/WebView.cpp:
(WebView::DragEnter):
(WebView::DragOver):
(WebView::DragLeave):
(WebView::Drop):

Use `WTFMove()` when handing the `DragData` over to `DragController`.

* Tools/TestWebKitAPI/Tests/WebKitCocoa/DragAndDropTests.mm:

Canonical link: https://commits.webkit.org/254380@main
  • Loading branch information
@whsieh
whsieh committed Sep 12, 2022
1 parent 5540bc1 commit a703b40
Show file tree
Hide file tree
Showing 16 changed files with 101 additions and 41 deletions.
7 changes: 4 additions & 3 deletions Source/WebCore/dom/DataTransfer.cpp
View file
Expand Up @@ -317,10 +317,11 @@ Vector<String> DataTransfer::types(AddFilesType addFilesType) const
if (!canReadTypes())
return { };

bool hideFilesType = !canWriteData() && !allowsFileAccess();
if (!DeprecatedGlobalSettings::customPasteboardDataEnabled()) {
auto types = m_pasteboard->typesForLegacyUnsafeBindings();
ASSERT(!types.contains("Files"_s));
if (m_pasteboard->fileContentState() != Pasteboard::FileContentState::NoFileOrImageData && addFilesType == AddFilesType::Yes)
if (!hideFilesType && m_pasteboard->fileContentState() != Pasteboard::FileContentState::NoFileOrImageData && addFilesType == AddFilesType::Yes)
types.append("Files"_s);
return types;
}
Expand All @@ -333,7 +334,7 @@ Vector<String> DataTransfer::types(AddFilesType addFilesType) const
auto fileContentState = m_pasteboard->fileContentState();
if (hasFileBackedItem || fileContentState != Pasteboard::FileContentState::NoFileOrImageData) {
Vector<String> types;
if (addFilesType == AddFilesType::Yes)
if (!hideFilesType && addFilesType == AddFilesType::Yes)
types.append("Files"_s);

if (fileContentState != Pasteboard::FileContentState::MayContainFilePaths) {
Expand All @@ -356,7 +357,7 @@ Vector<Ref<File>> DataTransfer::filesFromPasteboardAndItemList(ScriptExecutionCo
{
bool addedFilesFromPasteboard = false;
Vector<Ref<File>> files;
if ((!forDrag() || forFileDrag()) && m_pasteboard->fileContentState() != Pasteboard::FileContentState::NoFileOrImageData) {
if (allowsFileAccess() && m_pasteboard->fileContentState() != Pasteboard::FileContentState::NoFileOrImageData) {
WebCorePasteboardFileReader reader(context);
m_pasteboard->read(reader);
files = WTFMove(reader.files);
Expand Down
5 changes: 5 additions & 0 deletions Source/WebCore/dom/DataTransfer.h
View file
Expand Up @@ -119,6 +119,11 @@ class DataTransfer : public RefCounted<DataTransfer> {
enum class Type { CopyAndPaste, DragAndDropData, DragAndDropFiles, InputEvent };
DataTransfer(StoreMode, std::unique_ptr<Pasteboard>, Type = Type::CopyAndPaste, String&& effectAllowed = "uninitialized"_s);

bool allowsFileAccess() const
{
return !forDrag() || forFileDrag();
}

#if ENABLE(DRAG_SUPPORT)
bool forDrag() const { return m_type == Type::DragAndDropData || m_type == Type::DragAndDropFiles; }
bool forFileDrag() const { return m_type == Type::DragAndDropFiles; }
Expand Down
27 changes: 20 additions & 7 deletions Source/WebCore/page/DragController.cpp
View file
Expand Up @@ -216,13 +216,15 @@ void DragController::dragEnded()
client().dragEnded();
}

std::optional<DragOperation> DragController::dragEntered(const DragData& dragData)
std::optional<DragOperation> DragController::dragEntered(DragData&& dragData)
{
return dragEnteredOrUpdated(dragData);
return dragEnteredOrUpdated(WTFMove(dragData));
}

void DragController::dragExited(const DragData& dragData)
void DragController::dragExited(DragData&& dragData)
{
disallowFileAccessIfNeeded(dragData);

auto& mainFrame = m_page.mainFrame();
if (mainFrame.view())
mainFrame.eventHandler().cancelDragAndDrop(createMouseEvent(dragData), Pasteboard::create(dragData), dragData.draggingSourceOperationMask(), dragData.containsFiles());
Expand All @@ -232,17 +234,17 @@ void DragController::dragExited(const DragData& dragData)
m_fileInputElementUnderMouse = nullptr;
}

std::optional<DragOperation> DragController::dragUpdated(const DragData& dragData)
std::optional<DragOperation> DragController::dragUpdated(DragData&& dragData)
{
return dragEnteredOrUpdated(dragData);
return dragEnteredOrUpdated(WTFMove(dragData));
}

inline static bool dragIsHandledByDocument(DragHandlingMethod dragHandlingMethod)
{
return dragHandlingMethod != DragHandlingMethod::None && dragHandlingMethod != DragHandlingMethod::PageLoad;
}

bool DragController::performDragOperation(const DragData& dragData)
bool DragController::performDragOperation(DragData&& dragData)
{
if (!m_droppedImagePlaceholders.isEmpty() && m_droppedImagePlaceholderRange && tryToUpdateDroppedImagePlaceholders(dragData)) {
m_droppedImagePlaceholders.clear();
Expand All @@ -259,6 +261,8 @@ bool DragController::performDragOperation(const DragData& dragData)

m_documentUnderMouse = m_page.mainFrame().documentAtPoint(dragData.clientPosition());

disallowFileAccessIfNeeded(dragData);

ShouldOpenExternalURLsPolicy shouldOpenExternalURLsPolicy = ShouldOpenExternalURLsPolicy::ShouldNotAllow;
if (m_documentUnderMouse)
shouldOpenExternalURLsPolicy = m_documentUnderMouse->shouldOpenExternalURLsPolicyToPropagate();
Expand Down Expand Up @@ -314,7 +318,7 @@ void DragController::mouseMovedIntoDocument(Document* newDocument)
m_documentUnderMouse = newDocument;
}

std::optional<DragOperation> DragController::dragEnteredOrUpdated(const DragData& dragData)
std::optional<DragOperation> DragController::dragEnteredOrUpdated(DragData&& dragData)
{
mouseMovedIntoDocument(m_page.mainFrame().documentAtPoint(dragData.clientPosition()));

Expand All @@ -324,6 +328,8 @@ std::optional<DragOperation> DragController::dragEnteredOrUpdated(const DragData
return std::nullopt;
}

disallowFileAccessIfNeeded(dragData);

std::optional<DragOperation> dragOperation;
m_dragHandlingMethod = tryDocumentDrag(dragData, m_dragDestinationActionMask, dragOperation);
if (m_dragHandlingMethod == DragHandlingMethod::None && (m_dragDestinationActionMask.contains(DragDestinationAction::Load))) {
Expand All @@ -337,6 +343,13 @@ std::optional<DragOperation> DragController::dragEnteredOrUpdated(const DragData
return dragOperation;
}

void DragController::disallowFileAccessIfNeeded(DragData& dragData)
{
RefPtr frame = m_documentUnderMouse ? m_documentUnderMouse->frame() : nullptr;
if (frame && !frame->eventHandler().canDropCurrentlyDraggedImageAsFile())
dragData.disallowFileAccess();
}

static HTMLInputElement* asFileInput(Node& node)
{
if (!is<HTMLInputElement>(node))
Expand Down
11 changes: 6 additions & 5 deletions Source/WebCore/page/DragController.h
View file
Expand Up @@ -61,10 +61,10 @@ class DragController {

static DragOperation platformGenericDragOperation();

WEBCORE_EXPORT std::optional<DragOperation> dragEntered(const DragData&);
WEBCORE_EXPORT void dragExited(const DragData&);
WEBCORE_EXPORT std::optional<DragOperation> dragUpdated(const DragData&);
WEBCORE_EXPORT bool performDragOperation(const DragData&);
WEBCORE_EXPORT std::optional<DragOperation> dragEntered(DragData&&);
WEBCORE_EXPORT void dragExited(DragData&&);
WEBCORE_EXPORT std::optional<DragOperation> dragUpdated(DragData&&);
WEBCORE_EXPORT bool performDragOperation(DragData&&);
WEBCORE_EXPORT void dragCancelled();

bool mouseIsOverFileInput() const { return m_fileInputElementUnderMouse; }
Expand Down Expand Up @@ -110,7 +110,7 @@ class DragController {
bool dispatchTextInputEventFor(Frame*, const DragData&);
bool canProcessDrag(const DragData&);
bool concludeEditDrag(const DragData&);
std::optional<DragOperation> dragEnteredOrUpdated(const DragData&);
std::optional<DragOperation> dragEnteredOrUpdated(DragData&&);
std::optional<DragOperation> operationForLoad(const DragData&);
DragHandlingMethod tryDocumentDrag(const DragData&, OptionSet<DragDestinationAction>, std::optional<DragOperation>&);
bool tryDHTMLDrag(const DragData&, std::optional<DragOperation>&);
Expand All @@ -121,6 +121,7 @@ class DragController {

void mouseMovedIntoDocument(Document*);
bool shouldUseCachedImageForDragImage(const Image&) const;
void disallowFileAccessIfNeeded(DragData&);

std::optional<HitTestResult> hitTestResultForDragStart(Frame&, Element&, const IntPoint&) const;

Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/page/DragState.h
View file
Expand Up @@ -37,6 +37,7 @@ struct DragState {
bool shouldDispatchEvents;
OptionSet<DragSourceAction> type; // Should be std::optional<>. See bug 213086.
RefPtr<DataTransfer> dataTransfer; // Used on only the source side of dragging.
RefPtr<SecurityOrigin> restrictedOriginForImageData;
};

} // namespace WebCore
15 changes: 15 additions & 0 deletions Source/WebCore/page/EventHandler.cpp
View file
Expand Up @@ -2299,6 +2299,12 @@ Element* EventHandler::draggingElement() const
return dragState().source.get();
}

bool EventHandler::canDropCurrentlyDraggedImageAsFile() const
{
auto sourceOrigin = dragState().restrictedOriginForImageData;
return !sourceOrigin || m_frame.document()->securityOrigin().canReceiveDragData(*sourceOrigin);
}

static std::pair<bool, RefPtr<Frame>> contentFrameForNode(Node* target)
{
if (!is<HTMLFrameElementBase>(target))
Expand Down Expand Up @@ -4067,6 +4073,7 @@ bool EventHandler::handleDrag(const MouseEventWithHitTestResults& event, CheckDr
// Careful that the drag starting logic stays in sync with eventMayStartDrag().
if (m_mouseDownMayStartDrag && !dragState().source) {
dragState().shouldDispatchEvents = updateDragSourceActionsAllowed().contains(DragSourceAction::DHTML);
dragState().restrictedOriginForImageData = nullptr;

// Try to find an element that wants to be dragged.
HitTestResult result(m_mouseDownContentsPosition);
Expand Down Expand Up @@ -4166,6 +4173,14 @@ bool EventHandler::handleDrag(const MouseEventWithHitTestResults& event, CheckDr
}
}

if (dragState().source && dragState().type == DragSourceAction::Image) {
if (auto* renderImage = dynamicDowncast<RenderImage>(dragState().source->renderer())) {
auto* image = renderImage->cachedImage();
if (image && !image->isCORSSameOrigin())
dragState().restrictedOriginForImageData = SecurityOrigin::create(image->url());
}
}

dragState().dataTransfer->makeInvalidForSecurity();

if (m_mouseDownMayStartDrag) {
Expand Down
2 changes: 2 additions & 0 deletions Source/WebCore/page/EventHandler.h
View file
Expand Up @@ -353,6 +353,8 @@ class EventHandler {
Element* draggingElement() const;
#endif

bool canDropCurrentlyDraggedImageAsFile() const;

WEBCORE_EXPORT void invalidateClick();

#if ENABLE(IMAGE_ANALYSIS)
Expand Down
3 changes: 3 additions & 0 deletions Source/WebCore/page/SecurityOrigin.cpp
View file
Expand Up @@ -355,6 +355,9 @@ bool SecurityOrigin::canReceiveDragData(const SecurityOrigin& dragInitiator) con
if (this == &dragInitiator)
return true;

if (dragInitiator.isLocal() && isLocal())
return true;

return isSameOriginDomain(dragInitiator);
}

Expand Down
6 changes: 6 additions & 0 deletions Source/WebCore/platform/DragData.cpp
View file
Expand Up @@ -61,6 +61,12 @@ std::unique_ptr<PasteboardContext> DragData::createPasteboardContext() const
return PagePasteboardContext::create(std::optional<PageIdentifier> { m_pageID });
}

void DragData::disallowFileAccess()
{
m_fileNames = { };
m_disallowFileAccess = true;
}

} // namespace WebCore

#endif // ENABLE(DRAG_SUPPORT)
2 changes: 2 additions & 0 deletions Source/WebCore/platform/DragData.h
View file
Expand Up @@ -108,6 +108,7 @@ class DragData {
OptionSet<DragDestinationAction> dragDestinationActionMask() const { return m_dragDestinationActionMask; }
void setFileNames(Vector<String>& fileNames) { m_fileNames = WTFMove(fileNames); }
const Vector<String>& fileNames() const { return m_fileNames; }
void disallowFileAccess();
#if PLATFORM(COCOA)
const String& pasteboardName() const { return m_pasteboardName; }
bool containsURLTypeIdentifier() const;
Expand All @@ -133,6 +134,7 @@ class DragData {
#if PLATFORM(WIN)
DragDataMap m_dragDataMap;
#endif
bool m_disallowFileAccess { false };
};

} // namespace WebCore
Expand Down
12 changes: 9 additions & 3 deletions Source/WebCore/platform/cocoa/DragDataCocoa.mm
View file
Expand Up @@ -182,17 +182,21 @@ static inline String tiffPasteboardType()

bool DragData::containsFiles() const
{
return numberOfFiles();
return !m_disallowFileAccess && numberOfFiles();
}

unsigned DragData::numberOfFiles() const
{
if (m_disallowFileAccess)
return 0;
auto context = createPasteboardContext();
return platformStrategies()->pasteboardStrategy()->getNumberOfFiles(m_pasteboardName, context.get());
}

Vector<String> DragData::asFilenames() const
{
if (m_disallowFileAccess)
return { };
auto context = createPasteboardContext();
#if PLATFORM(MAC)
Vector<String> types;
Expand Down Expand Up @@ -264,8 +268,8 @@ static inline String tiffPasteboardType()
|| types.contains(htmlPasteboardType())
|| types.contains(String(kUTTypeWebArchive))
#if PLATFORM(MAC)
|| types.contains(String(legacyFilenamesPasteboardType()))
|| types.contains(String(legacyFilesPromisePasteboardType()))
|| (!m_disallowFileAccess && types.contains(String(legacyFilenamesPasteboardType())))
|| (!m_disallowFileAccess && types.contains(String(legacyFilesPromisePasteboardType())))
#endif
|| types.contains(tiffPasteboardType())
|| types.contains(pdfPasteboardType())
Expand All @@ -282,6 +286,8 @@ static inline String tiffPasteboardType()

bool DragData::containsPromise() const
{
if (m_disallowFileAccess)
return false;
auto context = createPasteboardContext();
// FIXME: legacyFilesPromisePasteboardType() contains UTIs, not path names. Also, why do we
// think promises should only contain one file (or UTI)?
Expand Down
18 changes: 9 additions & 9 deletions Source/WebKit/WebProcess/WebPage/WebPage.cpp
View file
Expand Up @@ -4643,28 +4643,28 @@ void WebPage::performDragControllerAction(DragControllerAction action, const Int
DragData dragData(&selectionData, clientPosition, globalPosition, draggingSourceOperationMask, flags, anyDragDestinationAction(), m_identifier);
switch (action) {
case DragControllerAction::Entered: {
auto resolvedDragOperation = m_page->dragController().dragEntered(dragData);
auto resolvedDragOperation = m_page->dragController().dragEntered(WTFMove(dragData));
send(Messages::WebPageProxy::DidPerformDragControllerAction(resolvedDragOperation, m_page->dragController().dragHandlingMethod(), m_page->dragController().mouseIsOverFileInput(), m_page->dragController().numberOfItemsToBeAccepted(), { }, { }));
return;
}
case DragControllerAction::Updated: {
auto resolvedDragOperation = m_page->dragController().dragUpdated(dragData);
auto resolvedDragOperation = m_page->dragController().dragUpdated(WTFMove(dragData));
send(Messages::WebPageProxy::DidPerformDragControllerAction(resolvedDragOperation, m_page->dragController().dragHandlingMethod(), m_page->dragController().mouseIsOverFileInput(), m_page->dragController().numberOfItemsToBeAccepted(), { }, { }));
return;
}
case DragControllerAction::Exited:
m_page->dragController().dragExited(dragData);
m_page->dragController().dragExited(WTFMove(dragData));
return;

case DragControllerAction::PerformDragOperation: {
m_page->dragController().performDragOperation(dragData);
m_page->dragController().performDragOperation(WTFMove(dragData));
return;
}
}
ASSERT_NOT_REACHED();
}
#else
void WebPage::performDragControllerAction(DragControllerAction action, const WebCore::DragData& dragData, SandboxExtension::Handle&& sandboxExtensionHandle, Vector<SandboxExtension::Handle>&& sandboxExtensionsHandleArray)
void WebPage::performDragControllerAction(DragControllerAction action, WebCore::DragData&& dragData, SandboxExtension::Handle&& sandboxExtensionHandle, Vector<SandboxExtension::Handle>&& sandboxExtensionsHandleArray)
{
if (!m_page) {
send(Messages::WebPageProxy::DidPerformDragControllerAction(std::nullopt, DragHandlingMethod::None, false, 0, { }, { }));
Expand All @@ -4673,17 +4673,17 @@ void WebPage::performDragControllerAction(DragControllerAction action, const Web

switch (action) {
case DragControllerAction::Entered: {
auto resolvedDragOperation = m_page->dragController().dragEntered(dragData);
auto resolvedDragOperation = m_page->dragController().dragEntered(WTFMove(dragData));
send(Messages::WebPageProxy::DidPerformDragControllerAction(resolvedDragOperation, m_page->dragController().dragHandlingMethod(), m_page->dragController().mouseIsOverFileInput(), m_page->dragController().numberOfItemsToBeAccepted(), m_page->dragCaretController().caretRectInRootViewCoordinates(), m_page->dragCaretController().editableElementRectInRootViewCoordinates()));
return;
}
case DragControllerAction::Updated: {
auto resolvedDragOperation = m_page->dragController().dragUpdated(dragData);
auto resolvedDragOperation = m_page->dragController().dragUpdated(WTFMove(dragData));
send(Messages::WebPageProxy::DidPerformDragControllerAction(resolvedDragOperation, m_page->dragController().dragHandlingMethod(), m_page->dragController().mouseIsOverFileInput(), m_page->dragController().numberOfItemsToBeAccepted(), m_page->dragCaretController().caretRectInRootViewCoordinates(), m_page->dragCaretController().editableElementRectInRootViewCoordinates()));
return;
}
case DragControllerAction::Exited:
m_page->dragController().dragExited(dragData);
m_page->dragController().dragExited(WTFMove(dragData));
send(Messages::WebPageProxy::DidPerformDragControllerAction(std::nullopt, DragHandlingMethod::None, false, 0, { }, { }));
return;

Expand All @@ -4696,7 +4696,7 @@ void WebPage::performDragControllerAction(DragControllerAction action, const Web
m_pendingDropExtensionsForFileUpload.append(extension);
}

bool handled = m_page->dragController().performDragOperation(dragData);
bool handled = m_page->dragController().performDragOperation(WTFMove(dragData));

// If we started loading a local file, the sandbox extension tracker would have adopted this
// pending drop sandbox extension. If not, we'll play it safe and clear it.
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/WebProcess/WebPage/WebPage.h
View file
Expand Up @@ -1027,7 +1027,7 @@ class WebPage : public API::ObjectImpl<API::Object::Type::BundlePage>, public IP
#endif

#if ENABLE(DRAG_SUPPORT) && !PLATFORM(GTK)
void performDragControllerAction(DragControllerAction, const WebCore::DragData&, SandboxExtension::Handle&&, Vector<SandboxExtension::Handle>&&);
void performDragControllerAction(DragControllerAction, WebCore::DragData&&, SandboxExtension::Handle&&, Vector<SandboxExtension::Handle>&&);
#endif

#if ENABLE(DRAG_SUPPORT)
Expand Down

0 comments on commit a703b40

Please sign in to comment.