Docs: Use ETags.

Docs: Use ETags.

BUG=270818
R=jyasskin@chromium.org
NOTRY=true

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=261222

[not at google - send to devlin, 2014-03-29 16:52:24]

So - the weird thing, and perhaps you can help with this, is that Chrome is
ignoring the ETags on the main requests, like
developer.chrome.com/extensions/storage. It respects the CSS and JS ETags
though.

I thought that maybe it was because I was testing by refreshing, but even when I
follow links around it seems to ignore them. What am I missing?

[not at google - send to devlin, 2014-04-01 20:53:38]

ping

[Jeffrey Yasskin, 2014-04-02 19:54:50]

https://codereview.chromium.org/218363002/diff/20001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/render_servlet.py (right):

https://codereview.chromium.org/218363002/diff/20001/chrome/common/extensions...
chrome/common/extensions/docs/server2/render_servlet.py:21: 'Cache-Control':
'public, max-age=0, no-cache',
Maybe link to http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.9.1
to explain that no-cache actually means "force revalidation".

https://codereview.chromium.org/218363002/diff/20001/chrome/common/extensions...
chrome/common/extensions/docs/server2/render_servlet.py:23: 'ETag': etag,
Remove this line, I think. you don't want to send this header if etag is None.

https://codereview.chromium.org/218363002/diff/20001/chrome/common/extensions...
chrome/common/extensions/docs/server2/render_servlet.py:130: # Content was
dynamic. The new etag is the md5 hash of the content.
Maybe comment here that we're relying on SSL for security; MD5 just avoids
accidental collisions.

https://codereview.chromium.org/218363002/diff/20001/chrome/common/extensions...
chrome/common/extensions/docs/server2/render_servlet.py:146: m = hashlib.md5()
You can just use hashlib.md5(content).hexdigest().

https://codereview.chromium.org/218363002/diff/20001/chrome/common/extensions...
chrome/common/extensions/docs/server2/render_servlet.py:152: return
Response.NotModified('Not Modified', headers=headers)
Since we've already done the work to generate the response, is this actually
going to save much latency for the client?

https://codereview.chromium.org/218363002/diff/20001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/servlet.py (right):

https://codereview.chromium.org/218363002/diff/20001/chrome/common/extensions...
chrome/common/extensions/docs/server2/servlet.py:6: class
RequestHeaders(object):
Surely appengine provides a request-headers class?

[not at google - send to devlin, 2014-04-02 22:52:53]

https://codereview.chromium.org/218363002/diff/20001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/render_servlet.py (right):

https://codereview.chromium.org/218363002/diff/20001/chrome/common/extensions...
chrome/common/extensions/docs/server2/render_servlet.py:21: 'Cache-Control':
'public, max-age=0, no-cache',
On 2014/04/02 19:54:50, Jeffrey Yasskin wrote:
> Maybe link to http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.9.1
> to explain that no-cache actually means "force revalidation".

I'll link to the spec.

https://codereview.chromium.org/218363002/diff/20001/chrome/common/extensions...
chrome/common/extensions/docs/server2/render_servlet.py:23: 'ETag': etag,
On 2014/04/02 19:54:50, Jeffrey Yasskin wrote:
> Remove this line, I think. you don't want to send this header if etag is None.

Oops. forgot to remove it from this list.

https://codereview.chromium.org/218363002/diff/20001/chrome/common/extensions...
chrome/common/extensions/docs/server2/render_servlet.py:130: # Content was
dynamic. The new etag is the md5 hash of the content.
On 2014/04/02 19:54:50, Jeffrey Yasskin wrote:
> Maybe comment here that we're relying on SSL for security; MD5 just avoids
> accidental collisions.

good point.

https://codereview.chromium.org/218363002/diff/20001/chrome/common/extensions...
chrome/common/extensions/docs/server2/render_servlet.py:146: m = hashlib.md5()
On 2014/04/02 19:54:50, Jeffrey Yasskin wrote:
> You can just use hashlib.md5(content).hexdigest().

Done.

https://codereview.chromium.org/218363002/diff/20001/chrome/common/extensions...
chrome/common/extensions/docs/server2/render_servlet.py:152: return
Response.NotModified('Not Modified', headers=headers)
On 2014/04/02 19:54:50, Jeffrey Yasskin wrote:
> Since we've already done the work to generate the response, is this actually
> going to save much latency for the client?

We might as well. The only reason not to do it is if md5 calculation >
populating a string and sending it over http. I don't know the answer to that.
Since this site is "mobile friendly" though I feel comfortable with saving
bandwidth.

https://codereview.chromium.org/218363002/diff/20001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/servlet.py (right):

https://codereview.chromium.org/218363002/diff/20001/chrome/common/extensions...
chrome/common/extensions/docs/server2/servlet.py:6: class
RequestHeaders(object):
On 2014/04/02 19:54:50, Jeffrey Yasskin wrote:
> Surely appengine provides a request-headers class?

I'm sure it does, but this is also used outside an appengine context.

[Jeffrey Yasskin, 2014-04-02 23:00:15]

lgtm

https://codereview.chromium.org/218363002/diff/20001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/servlet.py (right):

https://codereview.chromium.org/218363002/diff/20001/chrome/common/extensions...
chrome/common/extensions/docs/server2/servlet.py:6: class
RequestHeaders(object):
On 2014/04/02 22:52:53, kalman wrote:
> On 2014/04/02 19:54:50, Jeffrey Yasskin wrote:
> > Surely appengine provides a request-headers class?
> 
> I'm sure it does, but this is also used outside an appengine context.

Too bad.

[not at google - send to devlin, 2014-04-02 23:23:44]

The CQ bit was checked by kalman@chromium.org

[commit-bot: I haz the power, 2014-04-02 23:29:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kalman@chromium.org/218363002/60001

[commit-bot: I haz the power, 2014-04-02 23:33:20]

Change committed as 261222