Bug 1252565. Make dom::WarningOnlyErrorReporter handle workers. r=bholley
☠☠ backed out by 9716b46f3b60 ☠ ☠
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 01 Mar 2016 16:53:22 -0500
changeset 322699 96580db9b356aecb3f64739f5b93f5247e47c90b
parent 322698 ac15fe0e71fda9ccbe20732221eedbbbe6b54837
child 322700 aad89b3b0eb1db2a026b40f7825e563c3d861bf4
push id5913
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 16:57:49 +0000
treeherdermozilla-beta@dcaf0a6fa115 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley
bugs1252565
milestone47.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 1252565. Make dom::WarningOnlyErrorReporter handle workers. r=bholley
dom/base/ScriptSettings.cpp
dom/workers/RuntimeService.cpp
xpcom/base/CycleCollectedJSRuntime.cpp
--- a/dom/base/ScriptSettings.cpp
+++ b/dom/base/ScriptSettings.cpp
@@ -344,19 +344,20 @@ AutoJSAPI::InitInternal(JSObject* aGloba
     // be necessary.
     JS::Rooted<JSObject*> global(JS_GetRuntime(aCx), aGlobal);
     mCxPusher.emplace(mCx);
     mAutoNullableCompartment.emplace(mCx, global);
   } else {
     mAutoNullableCompartment.emplace(mCx, aGlobal);
   }
 
+  JSRuntime* rt = JS_GetRuntime(aCx);
+  mOldErrorReporter.emplace(JS_GetErrorReporter(rt));
+
   if (aIsMainThread) {
-    JSRuntime* rt = JS_GetRuntime(aCx);
-    mOldErrorReporter.emplace(JS_GetErrorReporter(rt));
     JS_SetErrorReporter(rt, xpc::SystemErrorReporter);
   }
 }
 
 AutoJSAPI::AutoJSAPI(nsIGlobalObject* aGlobalObject,
                      bool aIsMainThread,
                      JSContext* aCx)
   : mOwnErrorReporting(false)
@@ -460,18 +461,31 @@ AutoJSAPI::InitWithLegacyErrorReporting(
 // reports to the JSErrorReporter as soon as they are generated. These go
 // directly to the console, so we can handle them easily here.
 //
 // Eventually, SpiderMonkey will have a special-purpose callback for warnings
 // only.
 void
 WarningOnlyErrorReporter(JSContext* aCx, const char* aMessage, JSErrorReport* aRep)
 {
-  MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(JSREPORT_IS_WARNING(aRep->flags));
+  if (!NS_IsMainThread()) {
+    // Reporting a warning on workers is a bit complicated because we have to
+    // climb our parent chain until we get to the main thread.  So go ahead and
+    // just go through the worker ReportError codepath here.
+    //
+    // That said, it feels like we should be able to short-circuit things a bit
+    // here by posting an appropriate runnable to the main thread directly...
+    // Worth looking into sometime.
+    workers::WorkerPrivate* worker = workers::GetWorkerPrivateFromContext(aCx);
+    MOZ_ASSERT(worker);
+
+    worker->ReportError(aCx, aMessage, aRep);
+    return;
+  }
 
   RefPtr<xpc::ErrorReport> xpcReport = new xpc::ErrorReport();
   nsGlobalWindow* win = xpc::CurrentWindowOrNull(aCx);
   xpcReport->Init(aRep, aMessage, nsContentUtils::IsCallerChrome(),
                   win ? win->AsInner()->WindowID() : 0);
   xpcReport->LogToConsole();
 }
 
@@ -479,23 +493,17 @@ void
 AutoJSAPI::TakeOwnershipOfErrorReporting()
 {
   MOZ_ASSERT(!mOwnErrorReporting);
   mOwnErrorReporting = true;
 
   JSRuntime *rt = JS_GetRuntime(cx());
   mOldAutoJSAPIOwnsErrorReporting = JS::ContextOptionsRef(cx()).autoJSAPIOwnsErrorReporting();
   JS::ContextOptionsRef(cx()).setAutoJSAPIOwnsErrorReporting(true);
-  // Workers have their own error reporting mechanism which deals with warnings
-  // as well, so don't change the worker error reporter for now.  Once we switch
-  // all of workers to TakeOwnershipOfErrorReporting(), we will just make the
-  // default worker error reporter assert that it only sees warnings.
-  if (mIsMainThread) {
-    JS_SetErrorReporter(rt, WarningOnlyErrorReporter);
-  }
+  JS_SetErrorReporter(rt, WarningOnlyErrorReporter);
 }
 
 void
 AutoJSAPI::ReportException()
 {
   MOZ_ASSERT(OwnsErrorReporting(), "This is not our exception to report!");
   if (!HasException()) {
     return;
--- a/dom/workers/RuntimeService.cpp
+++ b/dom/workers/RuntimeService.cpp
@@ -554,25 +554,16 @@ LoadJSGCMemoryOptions(const char* aPrefN
     nsAutoCString message("Workers don't support the 'mem.");
     message.Append(memPrefName);
     message.AppendLiteral("' preference!");
     NS_WARNING(message.get());
 #endif
   }
 }
 
-void
-ErrorReporter(JSContext* aCx, const char* aMessage, JSErrorReport* aReport)
-{
-  WorkerPrivate* worker = GetWorkerPrivateFromContext(aCx);
-  MOZ_ASSERT(worker);
-
-  return worker->ReportError(aCx, aMessage, aReport);
-}
-
 bool
 InterruptCallback(JSContext* aCx)
 {
   WorkerPrivate* worker = GetWorkerPrivateFromContext(aCx);
   MOZ_ASSERT(worker);
 
   // Now is a good time to turn on profiling if it's pending.
   profiler_js_operation_callback();
@@ -807,18 +798,16 @@ CreateJSContextForWorker(WorkerPrivate* 
   JS::SetAsmJSCacheOps(aRuntime, &asmJSCacheOps);
 
   JSContext* workerCx = JS_NewContext(aRuntime, 0);
   if (!workerCx) {
     NS_WARNING("Could not create new context!");
     return nullptr;
   }
 
-  JS_SetErrorReporter(aRuntime, ErrorReporter);
-
   JS_SetInterruptCallback(aRuntime, InterruptCallback);
 
   js::SetCTypesActivityCallback(aRuntime, CTypesActivityCallback);
 
   JS::ContextOptionsRef(workerCx) = kRequiredContextOptions;
 
 #ifdef JS_GC_ZEAL
   JS_SetGCZeal(workerCx, settings.gcZeal, settings.gcZealFrequency);
--- a/xpcom/base/CycleCollectedJSRuntime.cpp
+++ b/xpcom/base/CycleCollectedJSRuntime.cpp
@@ -449,16 +449,22 @@ CycleCollectedJSRuntime::~CycleCollected
   nsCycleCollector_forgetJSRuntime();
 
   mozilla::dom::DestroyScriptSettings();
 
   mOwningThread->SetScriptObserver(nullptr);
   NS_RELEASE(mOwningThread);
 }
 
+static void
+MozCrashErrorReporter(JSContext*, const char*, JSErrorReport*)
+{
+  MOZ_CRASH("Why is someone touching JSAPI without an AutoJSAPI?");
+}
+
 nsresult
 CycleCollectedJSRuntime::Initialize(JSRuntime* aParentRuntime,
                                     uint32_t aMaxBytes,
                                     uint32_t aMaxNurseryBytes)
 {
   MOZ_ASSERT(!mJSRuntime);
 
   mOwningThread->SetScriptObserver(this);
@@ -492,16 +498,19 @@ CycleCollectedJSRuntime::Initialize(JSRu
 
   JS_SetObjectsTenuredCallback(mJSRuntime, JSObjectsTenuredCb, this);
   JS::SetOutOfMemoryCallback(mJSRuntime, OutOfMemoryCallback, this);
   JS::SetLargeAllocationFailureCallback(mJSRuntime,
                                         LargeAllocationFailureCallback, this);
   JS_SetContextCallback(mJSRuntime, ContextCallback, this);
   JS_SetDestroyZoneCallback(mJSRuntime, XPCStringConvert::FreeZoneCache);
   JS_SetSweepZoneCallback(mJSRuntime, XPCStringConvert::ClearZoneCache);
+  // XPCJSRuntime currently overrides this because we don't
+  // TakeOwnershipOfErrorReporting everwhere on the main thread yet.
+  JS_SetErrorReporter(mJSRuntime, MozCrashErrorReporter);
 
   static js::DOMCallbacks DOMcallbacks = {
     InstanceClassHasProtoAtDepth
   };
   SetDOMCallbacks(mJSRuntime, &DOMcallbacks);
 
 #ifdef SPIDERMONKEY_PROMISE
   JS::SetEnqueuePromiseJobCallback(mJSRuntime, EnqueuePromiseJobCallback, this);