Bug 1499821 - Introduce the FindDataFlavor helper function to nsTransferable. r=froydnj
authorTom Schuster <evilpies@gmail.com>
Thu, 25 Oct 2018 10:04:43 +0000
changeset 491304 113b4856d0f94b3e71bc24998971d67a00c75fcc
parent 491303 d4b2fecf181c6c89a205eaf0058bbf129f780faa
child 491305 04c752bcb18cbe9a4440d16dbb6cd84f1b21ea69
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersfroydnj
bugs1499821
milestone65.0a1
Bug 1499821 - Introduce the FindDataFlavor helper function to nsTransferable. r=froydnj This combines some of your ideas from D8074, in which I think is quite a big improvement to the overall code. The code for nsTransferable::GetTransferData, especially nsIFlavorDataProvider is a bit wonky, because I tried to preserve the current behavior. Differential Revision: https://phabricator.services.mozilla.com/D9599
widget/nsTransferable.cpp
widget/nsTransferable.h
--- a/widget/nsTransferable.cpp
+++ b/widget/nsTransferable.cpp
@@ -32,29 +32,19 @@ Notes to self:
 #include "nsCRT.h"
 #include "nsNetUtil.h"
 #include "nsIOutputStream.h"
 #include "nsIInputStream.h"
 #include "nsIWeakReferenceUtils.h"
 #include "nsILoadContext.h"
 #include "mozilla/UniquePtr.h"
 
-NS_IMPL_ISUPPORTS(nsTransferable, nsITransferable)
+using namespace mozilla;
 
-size_t
-GetDataForFlavor(const nsTArray<DataStruct>& aArray, const char* aDataFlavor)
-{
-  for (size_t i = 0; i < aArray.Length(); ++i) {
-    if (aArray[i].GetFlavor().Equals(aDataFlavor)) {
-      return i;
-    }
-  }
-
-  return aArray.NoIndex;
-}
+NS_IMPL_ISUPPORTS(nsTransferable, nsITransferable)
 
 DataStruct::DataStruct(DataStruct&& aRHS)
   : mData(aRHS.mData.forget())
   , mDataLen(aRHS.mDataLen)
   , mCacheFD(aRHS.mCacheFD)
   , mFlavor(aRHS.mFlavor)
 {
   aRHS.mCacheFD = nullptr;
@@ -162,17 +152,17 @@ DataStruct::ReadCache(nsISupports** aDat
   if (PR_GetOpenFileInfo(mCacheFD, &fileInfo) != PR_SUCCESS) {
     return NS_ERROR_FAILURE;
   }
   if (PR_Seek64(mCacheFD, 0, PR_SEEK_SET) == -1) {
     return NS_ERROR_FAILURE;
   }
   uint32_t fileSize = fileInfo.size;
 
-  auto data = mozilla::MakeUnique<char[]>(fileSize);
+  auto data = MakeUnique<char[]>(fileSize);
   if (!data) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   uint32_t actual = PR_Read(mCacheFD, data.get(), fileSize);
   if (actual != fileSize) {
     return NS_ERROR_FAILURE;
   }
@@ -230,16 +220,31 @@ nsTransferable::GetTransferDataFlavors(n
   MOZ_ASSERT(mInitialized);
 
   for (size_t i = 0; i < mDataArray.Length(); ++i) {
     DataStruct& data = mDataArray.ElementAt(i);
     aFlavors.AppendElement(data.GetFlavor());
   }
 }
 
+Maybe<size_t>
+nsTransferable::FindDataFlavor(const char* aFlavor)
+{
+  nsDependentCString flavor(aFlavor);
+
+  for (size_t i = 0; i < mDataArray.Length(); ++i) {
+    if (mDataArray[i].GetFlavor().Equals(flavor)) {
+      return Some(i);
+    }
+  }
+
+  return Nothing();
+}
+
+
 //
 // GetTransferData
 //
 // Returns the data of the requested flavor, obtained from either having the
 // data on hand or using a converter to get it. The data is wrapped in a
 // nsISupports primitive so that it is accessible from JS.
 //
 NS_IMETHODIMP
@@ -249,47 +254,43 @@ nsTransferable::GetTransferData(const ch
 {
   MOZ_ASSERT(mInitialized);
 
   *aData = nullptr;
   *aDataLen = 0;
 
   nsresult rv = NS_OK;
 
-  // first look and see if the data is present in one of the intrinsic flavors
-  for (size_t i = 0; i < mDataArray.Length(); ++i) {
-    DataStruct& data = mDataArray.ElementAt(i);
-    if (data.GetFlavor().Equals(aFlavor)) {
-      nsCOMPtr<nsISupports> dataBytes;
-      uint32_t len;
-      data.GetData(getter_AddRefs(dataBytes), &len);
+  // First look and see if the data is present in one of the intrinsic flavors.
+  if (Maybe<size_t> index = FindDataFlavor(aFlavor)) {
+    nsCOMPtr<nsISupports> dataBytes;
+    uint32_t len;
+    mDataArray[index.value()].GetData(getter_AddRefs(dataBytes), &len);
 
-      // Do we have a (lazy) data provider?
-      if (nsCOMPtr<nsIFlavorDataProvider> dataProvider =
-            do_QueryInterface(dataBytes)) {
-        rv = dataProvider->GetFlavorData(this, aFlavor,
-                                         getter_AddRefs(dataBytes), &len);
-        if (NS_FAILED(rv)) {
-          // The provider failed, fall into the converter code below.
-          break;
-        }
+    // Do we have a (lazy) data provider?
+    if (nsCOMPtr<nsIFlavorDataProvider> dataProvider =
+          do_QueryInterface(dataBytes)) {
+      rv = dataProvider->GetFlavorData(this, aFlavor,
+                                       getter_AddRefs(dataBytes), &len);
+      if (NS_FAILED(rv)) {
+        dataBytes = nullptr;
+        // The provider failed, fall into the converter code below.
       }
+    }
 
-      if (dataBytes) {
-        *aDataLen = len;
-        dataBytes.forget(aData);
-        return NS_OK;
-      }
+    if (dataBytes) {
+      *aDataLen = len;
+      dataBytes.forget(aData);
+      return NS_OK;
+    }
 
-      // Not found.
-      break;
-    }
+    // Empty data
   }
 
-  // if not, try using a format converter to get the requested flavor
+  // If not, try using a format converter to get the requested flavor.
   if (mFormatConv) {
     for (size_t i = 0; i < mDataArray.Length(); ++i) {
       DataStruct& data = mDataArray.ElementAt(i);
       bool canConvert = false;
       mFormatConv->CanConvert(data.GetFlavor().get(), aFlavor, &canConvert);
       if (canConvert) {
         nsCOMPtr<nsISupports> dataBytes;
         uint32_t len;
@@ -348,22 +349,20 @@ nsTransferable::GetAnyTransferData(nsACS
 NS_IMETHODIMP
 nsTransferable::SetTransferData(const char* aFlavor,
                                 nsISupports* aData,
                                 uint32_t aDataLen)
 {
   MOZ_ASSERT(mInitialized);
 
   // first check our intrinsic flavors to see if one has been registered.
-  for (size_t i = 0; i < mDataArray.Length(); ++i) {
-    DataStruct& data = mDataArray.ElementAt(i);
-    if (data.GetFlavor().Equals(aFlavor)) {
-      data.SetData(aData, aDataLen, mPrivateData);
-      return NS_OK;
-    }
+  if (Maybe<size_t> index = FindDataFlavor(aFlavor)) {
+    DataStruct& data = mDataArray.ElementAt(index.value());
+    data.SetData(aData, aDataLen, mPrivateData);
+    return NS_OK;
   }
 
   // if not, try using a format converter to find a flavor to put the data in
   if (mFormatConv) {
     for (size_t i = 0; i < mDataArray.Length(); ++i) {
       DataStruct& data = mDataArray.ElementAt(i);
       bool canConvert = false;
       mFormatConv->CanConvert(aFlavor, data.GetFlavor().get(), &canConvert);
@@ -397,42 +396,41 @@ nsTransferable::SetTransferData(const ch
 //
 // Adds a data flavor to our list with no data. Error if it already exists.
 //
 NS_IMETHODIMP
 nsTransferable::AddDataFlavor(const char* aDataFlavor)
 {
   MOZ_ASSERT(mInitialized);
 
-  if (GetDataForFlavor(mDataArray, aDataFlavor) != mDataArray.NoIndex) {
+  if (FindDataFlavor(aDataFlavor).isSome()) {
     return NS_ERROR_FAILURE;
   }
 
   // Create a new "slot" for the data
   mDataArray.AppendElement(DataStruct(aDataFlavor));
-
   return NS_OK;
 }
 
 //
 // RemoveDataFlavor
 //
 // Removes a data flavor (and causes the data to be destroyed). Error if
 // the requested flavor is not present.
 //
 NS_IMETHODIMP
 nsTransferable::RemoveDataFlavor(const char* aDataFlavor)
 {
   MOZ_ASSERT(mInitialized);
 
-  size_t idx = GetDataForFlavor(mDataArray, aDataFlavor);
-  if (idx != mDataArray.NoIndex) {
-    mDataArray.RemoveElementAt(idx);
+  if (Maybe<size_t> index = FindDataFlavor(aDataFlavor)) {
+    mDataArray.RemoveElementAt(index.value());
     return NS_OK;
   }
+
   return NS_ERROR_FAILURE;
 }
 
 NS_IMETHODIMP
 nsTransferable::SetConverter(nsIFormatConverter* aConverter)
 {
   MOZ_ASSERT(mInitialized);
 
--- a/widget/nsTransferable.h
+++ b/widget/nsTransferable.h
@@ -8,16 +8,17 @@
 
 #include "nsIFormatConverter.h"
 #include "nsITransferable.h"
 #include "nsCOMPtr.h"
 #include "nsString.h"
 #include "nsTArray.h"
 #include "nsIPrincipal.h"
 #include "prio.h"
+#include "mozilla/Maybe.h"
 
 class nsIMutableArray;
 
 //
 // DataStruct
 //
 // Holds a flavor (a mime type) that describes the data and the associated data.
 //
@@ -68,19 +69,22 @@ public:
 
     // nsISupports
   NS_DECL_ISUPPORTS
   NS_DECL_NSITRANSFERABLE
 
 protected:
   virtual ~nsTransferable();
 
-  // get flavors w/out converter
+  // Get flavors w/out converter
   void GetTransferDataFlavors(nsTArray<nsCString>& aFlavors);
- 
+
+  // Find index for data with the matching flavor in mDataArray.
+  mozilla::Maybe<size_t> FindDataFlavor(const char* aFlavor);
+
   nsTArray<DataStruct> mDataArray;
   nsCOMPtr<nsIFormatConverter> mFormatConv;
   bool mPrivateData;
   nsCOMPtr<nsIPrincipal> mRequestingPrincipal;
   nsContentPolicyType mContentPolicyType;
 #if DEBUG
   bool mInitialized;
 #endif