1057688 - per-app-offline: figure out why DNS sometimes still causes network traffic when offline

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Jason Duell]

Valentin also observed this.  Not critical because it's so little traffic, but it will cause the phone's network throbber to go off if nothing else, which is confusing (and also possible wake up the radio). So this is something to fix soon.

Comment 1

[Valentin Gosu [:valentin] (he/him)]

http://pastebin.mozilla.org/6266330

The issue seems to be that in nsHttpChannel::BeginConnect, we do a prefetch, regardless of the LOAD_NO_NETWORK_IO or LOAD_ONLY_FROM_CACHE flags being set. (nsHttpChannel.cpp:4613)

I'm looking into a way to fix this.

Comment 2

[Valentin Gosu [:valentin] (he/him)]

Attachment: nsHttpChannels still do DNSPrefetch when LOAD_NO_NETWORK_IO flag is present

Comment 3

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8483201 [details] [diff] [review]
nsHttpChannels still do DNSPrefetch when LOAD_NO_NETWORK_IO flag is present

This seems to get rid of the extra DNS request, but I'm not sure this is correct. Comments?

Comment 4

[Patrick McManus [:mcmanus]]

Comment on attachment 8483201 [details] [diff] [review]
nsHttpChannels still do DNSPrefetch when LOAD_NO_NETWORK_IO flag is present

Review of attachment 8483201 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/protocol/http/nsHttpChannel.cpp
@@ +4595,5 @@
>      if (mLoadFlags & VALIDATE_ALWAYS || BYPASS_LOCAL_CACHE(mLoadFlags))
>          mCaps |= NS_HTTP_REFRESH_DNS;
>  
> +    if (!mConnectionInfo->UsingHttpProxy() &&
> +        !(mLoadFlags & LOAD_NO_NETWORK_IO)) {

yes.. might as well make it mLoadFlags & (NO_NETWORK_IO | ONLY_FROM_CACHE)

@@ +4614,5 @@
>          mDNSPrefetch->PrefetchHigh(mCaps & NS_HTTP_REFRESH_DNS);
>      }
>  
> +    if (mLoadFlags & LOAD_NO_NETWORK_IO) {
> +        OnLookupComplete(nullptr, nullptr, NS_ERROR_NOT_CONNECTED);

I'm trying to figure out why you did this, and why it's not just the else() clause?

I think all it will accomplish is ~=NS_HTTP_REFRESH_DNS which shouldn't impact the NO_NETWORK_IO case anyhow after your patch...

Comment 5

[Honza Bambas (:mayhemer)]

Comment on attachment 8483201 [details] [diff] [review]
nsHttpChannels still do DNSPrefetch when LOAD_NO_NETWORK_IO flag is present

Review of attachment 8483201 [details] [diff] [review]:
-----------------------------------------------------------------

Yep and I please honor Patrick's comments, I agree with them and don't see a purpose of manual call to OnLookupComplete.

Comment 6

[Valentin Gosu [:valentin] (he/him)]

Attachment: Skip doing a DNSPrefetch when LOAD_NO_NETWORK_IO or ONLY_FROM_CACHE flags are present

Comment 7

[Valentin Gosu [:valentin] (he/him)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0c905641f867

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/0c905641f867