Improve weak pointer use (#3510)

Co-authored-by: Bart Louwers <bart@emeel.net>

include/mbgl/actor/scheduler.hpp

@@ -115,9 +115,9 @@ private:
                                ReplyFn&& reply,
                                mapbox::base::WeakPtr<Scheduler> replyScheduler) {
         schedule(tag, [replyScheduler = std::move(replyScheduler), tag, task, reply] {
-            auto lock = replyScheduler.lock();
-            if (!replyScheduler) return;
-            replyScheduler->schedule(tag, [reply, result = task()] { reply(result); });
+            if (auto guard = replyScheduler.lock(); replyScheduler) {
+                replyScheduler->schedule(tag, [reply, result = task()] { reply(result); });
+            }
         });
     }
 };

platform/android/MapLibreAndroid/src/cpp/snapshotter/map_snapshotter.cpp

@@ -66,14 +66,9 @@ MapSnapshotter::MapSnapshotter(jni::JNIEnv& _env,
 }
 
 MapSnapshotter::~MapSnapshotter() {
-    auto guard = weakScheduler.lock();
-    if (weakScheduler && weakScheduler.get() != mbgl::Scheduler::GetCurrent()) {
+    if (auto guard = weakScheduler.lock(); weakScheduler && weakScheduler.get() != Scheduler::GetCurrent()) {
         snapshotter->cancel();
-        weakScheduler->schedule([ptr = snapshotter.release()]() mutable {
-            if (ptr) {
-                delete ptr;
-            }
-        });
+        weakScheduler->schedule([ptr = snapshotter.release()]() { delete ptr; });
     }
 }
 

src/mbgl/actor/mailbox.cpp

@@ -69,7 +69,7 @@ void Mailbox::abandon() {
 }
 
 bool Mailbox::isOpen() const {
-    return bool(weakScheduler) && !closed;
+    return !closed && weakScheduler;
 }
 
 void Mailbox::push(std::unique_ptr<Message> message) {
@@ -152,8 +152,7 @@ void Mailbox::receive() {
 }
 
 void Mailbox::scheduleToRecieve(const std::optional<util::SimpleIdentity>& tag) {
-    auto guard = weakScheduler.lock();
-    if (weakScheduler) {
+    if (auto guard = weakScheduler.lock(); weakScheduler) {
         std::weak_ptr<Mailbox> mailbox = shared_from_this();
         auto setToRecieve = [mbox = std::move(mailbox)]() {
             if (auto locked = mbox.lock()) {

src/mbgl/actor/scheduler.cpp

@@ -9,8 +9,9 @@ std::function<void()> Scheduler::bindOnce(std::function<void()> fn) {
     assert(fn);
     return [scheduler = makeWeakPtr(), scheduled = std::move(fn)]() mutable {
         if (!scheduled) return; // Repeated call.
-        auto schedulerGuard = scheduler.lock();
-        if (scheduler) scheduler->schedule(std::move(scheduled));
+        if (auto guard = scheduler.lock(); scheduler) {
+            scheduler->schedule(std::move(scheduled));
+        }
     };
 }
 

src/mbgl/annotation/annotation_tile.cpp

@@ -12,15 +12,13 @@ AnnotationTile::AnnotationTile(const OverscaledTileID& overscaledTileID,
                                TileObserver* observer_)
     : GeometryTile(overscaledTileID, AnnotationManager::SourceID, parameters, observer_),
       annotationManager(parameters.annotationManager) {
-    auto guard = annotationManager.lock();
-    if (annotationManager) {
+    if (auto guard = annotationManager.lock(); annotationManager) {
         annotationManager->addTile(*this);
     }
 }
 
 AnnotationTile::~AnnotationTile() {
-    auto guard = annotationManager.lock();
-    if (annotationManager) {
+    if (auto guard = annotationManager.lock(); annotationManager) {
         annotationManager->removeTile(*this);
     }
 }

src/mbgl/renderer/render_orchestrator.cpp

@@ -165,8 +165,7 @@ std::unique_ptr<RenderTree> RenderOrchestrator::createRenderTree(
     }
 
     if (LayerManager::annotationsEnabled) {
-        auto guard = updateParameters->annotationManager.lock();
-        if (updateParameters->annotationManager) {
+        if (auto guard = updateParameters->annotationManager.lock(); updateParameters->annotationManager) {
             updateParameters->annotationManager->updateData();
         }
     }

src/mbgl/sprite/sprite_loader.cpp

@@ -99,20 +99,20 @@ void SpriteLoader::emitSpriteLoadedIfComplete(style::Sprite sprite) {
         /* parseClosure */
         [sprite = sprite, image = data->image, json = data->json]() -> ParseResult {
             try {
-                return {parseSprite(sprite.id, *image, *json), nullptr};
+                return {.images = parseSprite(sprite.id, *image, *json), .error = nullptr};
             } catch (...) {
-                return {{}, std::current_exception()};
+                return {.images = {}, .error = std::current_exception()};
             }
         },
         /* resultClosure */
-        [this, sprite = sprite, weak = weakFactory.makeWeakPtr()](ParseResult result) {
-            if (!weak) return; // This instance has been deleted.
-
-            if (result.error) {
-                observer->onSpriteError(std::optional(sprite), result.error);
-                return;
+        [this, sprite = sprite, factory = weakFactory.makeWeakPtr()](ParseResult result) {
+            if (auto guard = factory.lock(); factory) {
+                if (result.error) {
+                    observer->onSpriteError(std::optional(sprite), result.error);
+                } else {
+                    observer->onSpriteLoaded(std::optional(sprite), std::move(result.images));
+                }
             }
-            observer->onSpriteLoaded(std::optional(sprite), std::move(result.images));
         });
 }
 

src/mbgl/style/sources/geojson_source.cpp

@@ -101,12 +101,13 @@ void GeoJSONSource::loadDescription(FileSource& fileSource) {
                 /* onImplReady */
                 [this, self = makeWeakPtr(), capturedReq = req.get()](Immutable<Source::Impl> newImpl) {
                     assert(capturedReq);
-                    if (!self) return;                    // This source has been deleted.
-                    if (capturedReq != req.get()) return; // A new request is being processed, ignore this impl.
-
-                    baseImpl = std::move(newImpl);
-                    loaded = true;
-                    observer->onSourceLoaded(*this);
+                    if (auto guard = self.lock(); self) {
+                        if (capturedReq == req.get()) { // If a new request is being processed, ignore this impl.
+                            baseImpl = std::move(newImpl);
+                            loaded = true;
+                            observer->onSourceLoaded(*this);
+                        }
+                    }
                 });
         }
     });

src/mbgl/tile/geojson_tile.cpp

@@ -8,6 +8,8 @@
 
 namespace mbgl {
 
+using TileFeatures = style::GeoJSONData::TileFeatures;
+
 GeoJSONTile::GeoJSONTile(const OverscaledTileID& overscaledTileID,
                          std::string sourceID_,
                          const TileParameters& parameters,
@@ -23,14 +25,13 @@ void GeoJSONTile::updateData(std::shared_ptr<style::GeoJSONData> data_, bool nee
     assert(data_);
     data = std::move(data_);
     if (needsRelayout) reset();
-    data->getTile(
-        id.canonical,
-        [this, self = weakFactory.makeWeakPtr(), capturedData = data.get()](style::GeoJSONData::TileFeatures features) {
-            if (!self) return;
-            if (data.get() != capturedData) return;
-            auto tileData = std::make_unique<GeoJSONTileData>(std::move(features));
-            setData(std::move(tileData));
-        });
+    data->getTile(id.canonical,
+                  [this, self = weakFactory.makeWeakPtr(), capturedData = data.get()](TileFeatures features) {
+                      // If the data has changed, a new request is being processed, ignore this one
+                      if (auto guard = self.lock(); self && data.get() == capturedData) {
+                          setData(std::make_unique<GeoJSONTileData>(std::move(features)));
+                      }
+                  });
 }
 
 void GeoJSONTile::querySourceFeatures(std::vector<Feature>& result, const SourceQueryOptions& options) {