From 0007bbfa11e74f2a76aebc83995843ee8f6840ab Mon Sep 17 00:00:00 2001
From: Tobias Markmann <tm@ayena.de>
Date: Tue, 31 Jul 2018 17:42:11 +0200
Subject: Have StreamStack own intermediate layers via std::unique_ptr

Test-Information:

`./scons test=all` passes with no errors on macOS with clang
7 master.

Change-Id: I31765ac15750dc5af7b70d1a85171dc8e3590181

diff --git a/Swiften/Session/BasicSessionStream.cpp b/Swiften/Session/BasicSessionStream.cpp
index 54cd225..183b986 100644
--- a/Swiften/Session/BasicSessionStream.cpp
+++ b/Swiften/Session/BasicSessionStream.cpp
@@ -34,9 +34,6 @@ BasicSessionStream::BasicSessionStream(
             connection(connection),
             tlsContextFactory(tlsContextFactory),
             timerFactory(timerFactory),
-            compressionLayer(nullptr),
-            tlsLayer(nullptr),
-            whitespacePingLayer(nullptr),
             tlsOptions_(tlsOptions) {
     xmppLayer = new XMPPLayer(payloadParserFactories, payloadSerializers, xmlParserFactory, streamType);
     xmppLayer->onStreamStart.connect(boost::bind(&BasicSessionStream::handleStreamStartReceived, this, _1));
@@ -55,14 +52,11 @@ BasicSessionStream::BasicSessionStream(
 }
 
 BasicSessionStream::~BasicSessionStream() {
-    delete compressionLayer;
 
-    if (tlsLayer) {
+    if (auto tlsLayer = streamStack->getLayer<TLSLayer>()) {
         tlsLayer->onError.disconnect(boost::bind(&BasicSessionStream::handleTLSError, this, _1));
         tlsLayer->onConnected.disconnect(boost::bind(&BasicSessionStream::handleTLSConnected, this));
-        delete tlsLayer;
     }
-    delete whitespacePingLayer;
     delete streamStack;
 
     connection->onDisconnected.disconnect(boost::bind(&BasicSessionStream::handleConnectionFinished, this, _1));
@@ -112,12 +106,13 @@ bool BasicSessionStream::supportsTLSEncryption() {
 void BasicSessionStream::addTLSEncryption() {
     assert(available);
     auto tlsContext = tlsContextFactory->createTLSContext(tlsOptions_);
-    tlsLayer = new TLSLayer(std::move(tlsContext));
+    auto tlsLayer = std::make_unique<TLSLayer>(std::move(tlsContext));
     if (hasTLSCertificate() && !tlsLayer->setClientCertificate(getTLSCertificate())) {
         onClosed(std::make_shared<SessionStreamError>(SessionStreamError::InvalidTLSCertificateError));
     }
     else {
-        streamStack->addLayer(tlsLayer);
+        streamStack->addLayer(std::move(tlsLayer));
+        auto tlsLayer = streamStack->getLayer<TLSLayer>();
         tlsLayer->onError.connect(boost::bind(&BasicSessionStream::handleTLSError, this, _1));
         tlsLayer->onConnected.connect(boost::bind(&BasicSessionStream::handleTLSConnected, this));
         tlsLayer->connect();
@@ -125,23 +120,23 @@ void BasicSessionStream::addTLSEncryption() {
 }
 
 bool BasicSessionStream::isTLSEncrypted() {
-    return tlsLayer;
+    return streamStack->getLayer<TLSLayer>() != nullptr;
 }
 
 Certificate::ref BasicSessionStream::getPeerCertificate() const {
-    return tlsLayer->getPeerCertificate();
+    return streamStack->getLayer<TLSLayer>()->getPeerCertificate();
 }
 
 std::vector<Certificate::ref> BasicSessionStream::getPeerCertificateChain() const {
-    return tlsLayer->getPeerCertificateChain();
+    return streamStack->getLayer<TLSLayer>()->getPeerCertificateChain();
 }
 
 std::shared_ptr<CertificateVerificationError> BasicSessionStream::getPeerCertificateVerificationError() const {
-    return tlsLayer->getPeerCertificateVerificationError();
+    return streamStack->getLayer<TLSLayer>()->getPeerCertificateVerificationError();
 }
 
 ByteArray BasicSessionStream::getTLSFinishMessage() const {
-    return tlsLayer->getContext()->getFinishMessage();
+    return streamStack->getLayer<TLSLayer>()->getContext()->getFinishMessage();
 }
 
 bool BasicSessionStream::supportsZLibCompression() {
@@ -149,15 +144,16 @@ bool BasicSessionStream::supportsZLibCompression() {
 }
 
 void BasicSessionStream::addZLibCompression() {
-    compressionLayer = new CompressionLayer();
-    streamStack->addLayer(compressionLayer);
+    auto compressionLayer = std::make_unique<CompressionLayer>();
+    streamStack->addLayer(std::move(compressionLayer));
 }
 
 void BasicSessionStream::setWhitespacePingEnabled(bool enabled) {
+    auto whitespacePingLayer = streamStack->getLayer<WhitespacePingLayer>();
     if (enabled) {
         if (!whitespacePingLayer) {
-            whitespacePingLayer = new WhitespacePingLayer(timerFactory);
-            streamStack->addLayer(whitespacePingLayer);
+            streamStack->addLayer(std::make_unique<WhitespacePingLayer>(timerFactory));
+            whitespacePingLayer = streamStack->getLayer<WhitespacePingLayer>();
         }
         whitespacePingLayer->setActive();
     }
diff --git a/Swiften/Session/BasicSessionStream.h b/Swiften/Session/BasicSessionStream.h
index 48b3d63..472b5cc 100644
--- a/Swiften/Session/BasicSessionStream.h
+++ b/Swiften/Session/BasicSessionStream.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2016 Isode Limited.
+ * Copyright (c) 2010-2018 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
@@ -85,9 +85,6 @@ namespace Swift {
             TimerFactory* timerFactory;
             XMPPLayer* xmppLayer;
             ConnectionLayer* connectionLayer;
-            CompressionLayer* compressionLayer;
-            TLSLayer* tlsLayer;
-            WhitespacePingLayer* whitespacePingLayer;
             StreamStack* streamStack;
             TLSOptions tlsOptions_;
     };
diff --git a/Swiften/StreamStack/StreamStack.cpp b/Swiften/StreamStack/StreamStack.cpp
index 44a018d..f14f95c 100644
--- a/Swiften/StreamStack/StreamStack.cpp
+++ b/Swiften/StreamStack/StreamStack.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2016 Isode Limited.
+ * Copyright (c) 2010-2018 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
@@ -22,16 +22,16 @@ StreamStack::StreamStack(XMPPLayer* xmppLayer, LowLayer* physicalLayer) : xmppLa
 StreamStack::~StreamStack() {
 }
 
-void StreamStack::addLayer(StreamLayer* newLayer) {
-    LowLayer* lowLayer = layers_.empty() ? physicalLayer_ : *layers_.rbegin();
+void StreamStack::addLayer(std::unique_ptr<StreamLayer> streamLayer) {
+    LowLayer* lowLayer = layers_.empty() ? physicalLayer_ : layers_.rbegin()->get();
 
-    xmppLayer_->setChildLayer(newLayer);
-    newLayer->setParentLayer(xmppLayer_);
+    xmppLayer_->setChildLayer(streamLayer.get());
+    streamLayer->setParentLayer(xmppLayer_);
 
-    lowLayer->setParentLayer(newLayer);
-    newLayer->setChildLayer(lowLayer);
+    lowLayer->setParentLayer(streamLayer.get());
+    streamLayer->setChildLayer(lowLayer);
 
-    layers_.push_back(newLayer);
+    layers_.emplace_back(std::move(streamLayer));
 }
 
 }
diff --git a/Swiften/StreamStack/StreamStack.h b/Swiften/StreamStack/StreamStack.h
index b12a69f..5b77085 100644
--- a/Swiften/StreamStack/StreamStack.h
+++ b/Swiften/StreamStack/StreamStack.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2016 Isode Limited.
+ * Copyright (c) 2010-2018 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
@@ -24,15 +24,15 @@ namespace Swift {
             StreamStack(XMPPLayer* xmppLayer, LowLayer* physicalLayer);
             ~StreamStack();
 
-            void addLayer(StreamLayer*);
+            void addLayer(std::unique_ptr<StreamLayer> /* streamLayer */);
 
             XMPPLayer* getXMPPLayer() const {
                 return xmppLayer_;
             }
 
-            template<typename T> T* getLayer() {
-                for (auto& i : layers_) {
-                    T* layer = dynamic_cast<T*>(i);
+            template<typename T> T* getLayer() const {
+                for (const auto& i : layers_) {
+                    T* layer = dynamic_cast<T*>(i.get());
                     if (layer) {
                         return layer;
                     }
@@ -43,6 +43,6 @@ namespace Swift {
         private:
             XMPPLayer* xmppLayer_;
             LowLayer* physicalLayer_;
-            std::vector<StreamLayer*> layers_;
+            std::vector<std::unique_ptr<StreamLayer>> layers_;
     };
 }
diff --git a/Swiften/StreamStack/UnitTest/StreamStackTest.cpp b/Swiften/StreamStack/UnitTest/StreamStackTest.cpp
index f0f82c9..0b520f1 100644
--- a/Swiften/StreamStack/UnitTest/StreamStackTest.cpp
+++ b/Swiften/StreamStack/UnitTest/StreamStackTest.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2016 Isode Limited.
+ * Copyright (c) 2010-2018 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
@@ -61,8 +61,8 @@ class StreamStackTest : public CppUnit::TestFixture {
 
         void testWriteData_OneIntermediateStream() {
             StreamStack testling(xmppStream_, physicalStream_);
-            std::shared_ptr<MyStreamLayer> xStream(new MyStreamLayer("X"));
-            testling.addLayer(xStream.get());
+            std::unique_ptr<MyStreamLayer> xStream(new MyStreamLayer("X"));
+            testling.addLayer(std::move(xStream));
 
             xmppStream_->writeData("foo");
 
@@ -72,10 +72,10 @@ class StreamStackTest : public CppUnit::TestFixture {
 
         void testWriteData_TwoIntermediateStreamStack() {
             StreamStack testling(xmppStream_, physicalStream_);
-            std::shared_ptr<MyStreamLayer> xStream(new MyStreamLayer("X"));
-            std::shared_ptr<MyStreamLayer> yStream(new MyStreamLayer("Y"));
-            testling.addLayer(xStream.get());
-            testling.addLayer(yStream.get());
+            std::unique_ptr<MyStreamLayer> xStream(new MyStreamLayer("X"));
+            std::unique_ptr<MyStreamLayer> yStream(new MyStreamLayer("Y"));
+            testling.addLayer(std::move(xStream));
+            testling.addLayer(std::move(yStream));
 
             xmppStream_->writeData("foo");
 
@@ -95,8 +95,8 @@ class StreamStackTest : public CppUnit::TestFixture {
         void testReadData_OneIntermediateStream() {
             StreamStack testling(xmppStream_, physicalStream_);
             xmppStream_->onElement.connect(boost::bind(&StreamStackTest::handleElement, this, _1));
-            std::shared_ptr<MyStreamLayer> xStream(new MyStreamLayer("<"));
-            testling.addLayer(xStream.get());
+            std::unique_ptr<MyStreamLayer> xStream(new MyStreamLayer("<"));
+            testling.addLayer(std::move(xStream));
 
             physicalStream_->onDataRead(createSafeByteArray("stream:stream xmlns:stream='http://etherx.jabber.org/streams'><presence/>"));
 
@@ -106,10 +106,10 @@ class StreamStackTest : public CppUnit::TestFixture {
         void testReadData_TwoIntermediateStreamStack() {
             StreamStack testling(xmppStream_, physicalStream_);
             xmppStream_->onElement.connect(boost::bind(&StreamStackTest::handleElement, this, _1));
-            std::shared_ptr<MyStreamLayer> xStream(new MyStreamLayer("s"));
-            std::shared_ptr<MyStreamLayer> yStream(new MyStreamLayer("<"));
-            testling.addLayer(xStream.get());
-            testling.addLayer(yStream.get());
+            std::unique_ptr<MyStreamLayer> xStream(new MyStreamLayer("s"));
+            std::unique_ptr<MyStreamLayer> yStream(new MyStreamLayer("<"));
+            testling.addLayer(std::move(xStream));
+            testling.addLayer(std::move(yStream));
 
             physicalStream_->onDataRead(createSafeByteArray("tream:stream xmlns:stream='http://etherx.jabber.org/streams'><presence/>"));
 
@@ -119,8 +119,8 @@ class StreamStackTest : public CppUnit::TestFixture {
         void testAddLayer_ExistingOnWriteDataSlot() {
             StreamStack testling(xmppStream_, physicalStream_);
             xmppStream_->onWriteData.connect(boost::bind(&StreamStackTest::handleWriteData, this, _1));
-            std::shared_ptr<MyStreamLayer> xStream(new MyStreamLayer("X"));
-            testling.addLayer(xStream.get());
+            std::unique_ptr<MyStreamLayer> xStream(new MyStreamLayer("X"));
+            testling.addLayer(std::move(xStream));
 
             xmppStream_->writeData("foo");
 
-- 
cgit v0.10.2-6-g49f6