From 085ce9ea7b2c2f9a3603c63760467e86cb31d7c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Bargull?= Date: Wed, 9 Jan 2019 09:04:15 -0800 Subject: [PATCH 01/12] Bug 1518837: Reuse input register in LGuardToClass. r=jandem --- js/src/jit/CodeGenerator.cpp | 8 +++++--- js/src/jit/Lowering.cpp | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp index cf1a5f12bffd..d98e58b1e68f 100644 --- a/js/src/jit/CodeGenerator.cpp +++ b/js/src/jit/CodeGenerator.cpp @@ -12678,14 +12678,16 @@ void CodeGenerator::visitHasClass(LHasClass* ins) { void CodeGenerator::visitGuardToClass(LGuardToClass* ins) { Register lhs = ToRegister(ins->lhs()); - Register output = ToRegister(ins->output()); Register temp = ToRegister(ins->temp()); + // branchTestObjClass may zero the object register on speculative paths + // (we should have a defineReuseInput allocation in this case). + Register spectreRegToZero = lhs; + Label notEqual; masm.branchTestObjClass(Assembler::NotEqual, lhs, ins->mir()->getClass(), - temp, output, ¬Equal); - masm.mov(lhs, output); + temp, spectreRegToZero, ¬Equal); // Can't return null-return here, so bail. bailoutFrom(¬Equal, ins->snapshot()); diff --git a/js/src/jit/Lowering.cpp b/js/src/jit/Lowering.cpp index 008aa3c1a9b2..ad3fd9a979fa 100644 --- a/js/src/jit/Lowering.cpp +++ b/js/src/jit/Lowering.cpp @@ -4208,9 +4208,9 @@ void LIRGenerator::visitGuardToClass(MGuardToClass* ins) { MOZ_ASSERT(ins->object()->type() == MIRType::Object); MOZ_ASSERT(ins->type() == MIRType::Object); LGuardToClass* lir = - new (alloc()) LGuardToClass(useRegister(ins->object()), temp()); + new (alloc()) LGuardToClass(useRegisterAtStart(ins->object()), temp()); assignSnapshot(lir, Bailout_TypeBarrierO); - define(lir, ins); + defineReuseInput(lir, ins, 0); } void LIRGenerator::visitObjectClassToString(MObjectClassToString* ins) { From 47a7b896625dbca6a150fea2c2637863a7f64626 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Bargull?= Date: Wed, 9 Jan 2019 01:52:31 -0800 Subject: [PATCH 02/12] Bug 1394386: Don't enforce tenure allocation for TypedArrays from inlined constructor ool-path. r=jandem --- js/src/jit/MacroAssembler.cpp | 3 +- js/src/vm/TypedArrayObject.cpp | 54 ++++++++++++++++++++-------------- 2 files changed, 34 insertions(+), 23 deletions(-) diff --git a/js/src/jit/MacroAssembler.cpp b/js/src/jit/MacroAssembler.cpp index 3aa63afa03ba..ced36bb4d671 100644 --- a/js/src/jit/MacroAssembler.cpp +++ b/js/src/jit/MacroAssembler.cpp @@ -1177,7 +1177,8 @@ static void AllocateObjectBufferWithInit(JSContext* cx, TypedArrayObject* obj, obj->setFixedSlot(TypedArrayObject::LENGTH_SLOT, Int32Value(count)); size_t nbytes = count * obj->bytesPerElement(); - MOZ_ASSERT((CheckedUint32(nbytes) + sizeof(Value)).isValid()); + MOZ_ASSERT((CheckedUint32(nbytes) + sizeof(Value)).isValid(), + "JS_ROUNDUP must not overflow"); nbytes = JS_ROUNDUP(nbytes, sizeof(Value)); void* buf = cx->nursery().allocateZeroedBuffer(obj, nbytes, diff --git a/js/src/vm/TypedArrayObject.cpp b/js/src/vm/TypedArrayObject.cpp index 14d1194f38e2..a0bf60a8896d 100644 --- a/js/src/vm/TypedArrayObject.cpp +++ b/js/src/vm/TypedArrayObject.cpp @@ -8,6 +8,7 @@ #include "vm/TypedArrayObject.h" #include "mozilla/Alignment.h" +#include "mozilla/CheckedInt.h" #include "mozilla/FloatingPoint.h" #include "mozilla/PodOperations.h" #include "mozilla/TextUtils.h" @@ -55,6 +56,7 @@ using namespace js; using JS::CanonicalizeNaN; using JS::ToInt32; using JS::ToUint32; +using mozilla::CheckedUint32; using mozilla::IsAsciiDigit; /* @@ -159,9 +161,15 @@ void TypedArrayObject::finalize(FreeOp* fop, JSObject* obj) { return 0; } - Nursery& nursery = obj->runtimeFromMainThread()->gc.nursery(); void* buf = oldObj->elements(); + // Discarded objects (which didn't have enough room for inner elements) don't + // have any data to move. + if (!buf) { + return 0; + } + + Nursery& nursery = obj->runtimeFromMainThread()->gc.nursery(); if (!nursery.isInside(buf)) { nursery.removeMallocedBuffer(buf); return 0; @@ -190,6 +198,9 @@ void TypedArrayObject::finalize(FreeOp* fop, JSObject* obj) { newObj->setInlineElements(); } else { MOZ_ASSERT(!oldObj->hasInlineElements()); + MOZ_ASSERT((CheckedUint32(nbytes) + sizeof(Value)).isValid(), + "JS_ROUNDUP must not overflow"); + AutoEnterOOMUnsafeRegion oomUnsafe; nbytes = JS_ROUNDUP(nbytes, sizeof(Value)); void* data = newObj->zone()->pod_malloc( @@ -482,15 +493,9 @@ class TypedArrayObjectTemplate : public TypedArrayObject { #endif } - static void initTypedArrayData(JSContext* cx, TypedArrayObject* tarray, - int32_t len, void* buf, - gc::AllocKind allocKind) { + static void initTypedArrayData(TypedArrayObject* tarray, int32_t len, + void* buf, gc::AllocKind allocKind) { if (buf) { -#ifdef DEBUG - Nursery& nursery = cx->nursery(); - MOZ_ASSERT_IF(!nursery.isInside(buf) && !tarray->hasInlineElements(), - tarray->isTenured()); -#endif tarray->initPrivate(buf); } else { size_t nbytes = len * BYTES_PER_ELEMENT; @@ -524,25 +529,30 @@ class TypedArrayObjectTemplate : public TypedArrayObject { RootedObjectGroup group(cx, templateObj->group()); MOZ_ASSERT(group->clasp() == instanceClass()); - NewObjectKind newKind = TenuredObject; - - UniquePtr buf; - if (!fitsInline) { - MOZ_ASSERT(len > 0); - buf.reset(cx->pod_calloc(nbytes, js::ArrayBufferContentsArena)); - if (!buf) { - return nullptr; - } - } - TypedArrayObject* obj = - NewObjectWithGroup(cx, group, allocKind, newKind); + NewObjectWithGroup(cx, group, allocKind); if (!obj) { return nullptr; } initTypedArraySlots(obj, len); - initTypedArrayData(cx, obj, len, buf.release(), allocKind); + + void* buf = nullptr; + if (!fitsInline) { + MOZ_ASSERT(len > 0); + MOZ_ASSERT((CheckedUint32(nbytes) + sizeof(Value)).isValid(), + "JS_ROUNDUP must not overflow"); + + nbytes = JS_ROUNDUP(nbytes, sizeof(Value)); + buf = cx->nursery().allocateZeroedBuffer(obj, nbytes, + js::ArrayBufferContentsArena); + if (!buf) { + ReportOutOfMemory(cx); + return nullptr; + } + } + + initTypedArrayData(obj, len, buf, allocKind); return obj; } From ebb609373d32a05b8c36eec5e15995c09f47c969 Mon Sep 17 00:00:00 2001 From: Jed Davis Date: Thu, 10 Jan 2019 13:55:31 -0700 Subject: [PATCH 03/12] Backed out changeset a0cf88b1fe5b (bug 1487287) --- ipc/glue/GeckoChildProcessHost.cpp | 148 ++++------------------------- ipc/glue/GeckoChildProcessHost.h | 10 +- 2 files changed, 23 insertions(+), 135 deletions(-) diff --git a/ipc/glue/GeckoChildProcessHost.cpp b/ipc/glue/GeckoChildProcessHost.cpp index 07a36d51235d..70236897576d 100644 --- a/ipc/glue/GeckoChildProcessHost.cpp +++ b/ipc/glue/GeckoChildProcessHost.cpp @@ -33,16 +33,12 @@ #include "nsDirectoryServiceDefs.h" #include "nsIFile.h" #include "nsPrintfCString.h" -#include "nsIObserverService.h" +#include "mozilla/ClearOnShutdown.h" #include "mozilla/ipc/BrowserProcessSubThread.h" #include "mozilla/ipc/EnvironmentMap.h" #include "mozilla/Omnijar.h" -#include "mozilla/RecordReplay.h" #include "mozilla/Scoped.h" -#include "mozilla/Services.h" -#include "mozilla/SharedThreadPool.h" -#include "mozilla/StaticMutex.h" #include "mozilla/Telemetry.h" #include "ProtocolUtils.h" #include @@ -71,7 +67,6 @@ #include "private/pprio.h" using mozilla::MonitorAutoLock; -using mozilla::StaticMutexAutoLock; using mozilla::ipc::GeckoChildProcessHost; namespace mozilla { @@ -470,130 +465,26 @@ void GeckoChildProcessHost::GetChildLogName(const char* origLogName, buffer.AppendInt(mChildCounter); } -namespace { -// Windows needs a single dedicated thread for process launching, -// because of thread-safety restrictions/assertions in the sandbox -// code. (This implementation isn't itself Windows-specific, so -// the ifdef can be changed to test on other platforms.) -#ifdef XP_WIN - -static mozilla::StaticMutex gIPCLaunchThreadMutex; -static mozilla::StaticRefPtr gIPCLaunchThread; - -class IPCLaunchThreadObserver final : public nsIObserver { - public: - NS_DECL_ISUPPORTS - NS_DECL_NSIOBSERVER - protected: - virtual ~IPCLaunchThreadObserver() = default; -}; - -NS_IMPL_ISUPPORTS(IPCLaunchThreadObserver, nsIObserver, nsISupports) - -NS_IMETHODIMP -IPCLaunchThreadObserver::Observe(nsISupports* aSubject, const char* aTopic, - const char16_t* aData) { - MOZ_RELEASE_ASSERT(strcmp(aTopic, "xpcom-shutdown-threads") == 0); - StaticMutexAutoLock lock(gIPCLaunchThreadMutex); - - nsresult rv = NS_OK; - if (gIPCLaunchThread) { - rv = gIPCLaunchThread->Shutdown(); - gIPCLaunchThread = nullptr; - } - mozilla::Unused << NS_WARN_IF(NS_FAILED(rv)); - return rv; -} - -static nsCOMPtr GetIPCLauncher() { - StaticMutexAutoLock lock(gIPCLaunchThreadMutex); - if (!gIPCLaunchThread) { - nsCOMPtr thread; - nsresult rv = NS_NewNamedThread(NS_LITERAL_CSTRING("IPC Launch"), - getter_AddRefs(thread)); - if (!NS_WARN_IF(NS_FAILED(rv))) { - NS_DispatchToMainThread( - NS_NewRunnableFunction("GeckoChildProcessHost::GetIPCLauncher", [] { - nsCOMPtr obsService = - mozilla::services::GetObserverService(); - nsCOMPtr obs = new IPCLaunchThreadObserver(); - obsService->AddObserver(obs, "xpcom-shutdown-threads", false); - })); - gIPCLaunchThread = thread.forget(); - } - } - - nsCOMPtr thread = gIPCLaunchThread.get(); - return thread; -} - -#else // XP_WIN - -// Non-Windows platforms can use an on-demand thread pool. - -static nsCOMPtr GetIPCLauncher() { - nsCOMPtr pool = - mozilla::SharedThreadPool::Get(NS_LITERAL_CSTRING("IPC Launch")); - return pool; -} - -#endif // XP_WIN -} // anonymous namespace - -void GeckoChildProcessHost::RunPerformAsyncLaunch( +bool GeckoChildProcessHost::RunPerformAsyncLaunch( std::vector aExtraOpts) { - auto fail = [this] { + InitializeChannel(); + + bool ok = PerformAsyncLaunch(aExtraOpts); + if (!ok) { + // WaitUntilConnected might be waiting for us to signal. + // If something failed let's set the error state and notify. MonitorAutoLock lock(mMonitor); mProcessState = PROCESS_ERROR; mHandlePromise->Reject(LaunchError{}, __func__); lock.Notify(); - }; - - // This (probably?) needs to happen on the I/O thread. - InitializeChannel(); - - // But the rest of this doesn't, and shouldn't block IPC messages: - auto launchWrapper = [this, fail, aExtraOpts = std::move(aExtraOpts)]() { - bool ok = PerformAsyncLaunch(aExtraOpts); - - if (!ok) { - // WaitUntilConnected might be waiting for us to signal. - // If something failed let's set the error state and notify. - fail(); - CHROMIUM_LOG(ERROR) << "Failed to launch " - << XRE_ChildProcessTypeToString(mProcessType) - << " subprocess"; - Telemetry::Accumulate( - Telemetry::SUBPROCESS_LAUNCH_FAILURE, - nsDependentCString(XRE_ChildProcessTypeToString(mProcessType))); - } - }; - - // The Web Replay middleman process launches the actual content - // processes, and doesn't initialize enough of XPCOM to use thread - // pools. - if (!mozilla::recordreplay::IsMiddleman()) { - auto launcher = GetIPCLauncher(); - MOZ_DIAGNOSTIC_ASSERT(launcher != nullptr); - // Creating a thread pool shouldn't normally fail, but in case it - // does, use the fallback we already have for the middleman case. - if (launcher != nullptr) { - nsresult rv = launcher->Dispatch( - NS_NewRunnableFunction( - "ipc::GeckoChildProcessHost::PerformAsyncLaunch", launchWrapper), - NS_DISPATCH_NORMAL); - if (NS_FAILED(rv)) { - fail(); - CHROMIUM_LOG(ERROR) << "Failed to dispatch launch task for " - << XRE_ChildProcessTypeToString(mProcessType) - << " process; launching during shutdown?"; - } - return; - } + CHROMIUM_LOG(ERROR) << "Failed to launch " + << XRE_ChildProcessTypeToString(mProcessType) + << " subprocess"; + Telemetry::Accumulate( + Telemetry::SUBPROCESS_LAUNCH_FAILURE, + nsDependentCString(XRE_ChildProcessTypeToString(mProcessType))); } - - // Fall back to launching on the I/O thread. - launchWrapper(); + return ok; } void @@ -1224,12 +1115,7 @@ bool GeckoChildProcessHost::PerformAsyncLaunch( base::GetProcId(process), crashAnnotationReadPipe.forget()); MonitorAutoLock lock(mMonitor); - // This runs on a launch thread, but the OnChannel{Connected,Error} callbacks - // run on the I/O thread, so it's possible that the state already advanced - // beyond PROCESS_CREATED. - if (mProcessState < PROCESS_CREATED) { - mProcessState = PROCESS_CREATED; - } + mProcessState = PROCESS_CREATED; mHandlePromise->Resolve(process, __func__); lock.Notify(); @@ -1254,6 +1140,7 @@ void GeckoChildProcessHost::OnChannelConnected(int32_t peer_pid) { MOZ_CRASH("can't open handle to child process"); } MonitorAutoLock lock(mMonitor); + MOZ_DIAGNOSTIC_ASSERT(mProcessState == PROCESS_CREATED); mProcessState = PROCESS_CONNECTED; lock.Notify(); } @@ -1271,6 +1158,7 @@ void GeckoChildProcessHost::OnChannelError() { // in the FIXME comment below. MonitorAutoLock lock(mMonitor); if (mProcessState < PROCESS_CONNECTED) { + MOZ_DIAGNOSTIC_ASSERT(mProcessState == PROCESS_CREATED); mProcessState = PROCESS_ERROR; lock.Notify(); } diff --git a/ipc/glue/GeckoChildProcessHost.h b/ipc/glue/GeckoChildProcessHost.h index 6cdd090050a3..de06ca5a7e26 100644 --- a/ipc/glue/GeckoChildProcessHost.h +++ b/ipc/glue/GeckoChildProcessHost.h @@ -181,13 +181,13 @@ class GeckoChildProcessHost : public ChildProcessHost { private: DISALLOW_EVIL_CONSTRUCTORS(GeckoChildProcessHost); - // Does the actual work for AsyncLaunch; run in a thread pool - // (or, on Windows, a dedicated thread). + // Does the actual work for AsyncLaunch, on the IO thread. + // (TODO, bug 1487287: move this to its own thread(s).) bool PerformAsyncLaunch(StringVector aExtraOpts); - // Called on the I/O thread; creates channel, dispatches - // PerformAsyncLaunch, and consolidates error handling. - void RunPerformAsyncLaunch(StringVector aExtraOpts); + // Also called on the I/O thread; creates channel, launches, and + // consolidates error handling. + bool RunPerformAsyncLaunch(StringVector aExtraOpts); enum class BinaryPathType { Self, PluginContainer }; From fcefa80c7c9817f62823ffaf0b677e5aeb37f4b4 Mon Sep 17 00:00:00 2001 From: Nathan Froyd Date: Thu, 10 Jan 2019 16:13:37 -0500 Subject: [PATCH 04/12] Bug 1518922 - part 1 - remove dead code for extra crashreporting directory; r=Alex_Gaynor The command-line parameter used by nsEmbedFunctions.cpp is turned into an nsIFile, and then said nsIFile is never used. Its last use was deleted in bug 1407693, where we reworked how extra annotations were done. --- ipc/glue/GeckoChildProcessHost.cpp | 30 ------------------------------ toolkit/xre/nsEmbedFunctions.cpp | 16 ---------------- 2 files changed, 46 deletions(-) diff --git a/ipc/glue/GeckoChildProcessHost.cpp b/ipc/glue/GeckoChildProcessHost.cpp index 70236897576d..d23ac77e6c78 100644 --- a/ipc/glue/GeckoChildProcessHost.cpp +++ b/ipc/glue/GeckoChildProcessHost.cpp @@ -730,21 +730,6 @@ bool GeckoChildProcessHost::PerformAsyncLaunch( // Add the application directory path (-appdir path) AddAppDirToCommandLine(childArgv); - // Tmp dir that the GPU or RDD process should use for crash reports. - // This arg is always populated (but possibly with an empty value) for - // a GPU or RDD child process. - if (mProcessType == GeckoProcessType_GPU || - mProcessType == GeckoProcessType_RDD || - mProcessType == GeckoProcessType_VR) { - nsCOMPtr file; - CrashReporter::GetChildProcessTmpDir(getter_AddRefs(file)); - nsAutoCString path; - if (file) { - file->GetNativePath(path); - } - childArgv.push_back(path.get()); - } - childArgv.push_back(pidstring); if (!CrashReporter::IsDummy()) { @@ -1015,21 +1000,6 @@ bool GeckoChildProcessHost::PerformAsyncLaunch( // Win app model id cmdLine.AppendLooseValue(mGroupId.get()); - // Tmp dir that the GPU or RDD process should use for crash reports. - // This arg is always populated (but possibly with an empty value) for - // a GPU or RDD child process. - if (mProcessType == GeckoProcessType_GPU || - mProcessType == GeckoProcessType_RDD) { - nsCOMPtr file; - CrashReporter::GetChildProcessTmpDir(getter_AddRefs(file)); - nsString path; - if (file) { - MOZ_ALWAYS_SUCCEEDS(file->GetPath(path)); - } - std::wstring wpath(path.get()); - cmdLine.AppendLooseValue(wpath); - } - // Process id cmdLine.AppendLooseValue(UTF8ToWide(pidstring)); diff --git a/toolkit/xre/nsEmbedFunctions.cpp b/toolkit/xre/nsEmbedFunctions.cpp index d9cf2eff9d3b..9912a917dd67 100644 --- a/toolkit/xre/nsEmbedFunctions.cpp +++ b/toolkit/xre/nsEmbedFunctions.cpp @@ -594,22 +594,6 @@ nsresult XRE_InitChildProcess(int aArgc, char* aArgv[], base::ProcessId parentPID = strtol(parentPIDString, &end, 10); MOZ_ASSERT(!*end, "invalid parent PID"); - nsCOMPtr crashReportTmpDir; - if (XRE_GetProcessType() == GeckoProcessType_GPU || - XRE_GetProcessType() == GeckoProcessType_RDD) { - aArgc--; - if (strlen(aArgv[aArgc])) { // if it's empty, ignore it - nsresult rv = - XRE_GetFileFromPath(aArgv[aArgc], getter_AddRefs(crashReportTmpDir)); - if (NS_FAILED(rv)) { - // If we don't have a valid tmp dir we can probably still run ok, but - // crash report .extra files might not get picked up by the parent - // process. Debug-assert because this shouldn't happen in practice. - MOZ_ASSERT(false, "GPU process started without valid tmp dir!"); - } - } - } - // While replaying, use the parent PID that existed while recording. parentPID = recordreplay::RecordReplayValue(parentPID); From d983955203b8524b501794a3cdda9806331cf013 Mon Sep 17 00:00:00 2001 From: Nathan Froyd Date: Thu, 10 Jan 2019 16:13:37 -0500 Subject: [PATCH 05/12] Bug 1518922 - part 2 - remove GetChildProcessTmpDir; r=gsvelto After part 1, this function and the code associated with it is no longer used. --- .../crashreporter/nsDummyExceptionHandler.cpp | 2 -- toolkit/crashreporter/nsExceptionHandler.cpp | 34 ------------------- toolkit/crashreporter/nsExceptionHandler.h | 4 --- 3 files changed, 40 deletions(-) diff --git a/toolkit/crashreporter/nsDummyExceptionHandler.cpp b/toolkit/crashreporter/nsDummyExceptionHandler.cpp index c85f852cf9f8..d524e3c09a04 100644 --- a/toolkit/crashreporter/nsDummyExceptionHandler.cpp +++ b/toolkit/crashreporter/nsDummyExceptionHandler.cpp @@ -167,8 +167,6 @@ bool AppendExtraData(nsIFile* extraFile, const AnnotationTable& data) { void OOPInit() {} -void GetChildProcessTmpDir(nsIFile** aOutTmpDir) {} - #if defined(XP_WIN) || defined(XP_MACOSX) const char* GetChildNotificationPipe() { return nullptr; } #endif diff --git a/toolkit/crashreporter/nsExceptionHandler.cpp b/toolkit/crashreporter/nsExceptionHandler.cpp index c1f0e8ddcc8d..87beafeed65c 100644 --- a/toolkit/crashreporter/nsExceptionHandler.cpp +++ b/toolkit/crashreporter/nsExceptionHandler.cpp @@ -249,11 +249,6 @@ static std::map processToCrashFd; static std::terminate_handler oldTerminateHandler = nullptr; -#if (defined(XP_MACOSX) || defined(XP_WIN)) -// This field is valid in both chrome and content processes. -static xpstring* childProcessTmpDir = nullptr; -#endif - #if defined(XP_WIN) || defined(XP_MACOSX) // If crash reporting is disabled, we hand out this "null" pipe to the // child process and don't attempt to connect to a parent server. @@ -3067,26 +3062,6 @@ void OOPInit() { "attempt to initialize OOP crash reporter before in-process " "crashreporter!"); -#if (defined(XP_WIN) || defined(XP_MACOSX)) - nsCOMPtr tmpDir; -#if defined(MOZ_CONTENT_SANDBOX) - nsresult rv = NS_GetSpecialDirectory(NS_APP_CONTENT_PROCESS_TEMP_DIR, - getter_AddRefs(tmpDir)); - if (NS_FAILED(rv) && PR_GetEnv("XPCSHELL_TEST_PROFILE_DIR")) { - // Temporary hack for xpcshell, will be fixed in bug 1257098 - rv = NS_GetSpecialDirectory(NS_OS_TEMP_DIR, getter_AddRefs(tmpDir)); - } - if (NS_SUCCEEDED(rv)) { - childProcessTmpDir = CreatePathFromFile(tmpDir); - } -#else - if (NS_SUCCEEDED( - NS_GetSpecialDirectory(NS_OS_TEMP_DIR, getter_AddRefs(tmpDir)))) { - childProcessTmpDir = CreatePathFromFile(tmpDir); - } -#endif // defined(MOZ_CONTENT_SANDBOX) -#endif // (defined(XP_WIN) || defined(XP_MACOSX)) - #if defined(XP_WIN) childCrashNotifyPipe = mozilla::Smprintf("\\\\.\\pipe\\gecko-crash-server-pipe.%i", @@ -3175,15 +3150,6 @@ static void OOPDeinit() { #endif } -void GetChildProcessTmpDir(nsIFile** aOutTmpDir) { - MOZ_ASSERT(XRE_IsParentProcess()); -#if (defined(XP_MACOSX) || defined(XP_WIN)) - if (childProcessTmpDir) { - CreateFileFromPath(*childProcessTmpDir, aOutTmpDir); - } -#endif -} - #if defined(XP_WIN) || defined(XP_MACOSX) // Parent-side API for children const char* GetChildNotificationPipe() { diff --git a/toolkit/crashreporter/nsExceptionHandler.h b/toolkit/crashreporter/nsExceptionHandler.h index 0e7a15c0fa77..b60410777ae0 100644 --- a/toolkit/crashreporter/nsExceptionHandler.h +++ b/toolkit/crashreporter/nsExceptionHandler.h @@ -247,10 +247,6 @@ bool CreateAdditionalChildMinidump(ProcessHandle childPid, nsIFile* parentMinidump, const nsACString& name); -// Parent-side API, returns the tmp dir for child processes to use, accounting -// for sandbox considerations. -void GetChildProcessTmpDir(nsIFile** aOutTmpDir); - #if defined(XP_WIN32) || defined(XP_MACOSX) // Parent-side API for children const char* GetChildNotificationPipe(); From 56c0daa179e525debac57602aa38477e312d2aa2 Mon Sep 17 00:00:00 2001 From: Sean Stangl Date: Thu, 10 Jan 2019 23:16:27 +0200 Subject: [PATCH 06/12] Bug 1518957 - Implement ARM64 truncation and remove unused emitRoundDouble(). r=nbp --HG-- extra : amend_source : 6d7336b98d91c2f252431d38823731b394e30dd7 --- js/src/jit/arm/CodeGenerator-arm.cpp | 12 ---- js/src/jit/arm/CodeGenerator-arm.h | 2 - js/src/jit/arm64/CodeGenerator-arm64.cpp | 85 +++++++++++++++++++++--- js/src/jit/arm64/CodeGenerator-arm64.h | 2 - 4 files changed, 77 insertions(+), 24 deletions(-) diff --git a/js/src/jit/arm/CodeGenerator-arm.cpp b/js/src/jit/arm/CodeGenerator-arm.cpp index 9f163516c45c..77be4f4c1a68 100644 --- a/js/src/jit/arm/CodeGenerator-arm.cpp +++ b/js/src/jit/arm/CodeGenerator-arm.cpp @@ -1285,18 +1285,6 @@ void CodeGenerator::visitTruncF(LTruncF* lir) { bailoutFrom(&bail, lir->snapshot()); } -void CodeGeneratorARM::emitRoundDouble(FloatRegister src, Register dest, - Label* fail) { - ScratchDoubleScope scratch(masm); - ScratchRegisterScope scratchReg(masm); - - masm.ma_vcvt_F64_I32(src, scratch); - masm.ma_vxfer(scratch, dest); - masm.ma_cmp(dest, Imm32(0x7fffffff), scratchReg); - masm.ma_cmp(dest, Imm32(0x80000000), scratchReg, Assembler::NotEqual); - masm.ma_b(fail, Assembler::Equal); -} - void CodeGenerator::visitTruncateDToInt32(LTruncateDToInt32* ins) { emitTruncateDouble(ToFloatRegister(ins->input()), ToRegister(ins->output()), ins->mir()); diff --git a/js/src/jit/arm/CodeGenerator-arm.h b/js/src/jit/arm/CodeGenerator-arm.h index 8ff1aa3d440e..34359a51c86b 100644 --- a/js/src/jit/arm/CodeGenerator-arm.h +++ b/js/src/jit/arm/CodeGenerator-arm.h @@ -68,8 +68,6 @@ class CodeGeneratorARM : public CodeGeneratorShared { bool generateOutOfLineCode(); - void emitRoundDouble(FloatRegister src, Register dest, Label* fail); - // Emits a branch that directs control flow to the true block if |cond| is // true, and the false block if |cond| is false. void emitBranch(Assembler::Condition cond, MBasicBlock* ifTrue, diff --git a/js/src/jit/arm64/CodeGenerator-arm64.cpp b/js/src/jit/arm64/CodeGenerator-arm64.cpp index d07e7169e25f..006dfb9b4990 100644 --- a/js/src/jit/arm64/CodeGenerator-arm64.cpp +++ b/js/src/jit/arm64/CodeGenerator-arm64.cpp @@ -1056,7 +1056,7 @@ void CodeGenerator::visitRoundF(LRoundF* lir) { masm.Fcmp(input32, 0.0f); bailoutIf(Assembler::Overflow, lir->snapshot()); - // Move all 64 bits of the input into a scratch register to check for -0. + // Move all 32 bits of the input into a scratch register to check for -0. vixl::UseScratchRegisterScope temps(&masm.asVIXL()); const ARMRegister scratchGPR32 = temps.AcquireW(); masm.Fmov(scratchGPR32, input32); @@ -1098,9 +1098,83 @@ void CodeGenerator::visitRoundF(LRoundF* lir) { masm.bind(&done); } -void CodeGenerator::visitTrunc(LTrunc* lir) { MOZ_CRASH("visitTrunc"); } +void CodeGenerator::visitTrunc(LTrunc* lir) { + const FloatRegister input = ToFloatRegister(lir->input()); + const ARMFPRegister input64(input, 64); + const Register output = ToRegister(lir->output()); + const ARMRegister output32(output, 32); -void CodeGenerator::visitTruncF(LTruncF* lir) { MOZ_CRASH("visitTruncF"); } + Label done, zeroCase; + + // Convert scalar to signed 32-bit fixed-point, rounding toward zero. + // In the case of overflow, the output is saturated. + // In the case of NaN and -0, the output is zero. + masm.Fcvtzs(output32, input64); + + // If the output was zero, worry about special cases. + masm.branch32(Assembler::Equal, output, Imm32(0), &zeroCase); + + // Bail on overflow cases. + bailoutCmp32(Assembler::Equal, output, Imm32(INT_MAX), lir->snapshot()); + bailoutCmp32(Assembler::Equal, output, Imm32(INT_MIN), lir->snapshot()); + + // If the output was non-zero and wasn't saturated, just return it. + masm.jump(&done); + + // Handle the case of a zero output: + // 1. The input may have been NaN, requiring a bail. + // 2. The input may have been in (-1,-0], requiring a bail. + { + masm.bind(&zeroCase); + + // If input is a negative number that truncated to zero, the real + // output should be the non-integer -0. + // The use of "lt" instead of "lo" also catches unordered NaN input. + masm.Fcmp(input64, 0.0); + bailoutIf(vixl::lt, lir->snapshot()); + } + + masm.bind(&done); +} + +void CodeGenerator::visitTruncF(LTruncF* lir) { + const FloatRegister input = ToFloatRegister(lir->input()); + const ARMFPRegister input32(input, 32); + const Register output = ToRegister(lir->output()); + const ARMRegister output32(output, 32); + + Label done, zeroCase; + + // Convert scalar to signed 32-bit fixed-point, rounding toward zero. + // In the case of overflow, the output is saturated. + // In the case of NaN and -0, the output is zero. + masm.Fcvtzs(output32, input32); + + // If the output was zero, worry about special cases. + masm.branch32(Assembler::Equal, output, Imm32(0), &zeroCase); + + // Bail on overflow cases. + bailoutCmp32(Assembler::Equal, output, Imm32(INT_MAX), lir->snapshot()); + bailoutCmp32(Assembler::Equal, output, Imm32(INT_MIN), lir->snapshot()); + + // If the output was non-zero and wasn't saturated, just return it. + masm.jump(&done); + + // Handle the case of a zero output: + // 1. The input may have been NaN, requiring a bail. + // 2. The input may have been in (-1,-0], requiring a bail. + { + masm.bind(&zeroCase); + + // If input is a negative number that truncated to zero, the real + // output should be the non-integer -0. + // The use of "lt" instead of "lo" also catches unordered NaN input. + masm.Fcmp(input32, 0.0f); + bailoutIf(vixl::lt, lir->snapshot()); + } + + masm.bind(&done); +} void CodeGenerator::visitClzI(LClzI* lir) { ARMRegister input = toWRegister(lir->input()); @@ -1114,11 +1188,6 @@ void CodeGenerator::visitCtzI(LCtzI* lir) { masm.ctz32(input, output, /* knownNotZero = */ false); } -void CodeGeneratorARM64::emitRoundDouble(FloatRegister src, Register dest, - Label* fail) { - MOZ_CRASH("CodeGeneratorARM64::emitRoundDouble"); -} - void CodeGenerator::visitTruncateDToInt32(LTruncateDToInt32* ins) { emitTruncateDouble(ToFloatRegister(ins->input()), ToRegister(ins->output()), ins->mir()); diff --git a/js/src/jit/arm64/CodeGenerator-arm64.h b/js/src/jit/arm64/CodeGenerator-arm64.h index 04b9acb82226..394ca58f0214 100644 --- a/js/src/jit/arm64/CodeGenerator-arm64.h +++ b/js/src/jit/arm64/CodeGenerator-arm64.h @@ -60,8 +60,6 @@ class CodeGeneratorARM64 : public CodeGeneratorShared { bool generateOutOfLineCode(); - void emitRoundDouble(FloatRegister src, Register dest, Label* fail); - // Emits a branch that directs control flow to the true block if |cond| is // true, and the false block if |cond| is false. void emitBranch(Assembler::Condition cond, MBasicBlock* ifTrue, From e2c81d08b43d599c3b28664bc4231b04e5c266e3 Mon Sep 17 00:00:00 2001 From: Robert Strong Date: Thu, 10 Jan 2019 15:18:14 -0800 Subject: [PATCH 07/12] Bug 1517044 - Don't allow nsIWritablePropertyBag calls to overwrite nsIUpdate and nsIUpdatePatch attributes. r=mhowell --- toolkit/mozapps/update/nsUpdateService.js | 71 ++++++++++++++++--- .../tests/unit_aus_update/updateManagerXML.js | 51 +++++++++++++ 2 files changed, 114 insertions(+), 8 deletions(-) diff --git a/toolkit/mozapps/update/nsUpdateService.js b/toolkit/mozapps/update/nsUpdateService.js index 0de4e191311f..9f24f7c37f21 100644 --- a/toolkit/mozapps/update/nsUpdateService.js +++ b/toolkit/mozapps/update/nsUpdateService.js @@ -1233,13 +1233,21 @@ function UpdatePatch(patch) { this[attr.name] = attr.value; break; default: - // Set nsIPropertyBag properties that were read from the xml file. - this.setProperty(attr.name, attr.value); + if (!this._attrNames.includes(attr.name)) { + // Set nsIPropertyBag properties that were read from the xml file. + this.setProperty(attr.name, attr.value); + } break; } } } UpdatePatch.prototype = { + // nsIUpdatePatch attribute names used to prevent nsIWritablePropertyBag from + // over writing nsIUpdatePatch attributes. + _attrNames: [ + "errorCode", "finalURL", "selected", "size", "state", "type", "URL", + ], + /** * See nsIUpdateService.idl */ @@ -1266,7 +1274,7 @@ UpdatePatch.prototype = { } for (let [name, value] of Object.entries(this._properties)) { - if (value.present) { + if (value.present && !this._attrNames.includes(name)) { patch.setAttribute(name, value.data); } } @@ -1277,6 +1285,12 @@ UpdatePatch.prototype = { * See nsIWritablePropertyBag.idl */ setProperty: function UpdatePatch_setProperty(name, value) { + if (this._attrNames.includes(name)) { + throw Components.Exception( + "Illegal value '" + name + "' (attribute exists on nsIUpdatePatch) " + + "when calling method: [nsIWritablePropertyBag::setProperty]", + Cr.NS_ERROR_ILLEGAL_VALUE); + } this._properties[name] = { data: value, present: true }; }, @@ -1284,6 +1298,12 @@ UpdatePatch.prototype = { * See nsIWritablePropertyBag.idl */ deleteProperty: function UpdatePatch_deleteProperty(name) { + if (this._attrNames.includes(name)) { + throw Components.Exception( + "Illegal value '" + name + "' (attribute exists on nsIUpdatePatch) " + + "when calling method: [nsIWritablePropertyBag::deleteProperty]", + Cr.NS_ERROR_ILLEGAL_VALUE); + } if (name in this._properties) { this._properties[name].present = false; } else { @@ -1308,7 +1328,7 @@ UpdatePatch.prototype = { createInstance(Ci.nsISupportsInterfacePointer); let qi = ChromeUtils.generateQI([Ci.nsIProperty]); for (let [name, value] of Object.entries(this._properties)) { - if (value.present) { + if (value.present && !this._attrNames.includes(name)) { // The nsIPropertyBag enumerator returns a nsISimpleEnumerator whose // elements are nsIProperty objects. Calling QueryInterface for // nsIProperty on the object doesn't return to the caller an object that @@ -1327,6 +1347,12 @@ UpdatePatch.prototype = { * simplify code and to silence warnings in debug builds. */ getProperty: function UpdatePatch_getProperty(name) { + if (this._attrNames.includes(name)) { + throw Components.Exception( + "Illegal value '" + name + "' (attribute exists on nsIUpdatePatch) " + + "when calling method: [nsIWritablePropertyBag::getProperty]", + Cr.NS_ERROR_ILLEGAL_VALUE); + } if (name in this._properties && this._properties[name].present) { return this._properties[name].data; } @@ -1431,8 +1457,10 @@ function Update(update) { this[attr.name] = attr.value; break; default: - // Set nsIPropertyBag properties that were read from the xml file. - this.setProperty(attr.name, attr.value); + if (!this._attrNames.includes(attr.name)) { + // Set nsIPropertyBag properties that were read from the xml file. + this.setProperty(attr.name, attr.value); + } break; } } @@ -1470,6 +1498,15 @@ function Update(update) { } } Update.prototype = { + // nsIUpdate attribute names used to prevent nsIWritablePropertyBag from over + // writing nsIUpdate attributes. + _attrNames: [ + "appVersion", "buildID", "channel", "detailsURL", "displayVersion", + "elevationFailure", "errorCode", "installDate", "isCompleteUpdate", "name", + "previousAppVersion", "promptWaitTime", "serviceURL", "state", "statusText", + "type", "unsupported", + ], + /** * See nsIUpdateService.idl */ @@ -1563,7 +1600,7 @@ Update.prototype = { } for (let [name, value] of Object.entries(this._properties)) { - if (value.present) { + if (value.present && !this._attrNames.includes(name)) { update.setAttribute(name, value.data); } } @@ -1580,6 +1617,12 @@ Update.prototype = { * See nsIWritablePropertyBag.idl */ setProperty: function Update_setProperty(name, value) { + if (this._attrNames.includes(name)) { + throw Components.Exception( + "Illegal value '" + name + "' (attribute exists on nsIUpdate) " + + "when calling method: [nsIWritablePropertyBag::setProperty]", + Cr.NS_ERROR_ILLEGAL_VALUE); + } this._properties[name] = { data: value, present: true }; }, @@ -1587,6 +1630,12 @@ Update.prototype = { * See nsIWritablePropertyBag.idl */ deleteProperty: function Update_deleteProperty(name) { + if (this._attrNames.includes(name)) { + throw Components.Exception( + "Illegal value '" + name + "' (attribute exists on nsIUpdate) " + + "when calling method: [nsIWritablePropertyBag::deleteProperty]", + Cr.NS_ERROR_ILLEGAL_VALUE); + } if (name in this._properties) { this._properties[name].present = false; } else { @@ -1611,7 +1660,7 @@ Update.prototype = { createInstance(Ci.nsISupportsInterfacePointer); let qi = ChromeUtils.generateQI([Ci.nsIProperty]); for (let [name, value] of Object.entries(this._properties)) { - if (value.present) { + if (value.present && !this._attrNames.includes(name)) { // The nsIPropertyBag enumerator returns a nsISimpleEnumerator whose // elements are nsIProperty objects. Calling QueryInterface for // nsIProperty on the object doesn't return to the caller an object that @@ -1629,6 +1678,12 @@ Update.prototype = { * simplify code and to silence warnings in debug builds. */ getProperty: function Update_getProperty(name) { + if (this._attrNames.includes(name)) { + throw Components.Exception( + "Illegal value '" + name + "' (attribute exists on nsIUpdate) " + + "when calling method: [nsIWritablePropertyBag::getProperty]", + Cr.NS_ERROR_ILLEGAL_VALUE); + } if (name in this._properties && this._properties[name].present) { return this._properties[name].data; } diff --git a/toolkit/mozapps/update/tests/unit_aus_update/updateManagerXML.js b/toolkit/mozapps/update/tests/unit_aus_update/updateManagerXML.js index a8a81eef5682..0bd0ac22e0f3 100644 --- a/toolkit/mozapps/update/tests/unit_aus_update/updateManagerXML.js +++ b/toolkit/mozapps/update/tests/unit_aus_update/updateManagerXML.js @@ -261,5 +261,56 @@ function run_test() { Assert.equal(results[1].value, "custom4 patch value", "the second property value" + MSG_SHOULD_EQUAL); + let attrNames = [ + "appVersion", "buildID", "channel", "detailsURL", "displayVersion", + "elevationFailure", "errorCode", "installDate", "isCompleteUpdate", "name", + "previousAppVersion", "promptWaitTime", "serviceURL", "state", "statusText", + "type", "unsupported", + ]; + checkIllegalProperties(update, attrNames); + + attrNames = [ + "errorCode", "finalURL", "selected", "size", "state", "type", "URL", + ]; + checkIllegalProperties(patch, attrNames); + executeSoon(doTestFinish); } + +function checkIllegalProperties(object, propertyNames) { + let objectName = + object instanceof Ci.nsIUpdate ? "nsIUpdate" : "nsIUpdatePatch"; + propertyNames.forEach(function(name) { + // Check that calling getProperty, setProperty, and deleteProperty on an + // nsIUpdate attribute throws NS_ERROR_ILLEGAL_VALUE + let result = 0; + try { + object.getProperty(name); + } catch (e) { + result = e.result; + } + Assert.equal(result, Cr.NS_ERROR_ILLEGAL_VALUE, + "calling getProperty using an " + objectName + " attribute " + + "name should throw NS_ERROR_ILLEGAL_VALUE"); + + result = 0; + try { + object.setProperty(name, "value"); + } catch (e) { + result = e.result; + } + Assert.equal(result, Cr.NS_ERROR_ILLEGAL_VALUE, + "calling setProperty using an " + objectName + " attribute " + + "name should throw NS_ERROR_ILLEGAL_VALUE"); + + result = 0; + try { + object.deleteProperty(name); + } catch (e) { + result = e.result; + } + Assert.equal(result, Cr.NS_ERROR_ILLEGAL_VALUE, + "calling deleteProperty using an " + objectName + " attribute " + + "name should throw NS_ERROR_ILLEGAL_VALUE"); + }); +} From fd9a23b07384b67a9fe41689105f54250b15ca46 Mon Sep 17 00:00:00 2001 From: Bas Schouten Date: Thu, 10 Jan 2019 22:41:14 +0100 Subject: [PATCH 08/12] Bug 1519227: Register payload appropriately in CrossProcessCompositorBridgeParent. r=mstange Differential Revision: https://phabricator.services.mozilla.com/D16246 --HG-- extra : rebase_source : d3c9ad182bbb83931b95bff0b130a98c8a66b8e5 --- gfx/layers/ipc/CrossProcessCompositorBridgeParent.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gfx/layers/ipc/CrossProcessCompositorBridgeParent.cpp b/gfx/layers/ipc/CrossProcessCompositorBridgeParent.cpp index 337c413f72eb..45c215c0089e 100644 --- a/gfx/layers/ipc/CrossProcessCompositorBridgeParent.cpp +++ b/gfx/layers/ipc/CrossProcessCompositorBridgeParent.cpp @@ -389,6 +389,8 @@ void CrossProcessCompositorBridgeParent::ShadowLayersUpdated( static_cast( (endTime - aInfo.transactionStart()).ToMilliseconds())); + RegisterPayload(aLayerTree, aInfo.payload()); + aLayerTree->SetPendingTransactionId( aInfo.id(), aInfo.vsyncId(), aInfo.vsyncStart(), aInfo.refreshStart(), aInfo.transactionStart(), endTime, aInfo.url(), aInfo.fwdTime()); From f32fd66363106e6222c2ddd902815098031adf2c Mon Sep 17 00:00:00 2001 From: Eric Rahm Date: Thu, 10 Jan 2019 13:14:41 -0800 Subject: [PATCH 09/12] Bug 1519209 - Disable NSS_ALLOW_SSLKEYLOGFILE in beta and release. r=glandium This disables NSS_ALLOW_SSLKEYLOGFILE in beta in release in order to avoid shutdown hangs until the NSS project has time to fix the root cause of the issue. --HG-- extra : rebase_source : 51c84d4841308d283f993a7fda576031d7c4f449 --- python/mozbuild/mozbuild/frontend/gyp_reader.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/python/mozbuild/mozbuild/frontend/gyp_reader.py b/python/mozbuild/mozbuild/frontend/gyp_reader.py index a7f4f40febce..4d106f72d152 100644 --- a/python/mozbuild/mozbuild/frontend/gyp_reader.py +++ b/python/mozbuild/mozbuild/frontend/gyp_reader.py @@ -249,6 +249,10 @@ def process_gyp_result(gyp_result, gyp_dir_attrs, path, config, output, for define in defines: if '=' in define: name, value = define.split('=', 1) + # The NSS gyp file doesn't expose a way to override this + # currently, so we do so here. + if name == 'NSS_ALLOW_SSLKEYLOGFILE' and config.substs.get('RELEASE_OR_BETA', False): + continue context['DEFINES'][name] = value else: context['DEFINES'][define] = True From 03d98acfa88c6c340c4e94eebe8b0c285ef04a36 Mon Sep 17 00:00:00 2001 From: Shanavas M Date: Fri, 11 Jan 2019 00:57:02 +0100 Subject: [PATCH 10/12] Bug 1519269 - Remove OrderedMap in favor of IndexMap. r=emilio This cherry-picks https://github.com/servo/servo/pull/22656. --- Cargo.lock | 1 + servo/components/style/Cargo.toml | 1 + servo/components/style/custom_properties.rs | 141 ++------------------ servo/components/style/lib.rs | 1 + servo/ports/geckolib/glue.rs | 4 +- 5 files changed, 14 insertions(+), 134 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 173a75a300b2..ff6b24b7c944 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2425,6 +2425,7 @@ dependencies = [ "fallible 0.0.1", "fxhash 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "hashglobe 0.1.0", + "indexmap 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)", "itertools 0.7.6 (registry+https://github.com/rust-lang/crates.io-index)", "itoa 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)", "lazy_static 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/servo/components/style/Cargo.toml b/servo/components/style/Cargo.toml index d1cc1c601660..9b0b64c375cd 100644 --- a/servo/components/style/Cargo.toml +++ b/servo/components/style/Cargo.toml @@ -38,6 +38,7 @@ fallible = { path = "../fallible" } fxhash = "0.2" hashglobe = { path = "../hashglobe" } html5ever = {version = "0.22", optional = true} +indexmap = "1.0" itertools = "0.7.6" itoa = "0.4" lazy_static = "1" diff --git a/servo/components/style/custom_properties.rs b/servo/components/style/custom_properties.rs index 6bc2b331e35a..f21929c70798 100644 --- a/servo/components/style/custom_properties.rs +++ b/servo/components/style/custom_properties.rs @@ -8,17 +8,17 @@ use crate::hash::map::Entry; use crate::properties::{CSSWideKeyword, CustomDeclarationValue}; -use crate::selector_map::{PrecomputedHashMap, PrecomputedHashSet}; +use crate::selector_map::{PrecomputedHashMap, PrecomputedHashSet, PrecomputedHasher}; use crate::Atom; use cssparser::{Delimiter, Parser, ParserInput, SourcePosition, Token, TokenSerializationType}; -use precomputed_hash::PrecomputedHash; +use indexmap::IndexMap; use selectors::parser::SelectorParseErrorKind; use servo_arc::Arc; use smallvec::SmallVec; -use std::borrow::{Borrow, Cow}; +use std::borrow::Cow; use std::cmp; use std::fmt::{self, Write}; -use std::hash::Hash; +use std::hash::BuildHasherDefault; use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss}; /// The environment from which to get `env` function values. @@ -131,7 +131,8 @@ impl ToCss for SpecifiedValue { /// /// The variable values are guaranteed to not have references to other /// properties. -pub type CustomPropertiesMap = OrderedMap>; +pub type CustomPropertiesMap = + IndexMap, BuildHasherDefault>; /// Both specified and computed values are VariableValues, the difference is /// whether var() functions are expanded. @@ -140,130 +141,6 @@ pub type SpecifiedValue = VariableValue; /// whether var() functions are expanded. pub type ComputedValue = VariableValue; -/// A map that preserves order for the keys, and that is easily indexable. -#[derive(Clone, Debug, Eq, PartialEq)] -pub struct OrderedMap -where - K: PrecomputedHash + Hash + Eq + Clone, -{ - /// Key index. - index: Vec, - /// Key-value map. - values: PrecomputedHashMap, -} - -impl OrderedMap -where - K: Eq + PrecomputedHash + Hash + Clone, -{ - /// Creates a new ordered map. - pub fn new() -> Self { - OrderedMap { - index: Vec::new(), - values: PrecomputedHashMap::default(), - } - } - - /// Insert a new key-value pair. - /// - /// TODO(emilio): Remove unused_mut when Gecko and Servo agree in whether - /// it's necessary. - #[allow(unused_mut)] - pub fn insert(&mut self, key: K, value: V) { - let OrderedMap { - ref mut index, - ref mut values, - } = *self; - match values.entry(key) { - Entry::Vacant(mut entry) => { - index.push(entry.key().clone()); - entry.insert(value); - }, - Entry::Occupied(mut entry) => { - entry.insert(value); - }, - } - } - - /// Get a value given its key. - pub fn get(&self, key: &K) -> Option<&V> { - let value = self.values.get(key); - debug_assert_eq!(value.is_some(), self.index.contains(key)); - value - } - - /// Get whether there's a value on the map for `key`. - pub fn contains_key(&self, key: &K) -> bool { - self.values.contains_key(key) - } - - /// Get the key located at the given index. - pub fn get_key_at(&self, index: u32) -> Option<&K> { - self.index.get(index as usize) - } - - /// Get an ordered map iterator. - pub fn iter<'a>(&'a self) -> OrderedMapIterator<'a, K, V> { - OrderedMapIterator { - inner: self, - pos: 0, - } - } - - /// Get the count of items in the map. - pub fn len(&self) -> usize { - debug_assert_eq!(self.values.len(), self.index.len()); - self.values.len() - } - - /// Returns whether this map is empty. - pub fn is_empty(&self) -> bool { - self.len() == 0 - } - - /// Remove an item given its key. - fn remove(&mut self, key: &Q) -> Option - where - K: Borrow, - Q: PrecomputedHash + Hash + Eq, - { - let index = self.index.iter().position(|k| k.borrow() == key)?; - self.index.remove(index); - self.values.remove(key) - } -} - -/// An iterator for OrderedMap. -/// -/// The iteration order is determined by the order that the values are -/// added to the key-value map. -pub struct OrderedMapIterator<'a, K, V> -where - K: 'a + Eq + PrecomputedHash + Hash + Clone, - V: 'a, -{ - /// The OrderedMap itself. - inner: &'a OrderedMap, - /// The position of the iterator. - pos: usize, -} - -impl<'a, K, V> Iterator for OrderedMapIterator<'a, K, V> -where - K: Eq + PrecomputedHash + Hash + Clone, -{ - type Item = (&'a K, &'a V); - - fn next(&mut self) -> Option { - let key = self.inner.index.get(self.pos)?; - - self.pos += 1; - let value = &self.inner.values[key]; - - Some((key, value)) - } -} - /// A struct holding information about the external references to that a custom /// property value may have. #[derive(Default)] @@ -648,7 +525,7 @@ impl<'a> CustomPropertiesBuilder<'a> { if self.custom_properties.is_none() { self.custom_properties = Some(match self.inherited { Some(inherited) => (**inherited).clone(), - None => CustomPropertiesMap::new(), + None => CustomPropertiesMap::default(), }); } @@ -933,7 +810,7 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap, environment: // We have to clone the names so that we can mutably borrow the map // in the context we create for traversal. - let names = custom_properties_map.index.clone(); + let names: Vec<_> = custom_properties_map.keys().cloned().collect(); for name in names.into_iter() { let mut context = Context { count: 0, @@ -1112,7 +989,7 @@ pub fn substitute<'i>( let mut input = ParserInput::new(input); let mut input = Parser::new(&mut input); let mut position = (input.position(), first_token_type); - let empty_map = CustomPropertiesMap::new(); + let empty_map = CustomPropertiesMap::default(); let custom_properties = match computed_values_map { Some(m) => &**m, None => &empty_map, diff --git a/servo/components/style/lib.rs b/servo/components/style/lib.rs index 5c4b47193a5f..79829081b39f 100644 --- a/servo/components/style/lib.rs +++ b/servo/components/style/lib.rs @@ -48,6 +48,7 @@ extern crate hashglobe; #[cfg(feature = "servo")] #[macro_use] extern crate html5ever; +extern crate indexmap; extern crate itertools; extern crate itoa; #[macro_use] diff --git a/servo/ports/geckolib/glue.rs b/servo/ports/geckolib/glue.rs index 89c39bbc65b2..7e88c06ab5d8 100644 --- a/servo/ports/geckolib/glue.rs +++ b/servo/ports/geckolib/glue.rs @@ -5792,8 +5792,8 @@ pub extern "C" fn Servo_GetCustomPropertyNameAt( None => return false, }; - let property_name = match custom_properties.get_key_at(index) { - Some(n) => n, + let property_name = match custom_properties.get_index(index as usize) { + Some((key, _value)) => key, None => return false, }; From 06e203c285501680a0ac34ce64b86170426983d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 11 Jan 2019 00:57:26 +0100 Subject: [PATCH 11/12] Bug 1519269 - Rustfmt recent changes. --- servo/components/style/gecko_string_cache/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/servo/components/style/gecko_string_cache/mod.rs b/servo/components/style/gecko_string_cache/mod.rs index 197eca1640c0..9c33f25d9b85 100644 --- a/servo/components/style/gecko_string_cache/mod.rs +++ b/servo/components/style/gecko_string_cache/mod.rs @@ -14,10 +14,10 @@ use crate::gecko_bindings::bindings::Gecko_AddRefAtom; use crate::gecko_bindings::bindings::Gecko_Atomize; use crate::gecko_bindings::bindings::Gecko_Atomize16; use crate::gecko_bindings::bindings::Gecko_ReleaseAtom; -use crate::gecko_bindings::structs::{nsAtom, nsDynamicAtom, nsStaticAtom}; -use crate::gecko_bindings::structs::root::mozilla::detail::GkAtoms_Atoms_AtomsCount; use crate::gecko_bindings::structs::root::mozilla::detail::gGkAtoms; use crate::gecko_bindings::structs::root::mozilla::detail::kGkAtomsArrayOffset; +use crate::gecko_bindings::structs::root::mozilla::detail::GkAtoms_Atoms_AtomsCount; +use crate::gecko_bindings::structs::{nsAtom, nsDynamicAtom, nsStaticAtom}; use nsstring::{nsAString, nsStr}; use precomputed_hash::PrecomputedHash; use std::borrow::{Borrow, Cow}; From d85d8a3b574d030594525cb6fca08cd01a358d95 Mon Sep 17 00:00:00 2001 From: sotaro Date: Fri, 11 Jan 2019 09:04:26 +0900 Subject: [PATCH 12/12] Bug 1516787 - Add GLContextEGL::OnMarkDestroyed() r=jgilbert --- gfx/gl/GLContext.cpp | 2 ++ gfx/gl/GLContext.h | 3 +++ gfx/gl/GLContextEGL.h | 2 ++ gfx/gl/GLContextProviderEGL.cpp | 7 +++++++ gfx/gl/SharedSurfaceANGLE.cpp | 6 ++++-- 5 files changed, 18 insertions(+), 2 deletions(-) diff --git a/gfx/gl/GLContext.cpp b/gfx/gl/GLContext.cpp index 223d43b923d1..a47fda036c06 100644 --- a/gfx/gl/GLContext.cpp +++ b/gfx/gl/GLContext.cpp @@ -1923,6 +1923,8 @@ bool GLContext::AssembleOffscreenFBs(const GLuint colorMSRB, void GLContext::MarkDestroyed() { if (IsDestroyed()) return; + OnMarkDestroyed(); + // Null these before they're naturally nulled after dtor, as we want GLContext // to still be alive in *their* dtors. mScreen = nullptr; diff --git a/gfx/gl/GLContext.h b/gfx/gl/GLContext.h index 1d57c136b921..9591212d9810 100644 --- a/gfx/gl/GLContext.h +++ b/gfx/gl/GLContext.h @@ -3322,6 +3322,9 @@ class GLContext : public GLLibraryLoader, // the GL function pointers! void MarkDestroyed(); + protected: + virtual void OnMarkDestroyed() {} + // ----------------------------------------------------------------------------- // Everything that isn't standard GL APIs protected: diff --git a/gfx/gl/GLContextEGL.h b/gfx/gl/GLContextEGL.h index 82bbef50fb3d..95d5e0c02e23 100644 --- a/gfx/gl/GLContextEGL.h +++ b/gfx/gl/GLContextEGL.h @@ -95,6 +95,8 @@ class GLContextEGL : public GLContext { friend class GLContextProviderEGL; friend class GLContextEGLFactory; + virtual void OnMarkDestroyed() override; + public: const EGLConfig mConfig; diff --git a/gfx/gl/GLContextProviderEGL.cpp b/gfx/gl/GLContextProviderEGL.cpp index 18ac0bbed321..774eb34a8e87 100644 --- a/gfx/gl/GLContextProviderEGL.cpp +++ b/gfx/gl/GLContextProviderEGL.cpp @@ -310,6 +310,13 @@ GLContextEGL::GLContextEGL(CreateContextFlags flags, const SurfaceCaps& caps, #endif } +void +GLContextEGL::OnMarkDestroyed() { + if (mSurfaceOverride != EGL_NO_SURFACE) { + SetEGLSurfaceOverride(EGL_NO_SURFACE); + } +} + GLContextEGL::~GLContextEGL() { MarkDestroyed(); diff --git a/gfx/gl/SharedSurfaceANGLE.cpp b/gfx/gl/SharedSurfaceANGLE.cpp index c1bc1379c6b1..86491411da2c 100644 --- a/gfx/gl/SharedSurfaceANGLE.cpp +++ b/gfx/gl/SharedSurfaceANGLE.cpp @@ -98,8 +98,10 @@ SharedSurface_ANGLEShareHandle::SharedSurface_ANGLEShareHandle( mKeyedMutex(keyedMutex) {} SharedSurface_ANGLEShareHandle::~SharedSurface_ANGLEShareHandle() { - if (GLContextEGL::Cast(mGL)->GetEGLSurfaceOverride() == mPBuffer) { - GLContextEGL::Cast(mGL)->SetEGLSurfaceOverride(EGL_NO_SURFACE); + GLContext* gl = mGL; + + if (gl && GLContextEGL::Cast(gl)->GetEGLSurfaceOverride() == mPBuffer) { + GLContextEGL::Cast(gl)->SetEGLSurfaceOverride(EGL_NO_SURFACE); } mEGL->fDestroySurface(Display(), mPBuffer); }