1252565 - Make dom::WarningOnlyErrorReporter handle workers

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Now that we've done bug 1072144 we can do that.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Make dom::WarningOnlyErrorReporter handle workers

Comment 2

[Bobby Holley (:bholley)]

Comment on attachment 8725361 [details] [diff] [review]
Make dom::WarningOnlyErrorReporter handle workers

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

::: dom/workers/RuntimeService.cpp
@@ +802,5 @@
>      NS_WARNING("Could not create new context!");
>      return nullptr;
>    }
>  
> +  JS_SetErrorReporter(aRuntime, WarningOnlyErrorReporter);

Per IRC discussion, I think we should let ScriptSettings take care of this.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Make dom::WarningOnlyErrorReporter handle workers

Comment 4

[Bobby Holley (:bholley)]

Comment on attachment 8725386 [details] [diff] [review]
Make dom::WarningOnlyErrorReporter handle workers

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

::: xpcom/base/CycleCollectedJSRuntime.cpp
@@ +502,5 @@
>                                          LargeAllocationFailureCallback, this);
>    JS_SetContextCallback(mJSRuntime, ContextCallback, this);
>    JS_SetDestroyZoneCallback(mJSRuntime, XPCStringConvert::FreeZoneCache);
>    JS_SetSweepZoneCallback(mJSRuntime, XPCStringConvert::ClearZoneCache);
> +  JS_SetErrorReporter(mJSRuntime, MozCrashErrorReporter);

Add a comment here saying that XPCJSRuntime currently override this because we don't TakeOwnership everwhere on the main thread yet.

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/96580db9b356

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out along with the other things in the push for these failures in test_recusion.html: https://treeherder.mozilla.org/logviewer.html#?job_id=22804630&repo=mozilla-inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/9716b46f3b60

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I also see this failure: https://treeherder.mozilla.org/logviewer.html#?job_id=22807509&repo=mozilla-inbound

Comment 8

[Boris Zbarsky [:bzbarsky]]

Yep, the test_ctypes.xul fail is from this bug.  Will look into it.

Comment 9

[Boris Zbarsky [:bzbarsky]]

That failure happens because we have a test for using ctypes from a worker.  And ctypes uses js::PrepareScriptEnvironmentAndInvoke, which will report exceptions itself if there is no script environment preparer.  And right now we only set up an environment preparer for XPCJSRuntime.

I'm going to push that up to CycleCollectedJSRuntime, I think.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Push the script environment preparer bits up from XPCJSRuntime to CycleCollectedJSRuntime, because we need them on workers to do ctypes on workers properly

Note that I made some changes to the EnvironmentPreparer::invoke() impl to support running it on non-main threads.  So it's not just a cut/paste job there...

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fc5c4cb02d24
https://hg.mozilla.org/integration/mozilla-inbound/rev/9de2c10a1cc3

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Out again in https://hg.mozilla.org/integration/mozilla-inbound/rev/d7608e766470 for windows build bustage:
https://treeherder.mozilla.org/logviewer.html#?job_id=22861702&repo=mozilla-inbound

Comment 13

[Boris Zbarsky [:bzbarsky]]

Ugh, I suck.  I left in some debugging code...  Will reland with that taken out once the tree is reopened.

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e0a06595b43c
https://hg.mozilla.org/integration/mozilla-inbound/rev/a74dae924b1f

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e0a06595b43c
https://hg.mozilla.org/mozilla-central/rev/a74dae924b1f