Make OwnerPointer Movable & add test cases 41/191641/7
authorAdeel Kazmi <adeel.kazmi@samsung.com>
Fri, 19 Oct 2018 11:46:35 +0000 (12:46 +0100)
committerAdeel Kazmi <adeel.kazmi@samsung.com>
Wed, 24 Oct 2018 12:34:22 +0000 (13:34 +0100)
Change-Id: If01e25003536ff26ed67a620d094f2311a8e0340

automated-tests/src/dali-internal/CMakeLists.txt
automated-tests/src/dali-internal/utc-Dali-Internal-OwnerPointer.cpp [new file with mode: 0644]
dali/internal/common/owner-pointer.h

index 5fc4d99..64b34e7 100644 (file)
@@ -7,10 +7,11 @@ SET(CAPI_LIB "dali-internal")
 
 SET(TC_SOURCES
         utc-Dali-Internal-Core.cpp
-        utc-Dali-Internal-Handles.cpp
         utc-Dali-Internal-FixedSizeMemoryPool.cpp
-        utc-Dali-Internal-MemoryPoolObjectAllocator.cpp
         utc-Dali-Internal-FrustumCulling.cpp
+        utc-Dali-Internal-Handles.cpp
+        utc-Dali-Internal-MemoryPoolObjectAllocator.cpp
+        utc-Dali-Internal-OwnerPointer.cpp
 )
 
 LIST(APPEND TC_SOURCES
diff --git a/automated-tests/src/dali-internal/utc-Dali-Internal-OwnerPointer.cpp b/automated-tests/src/dali-internal/utc-Dali-Internal-OwnerPointer.cpp
new file mode 100644 (file)
index 0000000..1452895
--- /dev/null
@@ -0,0 +1,361 @@
+/*
+ * Copyright (c) 2018 Samsung Electronics Co., Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+// EXTERNAL INCLUDES
+#include <utility>
+
+// INTERNAL INCLUDES
+#include <dali-test-suite-utils.h>
+#include <dali/internal/common/owner-pointer.h>
+
+using namespace Dali::Internal;
+
+void utc_dali_internal_owner_pointer_startup(void)
+{
+  test_return_value = TET_UNDEF;
+}
+
+void utc_dali_internal_owner_pointer_cleanup(void)
+{
+  test_return_value = TET_PASS;
+}
+
+///////////////////////////////////////////////////////////////////////////////
+namespace
+{
+
+/// Takes in a reference to a bool which is set to true when the destructor is called
+class OwnedClass
+{
+public:
+
+  OwnedClass( bool& destructorCalled )
+  : mDestructorCalled( destructorCalled )
+  {
+    mDestructorCalled = false;
+  }
+
+  ~OwnedClass()
+  {
+    mDestructorCalled = true;
+  }
+
+private:
+
+  bool& mDestructorCalled;
+};
+
+/// Just a simple class with a function that marks a member boolean to true if that function is called
+class ClassWithFunction
+{
+public:
+  ClassWithFunction() = default;
+
+  void MyFunction()
+  {
+    functionCalled = true;
+  }
+
+  bool functionCalled{ false };
+};
+
+} // anon namespace
+
+///////////////////////////////////////////////////////////////////////////////
+
+int UtcDaliOwnerPointerEnsureDeletion(void)
+{
+  // Ensure that the object owned by the owner-pointer is deleted.
+
+  bool deleted = false;
+
+  {
+    OwnerPointer< OwnedClass > pointer( new OwnedClass( deleted ) );
+    DALI_TEST_EQUALS( deleted, false, TEST_LOCATION );
+  }
+
+  // OwnerPointer out-of-scope, object should be deleted.
+
+  DALI_TEST_EQUALS( deleted, true, TEST_LOCATION );
+
+  END_TEST;
+}
+
+int UtcDaliOwnerPointerDefaultConstructor(void)
+{
+  // Ensure the default constructor is created as expected.
+
+  OwnerPointer< OwnedClass > pointer;
+  DALI_TEST_CHECK( pointer.Get() == nullptr );
+
+  END_TEST;
+}
+
+int UtcDaliOwnerPointerCopy(void)
+{
+  // Call copy constructor and assignment operator
+
+  bool deleted = false;
+  OwnedClass* owned = new OwnedClass( deleted );
+
+  OwnerPointer< OwnedClass > first( owned );
+  DALI_TEST_CHECK( first.Get() == owned );
+
+  {
+    // Copy constructor, first should have a nullptr now, no object deletion
+    OwnerPointer< OwnedClass > second( first );
+    DALI_TEST_CHECK( first.Get() == nullptr );
+    DALI_TEST_CHECK( second.Get() == owned);
+    DALI_TEST_EQUALS( deleted, false, TEST_LOCATION );
+
+    // Self assignment, nothing should change or be deleted.
+    first = first;
+    second = second;
+    DALI_TEST_CHECK( first.Get() == nullptr );
+    DALI_TEST_CHECK( second.Get() == owned );
+    DALI_TEST_EQUALS( deleted, false, TEST_LOCATION );
+
+    // Assign second to first, no deletion, second should have a nullptr now
+    first = second;
+    DALI_TEST_CHECK( first.Get() == owned );
+    DALI_TEST_CHECK( second.Get() == nullptr );
+    DALI_TEST_EQUALS( deleted, false, TEST_LOCATION );
+  }
+
+  // second is out-of-scope now, no object deletion
+  DALI_TEST_EQUALS( deleted, false, TEST_LOCATION );
+
+  // Assign to an empty pointer, owned object should be deleted
+  OwnerPointer< OwnedClass > empty;
+  first = empty;
+  DALI_TEST_EQUALS( deleted, true, TEST_LOCATION );
+  DALI_TEST_CHECK( first.Get() == nullptr );
+  DALI_TEST_CHECK( empty.Get() == nullptr );
+
+  END_TEST;
+}
+
+int UtcDaliOwnerPointerMove(void)
+{
+  // Call move constructor and move assignment operator
+
+  bool deleted = false;
+  OwnedClass* owned = new OwnedClass( deleted );
+
+  OwnerPointer< OwnedClass > first( owned );
+  DALI_TEST_CHECK( first.Get() == owned );
+
+  {
+    // Move constructor, first should have a nullptr now, no object deletion
+    OwnerPointer< OwnedClass > second( std::move( first ) );
+    DALI_TEST_CHECK( first.Get() == nullptr );
+    DALI_TEST_CHECK( second.Get() == owned);
+    DALI_TEST_EQUALS( deleted, false, TEST_LOCATION );
+
+    // Self assignment, nothing should change or be deleted.
+    first = std::move( first );
+    second = std::move( second );
+    DALI_TEST_CHECK( first.Get() == nullptr );
+    DALI_TEST_CHECK( second.Get() == owned );
+    DALI_TEST_EQUALS( deleted, false, TEST_LOCATION );
+
+    // Assign second to first, no deletion, second should have a nullptr now
+    first = std::move( second );
+    DALI_TEST_CHECK( first.Get() == owned );
+    DALI_TEST_CHECK( second.Get() == nullptr );
+    DALI_TEST_EQUALS( deleted, false, TEST_LOCATION );
+  }
+
+  // second is out-of-scope now, no object deletion
+  DALI_TEST_EQUALS( deleted, false, TEST_LOCATION );
+
+  // Assign to an empty pointer, owned object should be deleted
+  OwnerPointer< OwnedClass > empty;
+  first = std::move( empty );
+  DALI_TEST_EQUALS( deleted, true, TEST_LOCATION );
+  DALI_TEST_CHECK( first.Get() == nullptr );
+  DALI_TEST_CHECK( empty.Get() == nullptr );
+
+  END_TEST;
+}
+
+int UtcDaliOwnerPointerIndirection(void)
+{
+  // Check the indirection operators
+
+  using Ptr = OwnerPointer< int >;
+
+  {
+    int* rawIntPtr( new int( 200 ) );
+    Ptr nonConstPtr( rawIntPtr );
+    DALI_TEST_CHECK( rawIntPtr == &( *nonConstPtr ) );
+    DALI_TEST_EQUALS( *nonConstPtr, 200, TEST_LOCATION );
+  }
+
+  {
+    int* rawIntPtr2( new int( 300 ) );
+    const Ptr constPtr( rawIntPtr2 );
+    DALI_TEST_CHECK( rawIntPtr2 == &( *constPtr ) );
+    DALI_TEST_EQUALS( *constPtr, 300, TEST_LOCATION );
+  }
+
+  END_TEST;
+}
+
+int UtcDaliOwnerPointerPointerOperator(void)
+{
+  // Check the pointer operators
+
+  using Ptr = OwnerPointer< ClassWithFunction >;
+
+  // Check if function is called as expected when using a const OwnerPointer
+  {
+    ClassWithFunction* rawPtr( new ClassWithFunction );
+    Ptr nonConstPtr( rawPtr );
+    DALI_TEST_EQUALS( rawPtr->functionCalled, false, TEST_LOCATION );
+    nonConstPtr->MyFunction();
+    DALI_TEST_EQUALS( rawPtr->functionCalled, true, TEST_LOCATION );
+  }
+
+
+  // Check if function is called as expected when using a const OwnerPointer
+  {
+    ClassWithFunction* rawPtr2( new ClassWithFunction );
+    const Ptr constPtr( rawPtr2 );
+    DALI_TEST_EQUALS( rawPtr2->functionCalled, false, TEST_LOCATION );
+    constPtr->MyFunction();
+    DALI_TEST_EQUALS( rawPtr2->functionCalled, true, TEST_LOCATION );
+  }
+  END_TEST;
+}
+
+int UtcDaliOwnerPointerComparisonOperator(void)
+{
+  // Check the comparison operator
+
+  using Ptr = OwnerPointer< int >;
+
+  int* rawIntPtr( new int( 200 ) );
+  Ptr ownerPtr( rawIntPtr );
+  DALI_TEST_CHECK( ownerPtr == rawIntPtr );
+  DALI_TEST_CHECK( ! ( ownerPtr == nullptr ) );
+
+  END_TEST;
+}
+
+int UtcDaliOwnerPointerReset(void)
+{
+  // Ensure that calling Reset deletes the object and sets the owner-pointer to NULL
+
+  bool deleted = false;
+
+  OwnerPointer< OwnedClass > pointer( new OwnedClass( deleted ) );
+  DALI_TEST_EQUALS( deleted, false, TEST_LOCATION );
+  pointer.Reset();
+  DALI_TEST_EQUALS( deleted, true, TEST_LOCATION );
+  DALI_TEST_CHECK( pointer.Get() == nullptr );
+
+  // Reset the empty pointer, should have no effect but there shouldn't be any crash
+  pointer.Reset();
+  DALI_TEST_CHECK( pointer.Get() == nullptr );
+
+  END_TEST;
+}
+
+int UtcDaliOwnerPointerRelease(void)
+{
+  // Ensure that calling Release does NOT delete the object but still sets the owner-pointer to NULL
+
+  bool deleted = false;
+
+  OwnedClass* rawPtr = new OwnedClass( deleted );
+  OwnerPointer< OwnedClass > pointer( rawPtr );
+
+  DALI_TEST_EQUALS( deleted, false, TEST_LOCATION );
+  DALI_TEST_CHECK( pointer.Release() == rawPtr );
+  DALI_TEST_EQUALS( deleted, false, TEST_LOCATION );
+  DALI_TEST_CHECK( pointer.Get() == nullptr );
+
+  // Release the empty pointer, should have no effect but there shouldn't be any crash
+  DALI_TEST_CHECK( pointer.Release() == nullptr );
+  DALI_TEST_CHECK( pointer.Get() == nullptr );
+
+  END_TEST;
+}
+
+int UtcDaliOwnerPointerGet(void)
+{
+  // Check the Get method
+
+  using Ptr = OwnerPointer< int >;
+
+  int* rawIntPtr( new int( 200 ) );
+  Ptr ownerPtr( rawIntPtr );
+  DALI_TEST_CHECK( ownerPtr.Get() == rawIntPtr );
+
+  END_TEST;
+}
+
+int UtcDaliOwnerPointerSwap(void)
+{
+  // Ensure the Swap method swaps the pointers and doesn't delete any objects
+
+  using Ptr = OwnerPointer< OwnedClass >;
+
+  bool firstObjectDeleted = false;
+  bool secondObjectDeleted = false;
+
+  OwnedClass* firstRawPtr = new OwnedClass( firstObjectDeleted );
+  OwnedClass* secondRawPtr = new OwnedClass( secondObjectDeleted );
+
+  Ptr firstPtr( firstRawPtr );
+  Ptr secondPtr( secondRawPtr );
+
+  // Check initial values
+  DALI_TEST_EQUALS( firstObjectDeleted, false, TEST_LOCATION );
+  DALI_TEST_EQUALS( secondObjectDeleted, false, TEST_LOCATION );
+  DALI_TEST_CHECK( firstPtr == firstRawPtr );
+  DALI_TEST_CHECK( secondPtr == secondRawPtr );
+
+  // Call Swap on first and ensure swap is done and there's no deletion
+  firstPtr.Swap( secondPtr );
+  DALI_TEST_EQUALS( firstObjectDeleted, false, TEST_LOCATION );
+  DALI_TEST_EQUALS( secondObjectDeleted, false, TEST_LOCATION );
+  DALI_TEST_CHECK( firstPtr == secondRawPtr );
+  DALI_TEST_CHECK( secondPtr == firstRawPtr );
+
+  // Swap back using second, again no deletion
+  secondPtr.Swap( firstPtr );
+  DALI_TEST_EQUALS( firstObjectDeleted, false, TEST_LOCATION );
+  DALI_TEST_EQUALS( secondObjectDeleted, false, TEST_LOCATION );
+  DALI_TEST_CHECK( firstPtr == firstRawPtr );
+  DALI_TEST_CHECK( secondPtr == secondRawPtr );
+
+  // Swap with self, nothing should change or be deleted
+  firstPtr.Swap( firstPtr );
+  DALI_TEST_EQUALS( firstObjectDeleted, false, TEST_LOCATION );
+  DALI_TEST_CHECK( firstPtr == firstRawPtr );
+
+  // Swap with an empty OwnerPointer, no deletion but firstPtr should be empty now
+  Ptr emptyPtr;
+  firstPtr.Swap( emptyPtr );
+  DALI_TEST_EQUALS( firstObjectDeleted, false, TEST_LOCATION );
+  DALI_TEST_CHECK( firstPtr == nullptr );
+  DALI_TEST_CHECK( emptyPtr == firstRawPtr );
+
+  END_TEST;
+}
index e780920..165747a 100644 (file)
@@ -2,7 +2,7 @@
 #define __DALI_INTERNAL_OWNER_POINTER_H__
 
 /*
- * Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2018 Samsung Electronics Co., Ltd.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -18,9 +18,6 @@
  *
  */
 
-// EXTERNAL INCLUDES
-#include <cstddef>    // NULL
-
 // INTERNAL INCLUDES
 #include <dali/public-api/common/dali-common.h>
 
@@ -30,15 +27,19 @@ namespace Dali
 namespace Internal
 {
 
-template < typename T >
+template< typename T >
 class OwnerPointer
 {
 public:
+
   /**
    * Default constructor. Creates an OwnerPointer that does not own any object.
+   * @note This does not protect against two different OwnerPointers pointing to the same object.
+   *       Both OwnerPointers will try to release the memory of the same object in that case which
+   *       could lead to a crash.
    */
   OwnerPointer()
-  : mObject(NULL)
+  : mObject( nullptr )
   {
   }
 
@@ -47,7 +48,7 @@ public:
    * @param[in] object A pointer to a heap allocated object.
    */
   OwnerPointer( T* object )
-  : mObject(object)
+  : mObject( object )
   {
   }
 
@@ -56,11 +57,20 @@ public:
    * @param[in] other The pointer that gives away the ownership.
    */
   OwnerPointer( const OwnerPointer& other )
-  : mObject(NULL)
+  : OwnerPointer( static_cast< OwnerPointer&& >( const_cast<OwnerPointer&>( other ) ) ) // Remove constness & cast to rvalue to use the move constructor
   {
     // other needs to be const for compiler to pick up this as copy constructor;
     // though we are using this as move as there can only be one owner
-    Swap( const_cast<OwnerPointer&>( other ) );
+  }
+
+  /**
+   * Move constructor. Passes the ownership of a pointer to another.
+   * @param[in] other The pointer that gives away the ownership.
+   */
+  OwnerPointer( OwnerPointer&& other )
+  : mObject( nullptr )
+  {
+    Swap( other );
   }
 
   /**
@@ -71,8 +81,9 @@ public:
   {
     if( this != &other )    // no self-assignment
     {
-      // Creation of temportaty object to prevent premature deletion of object
-      OwnerPointer(other).Swap(*this);
+      delete mObject;
+      mObject = other.mObject;
+      other.mObject = nullptr;
     }
 
     // return self
@@ -80,6 +91,16 @@ public:
   }
 
   /**
+   * Move assignment operator. Passes the ownership of a pointer to another.
+   * @param[in] other The pointer that gives away the ownership.
+   */
+  OwnerPointer& operator=( OwnerPointer&& other )
+  {
+    // Reuse operator=
+    return operator=( other );
+  }
+
+  /**
    * Assignment operator. Takes the ownership of the object.
    * If it owns an object already, it will be deleted.
    * @param[in] pointer A pointer to a heap allocated object.
@@ -109,7 +130,7 @@ public:
    */
   T& operator*()
   {
-    DALI_ASSERT_DEBUG( mObject != NULL );
+    DALI_ASSERT_DEBUG( mObject );
 
     return *mObject;
   }
@@ -120,7 +141,7 @@ public:
    */
   T& operator*() const
   {
-    DALI_ASSERT_DEBUG( mObject != NULL );
+    DALI_ASSERT_DEBUG( mObject );
 
     // Pointer semantics: A const pointer does not mean const data.
     return const_cast< T& >( *mObject );
@@ -159,11 +180,8 @@ public:
    */
   void Reset()
   {
-    if ( mObject != NULL )
-    {
-      delete mObject;
-      mObject = NULL;
-    }
+    delete mObject;
+    mObject = nullptr;
   }
 
   /**
@@ -173,7 +191,7 @@ public:
   T* Release()
   {
     T* tmp = mObject;
-    mObject = NULL;
+    mObject = nullptr;
     return tmp;
   }
 
@@ -188,12 +206,16 @@ public:
 
   /**
    * Swap owned objects
+   * @param[in] other The pointer to swap the owned objects with.
    */
   void Swap( OwnerPointer& other )
   {
-    T* tmp = mObject;
-    mObject = other.mObject;
-    other.mObject = tmp;
+    if( this != &other )
+    {
+      T* tmp = mObject;
+      mObject = other.mObject;
+      other.mObject = tmp;
+    }
   }
 
   // Handle comparisons - This is a variation of the safe bool idiom
@@ -209,7 +231,7 @@ public:
    */
   operator BooleanType() const
   {
-    return (mObject != NULL) ? &OwnerPointer::ThisIsSaferThanReturningVoidStar : NULL;
+    return ( mObject != nullptr ) ? &OwnerPointer::ThisIsSaferThanReturningVoidStar : nullptr;
   }
 
 private: