Make a slightly better 404 page for pub serve.

Make a slightly better 404 page for pub serve.

BUG=https://code.google.com/p/dart/issues/detail?id=17087
R=nweiz@google.com

Committed: https://code.google.com/p/dart/source/detail?r=34660

[nweiz, 2014-03-31 20:17:58]

https://codereview.chromium.org/219733002/diff/1/sdk/lib/_internal/pub/lib/sr...
File sdk/lib/_internal/pub/lib/src/barback/base_server.dart (right):

https://codereview.chromium.org/219733002/diff/1/sdk/lib/_internal/pub/lib/sr...
sdk/lib/_internal/pub/lib/src/barback/base_server.dart:71: void
notFound(HttpRequest request, {String error, AssetId asset}) {
This is complex enough now that we probably want a test for the 404 output.

https://codereview.chromium.org/219733002/diff/1/sdk/lib/_internal/pub/lib/sr...
sdk/lib/_internal/pub/lib/src/barback/base_server.dart:86:
"<tt>${asset.path}</tt> in package "
HTML-escape all of these interpolations.

Also, use "code" rather than "tt".

[Bob Nystrom, 2014-04-01 17:41:18]

https://codereview.chromium.org/219733002/diff/1/sdk/lib/_internal/pub/lib/sr...
File sdk/lib/_internal/pub/lib/src/barback/base_server.dart (right):

https://codereview.chromium.org/219733002/diff/1/sdk/lib/_internal/pub/lib/sr...
sdk/lib/_internal/pub/lib/src/barback/base_server.dart:71: void
notFound(HttpRequest request, {String error, AssetId asset}) {
On 2014/03/31 20:17:58, nweiz wrote:
> This is complex enough now that we probably want a test for the 404 output.

Done.

https://codereview.chromium.org/219733002/diff/1/sdk/lib/_internal/pub/lib/sr...
sdk/lib/_internal/pub/lib/src/barback/base_server.dart:86:
"<tt>${asset.path}</tt> in package "
On 2014/03/31 20:17:58, nweiz wrote:
> HTML-escape all of these interpolations.
> 
> Also, use "code" rather than "tt".

Done.

[nweiz, 2014-04-01 19:20:27]

https://codereview.chromium.org/219733002/diff/20001/sdk/lib/_internal/pub/li...
File sdk/lib/_internal/pub/lib/src/barback/base_server.dart (right):

https://codereview.chromium.org/219733002/diff/20001/sdk/lib/_internal/pub/li...
sdk/lib/_internal/pub/lib/src/barback/base_server.dart:84:
request.response.writeln("<h1>404 Not Found</h1>");
Now that this is HTML, it would be nice to provide at least a <title> tag.

https://codereview.chromium.org/219733002/diff/20001/sdk/lib/_internal/pub/li...
sdk/lib/_internal/pub/lib/src/barback/base_server.dart:92:
request.response.writeln("<p>Error: $error</p>");
Don't forget to escape this too!

https://codereview.chromium.org/219733002/diff/20001/sdk/lib/_internal/pub/te...
File sdk/lib/_internal/pub/test/serve/utils.dart (right):

https://codereview.chromium.org/219733002/diff/20001/sdk/lib/_internal/pub/te...
sdk/lib/_internal/pub/test/serve/utils.dart:210: void scheduleRequest(String
urlPath, callback(http.Response response),
You don't need to manually pass a callback here. [schedule] will forward the
value of the returned future, so you can just have this return a
[Future<http.Response>].

[Bob Nystrom, 2014-04-01 19:40:57]

https://codereview.chromium.org/219733002/diff/20001/sdk/lib/_internal/pub/li...
File sdk/lib/_internal/pub/lib/src/barback/base_server.dart (right):

https://codereview.chromium.org/219733002/diff/20001/sdk/lib/_internal/pub/li...
sdk/lib/_internal/pub/lib/src/barback/base_server.dart:84:
request.response.writeln("<h1>404 Not Found</h1>");
On 2014/04/01 19:20:28, nweiz wrote:
> Now that this is HTML, it would be nice to provide at least a <title> tag.

Done.

https://codereview.chromium.org/219733002/diff/20001/sdk/lib/_internal/pub/li...
sdk/lib/_internal/pub/lib/src/barback/base_server.dart:92:
request.response.writeln("<p>Error: $error</p>");
On 2014/04/01 19:20:28, nweiz wrote:
> Don't forget to escape this too!

Oops! Done.

https://codereview.chromium.org/219733002/diff/20001/sdk/lib/_internal/pub/te...
File sdk/lib/_internal/pub/test/serve/utils.dart (right):

https://codereview.chromium.org/219733002/diff/20001/sdk/lib/_internal/pub/te...
sdk/lib/_internal/pub/test/serve/utils.dart:210: void scheduleRequest(String
urlPath, callback(http.Response response),
On 2014/04/01 19:20:28, nweiz wrote:
> You don't need to manually pass a callback here. [schedule] will forward the
> value of the returned future, so you can just have this return a
> [Future<http.Response>].

Yeah, I thought about that, but I figured the point of this was to be a nice
DSL. But, now that I try it out, the Future looks cleaner.

Done.

[nweiz, 2014-04-02 03:13:15]

lgtm

[Bob Nystrom, 2014-04-02 18:22:55]

Committed patchset #3 manually as r34660 (presubmit successful).