From 10d7ccf23eda7fd9d411016459d0890f40841446 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Tue, 19 Mar 2019 15:14:11 +0000 Subject: [PATCH] Bug 1535384 part 5. Eliminate MOZ_CAN_RUN_SCRIPT_BOUNDARY for mutation callbacks. r=mccr8 Differential Revision: https://phabricator.services.mozilla.com/D23776 --HG-- extra : moz-landing-system : lando --- dom/base/nsDOMMutationObserver.cpp | 4 +++- dom/base/nsDOMMutationObserver.h | 4 +++- dom/bindings/CallbackObject.h | 2 +- dom/webidl/MutationObserver.webidl | 1 - dom/webidl/Promise.webidl | 1 - dom/workers/RuntimeService.cpp | 15 +++++++++++++-- dom/workers/WorkerPrivate.h | 4 +++- dom/workers/WorkerScope.cpp | 6 +++++- dom/workers/WorkerScope.h | 3 ++- dom/worklet/WorkletThread.cpp | 6 +++++- xpcom/base/CycleCollectedJSContext.cpp | 9 +++++++-- xpcom/base/CycleCollectedJSContext.h | 21 +++++++++++++++++++-- xpcom/base/nsCycleCollector.h | 2 ++ xpcom/build/nsXPCOM.h | 7 ++++++- xpcom/build/nsXPCOMPrivate.h | 2 ++ 15 files changed, 71 insertions(+), 16 deletions(-) diff --git a/dom/base/nsDOMMutationObserver.cpp b/dom/base/nsDOMMutationObserver.cpp index 7db9e6d92f33..10b835c8f2e7 100644 --- a/dom/base/nsDOMMutationObserver.cpp +++ b/dom/base/nsDOMMutationObserver.cpp @@ -536,6 +536,7 @@ void nsDOMMutationObserver::ScheduleForRun() { class MutationObserverMicroTask final : public MicroTaskRunnable { public: + MOZ_CAN_RUN_SCRIPT virtual void Run(AutoSlowOperation& aAso) override { nsDOMMutationObserver::HandleMutations(aAso); } @@ -804,7 +805,8 @@ void nsDOMMutationObserver::HandleMutation() { } ClearPendingRecords(); - mCallback->Call(this, mutations, *this); + RefPtr callback(mCallback); + callback->Call(this, mutations, *this); } void nsDOMMutationObserver::HandleMutationsInternal(AutoSlowOperation& aAso) { diff --git a/dom/base/nsDOMMutationObserver.h b/dom/base/nsDOMMutationObserver.h index 0095e2413989..f84a06d1cda1 100644 --- a/dom/base/nsDOMMutationObserver.h +++ b/dom/base/nsDOMMutationObserver.h @@ -467,7 +467,7 @@ class nsDOMMutationObserver final : public nsISupports, public nsWrapperCache { void TakeRecords(nsTArray>& aRetVal); - void HandleMutation(); + MOZ_CAN_RUN_SCRIPT void HandleMutation(); void GetObservingInfo( nsTArray>& aResult, @@ -514,6 +514,7 @@ class nsDOMMutationObserver final : public nsISupports, public nsWrapperCache { // static methods static void QueueMutationObserverMicroTask(); + MOZ_CAN_RUN_SCRIPT static void HandleMutations(mozilla::AutoSlowOperation& aAso); static bool AllScheduledMutationObserversAreSuppressed() { @@ -561,6 +562,7 @@ class nsDOMMutationObserver final : public nsISupports, public nsWrapperCache { return mOwner && nsGlobalWindowInner::Cast(mOwner)->IsInSyncOperation(); } + MOZ_CAN_RUN_SCRIPT static void HandleMutationsInternal(mozilla::AutoSlowOperation& aAso); static void AddCurrentlyHandlingObserver(nsDOMMutationObserver* aObserver, diff --git a/dom/bindings/CallbackObject.h b/dom/bindings/CallbackObject.h index b28c537addc9..3c02a133f08a 100644 --- a/dom/bindings/CallbackObject.h +++ b/dom/bindings/CallbackObject.h @@ -312,7 +312,7 @@ class CallbackObject : public nsISupports { const char* aExecutionReason, ExceptionHandling aExceptionHandling, JS::Realm* aRealm = nullptr, bool aIsJSImplementedWebIDL = false); - ~CallSetup(); + MOZ_CAN_RUN_SCRIPT ~CallSetup(); JSContext* GetContext() const { return mCx; } diff --git a/dom/webidl/MutationObserver.webidl b/dom/webidl/MutationObserver.webidl index 7b73e8efea8a..c179a64509a5 100644 --- a/dom/webidl/MutationObserver.webidl +++ b/dom/webidl/MutationObserver.webidl @@ -52,7 +52,6 @@ interface MutationObserver { attribute boolean mergeAttributeRecords; }; -[MOZ_CAN_RUN_SCRIPT_BOUNDARY] callback MutationCallback = void (sequence mutations, MutationObserver observer); dictionary MutationObserverInit { diff --git a/dom/webidl/Promise.webidl b/dom/webidl/Promise.webidl index d1f75bcc5dd8..e251cbbeb253 100644 --- a/dom/webidl/Promise.webidl +++ b/dom/webidl/Promise.webidl @@ -7,7 +7,6 @@ * Web IDL infrastructure. */ -[MOZ_CAN_RUN_SCRIPT_BOUNDARY] callback PromiseJobCallback = void(); [TreatNonCallableAsNull] diff --git a/dom/workers/RuntimeService.cpp b/dom/workers/RuntimeService.cpp index ebc668de588b..8ab431afe51f 100644 --- a/dom/workers/RuntimeService.cpp +++ b/dom/workers/RuntimeService.cpp @@ -35,6 +35,7 @@ #include "mozilla/AntiTrackingCommon.h" #include "mozilla/ArrayUtils.h" #include "mozilla/Atomics.h" +#include "mozilla/Attributes.h" #include "mozilla/CycleCollectedJSContext.h" #include "mozilla/CycleCollectedJSRuntime.h" #include "mozilla/Telemetry.h" @@ -931,7 +932,10 @@ class WorkerJSContext final : public mozilla::CycleCollectedJSContext { SetTargetedMicroTaskRecursionDepth(2); } - ~WorkerJSContext() { + // MOZ_CAN_RUN_SCRIPT_BOUNDARY because otherwise we have to annotate the + // SpiderMonkey JS::JobQueue's destructor as MOZ_CAN_RUN_SCRIPT, which is a + // bit of a pain. + MOZ_CAN_RUN_SCRIPT_BOUNDARY ~WorkerJSContext() { MOZ_COUNT_DTOR_INHERITED(WorkerJSContext, CycleCollectedJSContext); JSContext* cx = MaybeContext(); if (!cx) { @@ -2215,6 +2219,9 @@ bool LogViolationDetailsRunnable::MainThreadRun() { return true; } +// MOZ_CAN_RUN_SCRIPT_BOUNDARY until Runnable::Run is MOZ_CAN_RUN_SCRIPT. See +// bug 1535398. +MOZ_CAN_RUN_SCRIPT_BOUNDARY NS_IMETHODIMP WorkerThreadPrimaryRunnable::Run() { AUTO_PROFILER_LABEL_DYNAMIC_LOSSY_NSSTRING( @@ -2291,7 +2298,11 @@ WorkerThreadPrimaryRunnable::Run() { PROFILER_SET_JS_CONTEXT(cx); { - mWorkerPrivate->DoRunLoop(cx); + // We're on the worker thread here, and WorkerPrivate's refcounting is + // non-threadsafe: you can only do it on the parent thread. What that + // means in practice is that we're relying on it being kept alive while + // we run. Hopefully. + MOZ_KnownLive(mWorkerPrivate)->DoRunLoop(cx); // The AutoJSAPI in DoRunLoop should have reported any exceptions left // on cx. MOZ_ASSERT(!JS_IsExceptionPending(cx)); diff --git a/dom/workers/WorkerPrivate.h b/dom/workers/WorkerPrivate.h index 3b0f53091c2d..1fdd130474dd 100644 --- a/dom/workers/WorkerPrivate.h +++ b/dom/workers/WorkerPrivate.h @@ -8,6 +8,7 @@ #define mozilla_dom_workers_workerprivate_h__ #include "mozilla/dom/WorkerCommon.h" +#include "mozilla/Attributes.h" #include "mozilla/CondVar.h" #include "mozilla/DOMEventTargetHelper.h" #include "mozilla/RelativeTimeline.h" @@ -193,6 +194,7 @@ class WorkerPrivate : public RelativeTimeline { return std::move(mDefaultLocale); } + MOZ_CAN_RUN_SCRIPT void DoRunLoop(JSContext* aCx); bool InterruptCallback(JSContext* aCx); @@ -226,7 +228,7 @@ class WorkerPrivate : public RelativeTimeline { const Sequence& aTransferable, ErrorResult& aRv); - void EnterDebuggerEventLoop(); + MOZ_CAN_RUN_SCRIPT void EnterDebuggerEventLoop(); void LeaveDebuggerEventLoop(); diff --git a/dom/workers/WorkerScope.cpp b/dom/workers/WorkerScope.cpp index d66de6f8247c..35dcd96a83a0 100644 --- a/dom/workers/WorkerScope.cpp +++ b/dom/workers/WorkerScope.cpp @@ -894,7 +894,11 @@ void WorkerDebuggerGlobalScope::LoadSubScript( } void WorkerDebuggerGlobalScope::EnterEventLoop() { - mWorkerPrivate->EnterDebuggerEventLoop(); + // We're on the worker thread here, and WorkerPrivate's refcounting is + // non-threadsafe: you can only do it on the parent thread. What that + // means in practice is that we're relying on it being kept alive while + // we run. Hopefully. + MOZ_KnownLive(mWorkerPrivate)->EnterDebuggerEventLoop(); } void WorkerDebuggerGlobalScope::LeaveEventLoop() { diff --git a/dom/workers/WorkerScope.h b/dom/workers/WorkerScope.h index 6890cb8fae98..ec13f7fdb3fc 100644 --- a/dom/workers/WorkerScope.h +++ b/dom/workers/WorkerScope.h @@ -7,6 +7,7 @@ #ifndef mozilla_dom_workerscope_h__ #define mozilla_dom_workerscope_h__ +#include "mozilla/Attributes.h" #include "mozilla/dom/WorkerCommon.h" #include "mozilla/DOMEventTargetHelper.h" #include "mozilla/dom/DOMPrefs.h" @@ -312,7 +313,7 @@ class WorkerDebuggerGlobalScope final : public DOMEventTargetHelper, const Optional>& aSandbox, ErrorResult& aRv); - void EnterEventLoop(); + MOZ_CAN_RUN_SCRIPT void EnterEventLoop(); void LeaveEventLoop(); diff --git a/dom/worklet/WorkletThread.cpp b/dom/worklet/WorkletThread.cpp index 70c9a94ae97d..c367de8c42e2 100644 --- a/dom/worklet/WorkletThread.cpp +++ b/dom/worklet/WorkletThread.cpp @@ -9,6 +9,7 @@ #include "nsContentUtils.h" #include "nsCycleCollector.h" #include "mozilla/dom/AtomList.h" +#include "mozilla/Attributes.h" #include "mozilla/EventQueue.h" #include "mozilla/ThreadEventQueue.h" @@ -100,7 +101,10 @@ class WorkletJSContext final : public CycleCollectedJSContext { nsCycleCollector_startup(); } - ~WorkletJSContext() override { + // MOZ_CAN_RUN_SCRIPT_BOUNDARY because otherwise we have to annotate the + // SpiderMonkey JS::JobQueue's destructor as MOZ_CAN_RUN_SCRIPT, which is a + // bit of a pain. + MOZ_CAN_RUN_SCRIPT_BOUNDARY ~WorkletJSContext() override { MOZ_ASSERT(!NS_IsMainThread()); JSContext* cx = MaybeContext(); diff --git a/xpcom/base/CycleCollectedJSContext.cpp b/xpcom/base/CycleCollectedJSContext.cpp index f7eddb1daf84..df867b846cdc 100644 --- a/xpcom/base/CycleCollectedJSContext.cpp +++ b/xpcom/base/CycleCollectedJSContext.cpp @@ -219,6 +219,7 @@ class PromiseJobRunnable final : public MicroTaskRunnable { virtual ~PromiseJobRunnable() {} protected: + MOZ_CAN_RUN_SCRIPT virtual void Run(AutoSlowOperation& aAso) override { JSObject* callback = mCallback->CallbackPreserveColor(); nsIGlobalObject* global = callback ? xpc::NativeGlobal(callback) : nullptr; @@ -232,7 +233,8 @@ class PromiseJobRunnable final : public MicroTaskRunnable { AutoHandlingUserInputStatePusher userInpStatePusher( mPropagateUserInputEventHandling, nullptr, doc); - mCallback->Call("promise callback"); + // mCallback is const, so can't suddenly become null. + MOZ_KnownLive(mCallback)->Call("promise callback"); aAso.CheckForInterrupt(); } // Now that mCallback is no longer needed, clear any pointers it contains to @@ -250,7 +252,7 @@ class PromiseJobRunnable final : public MicroTaskRunnable { } private: - RefPtr mCallback; + const RefPtr mCallback; bool mPropagateUserInputEventHandling; }; @@ -525,6 +527,9 @@ class AsyncMutationHandler final : public mozilla::Runnable { public: AsyncMutationHandler() : mozilla::Runnable("AsyncMutationHandler") {} + // MOZ_CAN_RUN_SCRIPT_BOUNDARY until Runnable::Run is MOZ_CAN_RUN_SCRIPT. See + // bug 1535398. + MOZ_CAN_RUN_SCRIPT_BOUNDARY NS_IMETHOD Run() override { CycleCollectedJSContext* ccjs = CycleCollectedJSContext::Get(); if (ccjs) { diff --git a/xpcom/base/CycleCollectedJSContext.h b/xpcom/base/CycleCollectedJSContext.h index d57c34a00c64..c2a9795cc7fa 100644 --- a/xpcom/base/CycleCollectedJSContext.h +++ b/xpcom/base/CycleCollectedJSContext.h @@ -9,6 +9,7 @@ #include +#include "mozilla/Attributes.h" #include "mozilla/DeferredFinalize.h" #include "mozilla/LinkedList.h" #include "mozilla/mozalloc.h" @@ -75,7 +76,7 @@ class MicroTaskRunnable { public: MicroTaskRunnable() = default; NS_INLINE_DECL_REFCOUNTING(MicroTaskRunnable) - virtual void Run(AutoSlowOperation& aAso) = 0; + MOZ_CAN_RUN_SCRIPT virtual void Run(AutoSlowOperation& aAso) = 0; virtual bool Suppressed() { return false; } protected: @@ -166,7 +167,16 @@ class CycleCollectedJSContext public: // nsThread entrypoints + // + // MOZ_CAN_RUN_SCRIPT_BOUNDARY so we don't need to annotate + // nsThread::ProcessNextEvent and all its callers MOZ_CAN_RUN_SCRIPT for now. + // But we really should! + MOZ_CAN_RUN_SCRIPT_BOUNDARY virtual void BeforeProcessTask(bool aMightBlock); + // MOZ_CAN_RUN_SCRIPT_BOUNDARY so we don't need to annotate + // nsThread::ProcessNextEvent and all its callers MOZ_CAN_RUN_SCRIPT for now. + // But we really should! + MOZ_CAN_RUN_SCRIPT_BOUNDARY virtual void AfterProcessTask(uint32_t aRecursionDepth); // Check whether we need an idle GC task. @@ -196,6 +206,7 @@ class CycleCollectedJSContext // Usually the best way to do this is to use nsAutoMicroTask. void EnterMicroTask() { ++mMicroTaskLevel; } + MOZ_CAN_RUN_SCRIPT void LeaveMicroTask() { if (--mMicroTaskLevel == 0) { PerformMicroTaskCheckPoint(); @@ -208,8 +219,10 @@ class CycleCollectedJSContext void SetMicroTaskLevel(uint32_t aLevel) { mMicroTaskLevel = aLevel; } + MOZ_CAN_RUN_SCRIPT bool PerformMicroTaskCheckPoint(bool aForce = false); + MOZ_CAN_RUN_SCRIPT void PerformDebuggerMicroTaskCheckpoint(); bool IsInStableOrMetaStableState() const { return mDoingStableStates; } @@ -243,6 +256,10 @@ class CycleCollectedJSContext bool enqueuePromiseJob(JSContext* cx, JS::HandleObject promise, JS::HandleObject job, JS::HandleObject allocationSite, JS::HandleObject incumbentGlobal) override; + // MOZ_CAN_RUN_SCRIPT_BOUNDARY for now so we don't have to change SpiderMonkey + // headers. The caller presumably knows this can run script (like everything + // in SpiderMonkey!) and will deal. + MOZ_CAN_RUN_SCRIPT_BOUNDARY void runJobs(JSContext* cx) override; bool empty() const override; class SavedMicroTaskQueue; @@ -292,7 +309,7 @@ class MOZ_STACK_CLASS nsAutoMicroTask { ccjs->EnterMicroTask(); } } - ~nsAutoMicroTask() { + MOZ_CAN_RUN_SCRIPT ~nsAutoMicroTask() { CycleCollectedJSContext* ccjs = CycleCollectedJSContext::Get(); if (ccjs) { ccjs->LeaveMicroTask(); diff --git a/xpcom/base/nsCycleCollector.h b/xpcom/base/nsCycleCollector.h index 29b383730757..15b7a6dd6f94 100644 --- a/xpcom/base/nsCycleCollector.h +++ b/xpcom/base/nsCycleCollector.h @@ -16,6 +16,7 @@ struct already_AddRefed; #include "nsError.h" #include "nsID.h" +#include "mozilla/Attributes.h" #include "js/SliceBudget.h" namespace mozilla { @@ -59,6 +60,7 @@ uint32_t nsCycleCollector_suspectedCount(); // If aDoCollect is true, then run the GC and CC a few times before // shutting down the CC completely. +MOZ_CAN_RUN_SCRIPT void nsCycleCollector_shutdown(bool aDoCollect = true); // Helpers for interacting with JS diff --git a/xpcom/build/nsXPCOM.h b/xpcom/build/nsXPCOM.h index 9dbed182b7c2..ece4ce355db4 100644 --- a/xpcom/build/nsXPCOM.h +++ b/xpcom/build/nsXPCOM.h @@ -9,6 +9,7 @@ #include "nscore.h" #include "nsXPCOMCID.h" +#include "mozilla/Attributes.h" #ifdef __cplusplus # define DECL_CLASS(c) class c @@ -92,8 +93,12 @@ NS_InitMinimalXPCOM(); * * @return NS_OK for success; * other error codes indicate a failure during initialisation. + * + * MOZ_CAN_RUN_SCRIPT_BOUNDARY for now, but really it should maybe be + * MOZ_CAN_RUN_SCRIPT. */ -XPCOM_API(nsresult) NS_ShutdownXPCOM(nsIServiceManager* aServMgr); +XPCOM_API(MOZ_CAN_RUN_SCRIPT_BOUNDARY nsresult) +NS_ShutdownXPCOM(nsIServiceManager* aServMgr); /** * Public Method to access to the service manager. diff --git a/xpcom/build/nsXPCOMPrivate.h b/xpcom/build/nsXPCOMPrivate.h index 8de8145655fd..56c1cfc79bbf 100644 --- a/xpcom/build/nsXPCOMPrivate.h +++ b/xpcom/build/nsXPCOMPrivate.h @@ -9,6 +9,7 @@ #include "nscore.h" #include "nsXPCOM.h" +#include "mozilla/Attributes.h" /** * During this shutdown notification all threads which run XPCOM code must @@ -30,6 +31,7 @@ namespace mozilla { * other error codes indicate a failure during shutdown * */ +MOZ_CAN_RUN_SCRIPT nsresult ShutdownXPCOM(nsIServiceManager* aServMgr); void SetICUMemoryFunctions();