585244 - IPC DNS prefetching shouldn't bother sending zero-length hostnames

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Josh Matthews [:jdm]]

I end up seeing lots of these errors when browsing around in fennec:

WARNING: NS_ENSURE_TRUE(host && *host) failed: file /home/t_mattjo/src/firefox/mobilebase/netwerk/dns/nsHostResolver.cpp, line 501

They come from attempting to prefetch zero-length hostnames, such as javascript: links.  We know they're going to fail, so we might as well not bother sending zer-length hostnames over the wire.

Comment 1

[Josh Matthews [:jdm]]

Attachment: Patch

Comment 2

[Doug Turner (:dougt)]

Comment on attachment 463746 [details] [diff] [review]
Patch

How about using net_IsValidHostName?

Comment 3

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: Don't attempt to prefetch invalid hostnames from content processes

net_IsValidHostName() considers the empty string to be a valid hostname, so we still need to filter that.  However, net_IsValidHostName() is a good additional check to make.

Comment 4

[Doug Turner (:dougt)]

Comment on attachment 464725 [details] [diff] [review]
Don't attempt to prefetch invalid hostnames from content processes

I think we are all unhappy with the string conversions, and we are also unhappy with net_IsValidHostName(null|"") returning true.  But this patch improves what we have.

ooc, why the #ifdefs around:

+using namespace mozilla::net;

Does it hurt anything to not wrap it?

Comment 5

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

(In reply to comment #4)
> Comment on attachment 464725 [details] [diff] [review]
> Don't attempt to prefetch invalid hostnames from content processes
>
> ooc, why the #ifdefs around:
> 
> +using namespace mozilla::net;
> 
> Does it hurt anything to not wrap it?

I thought mozilla::net was all conditionally-declared necko IPC stuff (and if so, |using mozilla::net| would be a compile error), but I see that we forward-declare some necko things in mozilla::net outside of |ifdef MOZ_IPC|.  Good call, will fix.

Comment 6

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 464725 [details] [diff] [review]
Don't attempt to prefetch invalid hostnames from content processes

This is a safe patch that can eliminate a lot of wasted IPC traffic: ~70 messages while loading news.google.com, e.g.  (We might want to do something smarter for valid hostnames later, too, but that's a bug for another day.)

Comment 7

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

http://hg.mozilla.org/projects/cedar/rev/70094380fcbb