Bug 335545 - Store clipboard data in memory XOR file r=mstange
authorRob Wu <rob@robwu.nl>
Sun, 03 Sep 2017 03:21:45 +0200
changeset 406604 28a5bbda5231a3d0932fe80fdf0783e880ef2250
parent 406603 07e72c13b47641264629490a28894c6007fabe9d
child 406605 a9eca77bd638babb8de38824f1e61b4bbd8424f6
push id33571
push usercsabou@mozilla.com
push dateTue, 06 Mar 2018 02:49:43 +0000
treeherdermozilla-central@36c84344de23 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmstange
bugs335545
milestone60.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 335545 - Store clipboard data in memory XOR file r=mstange Ensure that only DataStruct::mData + mDataLen, XOR DataStruct::mCacheFD is used. (Previously it was possible that all of these members were populated, which is a waste of memory.) The effect of this change is visible when SetTransferData is called multiple times with the same flavor, but with one below the threshold for storing in-memory, and the other above (=store in a file). MozReview-Commit-ID: 4UlkKAYsjf
widget/nsTransferable.cpp
widget/nsTransferable.h
--- a/widget/nsTransferable.cpp
+++ b/widget/nsTransferable.cpp
@@ -63,42 +63,53 @@ DataStruct::~DataStruct()
 //-------------------------------------------------------------------------
 void
 DataStruct::SetData ( nsISupports* aData, uint32_t aDataLen, bool aIsPrivateData )
 {
   // Now, check to see if we consider the data to be "too large"
   // as well as ensuring that private browsing mode is disabled
   if (aDataLen > kLargeDatasetSize && !aIsPrivateData) {
     // if so, cache it to disk instead of memory
-    if ( NS_SUCCEEDED(WriteCache(aData, aDataLen)) )
+    if (NS_SUCCEEDED(WriteCache(aData, aDataLen))) {
+      // Clear previously set small data.
+      mData = nullptr;
+      mDataLen = 0;
       return;
-    else
-			NS_WARNING("Oh no, couldn't write data to the cache file");
+    }
+    NS_WARNING("Oh no, couldn't write data to the cache file");
+  }
+
+  if (mCacheFD) {
+    // Clear previously set big data.
+    PR_Close(mCacheFD);
+    mCacheFD = nullptr;
   }
 
   mData    = aData;
   mDataLen = aDataLen;
 }
 
 
 //-------------------------------------------------------------------------
 void
 DataStruct::GetData ( nsISupports** aData, uint32_t *aDataLen )
 {
   // check here to see if the data is cached on disk
-  if ( !mData && mCacheFD ) {
+  if (mCacheFD) {
     // if so, read it in and pass it back
     // ReadCache creates memory and copies the data into it.
     if ( NS_SUCCEEDED(ReadCache(aData, aDataLen)) )
       return;
     else {
       // oh shit, something went horribly wrong here.
       NS_WARNING("Oh no, couldn't read data in from the cache file");
       *aData = nullptr;
       *aDataLen = 0;
+      PR_Close(mCacheFD);
+      mCacheFD = nullptr;
       return;
     }
   }
 
   *aData = mData;
   if ( mData )
     NS_ADDREF(*aData);
   *aDataLen = mDataLen;
@@ -124,16 +135,18 @@ DataStruct::WriteCache(nsISupports* aDat
   nsPrimitiveHelpers::CreateDataFromPrimitive(mFlavor, aData, &buff, aDataLen);
   if (buff) {
     int32_t written = PR_Write(mCacheFD, buff, aDataLen);
     free(buff);
     if (written) {
       return NS_OK;
     }
   }
+  PR_Close(mCacheFD);
+  mCacheFD = nullptr;
   return NS_ERROR_FAILURE;
 }
 
 
 //-------------------------------------------------------------------------
 nsresult
 DataStruct::ReadCache(nsISupports** aData, uint32_t* aDataLen)
 {
--- a/widget/nsTransferable.h
+++ b/widget/nsTransferable.h
@@ -38,16 +38,17 @@ protected:
     // The size of data over which we write the data to disk rather than
     // keep it around in memory.
     kLargeDatasetSize = 1000000        // 1 million bytes
   };
   
   nsresult WriteCache(nsISupports* aData, uint32_t aDataLen );
   nsresult ReadCache(nsISupports** aData, uint32_t* aDataLen );
   
+  // mData + mDataLen OR mCacheFD should be used, not both.
   nsCOMPtr<nsISupports> mData;   // OWNER - some varient of primitive wrapper
   uint32_t mDataLen;
   PRFileDesc* mCacheFD;
   const nsCString mFlavor;
 
 };
 
 /**