From ce3e00458bdca20d55a50065439ecd800860501f Mon Sep 17 00:00:00 2001 From: Sebastian Hengst Date: Fri, 1 Sep 2017 15:54:56 +0200 Subject: [PATCH] Backed out changeset 44ff3bc7bee5 (bug 1063635) for failing xpcshell's toolkit/components/osfile/tests/xpcshell/test_read_write.js on Linux x64 debug. r=backout --- dom/webidl/NativeOSFileInternals.webidl | 37 -- .../osfile/NativeOSFileInternals.cpp | 348 ------------------ .../osfile/nsINativeOSFileInternals.idl | 27 -- 3 files changed, 412 deletions(-) diff --git a/dom/webidl/NativeOSFileInternals.webidl b/dom/webidl/NativeOSFileInternals.webidl index 934b6ae11805..36cc1a3a18c9 100644 --- a/dom/webidl/NativeOSFileInternals.webidl +++ b/dom/webidl/NativeOSFileInternals.webidl @@ -19,40 +19,3 @@ dictionary NativeOSFileReadOptions */ unsigned long long? bytes; }; - -/** - * Options for nsINativeOSFileInternals::WriteAtomic - */ -dictionary NativeOSFileWriteAtomicOptions -{ - /** - * If specified, specify the number of bytes to write. - * NOTE: This takes (and should take) a uint64 here but the actual - * value is limited to int32. This needs to be fixed, see Bug 1063635. - */ - unsigned long long? bytes; - - /** - * If specified, write all data to a temporary file in the - * |tmpPath|. Else, write to the given path directly. - */ - DOMString? tmpPath = null; - - /** - * If specified and true, a failure will occur if the file - * already exists in the given path. - */ - boolean noOverwrite = false; - - /** - * If specified and true, this will sync any buffered data - * for the file to disk. This might be slower, but safer. - */ - boolean flush = false; - - /** - * If specified, this will backup the destination file as - * specified. - */ - DOMString? backupTo = null; -}; diff --git a/toolkit/components/osfile/NativeOSFileInternals.cpp b/toolkit/components/osfile/NativeOSFileInternals.cpp index 2c47ba36002b..42c9e56c3e5d 100644 --- a/toolkit/components/osfile/NativeOSFileInternals.cpp +++ b/toolkit/components/osfile/NativeOSFileInternals.cpp @@ -26,7 +26,6 @@ #include "mozilla/Scoped.h" #include "mozilla/HoldDropJSObjects.h" #include "mozilla/TimeStamp.h" -#include "mozilla/UniquePtr.h" #include "prio.h" #include "prerror.h" @@ -34,7 +33,6 @@ #include "jsapi.h" #include "jsfriendapi.h" -#include "js/Conversions.h" #include "js/Utility.h" #include "xpcpublic.h" @@ -132,13 +130,11 @@ private: // errors, we need to map a few high-level errors to OS-level // constants. #if defined(XP_UNIX) -#define OS_ERROR_FILE_EXISTS EEXIST #define OS_ERROR_NOMEM ENOMEM #define OS_ERROR_INVAL EINVAL #define OS_ERROR_TOO_LARGE EFBIG #define OS_ERROR_RACE EIO #elif defined(XP_WIN) -#define OS_ERROR_FILE_EXISTS ERROR_ALREADY_EXISTS #define OS_ERROR_NOMEM ERROR_NOT_ENOUGH_MEMORY #define OS_ERROR_INVAL ERROR_BAD_ARGUMENTS #define OS_ERROR_TOO_LARGE ERROR_FILE_TOO_LARGE @@ -385,46 +381,6 @@ TypedArrayResult::GetCacheableResult(JSContext* cx, JS::MutableHandle return NS_OK; } -/** - * Return a result as an int32_t. - * - * In this implementation, attribute |result| is an int32_t. - */ -class Int32Result final: public AbstractResult -{ -public: - explicit Int32Result(TimeStamp aStartDate) - : AbstractResult(aStartDate) - , mContents(0) - { - } - - /** - * Initialize the object once the contents of the result are available. - * - * @param aContents The contents to pass to JS. This is an int32_t. - */ - void Init(TimeStamp aDispatchDate, - TimeDuration aExecutionDuration, - int32_t aContents) { - AbstractResult::Init(aDispatchDate, aExecutionDuration); - mContents = aContents; - } - -protected: - nsresult GetCacheableResult(JSContext* cx, JS::MutableHandleValue aResult) override; -private: - int32_t mContents; -}; - -nsresult -Int32Result::GetCacheableResult(JSContext* cx, JS::MutableHandleValue aResult) -{ - MOZ_ASSERT(NS_IsMainThread()); - aResult.set(JS::NumberValue(mContents)); - return NS_OK; -} - //////// Callback events /** @@ -905,222 +861,6 @@ protected: RefPtr mResult; }; -/** - * An event implenting writing atomically to a file. - */ -class DoWriteAtomicEvent: public AbstractDoEvent { -public: - /** - * @param aPath The path of the file. - */ - DoWriteAtomicEvent(const nsAString& aPath, - UniquePtr aBuffer, - const uint64_t aBytes, - const nsAString& aTmpPath, - const nsAString& aBackupTo, - const bool aFlush, - const bool aNoOverwrite, - nsMainThreadPtrHandle& aOnSuccess, - nsMainThreadPtrHandle& aOnError) - : AbstractDoEvent(aOnSuccess, aOnError) - , mPath(aPath) - , mBuffer(Move(aBuffer)) - , mBytes(aBytes) - , mTmpPath(aTmpPath) - , mBackupTo(aBackupTo) - , mFlush(aFlush) - , mNoOverwrite(aNoOverwrite) - , mResult(new Int32Result(TimeStamp::Now())) - { - MOZ_ASSERT(NS_IsMainThread()); - } - - ~DoWriteAtomicEvent() override { - // If Run() has bailed out, we may need to cleanup - // mResult, which is main-thread only data - if (!mResult) { - return; - } - NS_ReleaseOnMainThreadSystemGroup("DoWriteAtomicEvent::mResult", - mResult.forget()); - } - - NS_IMETHOD Run() override { - MOZ_ASSERT(!NS_IsMainThread()); - TimeStamp dispatchDate = TimeStamp::Now(); - int32_t bytesWritten; - - nsresult rv = WriteAtomic(&bytesWritten); - if (NS_FAILED(rv)) { - return NS_OK; - } - - AfterWriteAtomic(dispatchDate, bytesWritten); - return NS_OK; - } - -private: - /** - * Write atomically to a file. - * Must be called off the main thread. - * @param aBytesWritten will contain the total bytes written. - * This does not support compression in this implementation. - */ - nsresult WriteAtomic(int32_t* aBytesWritten) - { - MOZ_ASSERT(!NS_IsMainThread()); - - ScopedPRFileDesc file; - NS_ConvertUTF16toUTF8 path(mPath); - NS_ConvertUTF16toUTF8 tmpPath(mTmpPath); - NS_ConvertUTF16toUTF8 backupTo(mBackupTo); - bool fileExists = false; - - if (!mTmpPath.IsVoid() || !mBackupTo.IsVoid() || mNoOverwrite) { - // fileExists needs to be computed in the case of tmpPath, since - // the rename behaves differently depending on whether the - // file already exists. It's also computed for backupTo since the - // backup can be skipped if the file does not exist in the first place. - fileExists = PR_Access(path.get(), PR_ACCESS_EXISTS) == PR_SUCCESS; - } - - // Check noOverwrite. - if (mNoOverwrite && fileExists) { - Fail(NS_LITERAL_CSTRING("noOverwrite"), nullptr, OS_ERROR_FILE_EXISTS); - return NS_ERROR_FAILURE; - } - - // Backup the original file if it exists. - if (!mBackupTo.IsVoid() && fileExists) { - if (PR_Access(backupTo.get(), PR_ACCESS_EXISTS) == PR_SUCCESS) { - // The file specified by mBackupTo exists, so we need to delete it first. - if (PR_Delete(backupTo.get()) == PR_FAILURE) { - Fail(NS_LITERAL_CSTRING("delete"), nullptr, PR_GetOSError()); - return NS_ERROR_FAILURE; - } - } - if (PR_Rename(path.get(), backupTo.get()) == PR_FAILURE) { - Fail(NS_LITERAL_CSTRING("rename"), nullptr, PR_GetOSError()); - return NS_ERROR_FAILURE; - } - } - -#if defined(XP_WIN) - // On Windows, we can't use PR_OpenFile because it doesn't - // handle UTF-16 encoding, which is pretty bad. In addition, - // PR_OpenFile opens files without sharing, which is not the - // general semantics of OS.File. - HANDLE handle; - // if we're dealing with a tmpFile, we need to write there. - if (!mTmpPath.IsVoid()) { - handle = - ::CreateFileW(mTmpPath.get(), - GENERIC_WRITE, - FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, - /*Security attributes*/nullptr, - // CREATE_ALWAYS is used since since we need to create the temporary file, - // which we don't care about overwriting. - CREATE_ALWAYS, - FILE_ATTRIBUTE_NORMAL | FILE_FLAG_WRITE_THROUGH, - /*Template file*/ nullptr); - } else { - handle = - ::CreateFileW(mPath.get(), - GENERIC_WRITE, - FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, - /*Security attributes*/nullptr, - // CREATE_ALWAYS is used since since have already checked the noOverwrite - // condition, and thus can overwrite safely. - CREATE_ALWAYS, - FILE_ATTRIBUTE_NORMAL | FILE_FLAG_WRITE_THROUGH, - /*Template file*/ nullptr); - } - - if (handle == INVALID_HANDLE_VALUE) { - Fail(NS_LITERAL_CSTRING("open"), nullptr, ::GetLastError()); - return NS_ERROR_FAILURE; - } - - file = PR_ImportFile((PROsfd)handle); - if (!file) { - // |file| is closed by PR_ImportFile - Fail(NS_LITERAL_CSTRING("ImportFile"), nullptr, PR_GetOSError()); - return NS_ERROR_FAILURE; - } - -#else - // if we're dealing with a tmpFile, we need to write there. - if (!mTmpPath.IsVoid()) { - file = PR_OpenFile(tmpPath.get(), - PR_WRONLY | PR_CREATE_FILE, - PR_IRUSR | PR_IWUSR); - } else { - file = PR_OpenFile(path.get(), - PR_WRONLY | PR_CREATE_FILE, - PR_IRUSR | PR_IWUSR); - } - - if (!file) { - Fail(NS_LITERAL_CSTRING("open"), nullptr, PR_GetOSError()); - return NS_ERROR_FAILURE; - } -#endif // defined(XP_WIN) - - int32_t bytesWrittenSuccess = PR_Write(file, (void* )(mBuffer.get()), mBytes); - - if (bytesWrittenSuccess == -1) { - Fail(NS_LITERAL_CSTRING("write"), nullptr, PR_GetOSError()); - return NS_ERROR_FAILURE; - } - - // Apply any tmpPath renames. - if (!mTmpPath.IsVoid()) { - if (mBackupTo.IsVoid() && fileExists) { - // We need to delete the old file first, if it exists and we haven't already - // renamed it as a part of backing it up. - if (PR_Delete(path.get()) == PR_FAILURE) { - Fail(NS_LITERAL_CSTRING("delete"), nullptr, PR_GetOSError()); - return NS_ERROR_FAILURE; - } - } - - if(PR_Rename(tmpPath.get(), path.get()) == PR_FAILURE) { - Fail(NS_LITERAL_CSTRING("rename"), nullptr, PR_GetOSError()); - return NS_ERROR_FAILURE; - } - } - - if (mFlush) { - if (PR_Sync(file) == PR_FAILURE) { - Fail(NS_LITERAL_CSTRING("sync"), nullptr, PR_GetOSError()); - return NS_ERROR_FAILURE; - } - } - - *aBytesWritten = bytesWrittenSuccess; - return NS_OK; - } - -protected: - nsresult AfterWriteAtomic(TimeStamp aDispatchDate, int32_t aBytesWritten) { - MOZ_ASSERT(!NS_IsMainThread()); - mResult->Init(aDispatchDate, TimeStamp::Now() - aDispatchDate, aBytesWritten); - Succeed(mResult.forget()); - return NS_OK; - } - - const nsString mPath; - const UniquePtr mBuffer; - const int32_t mBytes; - const nsString mTmpPath; - const nsString mBackupTo; - const bool mFlush; - const bool mNoOverwrite; - -private: - RefPtr mResult; -}; - } // namespace // The OS.File service @@ -1183,92 +923,4 @@ NativeOSFileInternalsService::Read(const nsAString& aPath, return target->Dispatch(event, NS_DISPATCH_NORMAL); } -// Note: This method steals the contents of `aBuffer`. -NS_IMETHODIMP -NativeOSFileInternalsService::WriteAtomic(const nsAString& aPath, - JS::HandleValue aBuffer, - JS::HandleValue aOptions, - nsINativeOSFileSuccessCallback *aOnSuccess, - nsINativeOSFileErrorCallback *aOnError, - JSContext* cx) -{ - MOZ_ASSERT(NS_IsMainThread()); - // Extract typed-array/string into buffer. We also need to store the length - // of the buffer as that may be required if not provided in `aOptions`. - UniquePtr buffer; - int32_t bytes; - - // The incoming buffer must be an Object. - if (!aBuffer.isObject()) { - return NS_ERROR_INVALID_ARG; - } - - JS::RootedObject bufferObject(cx, nullptr); - if (!JS_ValueToObject(cx, aBuffer, &bufferObject)) { - return NS_ERROR_FAILURE; - } - if (!JS_IsArrayBufferObject(bufferObject.get())) { - return NS_ERROR_INVALID_ARG; - } - - bytes = JS_GetArrayBufferByteLength(bufferObject.get()); - buffer.reset(static_cast( - JS_StealArrayBufferContents(cx, bufferObject))); - - if (!buffer) { - return NS_ERROR_FAILURE; - } - - // Extract options. - dom::NativeOSFileWriteAtomicOptions dict; - - if (aOptions.isObject()) { - if (!dict.Init(cx, aOptions)) { - return NS_ERROR_INVALID_ARG; - } - } else { - // If an options object is not provided, initializing with a `null` - // value, which will give a set of defaults defined in the WebIDL binding. - if (!dict.Init(cx, JS::NullHandleValue)) { - return NS_ERROR_FAILURE; - } - } - - if (dict.mBytes.WasPassed() && !dict.mBytes.Value().IsNull()) { - // We need to check size and cast because NSPR and WebIDL have different types. - if (dict.mBytes.Value().Value() > PR_INT32_MAX) { - return NS_ERROR_INVALID_ARG; - } - bytes = (int32_t) (dict.mBytes.Value().Value()); - } - - // Prepare the off main thread event and dispatch it - nsCOMPtr onSuccess(aOnSuccess); - nsMainThreadPtrHandle onSuccessHandle( - new nsMainThreadPtrHolder( - "nsINativeOSFileSuccessCallback", onSuccess)); - nsCOMPtr onError(aOnError); - nsMainThreadPtrHandle onErrorHandle( - new nsMainThreadPtrHolder( - "nsINativeOSFileErrorCallback", onError)); - - RefPtr event = new DoWriteAtomicEvent(aPath, - Move(buffer), - bytes, - dict.mTmpPath, - dict.mBackupTo, - dict.mFlush, - dict.mNoOverwrite, - onSuccessHandle, - onErrorHandle); - nsresult rv; - nsCOMPtr target = do_GetService(NS_STREAMTRANSPORTSERVICE_CONTRACTID, &rv); - - if (NS_FAILED(rv)) { - return rv; - } - - return target->Dispatch(event, NS_DISPATCH_NORMAL); -} - } // namespace mozilla diff --git a/toolkit/components/osfile/nsINativeOSFileInternals.idl b/toolkit/components/osfile/nsINativeOSFileInternals.idl index 721540024fa2..c1bf8be147f1 100644 --- a/toolkit/components/osfile/nsINativeOSFileInternals.idl +++ b/toolkit/components/osfile/nsINativeOSFileInternals.idl @@ -82,33 +82,6 @@ interface nsINativeOSFileInternalsService: nsISupports void read(in AString path, in jsval options, in nsINativeOSFileSuccessCallback onSuccess, in nsINativeOSFileErrorCallback onError); - - /** - * Implementation of OS.File.writeAtomic - * - * @param path the absolute path of the file to write to. - * @param buffer the data as an array buffer to be written to the file. - * @param options An object that may contain the following fields - * - {number} bytes If provided, the number of bytes written is equal to this. - * The default value is the size of the |buffer|. - * - {string} tmpPath If provided and not null, first write to this path, and - * move to |path| after writing. - * - {string} backupPath if provided, backup file at |path| to this path - * before overwriting it. - * - {bool} flush if provided and true, flush the contents of the buffer after - * writing. This is slower, but safer. - * - {bool} noOverwrite if provided and true, do not write if a file already - * exists at |path|. - * @param onSuccess The success callback. - * @param onError The error callback. - */ - [implicit_jscontext] - void writeAtomic(in AString path, - in jsval buffer, - in jsval options, - in nsINativeOSFileSuccessCallback onSuccess, - in nsINativeOSFileErrorCallback onError); - };