Backed out 3 changesets (bug 335545) for asan failures in test_bug1123480.xul a=backout
authorWes Kocher <wkocher@mozilla.com>
Tue, 26 Sep 2017 15:26:07 -0700
changeset 383112 ffcfc5b577221d93aaaa52babc30af96328d2c05
parent 383111 ade76994700a497c337cd70ca360365b58da70c2
child 383113 ea48aa0eb4dc8888c7eef37571d330d41f35bcdb
push id32583
push userarchaeopteryx@coole-files.de
push dateWed, 27 Sep 2017 09:46:12 +0000
treeherdermozilla-central@5563e7da39b2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs335545, 1123480
milestone58.0a1
backs out25a686779a940f1478aa2f4d1595f81344caac5c
b83ddb70c8b515b89b3e385d5b46f22b165792ed
ea69ee15ed90aa56ea1811b6ec473b0b0c31bad8
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
Backed out 3 changesets (bug 335545) for asan failures in test_bug1123480.xul a=backout Backed out changeset 25a686779a94 (bug 335545) Backed out changeset b83ddb70c8b5 (bug 335545) Backed out changeset ea69ee15ed90 (bug 335545) MozReview-Commit-ID: LkJgt3eSs2J
widget/nsTransferable.cpp
widget/nsTransferable.h
widget/tests/chrome.ini
widget/tests/test_bug1123480.xul
--- a/widget/nsTransferable.cpp
+++ b/widget/nsTransferable.cpp
@@ -9,17 +9,16 @@ Notes to self:
 - at some point, strings will be accessible from JS, so we won't have to wrap
    flavors in an nsISupportsCString. Until then, we're kinda stuck with
    this crappy API of nsIArrays.
 
 */
 
 
 #include "nsTransferable.h"
-#include "nsAnonymousTemporaryFile.h"
 #include "nsArray.h"
 #include "nsArrayUtils.h"
 #include "nsString.h"
 #include "nsReadableUtils.h"
 #include "nsTArray.h"
 #include "nsIFormatConverter.h"
 #include "nsIContentPolicy.h"
 #include "nsIComponentManager.h"
@@ -31,16 +30,17 @@ Notes to self:
 #include "nsDirectoryServiceDefs.h"
 #include "nsDirectoryService.h"
 #include "nsCRT.h"
 #include "nsNetUtil.h"
 #include "nsIDOMNode.h"
 #include "nsIOutputStream.h"
 #include "nsIInputStream.h"
 #include "nsIWeakReferenceUtils.h"
+#include "nsIFile.h"
 #include "nsILoadContext.h"
 #include "mozilla/UniquePtr.h"
 
 NS_IMPL_ISUPPORTS(nsTransferable, nsITransferable)
 
 size_t GetDataForFlavor (const nsTArray<DataStruct>& aArray,
                            const char* aDataFlavor)
 {
@@ -50,126 +50,174 @@ size_t GetDataForFlavor (const nsTArray<
   }
 
   return aArray.NoIndex;
 }
 
 //-------------------------------------------------------------------------
 DataStruct::~DataStruct()
 {
-  if (mCacheFD) {
-    PR_Close(mCacheFD);
-  }
+  if (mCacheFileName) free(mCacheFileName);
 }
 
 //-------------------------------------------------------------------------
 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))) {
-      // Clear previously set small data.
-      mData = nullptr;
-      mDataLen = 0;
+    if ( NS_SUCCEEDED(WriteCache(aData, aDataLen)) )
       return;
-    }
-    NS_WARNING("Oh no, couldn't write data to the cache file");
+    else
+			NS_WARNING("Oh no, couldn't write data to the cache file");
   }
 
   mData    = aData;
   mDataLen = aDataLen;
 }
 
 
 //-------------------------------------------------------------------------
 void
 DataStruct::GetData ( nsISupports** aData, uint32_t *aDataLen )
 {
   // check here to see if the data is cached on disk
-  if (mCacheFD) {
+  if ( !mData && mCacheFileName ) {
     // 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;
 }
 
 
 //-------------------------------------------------------------------------
+already_AddRefed<nsIFile>
+DataStruct::GetFileSpec(const char* aFileName)
+{
+  nsCOMPtr<nsIFile> cacheFile;
+  NS_GetSpecialDirectory(NS_OS_TEMP_DIR, getter_AddRefs(cacheFile));
+
+  if (!cacheFile)
+    return nullptr;
+
+  // if the param aFileName contains a name we should use that
+  // because the file probably already exists
+  // otherwise create a unique name
+  if (!aFileName) {
+    cacheFile->AppendNative(NS_LITERAL_CSTRING("clipboardcache"));
+    nsresult rv = cacheFile->CreateUnique(nsIFile::NORMAL_FILE_TYPE, 0600);
+    if (NS_FAILED(rv))
+      return nullptr;
+  } else {
+    cacheFile->AppendNative(nsDependentCString(aFileName));
+  }
+
+  return cacheFile.forget();
+}
+
+
+//-------------------------------------------------------------------------
 nsresult
 DataStruct::WriteCache(nsISupports* aData, uint32_t aDataLen)
 {
-  nsresult rv;
-  if (!mCacheFD) {
-    rv = NS_OpenAnonymousTemporaryFile(&mCacheFD);
-    if (NS_FAILED(rv)) {
-      return NS_ERROR_FAILURE;
+  // Get a new path and file to the temp directory
+  nsCOMPtr<nsIFile> cacheFile = GetFileSpec(mCacheFileName);
+  if (cacheFile) {
+    // remember the file name
+    if (!mCacheFileName) {
+      nsCString fName;
+      cacheFile->GetNativeLeafName(fName);
+      mCacheFileName = strdup(fName.get());
     }
-  }
 
-  // write out the contents of the clipboard to the file
-  void* buff = nullptr;
-  nsPrimitiveHelpers::CreateDataFromPrimitive(mFlavor, aData, &buff, aDataLen);
-  if (buff) {
-    int32_t written = PR_Write(mCacheFD, buff, aDataLen);
-    free(buff);
-    if (written) {
+    // write out the contents of the clipboard
+    // to the file
+    //uint32_t bytes;
+    nsCOMPtr<nsIOutputStream> outStr;
+
+    NS_NewLocalFileOutputStream(getter_AddRefs(outStr),
+                                cacheFile);
+
+    if (!outStr) return NS_ERROR_FAILURE;
+
+    void* buff = nullptr;
+    nsPrimitiveHelpers::CreateDataFromPrimitive ( mFlavor, aData, &buff, aDataLen );
+    if ( buff ) {
+      uint32_t ignored;
+      outStr->Write(reinterpret_cast<char*>(buff), aDataLen, &ignored);
+      free(buff);
       return NS_OK;
     }
   }
-  PR_Close(mCacheFD);
-  mCacheFD = nullptr;
   return NS_ERROR_FAILURE;
 }
 
 
 //-------------------------------------------------------------------------
 nsresult
 DataStruct::ReadCache(nsISupports** aData, uint32_t* aDataLen)
 {
-  if (!mCacheFD) {
+  // if we don't have a cache filename we are out of luck
+  if (!mCacheFileName)
     return NS_ERROR_FAILURE;
+
+  // get the path and file name
+  nsCOMPtr<nsIFile> cacheFile = GetFileSpec(mCacheFileName);
+  bool exists;
+  if ( cacheFile && NS_SUCCEEDED(cacheFile->Exists(&exists)) && exists ) {
+    // get the size of the file
+    int64_t fileSize;
+    int64_t max32 = 0xFFFFFFFF;
+    cacheFile->GetFileSize(&fileSize);
+    if (fileSize > max32)
+      return NS_ERROR_OUT_OF_MEMORY;
+
+    uint32_t size = uint32_t(fileSize);
+    // create new memory for the large clipboard data
+    auto data = mozilla::MakeUnique<char[]>(size);
+    if ( !data )
+      return NS_ERROR_OUT_OF_MEMORY;
+
+    // now read it all in
+    nsCOMPtr<nsIInputStream> inStr;
+    NS_NewLocalFileInputStream( getter_AddRefs(inStr),
+                                cacheFile);
+
+    if (!cacheFile) return NS_ERROR_FAILURE;
+
+    nsresult rv = inStr->Read(data.get(), fileSize, aDataLen);
+
+    // make sure we got all the data ok
+    if (NS_SUCCEEDED(rv) && *aDataLen == size) {
+      nsPrimitiveHelpers::CreatePrimitiveForData(mFlavor, data.get(),
+                                                 fileSize, aData);
+      return *aData ? NS_OK : NS_ERROR_FAILURE;
+    }
+
+    // zero the return params
+    *aData    = nullptr;
+    *aDataLen = 0;
   }
 
-  PRFileInfo fileInfo;
-  if (PR_GetOpenFileInfo(mCacheFD, &fileInfo) != PR_SUCCESS) {
-    return NS_ERROR_FAILURE;
-  }
-  uint32_t fileSize = fileInfo.size;
-
-  auto data = mozilla::MakeUnique<char[]>(fileSize);
-  if (!data) {
-    return NS_ERROR_OUT_OF_MEMORY;
-  }
-
-  uint32_t actual = PR_Read(mCacheFD, data.get(), fileSize);
-  // actual = 0 means end of file.
-  if (actual != 0 && actual != fileSize) {
-    return NS_ERROR_FAILURE;
-  }
-
-  nsPrimitiveHelpers::CreatePrimitiveForData(mFlavor, data.get(), fileSize, aData);
-  return NS_OK;
+  return NS_ERROR_FAILURE;
 }
 
 
 //-------------------------------------------------------------------------
 //
 // Transferable constructor
 //
 //-------------------------------------------------------------------------
--- a/widget/nsTransferable.h
+++ b/widget/nsTransferable.h
@@ -7,52 +7,51 @@
 #define nsTransferable_h__
 
 #include "nsIFormatConverter.h"
 #include "nsITransferable.h"
 #include "nsCOMPtr.h"
 #include "nsString.h"
 #include "nsTArray.h"
 #include "nsIPrincipal.h"
-#include "prio.h"
 
 class nsIMutableArray;
 
 //
 // DataStruct
 //
 // Holds a flavor (a mime type) that describes the data and the associated data.
 //
 struct DataStruct
 {
   explicit DataStruct ( const char* aFlavor )
-    : mDataLen(0), mCacheFD(nullptr), mFlavor(aFlavor) { }
+    : mDataLen(0), mFlavor(aFlavor), mCacheFileName(nullptr) { }
   ~DataStruct();
   
   const nsCString& GetFlavor() const { return mFlavor; }
   void SetData( nsISupports* inData, uint32_t inDataLen, bool aIsPrivateData );
   void GetData( nsISupports** outData, uint32_t *outDataLen );
-  bool IsDataAvailable() const { return mData ? mDataLen > 0 : mCacheFD != nullptr; }
+  already_AddRefed<nsIFile> GetFileSpec(const char* aFileName);
+  bool IsDataAvailable() const { return (mData && mDataLen > 0) || (!mData && mCacheFileName); }
   
 protected:
 
   enum {
     // 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;
+  char *   mCacheFileName;
 
 };
 
 /**
  * XP Transferable wrapper
  */
 
 class nsTransferable : public nsITransferable
--- a/widget/tests/chrome.ini
+++ b/widget/tests/chrome.ini
@@ -97,11 +97,10 @@ support-files = chrome_context_menus_win
 [test_plugin_input_event.html]
 skip-if = toolkit != "windows"
 [test_mouse_scroll.xul]
 skip-if = toolkit != "windows"
 support-files = window_mouse_scroll_win.html
 
 # Privacy relevant
 [test_bug1123480.xul]
-skip-if = os == "win" # test uses /dev/fd, which is only available on *NIX
 subsuite = clipboard
 
--- a/widget/tests/test_bug1123480.xul
+++ b/widget/tests/test_bug1123480.xul
@@ -8,55 +8,48 @@ https://bugzilla.mozilla.org/show_bug.cg
         xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
         onload="RunTest();">
   <title>nsTransferable PBM Overflow Selection Test</title>
   <script type="application/javascript"
           src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"/>
 
   <script type="application/javascript">
   <![CDATA[
-  // Create 1 Mo of sample garbage text
-  // Since JS strings are saved as UTF16 strings, the string size is 2 MB,
-  // which is well above the kLargeDatasetSize threshold in nsTransferable.h
-  var Ipsum = "0123456789".repeat(100000);
-  var SHORT_STRING_NO_CACHE = "short string that will never be cached to the disk";
+  // Boilerplate constructs
+  var SmallDataset = 100000; // Hundred thousand bytes
 
-  // Get a list of open file descriptors, and ASSUME that any mutations in file
-  // descriptor counts are caused by our test. This is a bold assumption,
-  // but better than the alternatives (spawning lsof as a subprocess and parsing
-  // its output, or adding test-only code to nsAnonymousTemporaryFile).
-  function getOpenFileDescriptorCount() {
-    // Note: /dev/fd is only available on Linux and macOS, not Windows.
-    var file = Components.classes["@mozilla.org/file/local;1"].createInstance(Components.interfaces.nsIFile);
-    file.initWithPath("/dev/fd");
-    var count = 0;
-    for (var de = file.directoryEntries; de.hasMoreElements(); de.getNext()) {
-      ++count;
-    }
-    return count;
+  // Create 1 Mo of sample garbage text
+  var Ipsum = ""; // Select text from this
+  for (var Iter = 0; Iter < SmallDataset; Iter++) {
+      Ipsum += Math.random().toString(36) + ' ';
   }
 
   function RunTest() {
-    const gClipboardHelper = Components.classes["@mozilla.org/widget/clipboardhelper;1"].getService(Components.interfaces.nsIClipboardHelper);
+    // Construct a nsIFile object for access to file methods
+    Components.utils.import("resource://gre/modules/FileUtils.jsm");
+    var clipboardFile = FileUtils.getFile("TmpD", ["clipboardcache"]);
 
     // Sanitize environment
-    gClipboardHelper.copyString(SHORT_STRING_NO_CACHE);
-
-    var initialFdCount = getOpenFileDescriptorCount();
-    var expectedFdCount = initialFdCount + 1;
+    if (clipboardFile.exists()) {
+        clipboardFile.remove(false);
+    }
+    ok(!clipboardFile.exists(), "failed to presanitize the environment");
 
     // Overflow a nsTransferable region by using the clipboard helper
+    const gClipboardHelper = Components.classes["@mozilla.org/widget/clipboardhelper;1"].getService(Components.interfaces.nsIClipboardHelper);
     gClipboardHelper.copyString(Ipsum);
 
     // Disabled private browsing mode should cache large selections to disk
-    is(getOpenFileDescriptorCount(), expectedFdCount, "correctly saved memory by caching to disk");
+    ok(clipboardFile.exists(), "correctly saved memory by caching to disk");
 
     // Sanitize environment again
-    gClipboardHelper.copyString(SHORT_STRING_NO_CACHE);
-    is(getOpenFileDescriptorCount(), initialFdCount, "failed to evict the file descriptor of the clipboard cache");
+    if (clipboardFile.exists()) {
+        clipboardFile.remove(false);
+    }
+    ok(!clipboardFile.exists(), "failed to postsanitize the environment");
 
     // Repeat procedure of plain text selection with private browsing enabled
     Components.utils.import("resource://gre/modules/PrivateBrowsingUtils.jsm");
     var Winpriv = window.open("about:blank", "_blank", "chrome, width=500, height=200, private");
     ok(Winpriv, "failed to open private window");
     ok(PrivateBrowsingUtils.isContentWindowPrivate(Winpriv), "correctly used a private window context");
 
     // Select plaintext in private channel
@@ -68,17 +61,17 @@ https://bugzilla.mozilla.org/show_bug.cg
     var Suppstr = nsSupportsString();
     Suppstr.data = Ipsum;
     Transfer.init(Loadctx);
     Transfer.addDataFlavor("text/plain");
     Transfer.setTransferData("text/plain", Suppstr, Ipsum.length);
     Services.clipboard.setData(Transfer, null, Services.clipboard.kGlobalClipboard);
 
     // Enabled private browsing mode should not cache any selection to disk
-    is(getOpenFileDescriptorCount(), initialFdCount, "did not violate private browsing mode");
+    ok(!clipboardFile.exists(), "did not violate private browsing mode");
   }
   ]]>
   </script>
 
   <!-- test results are displayed in the html:body -->
   <body xmlns="http://www.w3.org/1999/xhtml">
   <a href="https://bugzilla.mozilla.org/show_bug.cgi?id=1123480"
      target="_blank">Mozilla Bug 1123480</a>