Closed Bug 459724 Opened 12 years ago Closed 11 years ago

DNS: page reload should set bypass cache flag

Categories

(Core :: Networking, defect)

x86
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9.1b2

People

(Reporter: mcmanus, Assigned: mcmanus)

References

Details

Attachments

(2 files, 3 obsolete files)

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.0.3) Gecko/2008092515 Ubuntu/8.10 (intrepid) Firefox/3.0.1
Build Identifier: 

this is a follow on bug to 208312

The DNS IDL defines RESOLVE_BYPASS_CACHE as a flag to bypass the DNS cache, but no code in mozilla-central invokes this.

A manual reload should force the cache entry to be refreshed.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.
Status: UNCONFIRMED → NEW
Ever confirmed: true
This bites me all the time, so would love a fix for this! :)
Flags: wanted1.9.1?
RES_BYPASS_CACHE, not RESOLVE_BYPASS_CACHE, correct?
n/m, I see there's both in the code.
just to be clear, this bug addresses bypassing of the mozilla dns cache service. Any caching resolver embedded in the OS in the local lookup chain will still be used.
This patch fixes a problem with the normalization of flags used in the DNS cache. 

While a bypass flag is already implemented in the class and in the DNS service IDL, the C++ implementation does not normalize it correctly. 

This patch is also included as part of the patch attached to 453403 (pre-fetch dns) as the bug gets in the way of any flag usage (and 453403 adds new flags). I'll update that bug as well to break it out there too.

This patch is necessary, but not sufficient, for closing this bug.
Attachment #343148 - Flags: review?(bzbarsky)
Attachment #343149 - Flags: review?(bzbarsky)
Comment on attachment 343149 [details] [diff] [review]
reload action will bypass dns cache

>+++ b/netwerk/base/public/nsISocketTransport.idl
>+    // connection flags

Probably document that these can be used to modify the behavior of the underlying socket connection?

>+    attribute unsigned long connectionFlags;
>+    const unsigned long BYPASS_CACHE = (1 << 0);

I'd separate this out with a comment like:

  // Values for connectionFlags

and then document what BYPASS_CACHE actually does if we can do that usefully (what cache does it bypass, exactly?).

>+++ b/netwerk/base/src/nsSocketTransport2.cpp
>@@ -759,17 +760,17 @@ nsSocketTransport::Init(const char **typ
>-
>+    

Please lose the whitespace change.

>+    rv = dns->AsyncResolve(SocketHost(), (mConnectionFlags & nsSocketTransport::BYPASS_CACHE) ? nsIDNSService::RESOLVE_BYPASS_CACHE : 0,

Can we keep this inside 80 chars?  Maybe have a dnsFlags temporary?

With those nits, looks good.  Sorry for the lag on this...
Attachment #343149 - Flags: superreview+
Attachment #343149 - Flags: review?(bzbarsky)
Attachment #343149 - Flags: review+
Attachment #343148 - Flags: superreview+
Attachment #343148 - Flags: review?(bzbarsky)
Attachment #343148 - Flags: review+
Comment on attachment 343148 [details] [diff] [review]
Fix normalization of flags in DNS cache

> HostDB_MatchEntry(PLDHashTable *table,
>-            he->rec->flags == hk->flags &&
>+        RES_KEY_FLAGS (he->rec->flags) == RES_KEY_FLAGS(hk->flags) &&

Fix the indent to match what we used to have here, and looks good.
addresses review comments from comment 8
Attachment #343148 - Attachment is obsolete: true
address review comments from comment 7. Thanks for taking a look.
Attachment #343149 - Attachment is obsolete: true
Comment on attachment 344481 [details] [diff] [review]
reload action will bypass dns cache (v2)

+     * BYPASS_CACHE will force the Necko DNS cache entry to be refreshed with
+     * a new call to NSPR

can you document that this has to be set before opening a stream?
update documentation to reflect Christian's comment in comment 11
Attachment #344481 - Attachment is obsolete: true
Blocks: 453403
Keywords: checkin-needed
Pushed changeset 4542793088dc for the flag normalization patch and changeset 39fd2b88d622 for the actual fix.
Assignee: nobody → mcmanus
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Attachment #344472 - Attachment description: Fix Normalization of flags in DNS Cache (v2) → Fix Normalization of flags in DNS Cache (v2) [Checkin: Comment 13]
Attachment #344781 - Attachment description: reload action will bypass dns cache (v3) → reload action will bypass dns cache (v3) [Checkin: Comment 13]
Keywords: checkin-needed
Target Milestone: --- → mozilla1.9.1b2
Version: unspecified → Trunk
You need to log in before you can comment on or make changes to this bug.