bug 1328964 don't allocate SourceBufferHolder on the heap r=baku
authorKarl Tomlinson <karlt+@karlt.net>
Tue, 27 Mar 2018 15:12:19 +1300
changeset 412954 c3c9f4525efb7898bab818432f44d9ee25ca4c11
parent 412953 bb29485e179b8dbbd42f0ae26f63d9ce173463b5
child 412955 06872fbe3904f267f66932d07d20f8311a14ae6f
push id33828
push userarchaeopteryx@coole-files.de
push dateThu, 12 Apr 2018 19:19:41 +0000
treeherdermozilla-central@6e22c4a726c2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku
bugs1328964, 987556
milestone61.0a1
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
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.