1180091 - make sure http cache follows normal HTTP semantics for signed packages

Status: RESOLVED WONTFIX

(Core :: Networking, defect, P2)

Description

[Paul Theriault [:pauljt] (no longer reading bugmail)]

Signed packages follow normal http semantics. I.e. if the package still exists in our http cache when the user revisits a signed page, but the cache headers indicate that the content needs to be updated, we do a normal GET request to see if a new version needs to be downloaded.

Comment 1

[Paul Theriault [:pauljt] (no longer reading bugmail)]

This might already be done, assigning to myself to verify.

Comment 2

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

Attachment: make sure http cache follows normal HTTP semantics for signed packages

test_package_caching() contains a bunch of tests to make sure nsIRequest flags are handled properly.
I don't test INHIBIT_CACHING or INHIBIT_PERSISTENT_CACHING since these flags don't really make sense in this context and should not be used. I'm considering throwing an error if they are used

Comment 3

[Paul Theriault [:pauljt] (no longer reading bugmail)]

(In reply to Valentin Gosu [:valentin] from comment #2)
> Created attachment 8646749 [details] [diff] [review]
> make sure http cache follows normal HTTP semantics for signed packages
> 
> test_package_caching() contains a bunch of tests to make sure nsIRequest
> flags are handled properly.
> I don't test INHIBIT_CACHING or INHIBIT_PERSISTENT_CACHING since these flags
> don't really make sense in this context and should not be used. I'm
> considering throwing an error if they are used

That sounds reasonable to me.

Comment 4

[Jason Duell]

Can we get review on this or find another reviewer?

Comment 5

[Paul Theriault [:pauljt] (no longer reading bugmail)]

Sorry, I'm not usually a reviewer. The tests look good to me, but I'm probably not qualified to review.

Comment 6

[Paul Theriault [:pauljt] (no longer reading bugmail)]

Henry, can you take a look at this please.

Comment 7

[Henry Chang [:hchang]]

I think I am also not qualified to review but the test cases look well test the requirements :) 

p.s. The use of |getResource| in the test cases needs to be updated to pass just channel and the callback in.

Comment 8

[Paul Theriault [:pauljt] (no longer reading bugmail)]

Comment on attachment 8646749 [details] [diff] [review]
make sure http cache follows normal HTTP semantics for signed packages

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

As above, this looks ok I'm not qualified to review this. Jonas can you review, or help find someone to do this? I'm not sure who to reach out to at this point.

Comment 9

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

Attachment: make sure http cache follows normal HTTP semantics for signed packages

Rebased against latest changes

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 8657973 [details] [diff] [review]
make sure http cache follows normal HTTP semantics for signed packages

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

I'm not quite following all that's going on in this test. (And I generally find it hard and not quite worth the time to review tests in detail). Mind grabbing me on irc and walking me through it?

Comment 11

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

Sure thing. How does Monday sound? Ping me when you're available.

Comment 12

[Paul Theriault [:pauljt] (no longer reading bugmail)]

I don't think this needs to block 2.5 but it would be nice to get fixed.

Comment 13

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

B2G code is going away.