Fix a possible polygon feature update after delete.

CHANGELOG.md

@@ -8,6 +8,7 @@
 
 ### Bug Fixes
 - Fixed an issue with overlapping, cropped tiles on old browsers (#901)
+- Fixed an issue where a `geo.gl.polygonFeature` could be updated after it was deleted (#913)
 
 ### Changes
 - Changed build process: optional dependencies are now included in the bundle by default (#890)

src/canvas/object.js

@@ -70,9 +70,11 @@ var canvas_object = function (arg) {
    * @returns {this} The current object.
    */
   this.draw = function () {
-    m_this._update();
-    m_this.renderer()._render();
-    s_draw();
+    if (m_this.ready) {
+      m_this._update();
+      m_this.renderer()._render();
+      s_draw();
+    }
     return m_this;
   };
 

src/d3/object.js

@@ -48,8 +48,10 @@ var d3_object = function (arg) {
    * @returns {this}
    */
   this.draw = function () {
-    m_this._update();
-    s_draw();
+    if (m_this.ready) {
+      m_this._update();
+      s_draw();
+    }
     return m_this;
   };
 

src/feature.js

@@ -105,6 +105,7 @@ var feature = function (arg) {
 
   var m_this = this,
       s_exit = this._exit,
+      m_ready,
       m_selectionAPI = arg.selectionAPI === undefined ? false : !!arg.selectionAPI,
       m_style = {},
       m_layer = arg.layer === undefined ? null : arg.layer,
@@ -122,16 +123,27 @@ var feature = function (arg) {
   // data items, such as individual vertices on lines or polygons.
   this._subfeatureStyles = {};
 
+  /**
+   * @property {boolean} ready `true` if this feature has been initialized,
+   *    `false` if it was destroyed, `undefined` if it was created but not
+   *    initialized.
+   * @name geo.feature#ready
+   */
+  Object.defineProperty(this, 'ready', {
+    get: function () {
+      return m_ready;
+    }
+  });
+
   /**
    * Private method to bind mouse handlers on the map element.  This does
    * nothing if the selectionAPI is turned off.  Otherwise, it first unbinds
    * any existing handlers and then binds handlers.
    */
   this._bindMouseHandlers = function () {
-
     // Don't bind handlers for improved performance on features that don't
     // require it.
-    if (!this.selectionAPI()) {
+    if (!m_this.selectionAPI()) {
       return;
     }
 
@@ -217,7 +229,7 @@ var feature = function (arg) {
     // evt.previous.
     if (over.index.length > 1) {
       m_this.geoTrigger(geo_event.feature.mouseover_order, {
-        feature: this,
+        feature: m_this,
         mouse: mouse,
         previous: m_selectedFeatures,
         over: over
@@ -334,7 +346,7 @@ var feature = function (arg) {
     // maintain evt.over.found.  Handlers should not modify evt.over.extra.
     if (over.index.length > 1) {
       m_this.geoTrigger(geo_event.feature.mouseclick_order, {
-        feature: this,
+        feature: m_this,
         mouse: mouse,
         over: over
       });
@@ -463,7 +475,7 @@ var feature = function (arg) {
       if (util.isFunction(m_style[key])) {
         out = function () {
           return util.convertColor(
-            m_style[key].apply(this, arguments)
+            m_style[key].apply(m_this, arguments)
           );
         };
       } else {
@@ -667,7 +679,7 @@ var feature = function (arg) {
   this.bin = function (val) {
     if (val === undefined) {
       if (m_bin === null) {
-        var parent = this.parent(),
+        var parent = m_this.parent(),
             idx = parent ? parent.children().indexOf(m_this) : -1;
         return idx >= 0 ? idx : 0;
       }
@@ -772,10 +784,10 @@ var feature = function (arg) {
     arg = !!arg;
     if (arg !== m_selectionAPI || direct) {
       m_selectionAPI = arg;
-      this._unbindMouseHandlers();
-      this._bindMouseHandlers();
+      m_this._unbindMouseHandlers();
+      m_this._bindMouseHandlers();
     }
-    return this;
+    return m_this;
   };
 
   /**
@@ -810,6 +822,7 @@ var feature = function (arg) {
                 {'opacity': 1.0}, arg.style === undefined ? {} :
                 arg.style);
     m_this._bindMouseHandlers();
+    m_ready = true;
   };
 
   /**
@@ -839,6 +852,7 @@ var feature = function (arg) {
     m_selectedFeatures = [];
     m_style = {};
     s_exit();
+    m_ready = false;
   };
 
   this._init(arg);

src/gl/object.js

@@ -27,9 +27,11 @@ var gl_object = function (arg) {
    * @returns {this}
    */
   this.draw = function () {
-    m_this._update({mayDelay: true});
-    m_this.renderer()._render();
-    s_draw();
+    if (m_this.ready) {
+      m_this._update({mayDelay: true});
+      m_this.renderer()._render();
+      s_draw();
+    }
     return m_this;
   };
 

src/gl/polygonFeature.js

@@ -339,6 +339,9 @@ var gl_polygonFeature = function (arg) {
    *    frame for the update.
    */
   this._update = function (opts) {
+    if (!m_this.ready) {
+      return;
+    }
     if (opts && opts.mayDelay && m_builtOnce) {
       m_updateAnimFrameRef = m_this.layer().map().scheduleAnimationFrame(m_this._update);
       return;
@@ -363,6 +366,10 @@ var gl_polygonFeature = function (arg) {
    * Destroy.
    */
   this._exit = function () {
+    if (m_updateAnimFrameRef && m_this.layer()) {
+      m_this.layer().map().scheduleAnimationFrame(m_this._update, 'remove');
+      m_updateAnimFrameRef = null;
+    }
     m_this.renderer().contextRenderer().removeActor(m_actor);
     s_exit();
   };

tests/cases/feature.js

@@ -363,5 +363,13 @@ describe('geo.feature', function () {
       expect(feat.selectionAPI(true, true)).toBe(feat);
       expect(feat.selectionAPI()).toBe(true);
     });
+    it('ready', function () {
+      feat = geo.feature({layer: layer});
+      expect(feat.ready).toBe(true);
+      feat._exit();
+      expect(feat.ready).toBe(false);
+      feat._init({});
+      expect(feat.ready).toBe(true);
+    });
   });
 });

tests/cases/polygonFeature.js

@@ -406,6 +406,7 @@ describe('geo.polygonFeature', function () {
       var buildTime = polygons.buildTime().getMTime();
       layer.deleteFeature(polygons);
       polygons.data(testPolygons);
+      polygons._update();
       map.draw();
       expect(buildTime).toEqual(polygons.buildTime().getMTime());
       destroyMap();