Target and listener in the event listener cannot be null (#6561)

* Target and listener in the event listener cannot be null * fixes * fixes
ampproject · Dec 8, 2016 · 27d29b4 · 27d29b4
1 parent 958e3e7
commit 27d29b4
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 22 deletions.
diff --git a/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js b/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js
@@ -150,7 +150,7 @@ class AmpStickyAd extends AMP.BaseElement {
     if (this.ad_.isBuilt()) {
       this.layoutAd_();
     } else {
-      listenOnce(this.ad_, 'amp:built', () => {
+      listenOnce(dev().assertElement(this.ad_), 'amp:built', () => {
         this.layoutAd_();
       });
     }
@@ -163,7 +163,7 @@ class AmpStickyAd extends AMP.BaseElement {
   layoutAd_() {
     this.updateInViewport(dev().assertElement(this.ad_), true);
     this.scheduleLayout(dev().assertElement(this.ad_));
-    listenOnce(this.ad_, 'amp:load:end', () => {
+    listenOnce(dev().assertElement(this.ad_), 'amp:load:end', () => {
       this.vsync_.mutate(() => {
         // Set sticky-ad to visible and change container style
         this.element.setAttribute('visible', '');

diff --git a/extensions/amp-sticky-ad/1.0/amp-sticky-ad.js b/extensions/amp-sticky-ad/1.0/amp-sticky-ad.js
@@ -152,7 +152,7 @@ class AmpStickyAd extends AMP.BaseElement {
     if (this.ad_.isBuilt()) {
       this.layoutAd_();
     } else {
-      listenOnce(this.ad_, 'amp:built', () => {
+      listenOnce(dev().assertElement(this.ad_), 'amp:built', () => {
         this.layoutAd_();
       });
     }
@@ -165,7 +165,7 @@ class AmpStickyAd extends AMP.BaseElement {
   layoutAd_() {
     this.updateInViewport(dev().assertElement(this.ad_), true);
     this.scheduleLayout(dev().assertElement(this.ad_));
-    listenOnce(this.ad_, 'amp:load:end', () => {
+    listenOnce(dev().assertElement(this.ad_), 'amp:load:end', () => {
       this.vsync_.mutate(() => {
         // Set sticky-ad to visible and change container style
         this.element.setAttribute('visible', '');

diff --git a/src/event-helper.js b/src/event-helper.js
@@ -23,50 +23,54 @@ const LOAD_FAILURE_PREFIX = 'Failed to load:';
 
 /**
  * Listens for the specified event on the element.
- * @param {?EventTarget} element
+ * @param {!EventTarget} element
  * @param {string} eventType
- * @param {?function(Event)} listener
+ * @param {function(!Event)} listener
  * @param {boolean=} opt_capture
  * @return {!UnlistenDef}
  */
 export function listen(element, eventType, listener, opt_capture) {
+  let localElement = element;
+  let localListener = listener;
   const capture = opt_capture || false;
-  element.addEventListener(eventType, listener, capture);
+  localElement.addEventListener(eventType, localListener, capture);
   return () => {
-    if (element) {
-      element.removeEventListener(eventType, listener, capture);
+    if (localElement) {
+      localElement.removeEventListener(eventType, localListener, capture);
     }
-    listener = null;
-    element = null;
+    localListener = null;
+    localElement = null;
   };
 }
 
 
 /**
  * Listens for the specified event on the element and removes the listener
  * as soon as event has been received.
- * @param {?EventTarget} element
+ * @param {!EventTarget} element
  * @param {string} eventType
- * @param {?function(Event)} listener
+ * @param {function(!Event)} listener
  * @param {boolean=} opt_capture
  * @return {!UnlistenDef}
  */
 export function listenOnce(element, eventType, listener, opt_capture) {
+  let localElement = element;
+  let localListener = listener;
   const capture = opt_capture || false;
   let unlisten;
   let proxy = event => {
-    listener(event);
+    localListener(event);
     unlisten();
   };
   unlisten = () => {
-    if (element) {
-      element.removeEventListener(eventType, proxy, capture);
+    if (localElement) {
+      localElement.removeEventListener(eventType, proxy, capture);
     }
-    element = null;
+    localElement = null;
     proxy = null;
-    listener = null;
+    localListener = null;
   };
-  element.addEventListener(eventType, proxy, capture);
+  localElement.addEventListener(eventType, proxy, capture);
   return unlisten;
 }
 

diff --git a/src/input.js b/src/input.js
@@ -44,7 +44,7 @@ export class Input {
     /** @private {!Function} */
     this.boundOnMouseDown_ = this.onMouseDown_.bind(this);
 
-    /** @private {?Function} */
+    /** @private {?function(!Event)} */
     this.boundOnMouseMove_ = null;
 
     /** @private {?Function} */
@@ -85,7 +85,7 @@ export class Input {
     if (this.hasTouch_) {
       this.hasMouse_ = !this.hasTouch_;
       this.boundOnMouseMove_ =
-        /** @private {!Function} */ (this.onMouseMove_.bind(this));
+          /** @private {function(!Event)} */ (this.onMouseMove_.bind(this));
       listenOnce(win.document, 'mousemove', this.boundOnMouseMove_);
     }
   }
@@ -240,7 +240,8 @@ export class Input {
     // Repeat, if attempts allow.
     this.mouseConfirmAttemptCount_++;
     if (this.mouseConfirmAttemptCount_ <= MAX_MOUSE_CONFIRM_ATTEMPS_) {
-      listenOnce(this.win.document, 'mousemove', this.boundOnMouseMove_);
+      listenOnce(this.win.document, 'mousemove',
+          /** @type {function(!Event)} */ (this.boundOnMouseMove_));
     } else {
       dev().fine(TAG_, 'mouse detection failed');
     }