Provide and respect HTTP cache headers in pub serve.

Provide and respect HTTP cache headers in pub serve.

In my local tests, this has a negligible performance impact on packages
with lots of small files, but is helpful for large unchanged assets.

R=nweiz@google.com

Committed: https://github.com/dart-lang/pub/commit/642addca734b05cb401e3ce656c6f5712adfd6d0

[Bob Nystrom, 2016-03-12 01:21:08]

rnystrom@google.com changed reviewers:
+ nweiz@google.com

[nweiz, 2016-03-12 01:30:51]

lgtm

https://codereview.chromium.org/1790503005/diff/1/lib/src/barback/barback_ser...
File lib/src/barback/barback_server.dart (right):

https://codereview.chromium.org/1790503005/diff/1/lib/src/barback/barback_ser...
lib/src/barback/barback_server.dart:170: var assetSha =
CryptoUtils.bytesToHex(sha.close());
CryptoUtils is going to get removed once I get around to it—use the hex
converter from package:convert instead.

https://codereview.chromium.org/1790503005/diff/1/lib/src/barback/barback_ser...
lib/src/barback/barback_server.dart:174: "Cache-Control": "max-age=3600",
It's not clear to me why this is being set. Consider adding a comment explaining
it.

[Bob Nystrom, 2016-03-14 19:15:01]

Description was changed from

==========
Provide and respect HTTP cache headers in pub serve.

In my local tests, this has a negligible performance impact on packages
with lots of small files, but is helpful for large unchanged assets.
==========

to

==========
Provide and respect HTTP cache headers in pub serve.

In my local tests, this has a negligible performance impact on packages
with lots of small files, but is helpful for large unchanged assets.

R=nweiz@google.com

Committed:
https://github.com/dart-lang/pub/commit/642addca734b05cb401e3ce656c6f5712adfd6d0
==========

[Bob Nystrom, 2016-03-14 19:15:02]

Committed patchset #2 (id:20001) manually as
642addca734b05cb401e3ce656c6f5712adfd6d0 (presubmit successful).

[Bob Nystrom, 2016-03-14 19:15:10]

Thanks!

https://codereview.chromium.org/1790503005/diff/1/lib/src/barback/barback_ser...
File lib/src/barback/barback_server.dart (right):

https://codereview.chromium.org/1790503005/diff/1/lib/src/barback/barback_ser...
lib/src/barback/barback_server.dart:170: var assetSha =
CryptoUtils.bytesToHex(sha.close());
On 2016/03/12 01:30:51, nweiz wrote:
> CryptoUtils is going to get removed once I get around to it—use the hex
> converter from package:convert instead.

I didn't see anything related to hex in there, so switched this to use base64
instead, but done.

https://codereview.chromium.org/1790503005/diff/1/lib/src/barback/barback_ser...
lib/src/barback/barback_server.dart:174: "Cache-Control": "max-age=3600",
On 2016/03/12 01:30:51, nweiz wrote:
> It's not clear to me why this is being set. Consider adding a comment
explaining
> it.

I'm an HTTP noob, but from what I read, you have to provide this header to
enable the browser to cache it at all. Added a comment.

[nweiz, 2016-03-14 19:29:13]

https://codereview.chromium.org/1790503005/diff/1/lib/src/barback/barback_ser...
File lib/src/barback/barback_server.dart (right):

https://codereview.chromium.org/1790503005/diff/1/lib/src/barback/barback_ser...
lib/src/barback/barback_server.dart:170: var assetSha =
CryptoUtils.bytesToHex(sha.close());
On 2016/03/14 19:15:10, Bob Nystrom wrote:
> On 2016/03/12 01:30:51, nweiz wrote:
> > CryptoUtils is going to get removed once I get around to it—use the hex
> > converter from package:convert instead.
> 
> I didn't see anything related to hex in there, so switched this to use base64
> instead, but done.

https://www.dartdocs.org/documentation/convert/1.0.1/convert/hex-constant.html

[Bob Nystrom, 2016-03-14 20:02:16]

On 2016/03/14 19:29:13, nweiz wrote:
>
https://codereview.chromium.org/1790503005/diff/1/lib/src/barback/barback_ser...
> File lib/src/barback/barback_server.dart (right):
> 
>
https://codereview.chromium.org/1790503005/diff/1/lib/src/barback/barback_ser...
> lib/src/barback/barback_server.dart:170: var assetSha =
> CryptoUtils.bytesToHex(sha.close());
> On 2016/03/14 19:15:10, Bob Nystrom wrote:
> > On 2016/03/12 01:30:51, nweiz wrote:
> > > CryptoUtils is going to get removed once I get around to it—use the hex
> > > converter from package:convert instead.
> > 
> > I didn't see anything related to hex in there, so switched this to use
base64
> > instead, but done.
> 
> https://www.dartdocs.org/documentation/convert/1.0.1/convert/hex-constant.html

Oh, I see. For some dumb reason I saw "package:convert" and read "dart:convert".
Either way, I think base64 is probably better anyway: it's smaller.

– bob