handle image download synchronizations issues

ArthurHub · ArthurHub · commit cd2eb12dc6ee · 2014-09-05T15:59:30.000+03:00
diff --git a/Source/HtmlRenderer/Core/Dom/CssBoxFrame.cs b/Source/HtmlRenderer/Core/Dom/CssBoxFrame.cs
@@ -420,12 +420,7 @@ private void HandlePostApiCall(object sender)
         {
             try
             {
-                if (_videoImageUrl != null)
-                {
-                    _imageLoadHandler = new ImageLoadHandler(HtmlContainer, OnLoadImageComplete);
-                    _imageLoadHandler.LoadImage(_videoImageUrl, HtmlTag != null ? HtmlTag.Attributes : null);
-                }
-                else
+                if (_videoImageUrl == null)
                 {
                     _imageLoadingComplete = true;
                     SetErrorBorder();
@@ -448,6 +443,12 @@ private void HandlePostApiCall(object sender)
         /// <param name="g">the device to draw to</param>
         protected override void PaintImp(RGraphics g)
         {
+            if (_videoImageUrl != null && _imageLoadHandler == null)
+            {
+                _imageLoadHandler = new ImageLoadHandler(HtmlContainer, OnLoadImageComplete);
+                _imageLoadHandler.LoadImage(_videoImageUrl, HtmlTag != null ? HtmlTag.Attributes : null);
+            }
+
             var rects = CommonUtils.GetFirstValueOrDefault(Rectangles);
 
             RPoint offset = HtmlContainer != null ? HtmlContainer.ScrollOffset : RPoint.Empty;
diff --git a/Source/HtmlRenderer/Core/Handlers/ImageDownloader.cs b/Source/HtmlRenderer/Core/Handlers/ImageDownloader.cs
@@ -30,7 +30,11 @@ namespace TheArtOfDev.HtmlRenderer.Core.Handlers
     public delegate void DownloadFileAsyncCallback(Uri imageUri, string filePath, Exception error, bool canceled);
 
     /// <summary>
-    /// 
+    /// Handler for downloading images from the web.<br/>
+    /// Single instance of the handler used for all images downloaded in a single html, this way if the html contains more
+    /// than one reference to the same image it will be downloaded only once.<br/>
+    /// Also handles corrupt, partial and canceled downloads by first downloading to temp file and only if successful moving to cached 
+    /// file location.
     /// </summary>
     internal sealed class ImageDownloader : IDisposable
     {
@@ -193,18 +197,21 @@ private void OnDownloadImageCompleted(WebClient client, Uri source, string tempP
             List<DownloadFileAsyncCallback> callbacksList;
             lock (_imageDownloadCallbacks)
             {
-                callbacksList = _imageDownloadCallbacks[filePath];
-                _imageDownloadCallbacks.Remove(filePath);
+                if (_imageDownloadCallbacks.TryGetValue(filePath, out callbacksList))
+                    _imageDownloadCallbacks.Remove(filePath);
             }
 
-            foreach (var cachedFileCallback in callbacksList)
+            if (callbacksList != null)
             {
-                try
+                foreach (var cachedFileCallback in callbacksList)
                 {
-                    cachedFileCallback(source, filePath, error, cancelled);
+                    try
+                    {
+                        cachedFileCallback(source, filePath, error, cancelled);
+                    }
+                    catch
+                    { }
                 }
-                catch
-                { }
             }
         }
 
diff --git a/Source/HtmlRenderer/Core/Handlers/ImageLoadHandler.cs b/Source/HtmlRenderer/Core/Handlers/ImageLoadHandler.cs
@@ -301,9 +301,14 @@ private void LoadImageFromFile(string source)
         {
             try
             {
-                _imageFileStream = File.Open(source, FileMode.Open, FileAccess.Read, FileShare.ReadWrite);
-                _image = _htmlContainer.Adapter.ImageFromStream(_imageFileStream);
-                _releaseImageObject = true;
+                var imageFileStream = File.Open(source, FileMode.Open, FileAccess.Read, FileShare.ReadWrite);
+                lock (_loadCompleteCallback)
+                {
+                    _imageFileStream = imageFileStream;
+                    if (!_disposed)
+                        _image = _htmlContainer.Adapter.ImageFromStream(_imageFileStream);
+                    _releaseImageObject = true;
+                }
                 ImageLoadComplete();
             }
             catch (Exception ex)
@@ -368,15 +373,18 @@ private void ImageLoadComplete(bool async = true)
         /// </summary>
         private void ReleaseObjects()
         {
-            if (_releaseImageObject && _image != null)
-            {
-                _image.Dispose();
-                _image = null;
-            }
-            if (_imageFileStream != null)
+            lock (_loadCompleteCallback)
             {
-                _imageFileStream.Dispose();
-                _imageFileStream = null;
+                if (_releaseImageObject && _image != null)
+                {
+                    _image.Dispose();
+                    _image = null;
+                }
+                if (_imageFileStream != null)
+                {
+                    _imageFileStream.Dispose();
+                    _imageFileStream = null;
+                }
             }
         }
 
diff --git a/Source/HtmlRenderer/Core/HtmlContainerInt.cs b/Source/HtmlRenderer/Core/HtmlContainerInt.cs
@@ -421,6 +421,7 @@ public void SetHtml(string htmlSource, CssData baseCssData = null)
                 if (_root != null)
                 {
                     _selectionHandler = new SelectionHandler(_root);
+                    _imageDownloader = new ImageDownloader();
                 }
             }
         }
@@ -879,8 +880,6 @@ internal void AddHoverBox(CssBox box, CssBlock block)
         /// </summary>
         internal ImageDownloader GetImageDownloader()
         {
-            if (_imageDownloader == null)
-                _imageDownloader = new ImageDownloader();
             return _imageDownloader;
         }
 

Original file line number	Diff line number	Diff line change
`@@ -30,7 +30,11 @@ namespace TheArtOfDev.HtmlRenderer.Core.Handlers`
`30`	`30`	`public delegate void DownloadFileAsyncCallback(Uri imageUri, string filePath, Exception error, bool canceled);`
`31`	`31`
`32`	`32`	`/// <summary>`
`33`		`- ///`
	`33`	`+ /// Handler for downloading images from the web.<br/>`
	`34`	`+ /// Single instance of the handler used for all images downloaded in a single html, this way if the html contains more`
	`35`	`+ /// than one reference to the same image it will be downloaded only once.<br/>`
	`36`	`+ /// Also handles corrupt, partial and canceled downloads by first downloading to temp file and only if successful moving to cached`
	`37`	`+ /// file location.`
`34`	`38`	`/// </summary>`
`35`	`39`	`internal sealed class ImageDownloader : IDisposable`
`36`	`40`	`{`
`@@ -193,18 +197,21 @@ private void OnDownloadImageCompleted(WebClient client, Uri source, string tempP`
`193`	`197`	`List<DownloadFileAsyncCallback> callbacksList;`
`194`	`198`	`lock (_imageDownloadCallbacks)`
`195`	`199`	`{`
`196`		`- callbacksList = _imageDownloadCallbacks[filePath];`
`197`		`- _imageDownloadCallbacks.Remove(filePath);`
	`200`	`+ if (_imageDownloadCallbacks.TryGetValue(filePath, out callbacksList))`
	`201`	`+ _imageDownloadCallbacks.Remove(filePath);`
`198`	`202`	`}`
`199`	`203`
`200`		`- foreach (var cachedFileCallback in callbacksList)`
	`204`	`+ if (callbacksList != null)`
`201`	`205`	`{`
`202`		`- try`
	`206`	`+ foreach (var cachedFileCallback in callbacksList)`
`203`	`207`	`{`
`204`		`- cachedFileCallback(source, filePath, error, cancelled);`
	`208`	`+ try`
	`209`	`+ {`
	`210`	`+ cachedFileCallback(source, filePath, error, cancelled);`
	`211`	`+ }`
	`212`	`+ catch`
	`213`	`+ { }`
`205`	`214`	`}`
`206`		`- catch`
`207`		`- { }`
`208`	`215`	`}`
`209`	`216`	`}`
`210`	`217`
Original file line number	Diff line number	Diff line change
`@@ -421,6 +421,7 @@ public void SetHtml(string htmlSource, CssData baseCssData = null)`
`421`	`421`	`if (_root != null)`
`422`	`422`	`{`
`423`	`423`	`_selectionHandler = new SelectionHandler(_root);`
	`424`	`+ _imageDownloader = new ImageDownloader();`
`424`	`425`	`}`
`425`	`426`	`}`
`426`	`427`	`}`
`@@ -879,8 +880,6 @@ internal void AddHoverBox(CssBox box, CssBlock block)`
`879`	`880`	`/// </summary>`
`880`	`881`	`internal ImageDownloader GetImageDownloader()`
`881`	`882`	`{`
`882`		`- if (_imageDownloader == null)`
`883`		`- _imageDownloader = new ImageDownloader();`
`884`	`883`	`return _imageDownloader;`
`885`	`884`	`}`
`886`	`885`