Fixed segfault if nine-patch image failed to load 84/24084/1
authorDavid Steele <david.steele@partner.samsung.com>
Fri, 20 Jun 2014 13:59:46 +0000 (14:59 +0100)
committerAdeel Kazmi <adeel.kazmi@samsung.com>
Tue, 8 Jul 2014 13:15:33 +0000 (14:15 +0100)
[problem]      If nine patch image fails to load, e.g. file not found,
then empty bitmap pointer is accessed when image actor tries to use it.
[cause]        Not checking bitmap pointer before dereference
[solution]     Ensured bitmap pointers are checked, if NULL, then an error is logged.

Change-Id: If42d2db4f662fc2323057f71b365a741e254d125
Signed-off-by: David Steele <david.steele@partner.samsung.com>
automated-tests/src/dali/tct-dali-core.h
automated-tests/src/dali/utc-Dali-ImageActor.cpp
dali/internal/event/actors/image-actor-impl.cpp
dali/internal/event/images/image-impl.cpp
dali/internal/event/images/nine-patch-image-impl.cpp

index 106da49..abfb082 100644 (file)
@@ -705,6 +705,8 @@ extern int UtcDaliImageActorNewNullWithArea(void);
 extern int UtcDaliImageActorSetImage(void);
 extern int UtcDaliImageActorPropertyIndices(void);
 extern int UtcDaliImageActorImageProperty(void);
+extern int UtcDaliImageActorNinePatch01(void);
+extern int UtcDaliImageActorNinePatch02(void);
 extern int UtcDaliImageAttributesConstructor(void);
 extern int UtcDaliImageAttributesLessThan(void);
 extern int UtcDaliImageAttributesEquality(void);
@@ -1928,6 +1930,8 @@ testcase tc_array[] = {
     {"UtcDaliImageActorSetImage", UtcDaliImageActorSetImage, image_actor_test_startup, image_actor_test_cleanup},
     {"UtcDaliImageActorPropertyIndices", UtcDaliImageActorPropertyIndices, image_actor_test_startup, image_actor_test_cleanup},
     {"UtcDaliImageActorImageProperty", UtcDaliImageActorImageProperty, image_actor_test_startup, image_actor_test_cleanup},
+    {"UtcDaliImageActorNinePatch01", UtcDaliImageActorNinePatch01, image_actor_test_startup, image_actor_test_cleanup},
+    {"UtcDaliImageActorNinePatch02", UtcDaliImageActorNinePatch02, image_actor_test_startup, image_actor_test_cleanup},
     {"UtcDaliImageAttributesConstructor", UtcDaliImageAttributesConstructor, utc_dali_image_attributes_startup, utc_dali_image_attributes_cleanup},
     {"UtcDaliImageAttributesLessThan", UtcDaliImageAttributesLessThan, utc_dali_image_attributes_startup, utc_dali_image_attributes_cleanup},
     {"UtcDaliImageAttributesEquality", UtcDaliImageAttributesEquality, utc_dali_image_attributes_startup, utc_dali_image_attributes_cleanup},
index 1396290..d78782e 100644 (file)
@@ -956,3 +956,94 @@ int UtcDaliImageActorImageProperty(void)
   END_TEST;
 }
 
+int UtcDaliImageActorNinePatch01(void)
+{
+  TestApplication application;
+  TestPlatformAbstraction& platform = application.GetPlatform();
+  TestGlAbstraction& glAbstraction = application.GetGlAbstraction();
+  TraceCallStack& textureTrace = glAbstraction.GetTextureTrace();
+  TraceCallStack& drawTrace = glAbstraction.GetDrawTrace();
+
+  tet_infoline("Test the successful loading of a nine-patch image\n");
+
+  platform.SetClosestImageSize(Vector2(4, 4));
+  Integration::Bitmap* bitmap = Integration::Bitmap::New( Integration::Bitmap::BITMAP_2D_PACKED_PIXELS, false );
+  Integration::PixelBuffer* pixels = bitmap->GetPackedPixelsProfile()->ReserveBuffer( Pixel::RGBA8888,  4,4,4,4 );
+  memset( pixels, 0, 64 );
+
+  Integration::ResourcePointer resourcePtr(bitmap); // reference it
+  platform.SetResourceLoaded( 0, Dali::Integration::ResourceBitmap, resourcePtr );
+
+  Image ninePatchImage = Image::New( "blah.#.png" );
+  DALI_TEST_CHECK( ninePatchImage );
+
+  ImageActor imageActor = ImageActor::New( ninePatchImage );
+  DALI_TEST_CHECK( imageActor );
+  Stage::GetCurrent().Add( imageActor );
+
+  drawTrace.Reset();
+  textureTrace.Reset();
+  drawTrace.Enable(true);
+  textureTrace.Enable(true);
+  glAbstraction.ClearBoundTextures();
+  std::vector<GLuint> ids;
+  ids.push_back( 23 );
+  glAbstraction.SetNextTextureIds( ids );
+
+  application.SendNotification();
+  application.Render();
+
+  DALI_TEST_CHECK( drawTrace.FindMethod( "DrawArrays" ) );
+  typedef std::vector<GLuint> TexVec;
+  const TexVec& textures = glAbstraction.GetBoundTextures(GL_TEXTURE0);
+  DALI_TEST_CHECK( textures.size() > 0 );
+  if( textures.size() > 0 )
+  {
+    DALI_TEST_EQUALS( textures[0], 23u, TEST_LOCATION );
+  }
+
+  END_TEST;
+}
+
+
+int UtcDaliImageActorNinePatch02(void)
+{
+  TestApplication application;
+  TestPlatformAbstraction& platform = application.GetPlatform();
+  TestGlAbstraction& glAbstraction = application.GetGlAbstraction();
+  TraceCallStack& textureTrace = glAbstraction.GetTextureTrace();
+  TraceCallStack& drawTrace = glAbstraction.GetDrawTrace();
+
+  tet_infoline("Test the failed loading of a nine-patch image\n");
+
+  platform.SetClosestImageSize(Vector2(0, 0));
+  Integration::ResourcePointer resourcePtr;
+  platform.SetResourceLoaded( 0, Dali::Integration::ResourceBitmap, resourcePtr );
+
+  Image ninePatchImage = Image::New( "blah.#.png" );
+  DALI_TEST_CHECK( ninePatchImage );
+
+  ImageActor imageActor = ImageActor::New( ninePatchImage );
+  DALI_TEST_CHECK( imageActor );
+  Stage::GetCurrent().Add( imageActor );
+
+  drawTrace.Reset();
+  textureTrace.Reset();
+  drawTrace.Enable(true);
+  textureTrace.Enable(true);
+  glAbstraction.ClearBoundTextures();
+  std::vector<GLuint> ids;
+  ids.push_back( 23 );
+  glAbstraction.SetNextTextureIds( ids );
+
+  application.SendNotification();
+  application.Render();
+
+  // Check that nothing was drawn.
+  DALI_TEST_CHECK( ! drawTrace.FindMethod( "DrawArrays" ) );
+  typedef std::vector<GLuint> TexVec;
+  const TexVec& textures = glAbstraction.GetBoundTextures(GL_TEXTURE0);
+  DALI_TEST_CHECK( textures.size() == 0u );
+
+  END_TEST;
+}
index 4a19fd3..f192359 100644 (file)
@@ -111,6 +111,7 @@ ImageActorPtr ImageActor::New( Image* anImage )
   if( ninePatchImage != NULL )
   {
     theImage = ninePatchImage->CreateCroppedBitmapImage();
+    // Note, if theImage is empty, the nine-patch image was not loaded
   }
 
   // Create the attachment
index 5c532b2..cdb355e 100644 (file)
@@ -356,7 +356,7 @@ bool Image::IsNinePatchFileName( std::string filename )
       break;
     }
 
-    // Satisfy prevnet
+    // Satisfy prevent
     if( state == DONE )
     {
       break;
index 3c8a069..65cbb72 100644 (file)
@@ -161,7 +161,14 @@ NinePatchImage::NinePatchImage( const std::string& filename, const Dali::ImageAt
 
   // Note, bitmap is only destroyed when the image is destroyed.
   Integration::ResourcePointer resource = platformAbstraction.LoadResourceSynchronously(resourceType, filename);
-  mBitmap = static_cast<Integration::Bitmap*>( resource.Get());
+  if( resource )
+  {
+    mBitmap = static_cast<Integration::Bitmap*>( resource.Get());
+  }
+  else
+  {
+    mBitmap.Reset();
+  }
 }
 
 NinePatchImage* NinePatchImage::GetNinePatchImage( Image* image)
@@ -193,43 +200,54 @@ Rect<int> NinePatchImage::GetChildRectangle()
 
 Internal::BitmapImagePtr NinePatchImage::CreateCroppedBitmapImage()
 {
-  Pixel::Format pixelFormat = mBitmap->GetPixelFormat();
-
-  BitmapImagePtr cropped = BitmapImage::New( mWidth-2, mHeight-2, pixelFormat,
-                                             Dali::Image::Immediate, Dali::Image::Never );
-
-  Integration::Bitmap::PackedPixelsProfile* srcProfile = mBitmap->GetPackedPixelsProfile();
-  DALI_ASSERT_DEBUG( srcProfile && "Wrong profile for source bitmap");
+  BitmapImagePtr cropped;
 
-  if( srcProfile )
+  if( ! mBitmap )
   {
-    PixelBuffer* destPixels = cropped->GetBuffer();
-    unsigned int destStride = cropped->GetBufferStride();
-    unsigned int pixelWidth = GetBytesPerPixel(pixelFormat);
+    DALI_LOG_ERROR( "NinePatchImage: Bitmap not loaded, cannot perform operation\n");
+  }
+  else
+  {
+    Pixel::Format pixelFormat = mBitmap->GetPixelFormat();
 
-    PixelBuffer* srcPixels = mBitmap->GetBuffer();
-    unsigned int srcStride = srcProfile->GetBufferStride();
+    cropped = BitmapImage::New( mWidth-2, mHeight-2, pixelFormat, Dali::Image::Immediate, Dali::Image::Never );
+
+    Integration::Bitmap::PackedPixelsProfile* srcProfile = mBitmap->GetPackedPixelsProfile();
+    DALI_ASSERT_DEBUG( srcProfile && "Wrong profile for source bitmap");
 
-    for( unsigned int row=1; row < mHeight-1; ++row )
+    if( srcProfile )
     {
-      PixelBuffer* src  = srcPixels + row*srcStride + pixelWidth;
-      PixelBuffer* dest = destPixels + (row-1)*destStride;
-      memcpy(dest, src, destStride );
+      PixelBuffer* destPixels = cropped->GetBuffer();
+      unsigned int destStride = cropped->GetBufferStride();
+      unsigned int pixelWidth = GetBytesPerPixel(pixelFormat);
+
+      PixelBuffer* srcPixels = mBitmap->GetBuffer();
+      unsigned int srcStride = srcProfile->GetBufferStride();
+
+      for( unsigned int row=1; row < mHeight-1; ++row )
+      {
+        PixelBuffer* src  = srcPixels + row*srcStride + pixelWidth;
+        PixelBuffer* dest = destPixels + (row-1)*destStride;
+        memcpy(dest, src, destStride );
+      }
     }
-  }
 
-  RectArea area;
-  cropped->Update(area); // default area has no width or height
+    RectArea area;
+    cropped->Update(area); // default area has no width or height
+  }
   return cropped;
 }
 
 void NinePatchImage::Connect()
 {
-  if( mConnectionCount == 0 && !mTicket )
+  if( !mTicket )
   {
-    const ImageTicketPtr& t = mResourceClient->AddBitmapImage(mBitmap.Get());
-    mTicket = t.Get();
-    mTicket->AddObserver(*this);
+    if( mBitmap )
+    {
+      const ImageTicketPtr& t = mResourceClient->AddBitmapImage(mBitmap.Get());
+      mTicket = t.Get();
+      mTicket->AddObserver(*this);
+    }
   }
 
   ++mConnectionCount;
@@ -246,6 +264,12 @@ void NinePatchImage::Disconnect()
 
 void NinePatchImage::ParseBorders()
 {
+  if( ! mBitmap )
+  {
+    DALI_LOG_ERROR( "NinePatchImage: Bitmap not loaded, cannot perform operation\n");
+    return;
+  }
+
   Pixel::Format pixelFormat = mBitmap->GetPixelFormat();
 
   Integration::Bitmap::PackedPixelsProfile* srcProfile = mBitmap->GetPackedPixelsProfile();