Closed Bug 493474 Opened 12 years ago Closed 10 years ago

Consider enabling DNS prefetch in camino

Categories

(Camino Graveyard :: General, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: bzbarsky, Assigned: cpeterson)

References

Details

(Whiteboard: 1.9.1)

Attachments

(1 file)

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.
Thanks for the heads-up. This is all 1.9.1+ though, yes?
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.
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".
Attachment #476724 - Flags: review?(stuart.morgan+bugzilla)
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.
Assignee: nobody → mozilla.org
Status: NEW → ASSIGNED
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.
Attachment #476724 - Flags: superreview?(mikepinkerton)
Attachment #476724 - Flags: review?(stuart.morgan+bugzilla)
Attachment #476724 - Flags: review+
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.
Attachment #476724 - Flags: superreview?(mikepinkerton)
Comment on attachment 476724 [details] [diff] [review]
enable-dns-prefetch-v1.patch

sr=pink
Attachment #476724 - Flags: superreview?(mikepinkerton) → superreview+
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.
(And by "pref" I meant the pair of prefetch prefs.)
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").
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
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
(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.
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 :(
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.
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.
Flags: camino2.1.1+
Whiteboard: 1.9.1 → 1.9.1 [camino-2.1.1]
(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.
Flags: camino2.1.1+
Whiteboard: 1.9.1 [camino-2.1.1] → 1.9.1
(Also, we should have caught the usage of string/name instead of constant for the pref, too :( All-around-fail on this one.)
You need to log in before you can comment on or make changes to this bug.