Bug 1298243 part 7. Change DataTransfer.types from being a DOMStringList to being a frozen array. r=mystor,gijs
authorBoris Zbarsky <bzbarsky@mit.edu>
Mon, 10 Oct 2016 21:07:47 -0400
changeset 317337 67d40adfa5e12f58326a00f27262036edd9386ba
parent 317336 1493789a0ce5976af2c78fb8b8f5dc3fff475ac7
child 317338 3e0c2cad052c8752f84ef746c2b54371971afb1f
push id82629
push userbzbarsky@mozilla.com
push dateTue, 11 Oct 2016 01:38:11 +0000
treeherdermozilla-inbound@0c1078ea4e6d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmystor, gijs
bugs1298243
milestone52.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1298243 part 7. Change DataTransfer.types from being a DOMStringList to being a frozen array. r=mystor,gijs
browser/base/content/newtab/dragDataHelper.js
browser/base/content/test/general/browser_clipboard_pastefile.js
browser/base/content/urlbarBindings.xml
browser/components/downloads/content/allDownloadsViewOverlay.js
browser/components/search/content/search.xml
dom/base/contentAreaDropListener.js
dom/events/DataTransfer.cpp
dom/events/DataTransfer.h
dom/events/test/test_dragstart.html
dom/webidl/DataTransfer.webidl
editor/libeditor/EditorEventListener.cpp
layout/forms/nsFileControlFrame.cpp
toolkit/components/viewsource/content/viewSource.js
toolkit/content/customizeToolbar.js
toolkit/content/widgets/browser.xml
toolkit/content/widgets/findbar.xml
toolkit/mozapps/downloads/content/downloads.js
toolkit/mozapps/extensions/content/extensions.js
--- a/browser/base/content/newtab/dragDataHelper.js
+++ b/browser/base/content/newtab/dragDataHelper.js
@@ -6,17 +6,17 @@
 
 var gDragDataHelper = {
   get mimeType() {
     return "text/x-moz-url";
   },
 
   getLinkFromDragEvent: function DragDataHelper_getLinkFromDragEvent(aEvent) {
     let dt = aEvent.dataTransfer;
-    if (!dt || !dt.types.contains(this.mimeType)) {
+    if (!dt || !dt.types.includes(this.mimeType)) {
       return null;
     }
 
     let data = dt.getData(this.mimeType) || "";
     let [url, title] = data.split(/[\r\n]+/);
     return {url: url, title: title};
   }
 };
--- a/browser/base/content/test/general/browser_clipboard_pastefile.js
+++ b/browser/base/content/test/general/browser_clipboard_pastefile.js
@@ -39,17 +39,17 @@ add_task(function*() {
   searchbar.focus();
 
   yield new Promise((resolve, reject) => {
     searchbar.addEventListener("paste", function copyEvent(event) {
       searchbar.removeEventListener("paste", copyEvent, true);
 
       let dt = event.clipboardData;
       is(dt.types.length, 3, "number of types");
-      ok(dt.types.contains("text/plain"), "text/plain exists in types");
+      ok(dt.types.includes("text/plain"), "text/plain exists in types");
       ok(dt.mozTypesAt(0).contains("text/plain"), "text/plain exists in mozTypesAt");
       is(dt.getData("text/plain"), "Alternate", "text/plain returned in getData");
       is(dt.mozGetDataAt("text/plain", 0), "Alternate", "text/plain returned in mozGetDataAt");
 
       resolve();
     }, true);
 
     EventUtils.synthesizeKey("v", { accelKey: true });
--- a/browser/base/content/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@@ -651,20 +651,20 @@ file, You can obtain one at http://mozil
           this.inputField.removeAttribute("tooltiptext");
         ]]></body>
       </method>
 
       <method name="onDragOver">
         <parameter name="aEvent"/>
         <body>
           var types = aEvent.dataTransfer.types;
-          if (types.contains("application/x-moz-file") ||
-              types.contains("text/x-moz-url") ||
-              types.contains("text/uri-list") ||
-              types.contains("text/unicode"))
+          if (types.includes("application/x-moz-file") ||
+              types.includes("text/x-moz-url") ||
+              types.includes("text/uri-list") ||
+              types.includes("text/unicode"))
             aEvent.preventDefault();
         </body>
       </method>
 
       <method name="onDrop">
         <parameter name="aEvent"/>
         <body><![CDATA[
           let links = browserDragAndDrop.dropLinks(aEvent);
--- a/browser/components/downloads/content/allDownloadsViewOverlay.js
+++ b/browser/components/downloads/content/allDownloadsViewOverlay.js
@@ -1386,19 +1386,19 @@ DownloadsPlacesView.prototype = {
     dt.setData("text/uri-list", url);
     dt.setData("text/plain", url);
     dt.effectAllowed = "copyMove";
     dt.addElement(selectedItem);
   },
 
   onDragOver(aEvent) {
     let types = aEvent.dataTransfer.types;
-    if (types.contains("text/uri-list") ||
-        types.contains("text/x-moz-url") ||
-        types.contains("text/plain")) {
+    if (types.includes("text/uri-list") ||
+        types.includes("text/x-moz-url") ||
+        types.includes("text/plain")) {
       aEvent.preventDefault();
     }
   },
 
   onDrop(aEvent) {
     let dt = aEvent.dataTransfer;
     // If dragged item is from our source, do not try to
     // redownload already downloaded file.
--- a/browser/components/search/content/search.xml
+++ b/browser/components/search/content/search.xml
@@ -851,17 +851,17 @@
 
       <handler event="keypress" keycode="VK_UP" modifiers="alt"
                phase="capturing"
                action="return this.openSearch();"/>
 
       <handler event="dragover">
       <![CDATA[
         var types = event.dataTransfer.types;
-        if (types.contains("text/plain") || types.contains("text/x-moz-text-internal"))
+        if (types.includes("text/plain") || types.includes("text/x-moz-text-internal"))
           event.preventDefault();
       ]]>
       </handler>
 
       <handler event="drop">
       <![CDATA[
         var dataTransfer = event.dataTransfer;
         var data = dataTransfer.getData("text/plain");
--- a/dom/base/contentAreaDropListener.js
+++ b/dom/base/contentAreaDropListener.js
@@ -133,21 +133,21 @@ ContentAreaDropListener.prototype =
 
   canDropLink: function(aEvent, aAllowSameDocument)
   {
     if (this._eventTargetIsDisabled(aEvent))
       return false;
 
     let dataTransfer = aEvent.dataTransfer;
     let types = dataTransfer.types;
-    if (!types.contains("application/x-moz-file") &&
-        !types.contains("text/x-moz-url") &&
-        !types.contains("text/uri-list") &&
-        !types.contains("text/x-moz-text-internal") &&
-        !types.contains("text/plain"))
+    if (!types.includes("application/x-moz-file") &&
+        !types.includes("text/x-moz-url") &&
+        !types.includes("text/uri-list") &&
+        !types.includes("text/x-moz-text-internal") &&
+        !types.includes("text/plain"))
       return false;
 
     if (aAllowSameDocument)
       return true;
 
     let sourceNode = dataTransfer.mozSourceNode;
     if (!sourceNode)
       return true;
--- a/dom/events/DataTransfer.cpp
+++ b/dom/events/DataTransfer.cpp
@@ -305,60 +305,54 @@ DataTransfer::GetFiles(nsIDOMFileList** 
   // This code is also called from C++ code, which expects it to have a System
   // Principal, and thus the SubjectPrincipal cannot be used.
   RefPtr<FileList> files = mItems->Files(nsContentUtils::GetSystemPrincipal());
 
   files.forget(aFileList);
   return NS_OK;
 }
 
-already_AddRefed<DOMStringList>
-DataTransfer::GetTypes(ErrorResult& aRv) const
+void
+DataTransfer::GetTypes(nsTArray<nsString>& aTypes) const
 {
-  RefPtr<DOMStringList> types = new DOMStringList();
-
+  // When called from bindings, aTypes will be empty, but since we might have
+  // Gecko-internal callers too, clear it to be safe.
+  aTypes.Clear();
+  
   const nsTArray<RefPtr<DataTransferItem>>* items = mItems->MozItemsAt(0);
   if (NS_WARN_IF(!items)) {
-    return types.forget();
+    return;
   }
 
   for (uint32_t i = 0; i < items->Length(); i++) {
     DataTransferItem* item = items->ElementAt(i);
     MOZ_ASSERT(item);
 
     if (item->ChromeOnly() && !nsContentUtils::LegacyIsCallerChromeOrNativeCode()) {
       continue;
     }
 
     nsAutoString type;
     item->GetType(type);
     if (item->Kind() == DataTransferItem::KIND_STRING || type.EqualsASCII(kFileMime)) {
       // If the entry has kind KIND_STRING, we want to add it to the list.
-      if (NS_WARN_IF(!types->Add(type))) {
-        aRv.Throw(NS_ERROR_FAILURE);
-        return nullptr;
-      }
+      aTypes.AppendElement(type);
     }
   }
 
   for (uint32_t i = 0; i < mItems->Length(); ++i) {
     bool found = false;
     DataTransferItem* item = mItems->IndexedGetter(i, found);
     MOZ_ASSERT(found);
     if (item->Kind() != DataTransferItem::KIND_FILE) {
       continue;
     }
-    if (NS_WARN_IF(!types->Add(NS_LITERAL_STRING("Files")))) {
-      aRv.Throw(NS_ERROR_FAILURE);
-      return nullptr;
-    }
+    aTypes.AppendElement(NS_LITERAL_STRING("Files"));
     break;
   }
-
-  return types.forget();
 }
 
 void
 DataTransfer::GetData(const nsAString& aFormat, nsAString& aData,
                       const Maybe<nsIPrincipal*>& aSubjectPrincipal,
                       ErrorResult& aRv)
 {
   MOZ_ASSERT(aSubjectPrincipal.isSome());
@@ -665,18 +659,17 @@ DataTransfer::PrincipalMaySetData(const 
     }
   }
   return true;
 }
 
 void
 DataTransfer::TypesListMayHaveChanged()
 {
-  // For now do nothing; we'll want to clear our cached types list once we start
-  // caching it.
+  DataTransferBinding::ClearCachedTypesValue(this);
 }
 
 nsresult
 DataTransfer::SetDataAtInternal(const nsAString& aFormat, nsIVariant* aData,
                                 uint32_t aIndex,
                                 nsIPrincipal* aSubjectPrincipal)
 {
   if (aFormat.IsEmpty()) {
--- a/dom/events/DataTransfer.h
+++ b/dom/events/DataTransfer.h
@@ -131,17 +131,17 @@ public:
     } else {
       aEffectAllowed.AssignASCII(sEffects[mEffectAllowed]);
     }
   }
 
   void SetDragImage(Element& aElement, int32_t aX, int32_t aY,
                     ErrorResult& aRv);
 
-  already_AddRefed<DOMStringList> GetTypes(ErrorResult& rv) const;
+  void GetTypes(nsTArray<nsString>& aTypes) const;
 
   void GetData(const nsAString& aFormat, nsAString& aData,
                const Maybe<nsIPrincipal*>& aSubjectPrincipal,
                ErrorResult& aRv);
 
   void SetData(const nsAString& aFormat, const nsAString& aData,
                const Maybe<nsIPrincipal*>& aSubjectPrincipal,
                ErrorResult& aRv);
--- a/dom/events/test/test_dragstart.html
+++ b/dom/events/test/test_dragstart.html
@@ -113,17 +113,17 @@ function doDragStartSelection(event)
   is(event.pageX, 14, "dragstart pageX");
   is(event.pageY, 14, "dragstart pageY");
 
   var dt = event.dataTransfer;
   ok(dt instanceof DataTransfer, "dataTransfer is DataTransfer");
   gDataTransfer = dt;
 
   var types = dt.types;
-  is(types instanceof DOMStringList, true, "initial types is a DOMStringList");
+  ok(Array.isArray(types), "initial types is an Array");
   checkTypes(dt, ["text/_moz_htmlcontext", "text/_moz_htmlinfo", "text/html", "text/plain"], 0, "initial selection");
 
   is(dt.getData("text/plain"), "This is a draggable bit of text.", "initial selection text/plain");
   is(dt.getData("text/html"), "<div id=\"draggable\" ondragstart=\"doDragStartSelection(event)\">This is a <em>draggable</em> bit of text.</div>",
      "initial selection text/html");
 
   // text/unicode and Text are available for compatibility. They retrieve the
   // text/plain data
@@ -143,17 +143,17 @@ function doDragStartSelection(event)
   setTimeout(afterDragTests, 0);
 }
 
 function test_DataTransfer(dt)
 {
   is(dt.mozItemCount, 0, "empty itemCount");
 
   var types = dt.types;
-  is(types instanceof DOMStringList, true, "empty types is a DOMStringList");
+  ok(Array.isArray(types), "empty types is an Array");
   checkTypes(dt, [], 0, "empty");
   is(dt.getData("text/plain"), "", "empty data is empty");
 
   // calling setDataAt requires an index that is 0 <= index <= dt.itemCount
   expectError(() => dt.mozSetDataAt("text/plain", "Some Text", 1),
               "IndexSizeError", "setDataAt index too high");
 
   is(dt.mozUserCancelled, false, "userCancelled");
--- a/dom/webidl/DataTransfer.webidl
+++ b/dom/webidl/DataTransfer.webidl
@@ -12,18 +12,18 @@ interface DataTransfer {
            attribute DOMString dropEffect;
            attribute DOMString effectAllowed;
 
   readonly attribute DataTransferItemList items;
 
   [Throws]
   void setDragImage(Element image, long x, long y);
 
-  [Throws]
-  readonly attribute DOMStringList types;
+  [Pure, Cached, Frozen]
+  readonly attribute sequence<DOMString> types;
   [Throws, NeedsSubjectPrincipal]
   DOMString getData(DOMString format);
   [Throws, NeedsSubjectPrincipal]
   void setData(DOMString format, DOMString data);
   [Throws, NeedsSubjectPrincipal]
   void clearData(optional DOMString format);
   [Throws, NeedsSubjectPrincipal]
   readonly attribute FileList? files;
--- a/editor/libeditor/EditorEventListener.cpp
+++ b/editor/libeditor/EditorEventListener.cpp
@@ -929,29 +929,26 @@ EditorEventListener::CanDrop(nsIDOMDragE
     return false;
   }
 
   nsCOMPtr<nsIDOMDataTransfer> domDataTransfer;
   aEvent->GetDataTransfer(getter_AddRefs(domDataTransfer));
   nsCOMPtr<DataTransfer> dataTransfer = do_QueryInterface(domDataTransfer);
   NS_ENSURE_TRUE(dataTransfer, false);
 
-  ErrorResult err;
-  RefPtr<DOMStringList> types = dataTransfer->GetTypes(err);
-  if (NS_WARN_IF(err.Failed())) {
-    return false;
-  }
+  nsTArray<nsString> types;
+  dataTransfer->GetTypes(types);
 
   // Plaintext editors only support dropping text. Otherwise, HTML and files
   // can be dropped as well.
-  if (!types->Contains(NS_LITERAL_STRING(kTextMime)) &&
-      !types->Contains(NS_LITERAL_STRING(kMozTextInternal)) &&
+  if (!types.Contains(NS_LITERAL_STRING(kTextMime)) &&
+      !types.Contains(NS_LITERAL_STRING(kMozTextInternal)) &&
       (mEditorBase->IsPlaintextEditor() ||
-       (!types->Contains(NS_LITERAL_STRING(kHTMLMime)) &&
-        !types->Contains(NS_LITERAL_STRING(kFileMime))))) {
+       (!types.Contains(NS_LITERAL_STRING(kHTMLMime)) &&
+        !types.Contains(NS_LITERAL_STRING(kFileMime))))) {
     return false;
   }
 
   // If there is no source node, this is probably an external drag and the
   // drop is allowed. The later checks rely on checking if the drag target
   // is the same as the drag source.
   nsCOMPtr<nsIDOMNode> sourceNode;
   dataTransfer->GetMozSourceNode(getter_AddRefs(sourceNode));
--- a/layout/forms/nsFileControlFrame.cpp
+++ b/layout/forms/nsFileControlFrame.cpp
@@ -374,24 +374,20 @@ nsFileControlFrame::DnDListener::GetBlob
 
 bool
 nsFileControlFrame::DnDListener::IsValidDropData(nsIDOMDataTransfer* aDOMDataTransfer)
 {
   nsCOMPtr<DataTransfer> dataTransfer = do_QueryInterface(aDOMDataTransfer);
   NS_ENSURE_TRUE(dataTransfer, false);
 
   // We only support dropping files onto a file upload control
-  ErrorResult rv;
-  RefPtr<DOMStringList> types = dataTransfer->GetTypes(rv);
-  if (NS_WARN_IF(rv.Failed())) {
-    rv.SuppressException();
-    return false;
-  }
+  nsTArray<nsString> types;
+  dataTransfer->GetTypes(types);
 
-  return types->Contains(NS_LITERAL_STRING("Files"));
+  return types.Contains(NS_LITERAL_STRING("Files"));
 }
 
 bool
 nsFileControlFrame::DnDListener::CanDropTheseFiles(nsIDOMDataTransfer* aDOMDataTransfer,
                                                    bool aSupportsMultiple)
 {
   nsCOMPtr<DataTransfer> dataTransfer = do_QueryInterface(aDOMDataTransfer);
   NS_ENSURE_TRUE(dataTransfer, false);
--- a/toolkit/components/viewsource/content/viewSource.js
+++ b/toolkit/components/viewsource/content/viewSource.js
@@ -510,17 +510,17 @@ ViewSourceChrome.prototype = {
   /**
    * Called when the user drags something over the content browser.
    */
   onDragOver(event) {
     // For drags that appear to be internal text (for example, tab drags),
     // set the dropEffect to 'none'. This prevents the drop even if some
     // other listener cancelled the event.
     let types = event.dataTransfer.types;
-    if (types.contains("text/x-moz-text-internal") && !types.contains("text/plain")) {
+    if (types.includes("text/x-moz-text-internal") && !types.includes("text/plain")) {
         event.dataTransfer.dropEffect = "none";
         event.stopPropagation();
         event.preventDefault();
     }
 
     let linkHandler = Cc["@mozilla.org/content/dropped-link-handler;1"]
                         .getService(Ci.nsIDroppedLinkHandler);
 
--- a/toolkit/content/customizeToolbar.js
+++ b/toolkit/content/customizeToolbar.js
@@ -650,17 +650,17 @@ function onToolbarDragStart(aEvent)
 
 function onToolbarDragOver(aEvent)
 {
   if (isUnwantedDragEvent(aEvent)) {
     return;
   }
 
   var documentId = gToolboxDocument.documentElement.id;
-  if (!aEvent.dataTransfer.types.contains("text/toolbarwrapper-id/" + documentId.toLowerCase()))
+  if (!aEvent.dataTransfer.types.includes("text/toolbarwrapper-id/" + documentId.toLowerCase()))
     return;
 
   var toolbar = aEvent.target;
   var dropTarget = aEvent.target;
   while (toolbar && toolbar.localName != "toolbar") {
     dropTarget = toolbar;
     toolbar = toolbar.parentNode;
   }
@@ -779,17 +779,17 @@ function onToolbarDrop(aEvent)
 }
 
 function onPaletteDragOver(aEvent)
 {
   if (isUnwantedDragEvent(aEvent)) {
     return;
   }
   var documentId = gToolboxDocument.documentElement.id;
-  if (aEvent.dataTransfer.types.contains("text/toolbarwrapper-id/" + documentId.toLowerCase()))
+  if (aEvent.dataTransfer.types.includes("text/toolbarwrapper-id/" + documentId.toLowerCase()))
     aEvent.preventDefault();
 }
 
 function onPaletteDrop(aEvent)
 {
   if (isUnwantedDragEvent(aEvent)) {
     return;
   }
--- a/toolkit/content/widgets/browser.xml
+++ b/toolkit/content/widgets/browser.xml
@@ -1440,17 +1440,17 @@
       <![CDATA[
         if (!this.droppedLinkHandler || event.defaultPrevented)
           return;
 
         // For drags that appear to be internal text (for example, tab drags),
         // set the dropEffect to 'none'. This prevents the drop even if some
         // other listener cancelled the event.
         var types = event.dataTransfer.types;
-        if (types.contains("text/x-moz-text-internal") && !types.contains("text/plain")) {
+        if (types.includes("text/x-moz-text-internal") && !types.includes("text/plain")) {
           event.dataTransfer.dropEffect = "none";
           event.stopPropagation();
           event.preventDefault();
         }
 
         // No need to handle "dragover" in e10s, since nsDocShellTreeOwner.cpp in the child process
         // handles that case using "@mozilla.org/content/dropped-link-handler;1" service.
         if (this.isRemoteBrowser)
--- a/toolkit/content/widgets/findbar.xml
+++ b/toolkit/content/widgets/findbar.xml
@@ -134,17 +134,17 @@
       <handler event="compositionend"><![CDATA[
         let findbar = this.findbar;
         findbar._isIMEComposing = false;
         if (findbar._findMode != findbar.FIND_NORMAL)
           findbar._setFindCloseTimeout();
       ]]></handler>
 
       <handler event="dragover"><![CDATA[
-        if (event.dataTransfer.types.contains("text/plain"))
+        if (event.dataTransfer.types.includes("text/plain"))
           event.preventDefault();
       ]]></handler>
 
       <handler event="drop"><![CDATA[
         let value = event.dataTransfer.getData("text/plain");
         this.value = value;
         this.findbar._find(value);
         event.stopPropagation();
--- a/toolkit/mozapps/downloads/content/downloads.js
+++ b/toolkit/mozapps/downloads/content/downloads.js
@@ -656,19 +656,19 @@ var gDownloadDNDObserver =
     dt.setData("text/plain", url);
     dt.effectAllowed = "copyMove";
     dt.addElement(dl);
   },
 
   onDragOver: function (aEvent)
   {
     var types = aEvent.dataTransfer.types;
-    if (types.contains("text/uri-list") ||
-        types.contains("text/x-moz-url") ||
-        types.contains("text/plain"))
+    if (types.includes("text/uri-list") ||
+        types.includes("text/x-moz-url") ||
+        types.includes("text/plain"))
       aEvent.preventDefault();
   },
 
   onDrop: function(aEvent)
   {
     var dt = aEvent.dataTransfer;
     // If dragged item is from our source, do not try to
     // redownload already downloaded file.
--- a/toolkit/mozapps/extensions/content/extensions.js
+++ b/toolkit/mozapps/extensions/content/extensions.js
@@ -3815,19 +3815,19 @@ var gUpdatesView = {
     if (aProperties.indexOf("applyBackgroundUpdates") != -1)
       this.updateAvailableCount();
   }
 };
 
 var gDragDrop = {
   onDragOver: function(aEvent) {
     var types = aEvent.dataTransfer.types;
-    if (types.contains("text/uri-list") ||
-        types.contains("text/x-moz-url") ||
-        types.contains("application/x-moz-file"))
+    if (types.includes("text/uri-list") ||
+        types.includes("text/x-moz-url") ||
+        types.includes("application/x-moz-file"))
       aEvent.preventDefault();
   },
 
   onDrop: function(aEvent) {
     var dataTransfer = aEvent.dataTransfer;
     var urls = [];
 
     // Convert every dropped item into a url