From 8b1effc544120a74564508800a22d35727da4b9d Mon Sep 17 00:00:00 2001
From: Adam Barth <abarth@chromium.org>
Date: Tue, 6 Oct 2015 10:37:55 -0700
Subject: [PATCH] NetworkImage occasionally does not grab the image

I haven't been able to reproduce this bug consistently, but my theory is that
the ImageDecoder was being garbage collected before it called its completion
callback. This patch prevents that by keeping a reference to the image decoder
while the callback is in flight.

Fixes #801
---
 packages/flutter/lib/services.dart            |  1 +
 .../lib/src/services/asset_bundle.dart        | 11 ++++---
 .../flutter/lib/src/services/image_cache.dart | 23 ++++++-------
 .../lib/src/services/image_decoder.dart       | 32 +++++++++++++++++++
 4 files changed, 51 insertions(+), 16 deletions(-)
 create mode 100644 packages/flutter/lib/src/services/image_decoder.dart

diff --git a/packages/flutter/lib/services.dart b/packages/flutter/lib/services.dart
index c5bc1d3818..67387b1d3b 100644
--- a/packages/flutter/lib/services.dart
+++ b/packages/flutter/lib/services.dart
@@ -16,6 +16,7 @@ export 'src/services/asset_bundle.dart';
 export 'src/services/embedder.dart';
 export 'src/services/fetch.dart';
 export 'src/services/image_cache.dart';
+export 'src/services/image_decoder.dart';
 export 'src/services/image_resource.dart';
 export 'src/services/keyboard.dart';
 export 'src/services/shell.dart';
diff --git a/packages/flutter/lib/src/services/asset_bundle.dart b/packages/flutter/lib/src/services/asset_bundle.dart
index aecc48a53b..0ecfa008bb 100644
--- a/packages/flutter/lib/src/services/asset_bundle.dart
+++ b/packages/flutter/lib/src/services/asset_bundle.dart
@@ -11,6 +11,7 @@ import 'package:mojo/core.dart' as core;
 import 'package:mojo_services/mojo/asset_bundle/asset_bundle.mojom.dart';
 import 'package:sky/src/services/fetch.dart';
 import 'package:sky/src/services/image_cache.dart';
+import 'package:sky/src/services/image_decoder.dart';
 import 'package:sky/src/services/image_resource.dart';
 import 'package:sky/src/services/shell.dart';
 
@@ -66,13 +67,13 @@ class MojoAssetBundle extends AssetBundle {
     _imageCache = null;
   }
 
+  Future<sky.Image> _fetchImage(String key) async {
+    return await decodeImageFromDataPipe(await load(key));
+  }
+
   ImageResource loadImage(String key) {
     return _imageCache.putIfAbsent(key, () {
-      Completer<sky.Image> completer = new Completer<sky.Image>();
-      load(key).then((assetData) {
-        new sky.ImageDecoder.consume(assetData.handle.h, completer.complete);
-      });
-      return new ImageResource(completer.future);
+      return new ImageResource(_fetchImage(key));
     });
   }
 
diff --git a/packages/flutter/lib/src/services/image_cache.dart b/packages/flutter/lib/src/services/image_cache.dart
index fdc7e37bd5..81b3d3f730 100644
--- a/packages/flutter/lib/src/services/image_cache.dart
+++ b/packages/flutter/lib/src/services/image_cache.dart
@@ -7,8 +7,18 @@ import 'dart:collection';
 import 'dart:sky' as sky;
 
 import 'package:mojo/mojo/url_response.mojom.dart';
-import 'package:sky/src/services/image_resource.dart';
 import 'package:sky/src/services/fetch.dart';
+import 'package:sky/src/services/image_decoder.dart';
+import 'package:sky/src/services/image_resource.dart';
+
+Future<sky.Image> _fetchImage(String url) async {
+  UrlResponse response = await fetchUrl(url);
+  if (response.statusCode >= 400) {
+    print("Failed (${response.statusCode}) to load image ${url}");
+    return null;
+  }
+  return await decodeImageFromDataPipe(response.body);
+}
 
 class _ImageCache {
   _ImageCache._();
@@ -17,16 +27,7 @@ class _ImageCache {
 
   ImageResource load(String url) {
     return _cache.putIfAbsent(url, () {
-      Completer<sky.Image> completer = new Completer<sky.Image>();
-      fetchUrl(url).then((UrlResponse response) {
-        if (response.statusCode >= 400) {
-          print("Failed (${response.statusCode}) to load image ${url}");
-          completer.complete(null);
-        } else {
-          new sky.ImageDecoder.consume(response.body.handle.h, completer.complete);
-        }
-      });
-      return new ImageResource(completer.future);
+      return new ImageResource(_fetchImage(url));
     });
   }
 }
diff --git a/packages/flutter/lib/src/services/image_decoder.dart b/packages/flutter/lib/src/services/image_decoder.dart
new file mode 100644
index 0000000000..375c31398e
--- /dev/null
+++ b/packages/flutter/lib/src/services/image_decoder.dart
@@ -0,0 +1,32 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'dart:async';
+import 'dart:sky' show Image, ImageDecoder, ImageDecoderCallback;
+import 'dart:typed_data';
+
+import 'package:mojo/core.dart' show MojoDataPipeConsumer;
+
+final Set<ImageDecoder> _activeDecoders = new Set<ImageDecoder>();
+
+typedef ImageDecoder _DecoderFactory(ImageDecoderCallback callback);
+
+Future<Image> _decode(_DecoderFactory createDecoder) {
+  Completer<Image> completer = new Completer<Image>();
+  ImageDecoder decoder;
+  decoder = createDecoder((Image image) {
+    _activeDecoders.remove(decoder);
+    completer.complete(image);
+  });
+  _activeDecoders.add(decoder);
+  return completer.future;
+}
+
+Future<Image> decodeImageFromDataPipe(MojoDataPipeConsumer consumerHandle) {
+  return _decode((ImageDecoderCallback callback) => new ImageDecoder.consume(consumerHandle.handle.h, callback));
+}
+
+Future<Image> decodeImageFromList(Uint8List list) {
+  return _decode((ImageDecoderCallback callback) => new ImageDecoder.fromList(list, callback));
+}