Closed Bug 1423550 Opened 2 years ago Closed 2 years ago

A load with an assigned application cache matching a NETWORK namespace doesn't lookup a regular HTTP cache entry

Categories

(Core :: Networking: HTTP, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: mayhemer, Assigned: mayhemer)

Details

(Whiteboard: [necko-triaged])

Attachments

(1 file, 1 obsolete file)

Reported via mozilla-microsoft-discuss forum.

When a sub-resource load from a context for which an appcache is assigned matches a NETWORK namespace, we don't query the regular HTTP cache for an entry.  This violates the spec and causes perf issues.

The fix would be to split (with few small tweaks) nsHttpChannel::OpenCacheEntry and call its second part w/o a reference to appcache again from nsHttpChannel::OnOfflineCacheEntryAvailable when NAMESPACE_BYPASS matches.
Copied from the original forum post:

Here is the minimal repro link:

https://ffappcache.azurewebsites.net/master2.html


Steps:

    Load above url in firefox
    It will install appcache (will cache 3 resources master2.html, repo2/jquery, repo2/1.js)
    Reload above url (F5)


Expected:

    master2.html will load from appcache
    jquery, 1.js will load from appcache
    2.js should load from browser cache


Actual:

    master2.html will load from appcache
    jquery, 1.js will load from appcache
    2.js is loaded from network



Now if you load below url again, which skips appcache all the resources are loaded from browser cache (including 2.js)
https://ffappcache.azurewebsites.net/master2.html?bO=1


This seems like an issue as other browser are working as per the below spec.

https://www.w3.org/TR/2011/WD-html5-20110525/offline.html#changesToNetworkingModel

For resource 2.js we will hit step 5 (from above spec) and should fetch the resource normally and abort the app-cache steps.

Can you please take a look at this issue.

Thanks.


Credit: Mihir Ray
Attached patch v1 (obsolete) — Splinter Review
This simply splits OpenCacheEntry to two halves.  When we match NETWORK names (internally still called BYPASS according the very first spec) we don't go to the network regularly but first check for existing HTTP cache entry.  This works well (no issue with wait flags) and preserves all code paths and checks.

I don't have time to intro an automated test for this, hence only tested manually.

let's see what breaks:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cf6d616fddc6d115f35c1bd750f115dd55823daa
Attachment #8934966 - Flags: review?(michal.novotny)
Attachment #8934966 - Flags: review?(michal.novotny) → review+
Attached patch v1.0.1Splinter Review
just minor change to a comment.
Attachment #8934966 - Attachment is obsolete: true
Attachment #8940748 - Flags: review+
Keywords: checkin-needed
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5a503850a497
Lookup regula HTTP cache on appcache NETWORK namespace match, r=michal
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/5a503850a497
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.