493474 - Consider enabling DNS prefetch in camino

Status: RESOLVED FIXED

(Camino Graveyard :: General, defect)

Description

[Boris Zbarsky [:bzbarsky]]

The frozen embedding API doesn't enable DNS prefetch by default; see bug 492196 comment 16.  Camino might want to enable it in their browser docshells.

Comment 1

[Stuart Morgan]

Thanks for the heads-up. This is all 1.9.1+ though, yes?

Comment 2

[Boris Zbarsky [:bzbarsky]]

That's correct.  So it doesn't affect 1.9.0-based camino at all; I just figured I'd let you guys know for when you switch to 1.9.1.

Comment 3

[Chris Peterson [:cpeterson]]

Attachment: enable-dns-prefetch-v1.patch

This patch enables Gecko's DNS prefetch by default (like Firefox). DNS prefetch can be disabled in about:config "network.dns.disablePrefetch" and "network.dns.disablePrefetchFromHTTPS".

Comment 4

[Chris Peterson [:cpeterson]]

I have confirmed that Gecko is now using its DNS prefetching code, but I am not sure how to quantify the performance benefit for Camino.

Comment 5

[Stuart Morgan]

Comment on attachment 476724 [details] [diff] [review]
enable-dns-prefetch-v1.patch

r=me. There's no change listener, but since we don't have UI for this pref I'm fine with changes only applying to new browser instances.

Comment 6

[Stuart Morgan]

Comment on attachment 476724 [details] [diff] [review]
enable-dns-prefetch-v1.patch

r=me. There's no change listener, but since we don't have UI for this pref I'm fine with changes only applying to new browser instances.

Comment 7

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 476724 [details] [diff] [review]
enable-dns-prefetch-v1.patch

sr=pink

Comment 8

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I didn't look at this patch before (sorry :( ), but I don't like that there's a Gecko pref that Camino itself checks for/obeys that doesn't exist by default and that didn't get added to all-camino.js (it also bypasses the GeckoPrefConstants stuff, but I don't know what the rules are for using that).

I understand that this is a slightly special case in that the pref is only to shut things off and that it's neither a pref used by Camino for Camino UI nor a pref read automatically by Gecko, but in order for someone to turn this off, they have to 1) quit Camino 2) edit user.js/prefs.js manually 3) restart Camino.  Even for something we don't expect users to turn off, this seems unnecessarily onerous, especially when we could just set the default values in all-camino.js and be done with it.

Comment 9

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(And by "pref" I meant the pair of prefetch prefs.)

Comment 10

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

http://wiki.caminobrowser.org/Status_Meetings:2010-11-03:Log#16:19

[4:19pm] smorgan: Oops :(
[4:19pm] smorgan: I should have caught that
[4:19pm] smorgan: Go ahead and add it; sr=me

I'll add the prefs to all-camino.js on checkin (I assume from their names that the default values, or at least the values we want to set, are "false").

Comment 11

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

http://hg.mozilla.org/camino/rev/009b114d9fa7 with the prefs added and set to false (confirmed with mZ's kb that false is, as the names imply, the default/desired setting).

Hoping this doesn't make bug 351678 worse /fingers crossed ;)


> fine with changes only applying to new browser instances.

For documentation purposes, "browser instances" here means "new tab" rather than the "new window" that camino.enable_plugins requires[1], right?

[1] http://caminobrowser.org/documentation/hiddenprefs/#DisablePlugins

Comment 12

[Stuart Morgan]

(In reply to comment #11)
> For documentation purposes, "browser instances" here means "new tab" rather
> than the "new window" that camino.enable_plugins requires[1], right?

Correct.

Comment 13

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

So, uh, the value for HTTPS is supposed to be "true", or off (the mZ pref documentation I consulted in comment 11 apparently didn't cover the HTTPS pref nor mention it behaved differently):
http://bitsup.blogspot.com/2008/11/dns-prefetching-for-firefox.html
https://developer.mozilla.org/En/Controlling_DNS_prefetching
and the massively-unclear-too-many-levels-of-indirection http://mxr.mozilla.org/mozilla1.9.2/source/content/html/content/src/nsHTMLDNSPrefetch.cpp#89-94 (which apparently means "read the pref using the ContentUtils method that sets a pref to false if not found and adds an observer, then read the pref again using the ContentUtils method that the previous ContentUtils method uses, but set the pref to true if not found).

So we need to change the default pref for HTTPS and consider respinning :(

Comment 14

[Stuart Morgan]

Ugh. sr=me on changing the default in all-camino.js

Respinning wouldn't be a bad idea, but this isn't so terrible that I think we couldn't wait for 2.1.1 instead if you want to avoid the extra work of the respin.

Comment 15

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

http://hg.mozilla.org/camino/rev/871f5baef229 to turn prefetch back off for HTTPS for 2.1.1; we'll make the call for 2.1 as things develop.

Comment 16

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #15)
> http://hg.mozilla.org/camino/rev/871f5baef229 to turn prefetch back off for
> HTTPS for 2.1.1; we'll make the call for 2.1 as things develop.

http://hg.mozilla.org/camino/rev/e9b6607c5fe9 on CAMINO_2_1_MINIBRANCH, since we'll be respinning to pick up the Gecko fixes.

Comment 17

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(Also, we should have caught the usage of string/name instead of constant for the pref, too :( All-around-fail on this one.)