1377484 - Label the use of NS_NewInputStreamPump in CacheScriptLoader.

Status: RESOLVED FIXED

(Core :: DOM: Workers, enhancement)

Description

[Bevis Tseng[:bevis][:btseng](Exited)]

NS_NewInputStreamPump() is used in CacheScriptLoader on the main thread. [1]
Besides, CacheScriptLoader is associated with a WorkerPrivate when creating [2], so we could use the WorkerPrivate::MainThreadEventTarget() for the labeling of NS_NewInputStreamPump().


[1] http://searchfox.org/mozilla-central/rev/17ebac68112bd635b458e831670c1e506ebc17ad/dom/workers/ScriptLoader.cpp#1704
[2] http://searchfox.org/mozilla-central/rev/17ebac68112bd635b458e831670c1e506ebc17ad/dom/workers/WorkerPrivate.cpp#5157

Comment 1

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: (v1) Patch: Label the use of NS_NewInputStreamPump in CacheScriptLoader.

Label the use of NS_NewInputStreamPump() with WorkerPrivate::MainThreadEventTarget().

Treeherder result looks good:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=92425608bb35c170f018c01a1da19149a61523a6

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8882777 [details] [diff] [review]
(v1) Patch: Label the use of NS_NewInputStreamPump in CacheScriptLoader.

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

Note, this will cause this data to load through a ThrottledEventQueue when it did not previously.  I think this is ok as I'd rather delay worker script load than jank the main thread if its busy.

Comment 3

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Ben Kelly [reviewing, but slowly][:bkelly] from comment #2)
> Note, this will cause this data to load through a ThrottledEventQueue when
> it did not previously.  I think this is ok as I'd rather delay worker script
> load than jank the main thread if its busy.

I have the same concern when modifying this. (I should rise my concern directly when asking the review. :\)
Thanks for your clarification on this! :)

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

In the future we may want to see if the data load can be redirected to a non-main thread target now that e10s necko supports that.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3d384edb68e6
Label the use of NS_NewInputStreamPump in CacheScriptLoader. r=bkelly

Comment 6

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/3d384edb68e6