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 442944 113b4856d0f94b3e71bc24998971d67a00c75fcc
parent 442943 d4b2fecf181c6c89a205eaf0058bbf129f780faa
child 442945 04c752bcb18cbe9a4440d16dbb6cd84f1b21ea69
push id34931
push userdvarga@mozilla.com
push dateThu, 25 Oct 2018 16:06:27 +0000
treeherdermozilla-central@88e1ecec651d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1499821
milestone65.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 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