545407 - Remove code that disables DNS pre-fetching for APP_TYPE_MAIL/EDITOR docshells

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Mark Banner (:standard8)]

Bug 492196 did some code to allow user-defined policies on DNS prefetching.

It attempted to make it so that if APP_TYPE_MAIL or APP_TYPE_EDITOR was set at the root docShell level, it would disable DNS prefetching down the docShell tree.

Unfortunately we don't set APP_TYPE_* early enough, so for the message pane, the DNS prefetching never gets disabled (xref bug 535976 and bug 544745). However for content tabs in Thunderbird it does get disabled.

Therefore we'd like just to remove this inheritance. As the two bugs cited will be fixing the setting on the message pane, removing the broken inheritance will remove redundant code, and allow us to gain the benefit of DNS prefetching on content tabs (which we're encouraging for extensions).

Comment 1

[Boris Zbarsky [:bzbarsky]]

Do you want to remove the _inheritance_, or the fact that APP_TYPE_MAIL disables dns prefetch?

I'm happy removing the latter if you don't want it.  I think the former is desirable, though (otherwise one just needs to stick an iframe in the docshell to get dns prefetching happening).

Comment 2

[Mark Banner (:standard8)]

Yes, my mistake, we'll just remove the code that disables prefetch for APP_TYPE_MAIL/EDITOR.

Comment 3

[Boris Zbarsky [:bzbarsky]]

OK, next question.  How does that work with Seamonkey?  And does Thunderbird actually use the APP_TYPE_MAIL thing for anything other than the window.opener stuff?

Comment 4

[[e-mail]]

The mailWindow.js part of attachment 426246 [details] [diff] [review] is just a straight port. For compose we should maybe disable prefetch in EditorSharedStartup (which would also affect both compose windows too.)

Comment 5

[Mark Banner (:standard8)]

(In reply to comment #3)
> OK, next question.  How does that work with Seamonkey?  And does Thunderbird
> actually use the APP_TYPE_MAIL thing for anything other than the window.opener
> stuff?

Well SeaMonkey is broken as well (bug 535976). SeaMonkey can use pretty much exactly the same patch as I'm providing in bug 544745 attachment 426246 [details] [diff] [review] (I just haven't ported it yet) - which is to set allowDNSPrefetch on the messagepane/editor docshells where we're going to display messages.

We use APP_TYPE_MAIL in various other situations, e.g. the mailnews specific content policy (though only SM uses it there), disabling HREF pre-fetching and the window.opener stuff.

However in all these other situations, the code is working up the docshell tree to the root docshell to discover if APP_TYPE_MAIL is set there and hence it doesn't matter if APP_TYPE_MAIL is set slightly later.

Comment 6

[Boris Zbarsky [:bzbarsky]]

> disabling HREF pre-fetching

Which doesn't work (what this bug is about), no?

But ok.  If it doesn't break Seamonkey, I'm happy to review a patch to remove the MAIL special-casing in docshell here.

Comment 7

[Mark Banner (:standard8)]

(In reply to comment #6)
> > disabling HREF pre-fetching
> 
> Which doesn't work (what this bug is about), no?

Nope, this is about DNS pre-fetching. HREF pre-fetching is something different AFAICT: http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsContentSink.cpp#851

Comment 8

[Boris Zbarsky [:bzbarsky]]

Ah, indeed.  We should move that to a frob on docshell too.  Separate bug, of course.

Comment 9

[Mark Banner (:standard8)]

Attachment: The fix

Removes the code to disable DNS prefetch in APP_TYPE_MAIL and APP_TYPE_EDITOR windows as previously discussed.

I'll file a separate bug for the patch that's required to stop the Thunderbird mozmill tests breaking.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 428421 [details] [diff] [review]
The fix

r=bzbarsky.  This doesn't need sr.

Comment 11

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/mozilla-central/rev/8163dc10e3da

Comment 12

[Mark Banner (:standard8)]

(In reply to comment #8)
> Ah, indeed.  We should move that to a frob on docshell too.  Separate bug, of
> course.

I raised bug 551423.