Fix to disable all tests for in-flight cancellation off the async loading path 61/25661/5
authorAndrew Cox <andrew.cox@partner.samsung.com>
Thu, 7 Aug 2014 16:05:48 +0000 (17:05 +0100)
committerAndrew Cox <andrew.cox@partner.samsung.com>
Mon, 11 Aug 2014 10:49:48 +0000 (11:49 +0100)
[problem]
When loading nine-patch images, an interruption exception could be
thrown on a thread that doesn't handle them.

[cause]
Our standard loading path involves two threads at a time coordinating on
cancellation of resource loads that have already been started.
Nine-patch images are a special case that execute image loading file IO
and decoding to bitmaps directly on the event thread, thus introducing a
third thread into this mix.
The event thread was not considered in the design or the reviews of the
late image cancelation feature.

[solution]
Make all image loading paths other than the main async one on a
background thread use a stubbed NOP implementation of the interface that
lets such loading paths interrupt themselves.
Thus the nine-patch loading path can never be interrupted.

No currently-commited demos show this issue but the following patch
fails without this one and works after it is applied:
https://review.tizen.org/gerrit/25392

Change-Id: Id5adeebb95b41eac1414e58b8ab61e560ad1a585
Signed-off-by: Andrew Cox <andrew.cox@partner.samsung.com>
platform-abstractions/slp/resource-loader/resource-loading-client.h
platform-abstractions/slp/resource-loader/resource-thread-image.cpp
platform-abstractions/slp/resource-loader/resource-thread-image.h

index df38279..b287281 100644 (file)
@@ -54,6 +54,30 @@ private:
   ResourceLoadingClient& operator =( ResourceLoadingClient& rhs );
 };
 
+/**
+ * @brief Default implementation of a caller of a low-level resource loading
+ * function which does nothing.
+ */
+class StubbedResourceLoadingClient : public ResourceLoadingClient
+{
+public:
+  /**
+   * @brief Check whether the current request has been cancelled.
+   *
+   * This does nothing and so can never throw an exception.
+   **/
+  virtual void InterruptionPoint() const {}
+
+  /** Construction is a NOP. */
+  StubbedResourceLoadingClient() {}
+  /** Destruction has no work to do. */
+  ~StubbedResourceLoadingClient() {}
+
+private:
+  StubbedResourceLoadingClient( const StubbedResourceLoadingClient& rhs );
+  StubbedResourceLoadingClient& operator =( StubbedResourceLoadingClient& rhs );
+};
+
 } /* namespace SlpPlatform */
 } /* namespace Dali */
 
index 3d25b1b..0fcaf1c 100755 (executable)
@@ -263,7 +263,7 @@ ResourcePointer ResourceThreadImage::LoadResourceSynchronously( const Integratio
   FILE * const fp = fopen( resourcePath.c_str(), "rb" );
   if( fp != NULL )
   {
-    bool result = ConvertStreamToBitmap( resourceType, resourcePath, fp, bitmap );
+    bool result = ConvertStreamToBitmap( resourceType, resourcePath, fp, StubbedResourceLoadingClient(), bitmap );
     if( result && bitmap )
     {
       resource.Reset(bitmap.Get());
@@ -379,7 +379,7 @@ void ResourceThreadImage::Load(const ResourceRequest& request)
 
   if( fp != NULL )
   {
-    result = ConvertStreamToBitmap( *request.GetType(), request.GetPath(), fp, bitmap );
+    result = ConvertStreamToBitmap( *request.GetType(), request.GetPath(), fp, *this, bitmap );
     // Last chance to interrupt a cancelled load before it is reported back to clients
     // which have already stopped tracking it:
     InterruptionPoint(); // Note: This can throw an exception.
@@ -443,7 +443,7 @@ void ResourceThreadImage::Decode(const ResourceRequest& request)
       FILE * const fp = fileCloser.GetFile();
       if ( fp != NULL )
       {
-        bool result = ConvertStreamToBitmap( *request.GetType(), request.GetPath(), fp, bitmap );
+        bool result = ConvertStreamToBitmap( *request.GetType(), request.GetPath(), fp, StubbedResourceLoadingClient(), bitmap );
 
         if ( result && bitmap )
         {
@@ -474,7 +474,7 @@ void ResourceThreadImage::Save(const Integration::ResourceRequest& request)
   DALI_LOG_WARNING( "Image saving not supported on background resource threads." );
 }
 
-bool ResourceThreadImage::ConvertStreamToBitmap(const ResourceType& resourceType, std::string path, FILE * const fp, BitmapPtr& ptr)
+bool ResourceThreadImage::ConvertStreamToBitmap(const ResourceType& resourceType, std::string path, FILE * const fp, const ResourceLoadingClient& client, BitmapPtr& ptr)
 {
   DALI_LOG_TRACE_METHOD(mLogFilter);
   DALI_ASSERT_DEBUG( ResourceBitmap == resourceType.id );
@@ -501,9 +501,9 @@ bool ResourceThreadImage::ConvertStreamToBitmap(const ResourceType& resourceType
       ImageAttributes attributes  = resType.imageAttributes;
 
       // Check for cancellation now we have hit the filesystem, done some allocation, and burned some cycles:
-      InterruptionPoint(); // Note: This can throw an exception.
+      client.InterruptionPoint(); // Note: This can throw an exception.
 
-      result = function( fp, *bitmap, attributes, *this );
+      result = function( fp, *bitmap, attributes, client );
 
       if (!result)
       {
@@ -550,7 +550,7 @@ bool ResourceThreadImage::ConvertStreamToBitmap(const ResourceType& resourceType
           // Make a new bitmap with the central part of the loaded one if required:
           if( scanlinesToTrim > 0 || columnsToTrim > 0 ) ///@ToDo: Make this test a bit fuzzy (allow say a 5% difference).
           {
-            InterruptionPoint(); // Note: This can throw an exception.
+            client.InterruptionPoint(); // Note: This can throw an exception.
 
             const unsigned newWidth = loadedWidth - 2 * columnsToTrim;
             const unsigned newHeight = loadedHeight - 2 * scanlinesToTrim;
index e9a4b36..364b039 100644 (file)
@@ -95,12 +95,14 @@ private:
    * @param[in] resourceType The type of resource to convert.
    * @param[in] path The path to the resource.
    * @param[in] fp File Pointer. Closed on exit.
+   * @param[in] client The component that is initiating the conversion.
    * @param[out] bitmap Pointer to write bitmap to
    * @return true on success, false on failure
    */
   bool ConvertStreamToBitmap( const Integration::ResourceType& resourceType,
                               std::string path,
                               FILE * const fp,
+                              const ResourceLoadingClient& client,
                               Integration::BitmapPtr& ptr );
 
 }; // class ResourceThreadImage