Implement a simple image cache for Sky in Dart (no eviction)

Implement a simple image cache for Sky in Dart. This doesn't yet do any eviction from the cache.

R=abarth@chromium.org, abarth

Committed: https://chromium.googlesource.com/external/mojo/+/514a50c7d3703d038429f877c0c690af4c9cc088

[abarth-chromium, 2015-06-02 00:52:48]

LGTM

https://codereview.chromium.org/1165753004/diff/1/sky/sdk/lib/framework/net/i...
File sky/sdk/lib/framework/net/image_cache.dart (right):

https://codereview.chromium.org/1165753004/diff/1/sky/sdk/lib/framework/net/i...
sky/sdk/lib/framework/net/image_cache.dart:8: class ImageCache {
There's no need for this class.  You can just put the state in the global scope
of this library and use _ prefixes to hide them.

Client then use the library as follows:

import "package:sky/framework/net/image_cache.dart" as image_cache;

image_cache.load(...)

https://codereview.chromium.org/1165753004/diff/1/sky/sdk/lib/framework/net/i...
sky/sdk/lib/framework/net/image_cache.dart:13: final HashMap completedRequests =
new HashMap<String, sky.Image>();
It's more important to put the types on the declaration than on the constructor:

final HashMap<String, sky.Image> completedRequests = new HashMap();

However, I think we should put the types everywhere given that this is framework
code that will be read 10x more often than it is written.

https://codereview.chromium.org/1165753004/diff/1/sky/sdk/lib/framework/net/i...
sky/sdk/lib/framework/net/image_cache.dart:18: } else if
(pendingRequests.containsKey(url)) {
Using putIfAbsent [1] is the idiomatic (and faster) way to implement this
pattern.


[1]
https://api.dartlang.org/apidocs/channels/stable/dartdoc-viewer/dart:core.Map...

[jackson, 2015-06-02 01:18:25]

https://codereview.chromium.org/1165753004/diff/1/sky/sdk/lib/framework/net/i...
File sky/sdk/lib/framework/net/image_cache.dart (right):

https://codereview.chromium.org/1165753004/diff/1/sky/sdk/lib/framework/net/i...
sky/sdk/lib/framework/net/image_cache.dart:8: class ImageCache {
On 2015/06/02 00:52:47, abarth wrote:
> There's no need for this class.  You can just put the state in the global
scope
> of this library and use _ prefixes to hide them.
> 
> Client then use the library as follows:
> 
> import "package:sky/framework/net/image_cache.dart" as image_cache;
> 
> image_cache.load(...)

Acknowledged.

https://codereview.chromium.org/1165753004/diff/1/sky/sdk/lib/framework/net/i...
sky/sdk/lib/framework/net/image_cache.dart:13: final HashMap completedRequests =
new HashMap<String, sky.Image>();
On 2015/06/02 00:52:47, abarth wrote:
> It's more important to put the types on the declaration than on the
constructor:
> 
> final HashMap<String, sky.Image> completedRequests = new HashMap();
> 
> However, I think we should put the types everywhere given that this is
framework
> code that will be read 10x more often than it is written.

Acknowledged.

https://codereview.chromium.org/1165753004/diff/1/sky/sdk/lib/framework/net/i...
sky/sdk/lib/framework/net/image_cache.dart:18: } else if
(pendingRequests.containsKey(url)) {
On 2015/06/02 00:52:47, abarth wrote:
> Using putIfAbsent [1] is the idiomatic (and faster) way to implement this
> pattern.
> 
> 
> [1]
>
https://api.dartlang.org/apidocs/channels/stable/dartdoc-viewer/dart:core.Map...

Acknowledged.

[jackson, 2015-06-02 02:15:09]

I simplified the ImageLoader API per your feedback on issue 1156003007

[jackson, 2015-06-02 17:44:52]

Committed patchset #7 (id:110001) manually as
514a50c7d3703d038429f877c0c690af4c9cc088 (presubmit successful).