Closed Bug 823648 Opened 12 years ago Closed 6 years ago

We should support Last-Modified headers in addition to ETag headers for manifest and package downloads


(Core Graveyard :: DOM: Apps, defect)

Gonk (Firefox OS)
Not set



Tracking Status
b2g18 + ---


(Reporter: julienw, Unassigned)



(1 obsolete file)

The rationale outlined in Bug 772364 comment 10 do not apply here :
* we don't care about subseconds problems, we definitely won't update less than one second after the download.
* clock warp problems are negligible (same device/same server). I can live with this risk.
* no need to parse, we just need to store the value of l-m as a string, and output the same value in i-m-s, just like ETag.

The Marketplace is generating a sensible ETag but not all apps will come from the Marketplace and I'm quite sure that we will hit this problem. So let's just store both if they're both present.

And because that's the way the web works since HTTP 1.1.
applies on b2g18

We should probably wait for Bug 820630 and add the support for package downloads as well. Or fix this now and support this header in Bug 820630.
Attachment #694508 - Flags: feedback?(fabrice)
What we want to know is if the *content* of the file has changed. This is not what LastModified gives us especially with static files I think.
Most out-of-marketplace app will not have an ETag with this information.

Of course, Last-Modified might change without the content changing, but I think this is still a better state than now where we're not supporting Last-Modified at all.

And it's ivery unlikely that Last-Modified does not change if the content is changing.
blocking-basecamp: ? → -
Blocks: 829853
As seen in Bug 829853, github servers do not send ETag:

$ curl -I
HTTP/1.1 200 OK
Date: Sat, 12 Jan 2013 12:36:27 GMT
Content-Type: application/x-web-app-manifest+json
Content-Length: 810
Last-Modified: Sat, 12 Jan 2013 03:44:17 GMT
Connection: keep-alive
Expires: Sun, 13 Jan 2013 12:36:27 GMT
Cache-Control: max-age=86400
Accept-Ranges: bytes

Therefore, I'd like that we reconsider to fix this bug for v1. Since I don't know the new stuff in blocking-b2g, I'm nominating for blocking-basecamp, but please choose what's appropriated here.
blocking-basecamp: - → ?
Switching flags per new usage for tracking? + tef? after retiring basecamp?
blocking-b2g: --- → tef?
blocking-basecamp: ? → ---
tracking-b2g18: --- → ?
Blocks: app-install
Fabrice said this is something we'd like to take for 1.x but not something we'd hold for.
blocking-b2g: tef? → -
No longer blocks: 829853
I a bit don't understand why you are handling ETag and Last-Modified (not talking about Expire) "manually" in a JS code when these headers are stored and correctly handled by the Necko cache natively for you.

If you want to check whether the content has changed on the server, then use following channel flags on the xhr:
    - this one will skip load from cache and just give OnStartRequest and OnStopRequest (no content load) when the cached content is found valid ; it means for you that there is no update
    - even there is no ETag or Last-Modified and the calculated expiration time indicates the resource is valid, we will recheck with the server, always
    - this is the most optimal way of loading always valid resource since when there is ETag or Last-Modified we use those for conditional requests (that you are bustling your self in JS)
    - with this flag you don't need to require ETag/Last-Modified being set by the serve

However, when the resource has not been modified on the server, you need to check whether the manifest (or which ever file you are checking on) has changed.  Thus a hash checker as in case of offline app cache manifest is what you need (bug 829934).

Putting back into the nom queue. If this is the direction we choose to go to fix the bug blocking this bug, then this blocks. If it isn't and the other bug ends up being the choice, then feel free to minus this and + the other bug (bug 829934).
blocking-b2g: - → tef?
Adding :sicking and :philikon because of the latest conversation on IRC, and I'd like to add some more information about server support.

Apache supports ETag out of the box (however AFAIK nginx support is trickier to get correct).

However all webperf best practices (most notably Yahoo and Google) advise to deactivate support in Apache and to rely only on Last-Modified headers which is sufficient for most cases.

Default configuration in Apache generates an ETag in the following format: INode-ModificationTime-Size. This is wrong in many cases, most notably when using load balancing (-> different INodes -> different ETag -> 200 instead of 304).

So ETag are only useful in specific cases, such as our Marketplace, but we can't expect people to do that, and most of the times it will be disabled because of webperf best practices.
Jonas - Do we need to block on this one or only block on the other bug for the hash verifier?
Flags: needinfo?(jonas)
I think we spend more time discussing that writing the code. That is silly. I'll happily review and land a patch.
I think bug 829934 is enough. Though doing this is a performance improvement which would also save users some bandwidth (in the order of 1kB/app/day)
Flags: needinfo?(jonas)
blocking-b2g: tef? → ---
No longer blocks: 829853
Attachment #694508 - Attachment is obsolete: true
Attachment #694508 - Flags: feedback?(fabrice)
Blocks: b2g-apps-v1-next
No longer blocks: app-install
No longer blocks: b2g-apps-v1-next
Product: Core → Core Graveyard
Core Graveyard / DOM: Apps is inactive. Closing all bugs in this component.
Closed: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.