Closed Bug 1064258 Opened 5 years ago Closed 5 years ago

Allow caching channels only store metadata

Categories

(Core :: Networking: HTTP, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla35

People

(Reporter: mayhemer, Assigned: mayhemer)

References

Details

Attachments

(1 file, 2 obsolete files)

No description provided.
Attached patch v1 (obsolete) — Splinter Review
- when storing new content, we close the output stream of the cache entry prior any writes happen (explained in the code why exactly)
- when loading a cached content, the channel first thinks the content is partial, hence we spacial-case for data-size == 0 && cache-only-metadata and go forward with loading the cached content with LOAD_ONLY_IF_MODIFIED flag set
- test included
Attachment #8485767 - Flags: review?(jduell.mcbugs)
Comment on attachment 8485767 [details] [diff] [review]
v1

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

Looks very good to me.

::: netwerk/test/unit/test_bug1064258.js
@@ +1,2 @@
> +/**
> + * Check how nsICachingChannel.cacheOnlyMetadata works.

really good notes--thanks! :)

@@ +101,5 @@
> +}
> +
> +function contentListener1b(request, buffer)
> +{
> +  do_check_eq(buffer, "");

Am I overly obsessive, or would it be nice to check in this function that the meta-data is present in the request?
Attachment #8485767 - Flags: review?(jduell.mcbugs) → review+
(In reply to Jason Duell (:jduell) from comment #2)
> > +function contentListener1b(request, buffer)
> > +{
> > +  do_check_eq(buffer, "");
> 
> Am I overly obsessive, or would it be nice to check in this function that
> the meta-data is present in the request?

Hmm.. and how would you do that? ;)  Checking the cacheToken?  The response status?
Attached patch v1 -> v1.1 idiff (obsolete) — Splinter Review
We missed something :)

https://tbpl.mozilla.org/?tree=Try&rev=2269db631ced
Attachment #8490025 - Flags: review?(jduell.mcbugs)
Attachment #8490025 - Flags: review?(jduell.mcbugs) → review+
(In reply to Ed Morley [:edmorley] from comment #6)
> Reverted for xpcshell failures:
> https://tbpl.mozilla.org/php/getParsedLog.php?id=49119912&tree=Mozilla-
> Inbound
> https://tbpl.mozilla.org/php/getParsedLog.php?id=49120971&tree=Mozilla-
> Inbound
> 
> https://hg.mozilla.org/integration/mozilla-inbound/rev/da12cd0ebe40

Grrr!!  Old version landed accidentally.  Going to land the final version.
https://hg.mozilla.org/mozilla-central/rev/62832afa4438
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Flags: qe-verify-
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.