Fix enabling polymorphic pimpl in QGeoRoute/Segment/Maneuver

Change 8ac6377e62af803b567449cdf30c669b92114cc4 allowed to subclass
private implementations of these 3 classes.
However, the pimpl was kept in a QExplicitlySharedDataPointer for
the first two, and QSharedDataPointer for the last class.

Failing to specialize the clone method of the pointer would result in
incorrect detach behavior, which would however only be visible with the
third class (the first two never call detach() internally, and the line
QEXPECT_FAIL("", "QGeoRoute equality operators broken", Continue); in
the tests make it clear.

This change also adds the equals virtual method also to QGeoRouteSegment
and QGeoManeuver private implementations.

Change-Id: If51dafebb19e4cb2d1bcf40c2b6f136f804198e0
Reviewed-by: Alex Blasche <alexander.blasche@qt.io>

6 files changed, 56 insertions, 9 deletions

diff --git a/src/location/maps/qgeomaneuver.cpp b/src/location/maps/qgeomaneuver.cpp
index 78efb971..9d6a4bf0 100644
--- a/src/location/maps/qgeomaneuver.cpp
+++ b/src/location/maps/qgeomaneuver.cpp
@@ -41,6 +41,12 @@
 
 QT_BEGIN_NAMESPACE
 
+template<>
+QGeoManeuverPrivate *QSharedDataPointer<QGeoManeuverPrivate>::clone()
+{
+    return d->clone();
+}
+
 /*!
     \class QGeoManeuver
     \inmodule QtLocation
@@ -146,7 +152,8 @@ QGeoManeuver &QGeoManeuver::operator= (const QGeoManeuver & other)
 */
 bool QGeoManeuver::operator== (const QGeoManeuver &other) const
 {
-    return (*(d_ptr.constData()) == *(other.d_ptr.constData()));
+    return ( (d_ptr.constData() == other.d_ptr.constData())
+            ||  (*(d_ptr.constData()) == *(other.d_ptr.constData())) );
 }
 
 /*!
@@ -308,6 +315,11 @@ QGeoManeuverPrivate::~QGeoManeuverPrivate()
 
 bool QGeoManeuverPrivate::operator==(const QGeoManeuverPrivate &other) const
 {
+    return equals(other);
+}
+
+bool QGeoManeuverPrivate::equals(const QGeoManeuverPrivate &other) const
+{
     return ((valid() == other.valid())
             && (position() == other.position())
             && (text() == other.text())
@@ -420,11 +432,9 @@ QGeoManeuverPrivateDefault::QGeoManeuverPrivateDefault(const QGeoManeuverPrivate
 
 QGeoManeuverPrivateDefault::~QGeoManeuverPrivateDefault() {}
 
-
-
-bool QGeoManeuverPrivateDefault::operator ==(const QGeoManeuverPrivateDefault &other) const
+QGeoManeuverPrivate *QGeoManeuverPrivateDefault::clone()
 {
-    return QGeoManeuverPrivateDefault::operator ==(other);
+    return new QGeoManeuverPrivateDefault(*this);
 }
 
 bool QGeoManeuverPrivateDefault::valid() const
diff --git a/src/location/maps/qgeomaneuver_p.h b/src/location/maps/qgeomaneuver_p.h
index 484354ba..e17bf880 100644
--- a/src/location/maps/qgeomaneuver_p.h
+++ b/src/location/maps/qgeomaneuver_p.h
@@ -63,6 +63,7 @@ public:
     QGeoManeuverPrivate();
     QGeoManeuverPrivate(const QGeoManeuverPrivate &other);
     virtual ~QGeoManeuverPrivate();
+    virtual QGeoManeuverPrivate *clone() = 0;
 
     bool operator== (const QGeoManeuverPrivate &other) const;
 
@@ -90,6 +91,8 @@ public:
     virtual QGeoCoordinate waypoint() const;
     virtual void setWaypoint(const QGeoCoordinate &waypoint);
 
+protected:
+    virtual bool equals(const QGeoManeuverPrivate &other) const;
 };
 
 class Q_LOCATION_PRIVATE_EXPORT QGeoManeuverPrivateDefault : public QGeoManeuverPrivate
@@ -98,8 +101,7 @@ public:
     QGeoManeuverPrivateDefault();
     QGeoManeuverPrivateDefault(const QGeoManeuverPrivateDefault &other);
     ~QGeoManeuverPrivateDefault();
-
-    bool operator== (const QGeoManeuverPrivateDefault &other) const;
+    virtual QGeoManeuverPrivate *clone() override;
 
     virtual bool valid() const override;
     virtual void setValid(bool valid) override;
diff --git a/src/location/maps/qgeoroute.cpp b/src/location/maps/qgeoroute.cpp
index 95603218..2e7b259a 100644
--- a/src/location/maps/qgeoroute.cpp
+++ b/src/location/maps/qgeoroute.cpp
@@ -44,6 +44,12 @@
 
 QT_BEGIN_NAMESPACE
 
+template<>
+QGeoRoutePrivate *QExplicitlySharedDataPointer<QGeoRoutePrivate>::clone()
+{
+    return d->clone();
+}
+
 /*!
     \class QGeoRoute
     \inmodule QtLocation
@@ -434,6 +440,11 @@ QGeoRoutePrivateDefault::QGeoRoutePrivateDefault(const QGeoRoutePrivateDefault &
 
 QGeoRoutePrivateDefault::~QGeoRoutePrivateDefault() {}
 
+QGeoRoutePrivate *QGeoRoutePrivateDefault::clone()
+{
+    return new QGeoRoutePrivateDefault(*this);
+}
+
 void QGeoRoutePrivateDefault::setId(const QString &id)
 {
     m_id = id;
diff --git a/src/location/maps/qgeoroute_p.h b/src/location/maps/qgeoroute_p.h
index 1a8f9b0c..26eb613c 100644
--- a/src/location/maps/qgeoroute_p.h
+++ b/src/location/maps/qgeoroute_p.h
@@ -66,6 +66,7 @@ public:
     QGeoRoutePrivate();
     QGeoRoutePrivate(const QGeoRoutePrivate &other);
     virtual ~QGeoRoutePrivate();
+    virtual QGeoRoutePrivate *clone() = 0;
 
     bool operator == (const QGeoRoutePrivate &other) const;
 
@@ -93,10 +94,11 @@ public:
     virtual void setFirstSegment(const QGeoRouteSegment &firstSegment);
     virtual QGeoRouteSegment firstSegment() const;
 
-    virtual bool equals(const QGeoRoutePrivate &other) const;
-
     virtual QString engineName() const = 0;
     virtual int segmentsCount() const = 0;
+
+protected:
+    virtual bool equals(const QGeoRoutePrivate &other) const;
 };
 
 class Q_LOCATION_PRIVATE_EXPORT  QGeoRoutePrivateDefault : public QGeoRoutePrivate
@@ -105,6 +107,7 @@ public:
     QGeoRoutePrivateDefault();
     QGeoRoutePrivateDefault(const QGeoRoutePrivateDefault &other);
     ~QGeoRoutePrivateDefault();
+    virtual QGeoRoutePrivate *clone() override;
 
     virtual void setId(const QString &id) override;
     virtual QString id() const override;
diff --git a/src/location/maps/qgeoroutesegment.cpp b/src/location/maps/qgeoroutesegment.cpp
index d9de9870..8dbe18fc 100644
--- a/src/location/maps/qgeoroutesegment.cpp
+++ b/src/location/maps/qgeoroutesegment.cpp
@@ -42,6 +42,12 @@
 
 QT_BEGIN_NAMESPACE
 
+template<>
+QGeoRouteSegmentPrivate *QExplicitlySharedDataPointer<QGeoRouteSegmentPrivate>::clone()
+{
+    return d->clone();
+}
+
 /*!
     \class QGeoRouteSegment
     \inmodule QtLocation
@@ -250,6 +256,11 @@ QGeoRouteSegmentPrivate::~QGeoRouteSegmentPrivate()
 
 bool QGeoRouteSegmentPrivate::operator ==(const QGeoRouteSegmentPrivate &other) const
 {
+    return equals(other);
+}
+
+bool QGeoRouteSegmentPrivate::equals(const QGeoRouteSegmentPrivate &other) const
+{
     return ((valid() == other.valid())
             && (travelTime() == other.travelTime())
             && (distance() == other.distance())
@@ -344,6 +355,11 @@ QGeoRouteSegmentPrivateDefault::~QGeoRouteSegmentPrivateDefault()
 
 }
 
+QGeoRouteSegmentPrivate *QGeoRouteSegmentPrivateDefault::clone()
+{
+    return new QGeoRouteSegmentPrivateDefault(*this);
+}
+
 bool QGeoRouteSegmentPrivateDefault::operator ==(const QGeoRouteSegmentPrivateDefault &other) const
 {
     return QGeoRouteSegmentPrivateDefault::operator ==(other);
diff --git a/src/location/maps/qgeoroutesegment_p.h b/src/location/maps/qgeoroutesegment_p.h
index 8f452896..f0b180da 100644
--- a/src/location/maps/qgeoroutesegment_p.h
+++ b/src/location/maps/qgeoroutesegment_p.h
@@ -67,6 +67,7 @@ public:
     QGeoRouteSegmentPrivate();
     QGeoRouteSegmentPrivate(const QGeoRouteSegmentPrivate &other);
     virtual ~QGeoRouteSegmentPrivate();
+    virtual QGeoRouteSegmentPrivate *clone() = 0;
 
     bool operator ==(const QGeoRouteSegmentPrivate &other) const;
 
@@ -89,6 +90,9 @@ public:
     virtual void setNextRouteSegment(const QExplicitlySharedDataPointer<QGeoRouteSegmentPrivate> &next);
 
     QExplicitlySharedDataPointer<QGeoRouteSegmentPrivate> m_nextSegment;
+
+protected:
+    virtual bool equals(const QGeoRouteSegmentPrivate &other) const;
 };
 
 
@@ -99,6 +103,7 @@ public:
     QGeoRouteSegmentPrivateDefault();
     QGeoRouteSegmentPrivateDefault(const QGeoRouteSegmentPrivateDefault &other);
     ~QGeoRouteSegmentPrivateDefault();
+    virtual QGeoRouteSegmentPrivate *clone() override;
 
     bool operator ==(const QGeoRouteSegmentPrivateDefault &other) const;