[core] Replace shared_ptr with unique_ptr in {Map,Resource}Optionsupstream/options-unique_ptr

9 files changed, 87 insertions, 75 deletions

diff --git a/benchmark/api/render.benchmark.cpp b/benchmark/api/render.benchmark.cpp
index 0e7a3be322..b66c3f228c 100644
--- a/benchmark/api/render.benchmark.cpp
+++ b/benchmark/api/render.benchmark.cpp
@@ -18,6 +18,10 @@ using namespace mbgl;
 
 namespace {
 
+static std::string cachePath { "benchmark/fixtures/api/cache.db" };
+constexpr double pixelRatio { 1.0 };
+constexpr Size size { 1000, 1000 };
+
 class RenderBenchmark {
 public:
     RenderBenchmark() {
@@ -27,22 +31,23 @@ public:
     util::RunLoop loop;
     ThreadPool threadPool { 4 };
 };
-    
+
 static void prepare(Map& map, optional<std::string> json = {}) {
     map.getStyle().loadJSON(json ? *json : util::read_file("benchmark/fixtures/api/style.json"));
     map.jumpTo(CameraOptions().withCenter(LatLng { 40.726989, -73.992857 }).withZoom(15.0)); // Manhattan
-    map.getStyle().addImage(std::make_unique<style::Image>("test-icon",
-                                                           decodeImage(util::read_file("benchmark/fixtures/api/default_marker.png")), 1.0));
+
+    auto image = decodeImage(util::read_file("benchmark/fixtures/api/default_marker.png"));
+    map.getStyle().addImage(std::make_unique<style::Image>("test-icon", std::move(image), 1.0));
 }
- 
+
 } // end namespace
 
 static void API_renderStill_reuse_map(::benchmark::State& state) {
     RenderBenchmark bench;
-    HeadlessFrontend frontend { { 1000, 1000 }, 1, bench.threadPool };
-    Map map { frontend, MapObserver::nullObserver(), frontend.getSize(), 1, bench.threadPool,
+    HeadlessFrontend frontend { size, pixelRatio, bench.threadPool };
+    Map map { frontend, MapObserver::nullObserver(), size, pixelRatio, bench.threadPool,
               MapOptions().withMapMode(MapMode::Static),
-              ResourceOptions().withCachePath("benchmark/fixtures/api/cache.db").withAssetPath(".").withAccessToken("foobar") };
+              ResourceOptions().withCachePath(cachePath).withAccessToken("foobar") };
     prepare(map);
 
     while (state.KeepRunning()) {
@@ -52,10 +57,9 @@ static void API_renderStill_reuse_map(::benchmark::State& state) {
 
 static void API_renderStill_reuse_map_formatted_labels(::benchmark::State& state) {
     RenderBenchmark bench;
-    HeadlessFrontend frontend { { 1000, 1000 }, 1, bench.threadPool };
-    Map map { frontend, MapObserver::nullObserver(), frontend.getSize(), 1, bench.threadPool,
-              MapOptions().withMapMode(MapMode::Static),
-              ResourceOptions().withCachePath("benchmark/fixtures/api/cache.db").withAssetPath(".").withAccessToken("foobar") };
+    HeadlessFrontend frontend { size, pixelRatio, bench.threadPool };
+    Map map { frontend, MapObserver::nullObserver(), size, pixelRatio, bench.threadPool,
+              MapOptions().withMapMode(MapMode::Static), ResourceOptions().withCachePath(cachePath).withAccessToken("foobar") };
     prepare(map, util::read_file("benchmark/fixtures/api/style_formatted_labels.json"));
 
     while (state.KeepRunning()) {
@@ -65,11 +69,10 @@ static void API_renderStill_reuse_map_formatted_labels(::benchmark::State& state
 
 static void API_renderStill_reuse_map_switch_styles(::benchmark::State& state) {
     RenderBenchmark bench;
-    HeadlessFrontend frontend { { 1000, 1000 }, 1, bench.threadPool };
-    Map map { frontend, MapObserver::nullObserver(), frontend.getSize(), 1, bench.threadPool,
-              MapOptions().withMapMode(MapMode::Static),
-              ResourceOptions().withCachePath("benchmark/fixtures/api/cache.db").withAssetPath(".").withAccessToken("foobar") };
-    
+    HeadlessFrontend frontend { size, pixelRatio, bench.threadPool };
+    Map map { frontend, MapObserver::nullObserver(), size, pixelRatio, bench.threadPool,
+              MapOptions().withMapMode(MapMode::Static), ResourceOptions().withCachePath(cachePath).withAccessToken("foobar") };
+
     while (state.KeepRunning()) {
         prepare(map, { "{}" });
         frontend.render(map);
@@ -80,11 +83,11 @@ static void API_renderStill_reuse_map_switch_styles(::benchmark::State& state) {
 
 static void API_renderStill_recreate_map(::benchmark::State& state) {
     RenderBenchmark bench;
-    auto mapOptions = MapOptions().withMapMode(MapMode::Static);
-    auto resourceOptions = ResourceOptions().withCachePath("benchmark/fixtures/api/cache.db").withAssetPath(".").withAccessToken("foobar");
+
     while (state.KeepRunning()) {
-        HeadlessFrontend frontend { { 1000, 1000 }, 1, bench.threadPool };
-        Map map { frontend, MapObserver::nullObserver(), frontend.getSize(), 1, bench.threadPool, mapOptions, resourceOptions };
+        HeadlessFrontend frontend { size, pixelRatio, bench.threadPool };
+        Map map { frontend, MapObserver::nullObserver(), size, pixelRatio, bench.threadPool,
+                  MapOptions().withMapMode(MapMode::Static), ResourceOptions().withCachePath(cachePath).withAccessToken("foobar") };
         prepare(map);
         frontend.render(map);
     }
diff --git a/include/mbgl/map/map_options.hpp b/include/mbgl/map/map_options.hpp
index f2297484a3..384b113dba 100644
--- a/include/mbgl/map/map_options.hpp
+++ b/include/mbgl/map/map_options.hpp
@@ -17,15 +17,18 @@ public:
     MapOptions();
     ~MapOptions();
 
+    MapOptions(MapOptions&&);
+    explicit MapOptions(const MapOptions&);
+
     /**
      * @brief Sets the map rendering mode. By default, it is set to Continuous
      * so the map will render as data arrives from the network and react
      * immediately to state changes.
      *
      * @param mode Map rendering mode.
-     * @return reference to MapOptions for chaining options together.
+     * @return MapOptions for chaining options together.
      */
-    MapOptions& withMapMode(MapMode mode);
+    MapOptions withMapMode(MapMode mode);
 
     /**
      * @brief Gets the previously set (or default) map mode.
@@ -40,9 +43,9 @@ public:
      * HeightOnly.
      *
      * @param mode Map constrain mode.
-     * @return reference to MapOptions for chaining options together.
+     * @return MapOptions for chaining options together.
      */
-    MapOptions& withConstrainMode(ConstrainMode mode);
+    MapOptions withConstrainMode(ConstrainMode mode);
 
     /**
      * @brief Gets the previously set (or default) constrain mode.
@@ -56,9 +59,9 @@ public:
      * orientation of the map as some devices may use inverted orientation.
      *
      * @param mode Viewport mode.
-     * @return reference to MapOptions for chaining options together.
+     * @return MapOptions for chaining options together.
      */
-    MapOptions& withViewportMode(ViewportMode mode);
+    MapOptions withViewportMode(ViewportMode mode);
 
     /**
      * @brief Gets the previously set (or default) viewport mode.
@@ -72,9 +75,9 @@ public:
      * or not. By default, it is set to true.
      *
      * @param enableCollisions true to enable, false to disable
-     * @return reference to MapOptions for chaining options together.
+     * @return MapOptions for chaining options together.
      */
-    MapOptions& withCrossSourceCollisions(bool enableCollisions);
+    MapOptions withCrossSourceCollisions(bool enableCollisions);
 
     /**
      * @brief Gets the previously set (or default) crossSourceCollisions value.
@@ -86,7 +89,7 @@ public:
 
 private:
     class Impl;
-    std::shared_ptr<Impl> impl_;
+    std::unique_ptr<Impl> impl_;
 };
 
 }  // namespace mbgl
diff --git a/include/mbgl/storage/resource_options.hpp b/include/mbgl/storage/resource_options.hpp
index 0a4669ea15..958392175c 100644
--- a/include/mbgl/storage/resource_options.hpp
+++ b/include/mbgl/storage/resource_options.hpp
@@ -16,13 +16,16 @@ public:
     ResourceOptions();
     ~ResourceOptions();
 
+    ResourceOptions(ResourceOptions&&);
+    explicit ResourceOptions(const ResourceOptions&);
+
     /**
      * @brief Sets the Mapbox access token - see https://docs.mapbox.com/help/how-mapbox-works/access-tokens/ for details.
      *
      * @param token Mapbox access token.
-     * @return reference to ResourceOptions for chaining options together.
+     * @return ResourceOptions for chaining options together.
      */
-    ResourceOptions& withAccessToken(std::string token);
+    ResourceOptions withAccessToken(std::string token);
 
     /**
      * @brief Gets the previously set (or default) Mapbox access token.
@@ -35,9 +38,9 @@ public:
      * @brief Sets the API base URL. Default is https://api.mapbox.com for Mapbox.
      *
      * @param baseURL API base URL.
-     * @return reference to ResourceOptions for chaining options together.
+     * @return ResourceOptions for chaining options together.
      */
-    ResourceOptions& withBaseURL(std::string baseURL);
+    ResourceOptions withBaseURL(std::string baseURL);
 
     /**
      * @brief Gets the previously set (or default) API base URL.
@@ -50,9 +53,9 @@ public:
      * @brief Sets the cache path.
      *
      * @param path Cache path.
-     * @return reference to ResourceOptions for chaining options together.
+     * @return ResourceOptions for chaining options together.
      */
-    ResourceOptions& withCachePath(std::string path);
+    ResourceOptions withCachePath(std::string path);
 
     /**
      * @brief Gets the previously set (or default) cache path.
@@ -66,9 +69,9 @@ public:
      * the asset:// scheme gets resolved in a style.
      *
      * @param path Asset path.
-     * @return reference to ResourceOptions for chaining options together.
+     * @return ResourceOptions for chaining options together.
      */
-    ResourceOptions& withAssetPath(std::string path);
+    ResourceOptions withAssetPath(std::string path);
 
     /**
      * @brief Gets the previously set (or default) asset path.
@@ -83,7 +86,7 @@ public:
      * @param size Cache maximum size in bytes.
      * @return reference to ResourceOptions for chaining options together.
      */
-    ResourceOptions& withMaximumCacheSize(uint64_t size);
+    ResourceOptions withMaximumCacheSize(uint64_t size);
 
     /**
      * @brief Gets the previously set (or default) maximum allowed cache size.
@@ -99,7 +102,7 @@ public:
      * @param context Platform context.
      * @return reference to ResourceOptions for chaining options together.
      */
-    ResourceOptions& withPlatformContext(void* context);
+    ResourceOptions withPlatformContext(void* context);
 
     /**
      * @brief Gets the previously set (or default) platform context.
@@ -110,7 +113,7 @@ public:
 
 private:
     class Impl;
-    std::shared_ptr<Impl> impl_;
+    std::unique_ptr<Impl> impl_;
 };
 
 } // namespace mbgl
diff --git a/platform/android/src/file_source.cpp b/platform/android/src/file_source.cpp
index bb1f30981b..94db563efc 100644
--- a/platform/android/src/file_source.cpp
+++ b/platform/android/src/file_source.cpp
@@ -31,7 +31,7 @@ FileSource::FileSource(jni::JNIEnv& _env,
     std::string path = jni::Make<std::string>(_env, _cachePath);
     mapbox::sqlite::setTempPath(path);
 
-    resourceOptions = mbgl::ResourceOptions()
+    resourceOptions
         .withAccessToken(accessToken ? jni::Make<std::string>(_env, accessToken) : "")
         .withCachePath(path + DATABASE_FILE)
         .withPlatformContext(reinterpret_cast<void*>(new AssetManagerFileSource(_env, assetManager)));
@@ -120,7 +120,7 @@ FileSource* FileSource::getNativePeer(jni::JNIEnv& env, const jni::Object<FileSo
 mbgl::ResourceOptions FileSource::getSharedResourceOptions(jni::JNIEnv& env, const jni::Object<FileSource>& jFileSource) {
     FileSource* fileSource = FileSource::getNativePeer(env, jFileSource);
     assert(fileSource != nullptr);
-    return fileSource->resourceOptions;
+    return mbgl::ResourceOptions(fileSource->resourceOptions);
 }
 
 void FileSource::registerNative(jni::JNIEnv& env) {
diff --git a/platform/glfw/main.cpp b/platform/glfw/main.cpp
index 8288a009f2..77585718e1 100644
--- a/platform/glfw/main.cpp
+++ b/platform/glfw/main.cpp
@@ -94,12 +94,14 @@ int main(int argc, char *argv[]) {
     view = &backend;
 
     // Set access token if present
-    const char *token = getenv("MAPBOX_ACCESS_TOKEN");
-    if (token == nullptr) {
+    std::string token(getenv("MAPBOX_ACCESS_TOKEN") ?: "");
+    if (token.empty()) {
         mbgl::Log::Warning(mbgl::Event::Setup, "no access token set. mapbox.com tiles won't work.");
     }
 
-    auto resourceOptions = mbgl::ResourceOptions().withCachePath(cacheDB).withAssetPath(".").withAccessToken(std::string(token));
+    mbgl::ResourceOptions resourceOptions;
+    resourceOptions.withCachePath(cacheDB).withAccessToken(token);
+
     auto fileSource = std::static_pointer_cast<mbgl::DefaultFileSource>(mbgl::FileSource::getSharedFileSource(resourceOptions));
     if (!settings.online) {
         fileSource->setOnlineStatus(false);
diff --git a/platform/ios/vendor/mapbox-events-ios b/platform/ios/vendor/mapbox-events-ios
-Subproject a75545ca01b3e567a989cd950b74f7865d3518c
+Subproject d79e62581df5f51a0064dd2f78972c489c71d41
diff --git a/platform/node/src/node_map.cpp b/platform/node/src/node_map.cpp
index 0b27d378a7..7ad9c362bf 100644
--- a/platform/node/src/node_map.cpp
+++ b/platform/node/src/node_map.cpp
@@ -631,14 +631,9 @@ void NodeMap::cancel() {
     });
 
     frontend = std::make_unique<mbgl::HeadlessFrontend>(mbgl::Size{ 256, 256 }, pixelRatio, threadpool);
-    auto mapOptions = mbgl::MapOptions()
-        .withMapMode(mode)
-        .withCrossSourceCollisions(crossSourceCollisions);
-
-    auto resourceOptions = mbgl::ResourceOptions().withPlatformContext(reinterpret_cast<void*>(this));
-
-    map = std::make_unique<mbgl::Map>(*frontend, mapObserver, frontend->getSize(), pixelRatio,
-                                      threadpool, mapOptions, resourceOptions);
+    map = std::make_unique<mbgl::Map>(*frontend, mapObserver, frontend->getSize(), pixelRatio, threadpool,
+                                      mbgl::MapOptions().withMapMode(mode).withCrossSourceCollisions(crossSourceCollisions),
+                                      mbgl::ResourceOptions().withPlatformContext(reinterpret_cast<void*>(this)));
 
     // FIXME: Reload the style after recreating the map. We need to find
     // a better way of canceling an ongoing rendering on the core level
diff --git a/src/mbgl/map/map_options.cpp b/src/mbgl/map/map_options.cpp
index ddb3f8e3be..c1146ab0bc 100644
--- a/src/mbgl/map/map_options.cpp
+++ b/src/mbgl/map/map_options.cpp
@@ -10,39 +10,42 @@ public:
     bool crossSourceCollisions = true;
 };
 
-MapOptions::MapOptions() : impl_(std::make_shared<MapOptions::Impl>()) {}
+// These requires the complete type of Impl.
+MapOptions::MapOptions() : impl_(std::make_unique<Impl>()) {}
 MapOptions::~MapOptions() = default;
+MapOptions::MapOptions(MapOptions&&) = default;
+MapOptions::MapOptions(const MapOptions& other) : impl_(std::make_unique<Impl>(*other.impl_)) {}
 
-MapOptions& MapOptions::withMapMode(MapMode mode) {
+MapOptions MapOptions::withMapMode(MapMode mode) {
     impl_->mapMode = mode;
-    return *this;
+    return std::move(*this);
 }
 
 MapMode MapOptions::mapMode() const {
     return impl_->mapMode;
 }
 
-MapOptions& MapOptions::withConstrainMode(ConstrainMode mode) {
+MapOptions MapOptions::withConstrainMode(ConstrainMode mode) {
     impl_->constrainMode = mode;
-    return *this;
+    return std::move(*this);
 }
 
 ConstrainMode MapOptions::constrainMode() const {
     return impl_->constrainMode;
 }
 
-MapOptions& MapOptions::withViewportMode(ViewportMode mode) {
+MapOptions MapOptions::withViewportMode(ViewportMode mode) {
     impl_->viewportMode = mode;
-    return *this;
+    return std::move(*this);
 }
 
 ViewportMode MapOptions::viewportMode() const {
     return impl_->viewportMode;
 }
 
-MapOptions& MapOptions::withCrossSourceCollisions(bool enableCollisions) {
+MapOptions MapOptions::withCrossSourceCollisions(bool enableCollisions) {
     impl_->crossSourceCollisions = enableCollisions;
-    return *this;
+    return std::move(*this);
 }
 
 bool MapOptions::crossSourceCollisions() const {
diff --git a/src/mbgl/storage/resource_options.cpp b/src/mbgl/storage/resource_options.cpp
index ce82b51a14..dcd7f84f7e 100644
--- a/src/mbgl/storage/resource_options.cpp
+++ b/src/mbgl/storage/resource_options.cpp
@@ -13,57 +13,60 @@ public:
     void* platformContext = nullptr;
 };
 
-ResourceOptions::ResourceOptions() : impl_(std::make_shared<ResourceOptions::Impl>()) {}
+// These requires the complete type of Impl.
+ResourceOptions::ResourceOptions() : impl_(std::make_unique<Impl>()) {}
 ResourceOptions::~ResourceOptions() = default;
+ResourceOptions::ResourceOptions(ResourceOptions&&)  = default;
+ResourceOptions::ResourceOptions(const ResourceOptions& other) : impl_(std::make_unique<Impl>(*other.impl_)) {}
 
-ResourceOptions& ResourceOptions::withAccessToken(std::string token) {
+ResourceOptions ResourceOptions::withAccessToken(std::string token) {
     impl_->accessToken = std::move(token);
-    return *this;
+    return std::move(*this);
 }
 
 const std::string& ResourceOptions::accessToken() const {
     return impl_->accessToken;
 }
 
-ResourceOptions& ResourceOptions::withBaseURL(std::string url) {
+ResourceOptions ResourceOptions::withBaseURL(std::string url) {
     impl_->baseURL = std::move(url);
-    return *this;
+    return std::move(*this);
 }
 
 const std::string& ResourceOptions::baseURL() const {
     return impl_->baseURL;
 }
 
-ResourceOptions& ResourceOptions::withCachePath(std::string path) {
+ResourceOptions ResourceOptions::withCachePath(std::string path) {
     impl_->cachePath = std::move(path);
-    return *this;
+    return std::move(*this);
 }
 
 const std::string& ResourceOptions::cachePath() const {
     return impl_->cachePath;
 }
 
-ResourceOptions& ResourceOptions::withAssetPath(std::string path) {
+ResourceOptions ResourceOptions::withAssetPath(std::string path) {
     impl_->assetPath = std::move(path);
-    return *this;
+    return std::move(*this);
 }
 
 const std::string& ResourceOptions::assetPath() const {
     return impl_->assetPath;
 }
 
-ResourceOptions& ResourceOptions::withMaximumCacheSize(uint64_t size) {
+ResourceOptions ResourceOptions::withMaximumCacheSize(uint64_t size) {
     impl_->maximumSize = size;
-    return *this;
+    return std::move(*this);
 }
 
 uint64_t ResourceOptions::maximumCacheSize() const {
     return impl_->maximumSize;
 }
 
-ResourceOptions& ResourceOptions::withPlatformContext(void* context) {
+ResourceOptions ResourceOptions::withPlatformContext(void* context) {
     impl_->platformContext = context;
-    return *this;
+    return std::move(*this);
 }
 
 void* ResourceOptions::platformContext() const {