Revert "Fix wp and sp comparison bugs"

This reverts commit 029b12ebde0b5a13b4b1252dc20b2ecc8194d5b7.

Reason for revert: There appear to be problems with null comparisons. Reported failure in HwcBufferCacheTest.

Change-Id: I19745bb281dabe8b05c2df3fe95e7be7a49dcd51
(cherry picked from commit a2a2ad805775ea88f25388677aa37e0492a492c4)
diff --git a/libutils/RefBase_test.cpp b/libutils/RefBase_test.cpp
index b594ca5..2e0cf6e 100644
--- a/libutils/RefBase_test.cpp
+++ b/libutils/RefBase_test.cpp
@@ -45,44 +45,6 @@
     bool* mDeleted;
 };
 
-// A version of Foo that ensures that all objects are allocated at the same
-// address. No more than one can be allocated at a time. Thread-hostile.
-class FooFixedAlloc : public RefBase {
-public:
-    static void* operator new(size_t size) {
-        if (mAllocCount != 0) {
-            abort();
-        }
-        mAllocCount = 1;
-        if (theMemory == nullptr) {
-            theMemory = malloc(size);
-        }
-        return theMemory;
-    }
-
-    static void operator delete(void *p) {
-        if (mAllocCount != 1 || p != theMemory) {
-            abort();
-        }
-        mAllocCount = 0;
-    }
-
-    FooFixedAlloc(bool* deleted_check) : mDeleted(deleted_check) {
-        *mDeleted = false;
-    }
-
-    ~FooFixedAlloc() {
-        *mDeleted = true;
-    }
-private:
-    bool* mDeleted;
-    static int mAllocCount;
-    static void* theMemory;
-};
-
-int FooFixedAlloc::mAllocCount(0);
-void* FooFixedAlloc::theMemory(nullptr);
-
 TEST(RefBase, StrongMoves) {
     bool isDeleted;
     Foo* foo = new Foo(&isDeleted);
@@ -128,91 +90,6 @@
     ASSERT_FALSE(isDeleted) << "Deletion on wp destruction should no longer occur";
 }
 
-TEST(RefBase, Comparisons) {
-    bool isDeleted, isDeleted2;
-    Foo* foo = new Foo(&isDeleted);
-    Foo* foo2 = new Foo(&isDeleted2);
-    sp<Foo> sp1(foo);
-    sp<Foo> sp2(foo2);
-    wp<Foo> wp1(sp1);
-    wp<Foo> wp2(sp1);
-    wp<Foo> wp3(sp2);
-    ASSERT_TRUE(wp1 == wp2);
-    ASSERT_TRUE(wp1 == sp1);
-    ASSERT_TRUE(wp3 == sp2);
-    ASSERT_TRUE(wp1 != sp2);
-    ASSERT_TRUE(wp1 <= wp2);
-    ASSERT_TRUE(wp1 >= wp2);
-    ASSERT_FALSE(wp1 != wp2);
-    ASSERT_FALSE(wp1 > wp2);
-    ASSERT_FALSE(wp1 < wp2);
-    ASSERT_FALSE(sp1 == sp2);
-    ASSERT_TRUE(sp1 != sp2);
-    bool sp1_smaller = sp1 < sp2;
-    wp<Foo>wp_smaller = sp1_smaller ? wp1 : wp3;
-    wp<Foo>wp_larger = sp1_smaller ? wp3 : wp1;
-    ASSERT_TRUE(wp_smaller < wp_larger);
-    ASSERT_TRUE(wp_smaller != wp_larger);
-    ASSERT_TRUE(wp_smaller <= wp_larger);
-    ASSERT_FALSE(wp_smaller == wp_larger);
-    ASSERT_FALSE(wp_smaller > wp_larger);
-    ASSERT_FALSE(wp_smaller >= wp_larger);
-    sp2 = nullptr;
-    ASSERT_TRUE(isDeleted2);
-    ASSERT_FALSE(isDeleted);
-    ASSERT_FALSE(wp3 == sp2);
-    // Comparison results on weak pointers should not be affected.
-    ASSERT_TRUE(wp_smaller < wp_larger);
-    ASSERT_TRUE(wp_smaller != wp_larger);
-    ASSERT_TRUE(wp_smaller <= wp_larger);
-    ASSERT_FALSE(wp_smaller == wp_larger);
-    ASSERT_FALSE(wp_smaller > wp_larger);
-    ASSERT_FALSE(wp_smaller >= wp_larger);
-    wp2 = nullptr;
-    ASSERT_FALSE(wp1 == wp2);
-    ASSERT_TRUE(wp1 != wp2);
-    wp1.clear();
-    ASSERT_TRUE(wp1 == wp2);
-    ASSERT_FALSE(wp1 != wp2);
-    wp3.clear();
-    ASSERT_TRUE(wp1 == wp3);
-    ASSERT_FALSE(wp1 != wp3);
-    ASSERT_FALSE(isDeleted);
-    sp1.clear();
-    ASSERT_TRUE(isDeleted);
-    ASSERT_TRUE(sp1 == sp2);
-}
-
-// Check whether comparison against dead wp works, even if the object referenced
-// by the new wp happens to be at the same address.
-TEST(RefBase, ReplacedComparison) {
-    bool isDeleted, isDeleted2;
-    FooFixedAlloc* foo = new FooFixedAlloc(&isDeleted);
-    sp<FooFixedAlloc> sp1(foo);
-    wp<FooFixedAlloc> wp1(sp1);
-    ASSERT_TRUE(wp1 == sp1);
-    sp1.clear();  // Deallocates the object.
-    ASSERT_TRUE(isDeleted);
-    FooFixedAlloc* foo2 = new FooFixedAlloc(&isDeleted2);
-    ASSERT_FALSE(isDeleted2);
-    ASSERT_EQ(foo, foo2);  // Not technically a legal comparison, but ...
-    sp<FooFixedAlloc> sp2(foo2);
-    wp<FooFixedAlloc> wp2(sp2);
-    ASSERT_TRUE(sp2 == wp2);
-    ASSERT_FALSE(sp2 != wp2);
-    ASSERT_TRUE(sp2 != wp1);
-    ASSERT_FALSE(sp2 == wp1);
-    ASSERT_FALSE(sp2 == sp1);  // sp1 is null.
-    ASSERT_FALSE(wp1 == wp2);  // wp1 refers to old object.
-    ASSERT_TRUE(wp1 != wp2);
-    ASSERT_TRUE(wp1 > wp2 || wp1 < wp2);
-    ASSERT_TRUE(wp1 >= wp2 || wp1 <= wp2);
-    ASSERT_FALSE(wp1 >= wp2 && wp1 <= wp2);
-    ASSERT_FALSE(wp1 == nullptr);
-    wp1 = sp2;
-    ASSERT_TRUE(wp1 == wp2);
-    ASSERT_FALSE(wp1 != wp2);
-}
 
 // Set up a situation in which we race with visit2AndRremove() to delete
 // 2 strong references.  Bar destructor checks that there are no early
diff --git a/libutils/include/utils/RefBase.h b/libutils/include/utils/RefBase.h
index 730d631..1780cf2 100644
--- a/libutils/include/utils/RefBase.h
+++ b/libutils/include/utils/RefBase.h
@@ -171,8 +171,6 @@
 #define ANDROID_REF_BASE_H
 
 #include <atomic>
-#include <functional>
-#include <type_traits>  // for common_type.
 
 #include <stdint.h>
 #include <sys/types.h>
@@ -194,26 +192,19 @@
 // ---------------------------------------------------------------------------
 
 #define COMPARE_WEAK(_op_)                                      \
+inline bool operator _op_ (const sp<T>& o) const {              \
+    return m_ptr _op_ o.m_ptr;                                  \
+}                                                               \
+inline bool operator _op_ (const T* o) const {                  \
+    return m_ptr _op_ o;                                        \
+}                                                               \
+template<typename U>                                            \
+inline bool operator _op_ (const sp<U>& o) const {              \
+    return m_ptr _op_ o.m_ptr;                                  \
+}                                                               \
 template<typename U>                                            \
 inline bool operator _op_ (const U* o) const {                  \
     return m_ptr _op_ o;                                        \
-}                                                               \
-/* Needed to handle type inference for nullptr: */              \
-inline bool operator _op_ (const T* o) const {                  \
-    return m_ptr _op_ o;                                        \
-}
-
-template<template<typename C> class comparator, typename T, typename U>
-static inline bool _wp_compare_(T* a, U* b) {
-    return comparator<typename std::common_type<T*, U*>::type>()(a, b);
-}
-
-// Use std::less and friends to avoid undefined behavior when ordering pointers
-// to different objects.
-#define COMPARE_WEAK_FUNCTIONAL(_op_, _compare_)                 \
-template<typename U>                                             \
-inline bool operator _op_ (const U* o) const {                   \
-    return _wp_compare_<_compare_>(m_ptr, o);                    \
 }
 
 // ---------------------------------------------------------------------------
@@ -404,51 +395,39 @@
 
     COMPARE_WEAK(==)
     COMPARE_WEAK(!=)
-    COMPARE_WEAK_FUNCTIONAL(>, std::greater)
-    COMPARE_WEAK_FUNCTIONAL(<, std::less)
-    COMPARE_WEAK_FUNCTIONAL(<=, std::less_equal)
-    COMPARE_WEAK_FUNCTIONAL(>=, std::greater_equal)
+    COMPARE_WEAK(>)
+    COMPARE_WEAK(<)
+    COMPARE_WEAK(<=)
+    COMPARE_WEAK(>=)
 
+    inline bool operator == (const wp<T>& o) const {
+        return (m_ptr == o.m_ptr) && (m_refs == o.m_refs);
+    }
     template<typename U>
     inline bool operator == (const wp<U>& o) const {
-        return m_refs == o.m_refs;  // Implies m_ptr == o.mptr; see invariants below.
+        return m_ptr == o.m_ptr;
     }
 
-    template<typename U>
-    inline bool operator == (const sp<U>& o) const {
-        // Just comparing m_ptr fields is often dangerous, since wp<> may refer to an older
-        // object at the same address.
-        if (o == nullptr) {
-          return m_ptr == nullptr;
-        } else {
-          return m_refs == o->getWeakRefs();  // Implies m_ptr == o.mptr.
-        }
+    inline bool operator > (const wp<T>& o) const {
+        return (m_ptr == o.m_ptr) ? (m_refs > o.m_refs) : (m_ptr > o.m_ptr);
     }
-
-    template<typename U>
-    inline bool operator != (const sp<U>& o) const {
-        return !(*this == o);
-    }
-
     template<typename U>
     inline bool operator > (const wp<U>& o) const {
-        if (m_ptr == o.m_ptr) {
-            return _wp_compare_<std::greater>(m_refs, o.m_refs);
-        } else {
-            return _wp_compare_<std::greater>(m_ptr, o.m_ptr);
-        }
+        return (m_ptr == o.m_ptr) ? (m_refs > o.m_refs) : (m_ptr > o.m_ptr);
     }
 
+    inline bool operator < (const wp<T>& o) const {
+        return (m_ptr == o.m_ptr) ? (m_refs < o.m_refs) : (m_ptr < o.m_ptr);
+    }
     template<typename U>
     inline bool operator < (const wp<U>& o) const {
-        if (m_ptr == o.m_ptr) {
-            return _wp_compare_<std::less>(m_refs, o.m_refs);
-        } else {
-            return _wp_compare_<std::less>(m_ptr, o.m_ptr);
-        }
+        return (m_ptr == o.m_ptr) ? (m_refs < o.m_refs) : (m_ptr < o.m_ptr);
     }
+                         inline bool operator != (const wp<T>& o) const { return m_refs != o.m_refs; }
     template<typename U> inline bool operator != (const wp<U>& o) const { return !operator == (o); }
+                         inline bool operator <= (const wp<T>& o) const { return !operator > (o); }
     template<typename U> inline bool operator <= (const wp<U>& o) const { return !operator > (o); }
+                         inline bool operator >= (const wp<T>& o) const { return !operator < (o); }
     template<typename U> inline bool operator >= (const wp<U>& o) const { return !operator < (o); }
 
 private:
@@ -467,22 +446,6 @@
 // ---------------------------------------------------------------------------
 // No user serviceable parts below here.
 
-// Implementation invariants:
-// Either
-// 1) m_ptr and m_refs are both null, or
-// 2) m_refs == m_ptr->mRefs, or
-// 3) *m_ptr is no longer live, and m_refs points to the weakref_type object that corresponded
-//    to m_ptr while it was live. *m_refs remains live while a wp<> refers to it.
-//
-// The m_refs field in a RefBase object is allocated on construction, unique to that RefBase
-// object, and never changes. Thus if two wp's have identical m_refs fields, they are either both
-// null or point to the same object. If two wp's have identical m_ptr fields, they either both
-// point to the same live object and thus have the same m_ref fields, or at least one of the
-// objects is no longer live.
-//
-// Note that the above comparison operations go out of their way to provide an ordering consistent
-// with ordinary pointer comparison; otherwise they could ignore m_ptr, and just compare m_refs.
-
 template<typename T>
 wp<T>::wp(T* other)
     : m_ptr(other)
@@ -632,7 +595,6 @@
 {
     if (m_ptr) {
         m_refs->decWeak(this);
-        m_refs = 0;
         m_ptr = 0;
     }
 }
diff --git a/libutils/include/utils/StrongPointer.h b/libutils/include/utils/StrongPointer.h
index 9cd7c75..1571129 100644
--- a/libutils/include/utils/StrongPointer.h
+++ b/libutils/include/utils/StrongPointer.h
@@ -17,9 +17,6 @@
 #ifndef ANDROID_STRONG_POINTER_H
 #define ANDROID_STRONG_POINTER_H
 
-#include <functional>
-#include <type_traits>  // for common_type.
-
 // ---------------------------------------------------------------------------
 namespace android {
 
@@ -27,12 +24,13 @@
 
 // ---------------------------------------------------------------------------
 
-// TODO: Maybe remove sp<> ? wp<> comparison? These are dangerous: If the wp<>
-// was created before the sp<>, and they point to different objects, they may
-// compare equal even if they are entirely unrelated. E.g. CameraService
-// currently performa such comparisons.
-
-#define COMPARE_STRONG(_op_)                                           \
+#define COMPARE(_op_)                                           \
+inline bool operator _op_ (const sp<T>& o) const {              \
+    return m_ptr _op_ o.m_ptr;                                  \
+}                                                               \
+inline bool operator _op_ (const T* o) const {                  \
+    return m_ptr _op_ o;                                        \
+}                                                               \
 template<typename U>                                            \
 inline bool operator _op_ (const sp<U>& o) const {              \
     return m_ptr _op_ o.m_ptr;                                  \
@@ -41,27 +39,14 @@
 inline bool operator _op_ (const U* o) const {                  \
     return m_ptr _op_ o;                                        \
 }                                                               \
-/* Needed to handle type inference for nullptr: */              \
-inline bool operator _op_ (const T* o) const {                  \
-    return m_ptr _op_ o;                                        \
+inline bool operator _op_ (const wp<T>& o) const {              \
+    return m_ptr _op_ o.m_ptr;                                  \
+}                                                               \
+template<typename U>                                            \
+inline bool operator _op_ (const wp<U>& o) const {              \
+    return m_ptr _op_ o.m_ptr;                                  \
 }
 
-template<template<typename C> class comparator, typename T, typename U>
-static inline bool _sp_compare_(T* a, U* b) {
-    return comparator<typename std::common_type<T*, U*>::type>()(a, b);
-}
-
-// Use std::less and friends to avoid undefined behavior when ordering pointers
-// to different objects.
-#define COMPARE_STRONG_FUNCTIONAL(_op_, _compare_)               \
-template<typename U>                                             \
-inline bool operator _op_ (const sp<U>& o) const {               \
-    return _sp_compare_<_compare_>(m_ptr, o.m_ptr);              \
-}                                                                \
-template<typename U>                                             \
-inline bool operator _op_ (const U* o) const {                   \
-    return _sp_compare_<_compare_>(m_ptr, o);                    \
-}
 // ---------------------------------------------------------------------------
 
 template<typename T>
@@ -104,23 +89,12 @@
 
     // Operators
 
-    COMPARE_STRONG(==)
-    COMPARE_STRONG(!=)
-    COMPARE_STRONG_FUNCTIONAL(>, std::greater)
-    COMPARE_STRONG_FUNCTIONAL(<, std::less)
-    COMPARE_STRONG_FUNCTIONAL(<=, std::less_equal)
-    COMPARE_STRONG_FUNCTIONAL(>=, std::greater_equal)
-
-    // Punt these to the wp<> implementation.
-    template<typename U>
-    inline bool operator == (const wp<U>& o) const {
-        return o == *this;
-    }
-
-    template<typename U>
-    inline bool operator != (const wp<U>& o) const {
-        return o != *this;
-    }
+    COMPARE(==)
+    COMPARE(!=)
+    COMPARE(>)
+    COMPARE(<)
+    COMPARE(<=)
+    COMPARE(>=)
 
 private:    
     template<typename Y> friend class sp;