Docs: Use ETags.

chrome/common/extensions/docs/server2/render_servlet.py

Index: chrome/common/extensions/docs/server2/render_servlet.py
diff --git a/chrome/common/extensions/docs/server2/render_servlet.py b/chrome/common/extensions/docs/server2/render_servlet.py
index c714b5de031f8ca7e8b0cac2a10e01e2266ae6f1..cfda81c8f4f3361c868ba9e18649187c01960d02 100644
--- a/chrome/common/extensions/docs/server2/render_servlet.py
+++ b/chrome/common/extensions/docs/server2/render_servlet.py
@@ -2,6 +2,7 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import hashlib
 import logging
 import posixpath
 import traceback
@@ -15,12 +16,16 @@ from special_paths import SITE_VERIFICATION_FILE
 from third_party.handlebar import Handlebar
 
 
-def _MakeHeaders(content_type):
-  return {
-    'X-Frame-Options': 'sameorigin',
+def _MakeHeaders(content_type, etag=None):
+  headers = {
+    'Cache-Control': 'public, max-age=0, no-cache',

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

Maybe link to http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.9.1
to explain that no-cache actually means "force revalidation".

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

I'll link to the spec.

     'Content-Type': content_type,
-    'Cache-Control': 'max-age=300',
+    'ETag': etag,

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

Remove this line, I think. you don't want to send this header if etag is None.

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

Oops. forgot to remove it from this list.

+    'X-Frame-Options': 'sameorigin',
   }
+  if etag is not None:
+    headers['ETag'] = etag
+  return headers
 
 
 class RenderServlet(Servlet):
@@ -122,10 +127,27 @@ class RenderServlet(Servlet):
       if warnings:
         sep = '\n - '
         logging.warning('Rendering %s:%s%s' % (path, sep, sep.join(warnings)))
+      # Content was dynamic. The new etag is the md5 hash of the content.

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

Maybe comment here that we're relying on SSL for security; MD5 just avoids
accidental collisions.

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

good point.

+      etag = None
+    elif content_and_type.version is not None:
+      # Content was static. The new etag is the version of the content.
+      etag = '"%s"' % content_and_type.version
+    else:
+      # Sometimes non-dynamic content does not have a version, for example
+      # .zip files. The new etag is the md5 hash of the content.
+      etag = None
 
     content_type = content_and_type.content_type
     if isinstance(content, unicode):
       content = content.encode('utf-8')
       content_type += '; charset=utf-8'
 
-    return Response.Ok(content, headers=_MakeHeaders(content_type))
+    if etag is None:
+      m = hashlib.md5()

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

You can just use hashlib.md5(content).hexdigest().

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

Done.

+      m.update(content)
+      etag = '"%s"' % m.hexdigest()
+
+    headers = _MakeHeaders(content_type, etag=etag)
+    if etag == self._request.headers.get('If-None-Match'):
+      return Response.NotModified('Not Modified', headers=headers)

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

Since we've already done the work to generate the response, is this actually
going to save much latency for the client?

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

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.

+    return Response.Ok(content, headers=headers)