Generate an etag for chrome-extension:// file requests, so caching can be optional

Generate an etag for chrome-extension:// file requests.

This allows code downstream to use those headers as caching
signals, and optionally cache extension files, or files
derived from extension files.

Whenever an etag is sent, send "cache-control: no-cache"
so that downstream users know it's a good idea to
revalidate.

One downstream is PNaCl, which would like to use standard
cache-related http response headers to determine how
to cache translations of bitcode.  Currently, only bitcode
downloaded from web servers have such headers, and this CL
adds headers for bitcode "downloaded" from chrome extensions.

See chromium-dev discussion: "Add to HttpResponse headers
for chrome/browser/extensions/extension_protocols"


BUG=https://code.google.com/p/nativeclient/issues/detail?id=2992

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

[jvoung (off chromium), 2013-06-03 17:26:23]

So did this seem okay?

If so, Matt would you be able to review?

[Matt Perry, 2013-06-03 18:13:55]

LGTM, though I didn't closely follow the discussion on chromium-dev.

https://codereview.chromium.org/15963003/diff/20001/chrome/browser/extensions...
File chrome/browser/extensions/extension_protocols.cc (right):

https://codereview.chromium.org/15963003/diff/20001/chrome/browser/extensions...
chrome/browser/extensions/extension_protocols.cc:82:
raw_headers.append(etag.c_str());
No need to take c_str() here

https://codereview.chromium.org/15963003/diff/20001/chrome/browser/extensions...
File chrome/browser/extensions/extension_protocols_unittest.cc (right):

https://codereview.chromium.org/15963003/diff/20001/chrome/browser/extensions...
chrome/browser/extensions/extension_protocols_unittest.cc:249: extension,
base::Time::Now(), false);
nit: looks like this doesn't need to wrap

[jvoung (off chromium), 2013-06-03 20:21:52]

https://codereview.chromium.org/15963003/diff/20001/chrome/browser/extensions...
File chrome/browser/extensions/extension_protocols.cc (right):

https://codereview.chromium.org/15963003/diff/20001/chrome/browser/extensions...
chrome/browser/extensions/extension_protocols.cc:82:
raw_headers.append(etag.c_str());
On 2013/06/03 18:13:55, Matt Perry wrote:
> No need to take c_str() here

Done.

https://codereview.chromium.org/15963003/diff/20001/chrome/browser/extensions...
File chrome/browser/extensions/extension_protocols_unittest.cc (right):

https://codereview.chromium.org/15963003/diff/20001/chrome/browser/extensions...
chrome/browser/extensions/extension_protocols_unittest.cc:249: extension,
base::Time::Now(), false);
On 2013/06/03 18:13:55, Matt Perry wrote:
> nit: looks like this doesn't need to wrap

Done.

[rvargas (doing something else), 2013-06-03 21:12:37]

lgtm

https://chromiumcodereview.appspot.com/15963003/diff/26001/chrome/browser/ext...
File chrome/browser/extensions/extension_protocols.cc (right):

https://chromiumcodereview.appspot.com/15963003/diff/26001/chrome/browser/ext...
chrome/browser/extensions/extension_protocols.cc:76: std::string hash =
base::StringPrintf("%f", last_modified_time.ToDoubleT());
nit: ToInternalValue ?

https://chromiumcodereview.appspot.com/15963003/diff/26001/chrome/browser/ext...
File chrome/browser/extensions/extension_protocols_unittest.cc (right):

https://chromiumcodereview.appspot.com/15963003/diff/26001/chrome/browser/ext...
chrome/browser/extensions/extension_protocols_unittest.cc:82:
Extension::Create(path, Manifest::UNPACKED, manifest,
I'm assuming that the extension destructor does cleanup of the files on disk.

[jvoung (off chromium), 2013-06-03 22:08:22]

https://chromiumcodereview.appspot.com/15963003/diff/26001/chrome/browser/ext...
File chrome/browser/extensions/extension_protocols.cc (right):

https://chromiumcodereview.appspot.com/15963003/diff/26001/chrome/browser/ext...
chrome/browser/extensions/extension_protocols.cc:76: std::string hash =
base::StringPrintf("%f", last_modified_time.ToDoubleT());
On 2013/06/03 21:12:37, rvargas wrote:
> nit: ToInternalValue ?

Done.

https://chromiumcodereview.appspot.com/15963003/diff/26001/chrome/browser/ext...
File chrome/browser/extensions/extension_protocols_unittest.cc (right):

https://chromiumcodereview.appspot.com/15963003/diff/26001/chrome/browser/ext...
chrome/browser/extensions/extension_protocols_unittest.cc:82:
Extension::Create(path, Manifest::UNPACKED, manifest,
On 2013/06/03 21:12:37, rvargas wrote:
> I'm assuming that the extension destructor does cleanup of the files on disk.

It doesn't look like the destructor does anything, but it didn't look like the
constructor would put anything new on disk either.

I thought it just refers to a path to some directory (where the directory holds
the extension files prior to packing it up into a .zip or .crx).

[Matt Perry, 2013-06-03 22:17:19]

https://chromiumcodereview.appspot.com/15963003/diff/26001/chrome/browser/ext...
File chrome/browser/extensions/extension_protocols_unittest.cc (right):

https://chromiumcodereview.appspot.com/15963003/diff/26001/chrome/browser/ext...
chrome/browser/extensions/extension_protocols_unittest.cc:82:
Extension::Create(path, Manifest::UNPACKED, manifest,
On 2013/06/03 22:08:22, jvoung (cr) wrote:
> On 2013/06/03 21:12:37, rvargas wrote:
> > I'm assuming that the extension destructor does cleanup of the files on
disk.
> 
> It doesn't look like the destructor does anything, but it didn't look like the
> constructor would put anything new on disk either.
> 
> I thought it just refers to a path to some directory (where the directory
holds
> the extension files prior to packing it up into a .zip or .crx).

Yep, no new files are created by this test.

[commit-bot: I haz the power, 2013-06-03 22:33:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jvoung@chromium.org/15963003/30002

[commit-bot: I haz the power, 2013-06-03 22:50:37]

Retried try job too often on mac_rel for step(s) base_unittests, browser_tests,
cacheinvalidation_unittests, cc_unittests, check_deps, chromedriver2_unittests,
components_unittests, content_browsertests, content_unittests, crypto_unittests,
gpu_unittests, interactive_ui_tests, ipc_tests, jingle_unittests,
media_unittests, nacl_integration, net_unittests, ppapi_unittests,
printing_unittests, remoting_unittests, sql_unittests, sync_integration_tests,
sync_unit_tests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2013-06-03 23:58:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jvoung@chromium.org/15963003/30002

[commit-bot: I haz the power, 2013-06-04 03:49:35]

Change committed as 203830