632490 - Use a content-type hint for the channel in mozJSSubScriptLoader::LoadSubScript

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Matt Brubeck (:mbrubeck)]

Attachment: patch

Bug 632177 shows that GetMIMEInfoFromOS can be very expensive for Fennec.  This function is called by nsFileChannel::MakeFileInputStream, which is triggered by NS_OpenURI when a file: URI is passed to LoadSubScript.

This patch avoids the expensive GetMIMEInfoFromOS call by setting a MIME-type "hint" on the channel.

Nominating for blocking-fennec because this impacts Fennec startup time (which makes heavy use of LoadSubScript).  With this patch plus the patch from bug 	632417, I can get all the way through Fennec startup without any calls to GetMIMEInfoFromOS.

Comment 1

[Mike Shaver (:shaver emeritus)]

Comment on attachment 510695 [details] [diff] [review]
patch

>-    rv = NS_OpenURI(getter_AddRefs(instream), uri, serv,
>-                    nsnull, nsnull, nsIRequest::LOAD_NORMAL,
>-                    getter_AddRefs(chan));
>+    }
>+
>+    rv = NS_NewChannel(getter_AddRefs(chan), uri, serv,
>+                       nsnull, nsnull, nsIRequest::LOAD_NORMAL);
>+    if (NS_SUCCEEDED(rv))
>+    {
>+        chan->SetContentType(NS_LITERAL_CSTRING("application/javascript"));
>+        rv = chan->Open(getter_AddRefs(instream));
>+    }
>+

Add a comment here explaining why we don't use the more-concise OpenURI (which would probably like a content-hint overload later?), and r=shaver.

Comment 2

[(dormant account)]

(In reply to comment #1)

> 
> Add a comment here explaining why we don't use the more-concise OpenURI (which
> would probably like a content-hint overload later?), and r=shaver.

What would be even nice is having  NS_OpenURI do a WARNING() that it's slow :)

Comment 3

[Matt Brubeck (:mbrubeck)]

Attachment: patch for checkin

Comment 4

[Matt Brubeck (:mbrubeck)]

This looked good on try; pushing to m-c:

http://hg.mozilla.org/mozilla-central/rev/682d008059c6

Comment 5

[Marco Bonardo [:mak]]

looks like checkin-needed is not useful anymore? Set it back if I'm wrong.