bug 1328964 don't allocate SourceBufferHolder on the heap r?baku draft
authorKarl Tomlinson <karlt+@karlt.net>
Tue, 27 Mar 2018 15:12:19 +1300
changeset 780861 e443ea752776949f9b32faceec79f2510ea2b5b6
parent 780860 0dd362f0b328fee755b07372f1d8a1df41dce1af
child 780862 f4917bf3e0eef1ffed9b06f60aef89cbebfe53b8
push id106145
push userktomlinson@mozilla.com
push dateThu, 12 Apr 2018 05:09:40 +0000
reviewersbaku
bugs1328964, 987556
milestone61.0a1
bug 1328964 don't allocate SourceBufferHolder on the heap r?baku I'm not aware of any reason why SourceBufferHolder must be allocated only on the stack, but bz and bkelly seemed to agree that it should be MOZ_STACK_CLASS in https://bugzilla.mozilla.org/show_bug.cgi?id=987556#c72 FWIW avoiding SourceBufferHolder saves by not needing to store GiveOwnership. UniquePtr makes the transfer of ownership clearer. MozReview-Commit-ID: 2wxkW75TNUM
dom/worklet/Worklet.cpp
--- a/dom/worklet/Worklet.cpp
+++ b/dom/worklet/Worklet.cpp
@@ -25,41 +25,42 @@
 
 namespace mozilla {
 namespace dom {
 
 class ExecutionRunnable final : public Runnable
 {
 public:
   ExecutionRunnable(WorkletFetchHandler* aHandler, Worklet::WorkletType aType,
-                    char16_t* aScriptBuffer, size_t aScriptLength,
+                    JS::UniqueTwoByteChars aScriptBuffer, size_t aScriptLength,
                     const WorkletLoadInfo& aWorkletLoadInfo)
     : Runnable("Worklet::ExecutionRunnable")
     , mHandler(aHandler)
+    , mScriptBuffer(Move(aScriptBuffer))
+    , mScriptLength(aScriptLength)
     , mWorkletType(aType)
-    , mBuffer(aScriptBuffer, aScriptLength,
-              JS::SourceBufferHolder::GiveOwnership)
     , mResult(NS_ERROR_FAILURE)
   {
     MOZ_ASSERT(NS_IsMainThread());
   }
 
   NS_IMETHOD
   Run() override;
 
 private:
   void
   RunOnWorkletThread();
 
   void
   RunOnMainThread();
 
   RefPtr<WorkletFetchHandler> mHandler;
+  JS::UniqueTwoByteChars mScriptBuffer;
+  size_t mScriptLength;
   Worklet::WorkletType mWorkletType;
-  JS::SourceBufferHolder mBuffer;
   nsresult mResult;
 };
 
 // ---------------------------------------------------------------------------
 // WorkletFetchHandler
 
 class WorkletFetchHandler final : public PromiseNativeHandler
                                 , public nsIStreamLoaderObserver
@@ -205,30 +206,30 @@ public:
   {
     MOZ_ASSERT(NS_IsMainThread());
 
     if (NS_FAILED(aStatus)) {
       RejectPromises(aStatus);
       return NS_OK;
     }
 
-    char16_t* scriptTextBuf;
+    JS::UniqueTwoByteChars scriptTextBuf;
     size_t scriptTextLength;
     nsresult rv =
       ScriptLoader::ConvertToUTF16(nullptr, aString, aStringLen,
                                    NS_LITERAL_STRING("UTF-8"), nullptr,
                                    scriptTextBuf, scriptTextLength);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       RejectPromises(rv);
       return NS_OK;
     }
 
     // Moving the ownership of the buffer
     nsCOMPtr<nsIRunnable> runnable =
-      new ExecutionRunnable(this, mWorklet->Type(), scriptTextBuf,
+      new ExecutionRunnable(this, mWorklet->Type(), Move(scriptTextBuf),
                             scriptTextLength, mWorklet->LoadInfo());
 
     RefPtr<WorkletThread> thread = mWorklet->GetOrCreateThread();
     if (!thread) {
       RejectPromises(NS_ERROR_FAILURE);
       return NS_OK;
     }
 
@@ -395,18 +396,20 @@ ExecutionRunnable::RunOnWorkletThread()
   JS::CompileOptions compileOptions(cx);
   compileOptions.setIntroductionType("Worklet");
   compileOptions.setFileAndLine(url.get(), 0);
   compileOptions.setIsRunOnce(true);
   compileOptions.setNoScriptRval(true);
 
   JSAutoCompartment comp(cx, globalObj);
 
+  JS::SourceBufferHolder buffer(mScriptBuffer.release(), mScriptLength,
+                                JS::SourceBufferHolder::GiveOwnership);
   JS::Rooted<JS::Value> unused(cx);
-  if (!JS::Evaluate(cx, compileOptions, mBuffer, &unused)) {
+  if (!JS::Evaluate(cx, compileOptions, buffer, &unused)) {
     ErrorResult error;
     error.MightThrowJSException();
     error.StealExceptionFromJSContext(cx);
     mResult = error.StealNSResult();
     return;
   }
 
   // All done.