Address post-review feedback following r222885
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 5 Oct 2017 08:06:16 +0000 (08:06 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 5 Oct 2017 08:06:16 +0000 (08:06 +0000)
https://bugs.webkit.org/show_bug.cgi?id=177853
<rdar://problem/34807346>

Reviewed by Ryosuke Niwa and Dan Bates.

* dom/DataTransfer.cpp:
(WebCore::DataTransfer::updateFileList):
(WebCore::DataTransfer::types const):
(WebCore::DataTransfer::filesFromPasteboardAndItemList const):
* dom/DataTransferItem.h:
(WebCore::DataTransferItem::file):
(WebCore::DataTransferItem::file const): Deleted.
* dom/DataTransferItemList.cpp:
(WebCore::DataTransferItemList::add):
(WebCore::DataTransferItemList::remove):

When removing a data transfer item, capture it in a Ref for the scope of remove(), so that it won't be destroyed
immediately after removing from the item list.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@222904 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/dom/DataTransfer.cpp
Source/WebCore/dom/DataTransferItem.h
Source/WebCore/dom/DataTransferItemList.cpp

index 0e574373b9b95e461f05d353b4200563852fe9bd..34adbc2dfd81ce613f8fade403d9fdb0ec63510c 100644 (file)
@@ -1,3 +1,25 @@
+2017-10-05  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Address post-review feedback following r222885
+        https://bugs.webkit.org/show_bug.cgi?id=177853
+        <rdar://problem/34807346>
+
+        Reviewed by Ryosuke Niwa and Dan Bates.
+
+        * dom/DataTransfer.cpp:
+        (WebCore::DataTransfer::updateFileList):
+        (WebCore::DataTransfer::types const):
+        (WebCore::DataTransfer::filesFromPasteboardAndItemList const):
+        * dom/DataTransferItem.h:
+        (WebCore::DataTransferItem::file):
+        (WebCore::DataTransferItem::file const): Deleted.
+        * dom/DataTransferItemList.cpp:
+        (WebCore::DataTransferItemList::add):
+        (WebCore::DataTransferItemList::remove):
+
+        When removing a data transfer item, capture it in a Ref for the scope of remove(), so that it won't be destroyed
+        immediately after removing from the item list.
+
 2017-10-05  Youenn Fablet  <youenn@apple.com>
 
         Make LibWebRTCProvider port agnostic
index f982d57e774efa7fc0f2060155e5346067c4ab9f..d9608a9e596733d0863ab901de4bdb2d93686e1c 100644 (file)
@@ -164,8 +164,6 @@ void DataTransfer::setData(const String& type, const String& data)
 
 void DataTransfer::updateFileList()
 {
-    // If we're removing an item, then the item list must exist, which implies that the file list must have been initialized already.
-    ASSERT(m_fileList);
     ASSERT(canWriteData());
 
     m_fileList->m_files = filesFromPasteboardAndItemList();
@@ -203,11 +201,11 @@ Vector<String> DataTransfer::types() const
     }
 
     if (m_itemList && m_itemList->hasItems() && m_itemList->items().findMatching([] (const auto& item) { return item->isFile(); }) != notFound)
-        return { "Files" };
+        return { ASCIILiteral("Files") };
 
     if (m_pasteboard->containsFiles()) {
         ASSERT(!m_pasteboard->typesSafeForBindings().contains("Files"));
-        return { "Files" };
+        return { ASCIILiteral("Files") };
     }
 
     auto types = m_pasteboard->typesSafeForBindings();
@@ -229,9 +227,8 @@ Vector<Ref<File>> DataTransfer::filesFromPasteboardAndItemList() const
     bool itemListContainsItems = false;
     if (m_itemList && m_itemList->hasItems()) {
         for (auto& item : m_itemList->items()) {
-            if (auto file = item->file()) {
-                files.append(*file);
-            }
+            if (auto file = item->file())
+                files.append(file.releaseNonNull());
         }
         itemListContainsItems = true;
     }
index 9685529187e66d439bbf502a7076d06b488c9746..1f2809ff6b1eb9a2dae2f4cfddb62fc4e8f3b91f 100644 (file)
@@ -54,7 +54,7 @@ public:
 
     ~DataTransferItem();
 
-    RefPtr<File> file() const { return m_file; }
+    RefPtr<File> file() { return m_file; }
     void clearListAndPutIntoDisabledMode();
 
     bool isFile() const { return m_file; }
index 690a2ab0795e29bbacfa9eb697ac431e09632eaa..8b273874a89444a65027a3d18762f55ff60c9b11 100644 (file)
@@ -91,7 +91,7 @@ RefPtr<DataTransferItem> DataTransferItemList::add(Ref<File>&& file)
 
     ensureItems().append(DataTransferItem::create(m_weakPtrFactory.createWeakPtr(*this), file->type(), file.copyRef()));
     m_dataTransfer.didAddFileToItemList();
-    return RefPtr<DataTransferItem> { m_items->last().ptr() };
+    return m_items->last().ptr();
 }
 
 ExceptionOr<void> DataTransferItemList::remove(unsigned index)
@@ -103,15 +103,13 @@ ExceptionOr<void> DataTransferItemList::remove(unsigned index)
     if (items.size() <= index)
         return Exception { IndexSizeError }; // Matches Gecko. See https://github.com/whatwg/html/issues/2925
 
-    // Since file-backed DataTransferItems are not actually written to the pasteboard yet, we don't need to remove any
-    // temporary files. When we support writing file-backed DataTransferItems to the platform pasteboard, we will need
-    // to clean up here.
-    auto& removedItem = items[index].get();
-    if (!removedItem.isFile())
-        m_dataTransfer.pasteboard().clear(removedItem.type());
-    removedItem.clearListAndPutIntoDisabledMode();
+    // FIXME: Remove the file from the pasteboard object once we add support for it.
+    Ref<DataTransferItem> removedItem = items[index].copyRef();
+    if (!removedItem->isFile())
+        m_dataTransfer.pasteboard().clear(removedItem->type());
+    removedItem->clearListAndPutIntoDisabledMode();
     items.remove(index);
-    if (removedItem.isFile())
+    if (removedItem->isFile())
         m_dataTransfer.updateFileList();
 
     return { };