Cherry pick #6555 fix from master.

Don't load FeatureIndex#vtLayers from outside FeatureIndex code. (#6559) * Don't load FeatureIndex#vtLayers from outside FeatureIndex code. Fixes issue #6555 (a possible crash when calling queryRenderedFeatures after querySourceFeatures) The actual contents of the regression test are not that important -- before this fix, the test would crash. * Fix Tile unit test by deserializing the FeatureIndex before using it.
mapbox · Apr 30, 2018 · 90121ad · 90121ad
1 parent dfed05e
commit 90121ad
Show file tree

Hide file tree

Showing 5 changed files with 88 additions and 18 deletions.
diff --git a/src/data/feature_index.js b/src/data/feature_index.js
@@ -84,12 +84,17 @@ class FeatureIndex {
         }
     }
 
-    // Finds non-symbol features in this tile at a particular position.
-    query(args: QueryParameters, styleLayers: {[string]: StyleLayer}): {[string]: Array<{ featureIndex: number, feature: GeoJSONFeature }>} {
+    loadVTLayers(): {[string]: VectorTileLayer} {
         if (!this.vtLayers) {
             this.vtLayers = new vt.VectorTile(new Protobuf(this.rawTileData)).layers;
             this.sourceLayerCoder = new DictionaryCoder(this.vtLayers ? Object.keys(this.vtLayers).sort() : ['_geojsonTileLayer']);
         }
+        return this.vtLayers;
+    }
+
+    // Finds non-symbol features in this tile at a particular position.
+    query(args: QueryParameters, styleLayers: {[string]: StyleLayer}): {[string]: Array<{ featureIndex: number, feature: GeoJSONFeature }>} {
+        this.loadVTLayers();
 
         const params = args.params || {},
             pixelsToTileUnits = EXTENT / args.tileSize / args.scale,
@@ -201,10 +206,7 @@ class FeatureIndex {
                          filterLayerIDs: Array<string>,
                          styleLayers: {[string]: StyleLayer}) {
         const result = {};
-        if (!this.vtLayers) {
-            this.vtLayers = new vt.VectorTile(new Protobuf(this.rawTileData)).layers;
-            this.sourceLayerCoder = new DictionaryCoder(this.vtLayers ? Object.keys(this.vtLayers).sort() : ['_geojsonTileLayer']);
-        }
+        this.loadVTLayers();
 
         const filter = featureFilter(filterSpec);
 

diff --git a/src/source/tile.js b/src/source/tile.js
@@ -3,8 +3,6 @@
 import { uniqueId, deepEqual, parseCacheControl } from '../util/util';
 import { deserialize as deserializeBucket } from '../data/bucket';
 import FeatureIndex from '../data/feature_index';
-import vt from '@mapbox/vector-tile';
-import Protobuf from 'pbf';
 import GeoJSONFeature from '../util/vectortile_to_geojson';
 import featureFilter from '../style-spec/feature_filter';
 import SymbolBucket from '../data/bucket/symbol_bucket';
@@ -266,11 +264,7 @@ class Tile {
     querySourceFeatures(result: Array<GeoJSONFeature>, params: any) {
         if (!this.latestFeatureIndex || !this.latestFeatureIndex.rawTileData) return;
 
-        if (!this.latestFeatureIndex.vtLayers) {
-            this.latestFeatureIndex.vtLayers =
-                new vt.VectorTile(new Protobuf(this.latestFeatureIndex.rawTileData)).layers;
-        }
-        const vtLayers = this.latestFeatureIndex.vtLayers;
+        const vtLayers = this.latestFeatureIndex.loadVTLayers();
 
         const sourceLayer = params ? params.sourceLayer : '';
         const layer = vtLayers._geojsonTileLayer || vtLayers[sourceLayer];

diff --git a/test/integration/query-tests/regressions/mapbox-gl-js#6555/expected.json b/test/integration/query-tests/regressions/mapbox-gl-js#6555/expected.json
@@ -0,0 +1,13 @@
+[
+  {
+    "geometry": {
+      "type": "Point",
+      "coordinates": [
+        0,
+        0
+      ]
+    },
+    "type": "Feature",
+    "properties": {}
+  }
+]
diff --git a/test/integration/query-tests/regressions/mapbox-gl-js#6555/style.json b/test/integration/query-tests/regressions/mapbox-gl-js#6555/style.json
@@ -0,0 +1,59 @@
+{
+    "version": 8,
+    "metadata": {
+        "test": {
+            "width": 128,
+            "height": 128,
+            "operations": [
+              [
+                "querySourceFeatures",
+                [
+                  "source"
+                ]
+              ]
+            ],
+            "queryGeometry": [
+                79,
+                64
+            ],
+            "queryOptions": {
+            }
+        }
+    },
+    "zoom": 0,
+    "glyphs": "local://glyphs/{fontstack}/{range}.pbf",
+    "sprite": "local://sprites/sprite",
+    "sources": {
+        "source": {
+            "type": "geojson",
+            "data": {
+              "type": "FeatureCollection",
+              "features": [{
+                "type": "Feature",
+                "geometry": {
+                  "type": "Point",
+                  "coordinates": [0, 0]
+                }
+              }]
+            }
+        }
+    },
+    "layers": [
+        {
+          "id": "background",
+          "type": "background",
+          "paint": {
+            "background-color": "white"
+          }
+        },
+        {
+          "id": "iconlayer",
+          "type": "symbol",
+          "source": "source",
+          "layout": {
+            "icon-image": "building-12",
+            "icon-offset": [15, 0]
+          }
+        }
+    ]
+}
diff --git a/test/unit/source/tile.test.js b/test/unit/source/tile.test.js
@@ -9,7 +9,7 @@ import FeatureIndex from '../../../src/data/feature_index';
 import { CollisionBoxArray } from '../../../src/data/array_types';
 import { extend } from '../../../src/util/util';
 import Context from '../../../src/gl/context';
-import { serialize } from '../../../src/util/web_worker_transfer';
+import { serialize, deserialize } from '../../../src/util/web_worker_transfer';
 
 test('querySourceFeatures', (t) => {
     const features = [{
@@ -21,7 +21,6 @@ test('querySourceFeatures', (t) => {
 
     t.test('geojson tile', (t) => {
         const tile = new Tile(new OverscaledTileID(1, 0, 1, 1, 1));
-        tile.latestFeatureIndex = new FeatureIndex(tile.tileID);
         let result;
 
         result = [];
@@ -30,7 +29,10 @@ test('querySourceFeatures', (t) => {
 
         const geojsonWrapper = new GeoJSONWrapper(features);
         geojsonWrapper.name = '_geojsonTileLayer';
-        tile.latestFeatureIndex.rawTileData = vtpbf({ layers: { '_geojsonTileLayer': geojsonWrapper }});
+        tile.loadVectorData(
+            createVectorData({rawTileData: vtpbf({ layers: { '_geojsonTileLayer': geojsonWrapper }})}),
+            createPainter()
+        );
 
         result = [];
         tile.querySourceFeatures(result);
@@ -309,8 +311,8 @@ function createRawTileData() {
 function createVectorData(options) {
     const collisionBoxArray = new CollisionBoxArray();
     return extend({
-        collisionBoxArray: serialize(collisionBoxArray),
-        featureIndex: serialize(new FeatureIndex(new OverscaledTileID(1, 0, 1, 1, 1))),
+        collisionBoxArray: deserialize(serialize(collisionBoxArray)),
+        featureIndex: deserialize(serialize(new FeatureIndex(new OverscaledTileID(1, 0, 1, 1, 1)))),
         buckets: []
     }, options);
 }