Closed Bug 1250291 Opened 8 years ago Closed 8 years ago

Get rid of MaybeReportMainThreadException

Categories

(Core :: DOM: Workers, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

Attachments

(3 files)

It looks like it's reporting things, but it's a lie.  An inconsistently applied lie, afaict.
Comment on attachment 8722150 [details] [diff] [review]
part 1.  Stop pretending to report exceptions in MainThreadStopSyncLoopRunnable::PostDispatch

Review of attachment 8722150 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/workers/WorkerRunnable.cpp
@@ +532,5 @@
> +  //
> +  // Our PreDispatch always returns true and is final.  We inherit
> +  // DispatchInternal from StopSyncLoopRunnable, and that itself is final and
> +  // only returns false if dispatch to the syncloop target fails.  So we can
> +  // never have a JS exception here.

FWIW, I'm fairly confident that dispatching to the sync loop target (nsNestedEventTarget) can never fail, but that makes a lot of assumptions about nsThread internals that we probably shouldn't do here.
Attachment #8722150 - Flags: review?(khuey) → review+
Comment on attachment 8722151 [details] [diff] [review]
part 2.  Stop pretending to report exceptions in MainThreadWorkerSyncRunnable::PostDispatch

Review of attachment 8722151 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/workers/XMLHttpRequest.cpp
@@ +1181,5 @@
> +  DebugOnly<bool> ok =
> +    jsapi.Init(xpc::NativeGlobal(JS::CurrentGlobalOrNull(aCx)), aCx);
> +  MOZ_ASSERT(ok);
> +  MOZ_ASSERT(jsapi.cx() == aCx);
> +  jsapi.TakeOwnershipOfErrorReporting();

So there's JSAPI in here that can definitely throw. (e.g. JS_NewArrayObject)  You correctly point out that we weren't reporting that exception before (because PreDispatch still returns true) but presumably we need to do *something* with it ...?
Attachment #8722151 - Flags: review?(khuey) → review+
> but presumably we need to do *something* with it ...?

Sure.  ~AutoJSAPI will report it, since we called TakeOwnershipOfErrorReporting().
You need to log in before you can comment on or make changes to this bug.