[all] Don't interpret 404s on non-tile resources as "no content"

7 files changed, 121 insertions, 144 deletions

diff --git a/src/mbgl/map/map_context.cpp b/src/mbgl/map/map_context.cpp
index 03da5ee321..6b4657f0af 100644
--- a/src/mbgl/map/map_context.cpp
+++ b/src/mbgl/map/map_context.cpp
@@ -113,14 +113,11 @@ void MapContext::setStyleURL(const std::string& url) {
                 Log::Error(Event::Setup, "loading style failed: %s", res.error->message.c_str());
                 data.loading = false;
             }
+        } else if (res.notModified || res.noContent) {
             return;
+        } else {
+            loadStyleJSON(*res.data, base);
         }
-
-        if (res.notModified) {
-            return;
-        }
-
-        loadStyleJSON(*res.data, base);
     });
 }
 
diff --git a/src/mbgl/map/raster_tile_data.cpp b/src/mbgl/map/raster_tile_data.cpp
index f9f5480197..8df4e05b62 100644
--- a/src/mbgl/map/raster_tile_data.cpp
+++ b/src/mbgl/map/raster_tile_data.cpp
@@ -22,52 +22,46 @@ RasterTileData::RasterTileData(const TileID& id_,
     const Resource resource = Resource::tile(urlTemplate, pixelRatio, id.x, id.y, id.sourceZ);
     req = util::ThreadContext::getFileSource()->request(resource, [callback, this](Response res) {
         if (res.error) {
-            std::exception_ptr error;
-            if (res.error->reason == Response::Error::Reason::NotFound) {
-                // This is a 404 response. We're treating these as empty tiles.
-                workRequest.reset();
-                state = State::parsed;
-                bucket.reset();
-            } else {
-                // This is a different error, e.g. a connection or server error.
-                error = std::make_exception_ptr(std::runtime_error(res.error->message));
-            }
-            callback(error);
-            return;
-        }
-
-        modified = res.modified;
-        expires = res.expires;
-
-        if (res.notModified) {
-            // We got the same data again. Abort early.
-            return;
-        }
+            callback(std::make_exception_ptr(std::runtime_error(res.error->message)));
+        } else if (res.notModified) {
+            modified = res.modified;
+            expires = res.expires;
+        } else if (res.noContent) {
+            state = State::parsed;
+            modified = res.modified;
+            expires = res.expires;
+            workRequest.reset();
+            bucket.reset();
+            callback(nullptr);
+        } else {
+            modified = res.modified;
+            expires = res.expires;
 
-        if (state == State::loading) {
             // Only overwrite the state when we didn't have a previous tile.
-            state = State::loaded;
-        }
+            if (state == State::loading) {
+                state = State::loaded;
+            }
 
-        workRequest.reset();
-        workRequest = worker.parseRasterTile(std::make_unique<RasterBucket>(texturePool), res.data, [this, callback] (RasterTileParseResult result) {
             workRequest.reset();
-            if (state != State::loaded) {
-                return;
-            }
+            workRequest = worker.parseRasterTile(std::make_unique<RasterBucket>(texturePool), res.data, [this, callback] (RasterTileParseResult result) {
+                workRequest.reset();
+                if (state != State::loaded) {
+                    return;
+                }
 
-            std::exception_ptr error;
-            if (result.is<std::unique_ptr<Bucket>>()) {
-                state = State::parsed;
-                bucket = std::move(result.get<std::unique_ptr<Bucket>>());
-            } else {
-                error = result.get<std::exception_ptr>();
-                state = State::obsolete;
-                bucket.reset();
-            }
+                std::exception_ptr error;
+                if (result.is<std::unique_ptr<Bucket>>()) {
+                    state = State::parsed;
+                    bucket = std::move(result.get<std::unique_ptr<Bucket>>());
+                } else {
+                    error = result.get<std::exception_ptr>();
+                    state = State::obsolete;
+                    bucket.reset();
+                }
 
-            callback(error);
-        });
+                callback(error);
+            });
+        }
     });
 }
 
diff --git a/src/mbgl/map/source.cpp b/src/mbgl/map/source.cpp
index e763674040..1a7933f53e 100644
--- a/src/mbgl/map/source.cpp
+++ b/src/mbgl/map/source.cpp
@@ -90,76 +90,74 @@ void Source::load() {
     req = fs->request(Resource::source(url), [this](Response res) {
         if (res.error) {
             observer->onSourceError(*this, std::make_exception_ptr(std::runtime_error(res.error->message)));
+        } else if (res.notModified) {
             return;
-        }
+        } else if (res.noContent) {
+            observer->onSourceError(*this, std::make_exception_ptr(std::runtime_error("unexpectedly empty source")));
+        } else {
+            bool reloadTiles = false;
+
+            if (type == SourceType::Vector || type == SourceType::Raster) {
+                std::unique_ptr<SourceInfo> newInfo;
+
+                // Create a new copy of the SourceInfo object that holds the base values we've parsed
+                // from the stylesheet. Then merge in the values parsed from the TileJSON we retrieved
+                // via the URL.
+                try {
+                    newInfo = StyleParser::parseTileJSON(*res.data, url, type);
+                } catch (...) {
+                    observer->onSourceError(*this, std::current_exception());
+                    return;
+                }
 
-        if (res.notModified) {
-            // We got the same data back as last time. Abort early.
-            return;
-        }
+                // Check whether previous information specifies different tile
+                if (info && info->tiles != newInfo->tiles) {
+                    reloadTiles = true;
 
-        bool reloadTiles = false;
+                    // Tile size changed: We need to recalculate the tiles we need to load because we
+                    // might have to load tiles for a different zoom level
+                    // This is done automatically when we trigger the onSourceLoaded observer below.
 
-        if (type == SourceType::Vector || type == SourceType::Raster) {
-            std::unique_ptr<SourceInfo> newInfo;
+                    // Min/Max zoom changed: We need to recalculate what tiles to load, if we have tiles
+                    // loaded that are outside the new zoom range
+                    // This is done automatically when we trigger the onSourceLoaded observer below.
 
-            // Create a new copy of the SourceInfo object that holds the base values we've parsed
-            // from the stylesheet. Then merge in the values parsed from the TileJSON we retrieved
-            // via the URL.
-            try {
-                newInfo = StyleParser::parseTileJSON(*res.data, url, type);
-            } catch (...) {
-                observer->onSourceError(*this, std::current_exception());
-                return;
-            }
+                    // Attribution changed: We need to notify the embedding application that this
+                    // changed. See https://github.com/mapbox/mapbox-gl-native/issues/2723
+                    // This is not yet implemented.
 
-            // Check whether previous information specifies different tile
-            if (info && info->tiles != newInfo->tiles) {
-                reloadTiles = true;
+                    // Center/bounds changed: We're not using these values currently
+                }
 
-                // Tile size changed: We need to recalculate the tiles we need to load because we
-                // might have to load tiles for a different zoom level
-                // This is done automatically when we trigger the onSourceLoaded observer below.
+                info = std::move(newInfo);
+            } else if (type == SourceType::GeoJSON) {
+                info = std::make_unique<SourceInfo>();
 
-                // Min/Max zoom changed: We need to recalculate what tiles to load, if we have tiles
-                // loaded that are outside the new zoom range
-                // This is done automatically when we trigger the onSourceLoaded observer below.
+                rapidjson::GenericDocument<rapidjson::UTF8<>, rapidjson::CrtAllocator> d;
+                d.Parse<0>(res.data->c_str());
 
-                // Attribution changed: We need to notify the embedding application that this
-                // changed. See https://github.com/mapbox/mapbox-gl-native/issues/2723
-                // This is not yet implemented.
+                if (d.HasParseError()) {
+                    std::stringstream message;
+                    message << d.GetErrorOffset() << " - " << rapidjson::GetParseError_En(d.GetParseError());
+                    observer->onSourceError(*this, std::make_exception_ptr(std::runtime_error(message.str())));
+                    return;
+                }
 
-                // Center/bounds changed: We're not using these values currently
+                geojsonvt = StyleParser::parseGeoJSON(d);
+                reloadTiles = true;
             }
 
-            info = std::move(newInfo);
-        } else if (type == SourceType::GeoJSON) {
-            info = std::make_unique<SourceInfo>();
-
-            rapidjson::GenericDocument<rapidjson::UTF8<>, rapidjson::CrtAllocator> d;
-            d.Parse<0>(res.data->c_str());
-
-            if (d.HasParseError()) {
-                std::stringstream message;
-                message << d.GetErrorOffset() << " - " << rapidjson::GetParseError_En(d.GetParseError());
-                observer->onSourceError(*this, std::make_exception_ptr(std::runtime_error(message.str())));
-                return;
+            if (reloadTiles) {
+                // Tile information changed because we got new GeoJSON data, or a new tile URL.
+                tilePtrs.clear();
+                tileDataMap.clear();
+                tiles.clear();
+                cache.clear();
             }
 
-            geojsonvt = StyleParser::parseGeoJSON(d);
-            reloadTiles = true;
+            loaded = true;
+            observer->onSourceLoaded(*this);
         }
-
-        if (reloadTiles) {
-            // Tile information changed because we got new GeoJSON data, or a new tile URL.
-            tilePtrs.clear();
-            tileDataMap.clear();
-            tiles.clear();
-            cache.clear();
-        }
-
-        loaded = true;
-        observer->onSourceLoaded(*this);
     });
 }
 
diff --git a/src/mbgl/map/vector_tile.cpp b/src/mbgl/map/vector_tile.cpp
index 17746a26a1..6de3b76b4f 100644
--- a/src/mbgl/map/vector_tile.cpp
+++ b/src/mbgl/map/vector_tile.cpp
@@ -191,22 +191,15 @@ VectorTileMonitor::VectorTileMonitor(const TileID& tileID_, float pixelRatio_, c
 std::unique_ptr<FileRequest> VectorTileMonitor::monitorTile(const GeometryTileMonitor::Callback& callback) {
     const Resource resource = Resource::tile(urlTemplate, pixelRatio, tileID.x, tileID.y, tileID.sourceZ);
     return util::ThreadContext::getFileSource()->request(resource, [callback, this](Response res) {
-        if (res.notModified) {
-            // We got the same data again. Abort early.
-            return;
-        }
-
         if (res.error) {
-            if (res.error->reason == Response::Error::Reason::NotFound) {
-                callback(nullptr, nullptr, res.modified, res.expires);
-                return;
-            } else {
-                callback(std::make_exception_ptr(std::runtime_error(res.error->message)), nullptr, res.modified, res.expires);
-                return;
-            }
+            callback(std::make_exception_ptr(std::runtime_error(res.error->message)), nullptr, res.modified, res.expires);
+        } else if (res.notModified) {
+            return;
+        } else if (res.noContent) {
+            callback(nullptr, nullptr, res.modified, res.expires);
+        } else {
+            callback(nullptr, std::make_unique<VectorTile>(res.data), res.modified, res.expires);
         }
-
-        callback(nullptr, std::make_unique<VectorTile>(res.data), res.modified, res.expires);
     });
 }
 
diff --git a/src/mbgl/sprite/sprite_store.cpp b/src/mbgl/sprite/sprite_store.cpp
index 15a345cc4d..38aa7009ca 100644
--- a/src/mbgl/sprite/sprite_store.cpp
+++ b/src/mbgl/sprite/sprite_store.cpp
@@ -38,15 +38,12 @@ void SpriteStore::setURL(const std::string& url) {
     loader->jsonRequest = fs->request(Resource::spriteJSON(url, pixelRatio), [this](Response res) {
         if (res.error) {
             observer->onSpriteError(std::make_exception_ptr(std::runtime_error(res.error->message)));
+        } else if (res.notModified) {
             return;
-        }
-
-        if (res.notModified) {
-            // We got the same data back as last time. Abort early.
-            return;
-        }
-
-        if (!loader->json || *loader->json != *res.data) {
+        } else if (res.noContent) {
+            loader->json = std::make_shared<const std::string>();
+            emitSpriteLoadedIfComplete();
+        } else {
             // Only trigger a sprite loaded event we got new data.
             loader->json = res.data;
             emitSpriteLoadedIfComplete();
@@ -56,15 +53,12 @@ void SpriteStore::setURL(const std::string& url) {
     loader->spriteRequest = fs->request(Resource::spriteImage(url, pixelRatio), [this](Response res) {
         if (res.error) {
             observer->onSpriteError(std::make_exception_ptr(std::runtime_error(res.error->message)));
+        } else if (res.notModified) {
             return;
-        }
-
-        if (res.notModified) {
-            // We got the same data back as last time. Abort early.
-            return;
-        }
-
-        if (!loader->image || *loader->image != *res.data) {
+        } else if (res.noContent) {
+            loader->image = std::make_shared<const std::string>();
+            emitSpriteLoadedIfComplete();
+        } else {
             loader->image = res.data;
             emitSpriteLoadedIfComplete();
         }
diff --git a/src/mbgl/storage/response.cpp b/src/mbgl/storage/response.cpp
index 22263d2ebb..09c43c8a6a 100644
--- a/src/mbgl/storage/response.cpp
+++ b/src/mbgl/storage/response.cpp
@@ -9,6 +9,7 @@ Response::Response(const Response& res) {
 
 Response& Response::operator=(const Response& res) {
     error = res.error ? std::make_unique<Error>(*res.error) : nullptr;
+    noContent = res.noContent;
     notModified = res.notModified;
     data = res.data;
     modified = res.modified;
diff --git a/src/mbgl/text/glyph_pbf.cpp b/src/mbgl/text/glyph_pbf.cpp
index 0dfed6b430..5973c0ce14 100644
--- a/src/mbgl/text/glyph_pbf.cpp
+++ b/src/mbgl/text/glyph_pbf.cpp
@@ -71,22 +71,22 @@ GlyphPBF::GlyphPBF(GlyphStore* store,
     req = fs->request(Resource::glyphs(store->getURL(), fontStack, glyphRange), [this, store, fontStack, glyphRange](Response res) {
         if (res.error) {
             observer->onGlyphsError(fontStack, glyphRange, std::make_exception_ptr(std::runtime_error(res.error->message)));
+        } else if (res.notModified) {
             return;
-        }
-
-        if (res.notModified) {
-            return;
-        }
+        } else if (res.noContent) {
+            parsed = true;
+            observer->onGlyphsLoaded(fontStack, glyphRange);
+        } else {
+            try {
+                parseGlyphPBF(**store->getFontStack(fontStack), *res.data);
+            } catch (...) {
+                observer->onGlyphsError(fontStack, glyphRange, std::current_exception());
+                return;
+            }
 
-        try {
-            parseGlyphPBF(**store->getFontStack(fontStack), *res.data);
-        } catch (...) {
-            observer->onGlyphsError(fontStack, glyphRange, std::current_exception());
-            return;
+            parsed = true;
+            observer->onGlyphsLoaded(fontStack, glyphRange);
         }
-
-        parsed = true;
-        observer->onGlyphsLoaded(fontStack, glyphRange);
     });
 }