Bug 1266456 - part3: HTMLTooltip show() clear timeout to avoid window leaks;r=jsnajdr draft
authorJulian Descottes <jdescottes@mozilla.com>
Fri, 24 Jun 2016 16:47:01 +0200
changeset 381150 c99aba735f0ed75fb79ff161afd1a848688a45b6
parent 380919 23e7db84e99d2be9471ed0db3dcb546ee1b162f6
child 381151 3740bcf6a94faae214336f5dc0cdfd1eea0f346e
push id21409
push userjdescottes@mozilla.com
push dateFri, 24 Jun 2016 14:54:55 +0000
reviewersjsnajdr
bugs1266456
milestone50.0a1
Bug 1266456 - part3: HTMLTooltip show() clear timeout to avoid window leaks;r=jsnajdr When calling HTMLTooltip::show() consecutively, we were leaking a window object. The timeout responsible for attaching the click event on window is now cleared before attaching a new one. MozReview-Commit-ID: 3ccIHM2QNlp
devtools/client/shared/test/browser.ini
devtools/client/shared/test/browser_html_tooltip_consecutive-show.js
devtools/client/shared/widgets/HTMLTooltip.js
--- a/devtools/client/shared/test/browser.ini
+++ b/devtools/client/shared/test/browser.ini
@@ -115,16 +115,17 @@ skip-if = e10s # Bug 1221911, bug 122228
 skip-if = e10s # Bug 1221911, bug 1222289, frequent e10s timeouts
 [browser_html_tooltip-01.js]
 [browser_html_tooltip-02.js]
 [browser_html_tooltip-03.js]
 [browser_html_tooltip-04.js]
 [browser_html_tooltip-05.js]
 [browser_html_tooltip_arrow-01.js]
 [browser_html_tooltip_arrow-02.js]
+[browser_html_tooltip_consecutive-show.js]
 [browser_html_tooltip_variable-height.js]
 [browser_inplace-editor-01.js]
 [browser_inplace-editor-02.js]
 [browser_inplace-editor_autocomplete_01.js]
 [browser_inplace-editor_autocomplete_02.js]
 [browser_inplace-editor_autocomplete_offset.js]
 [browser_inplace-editor_maxwidth.js]
 [browser_key_shortcuts.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/shared/test/browser_html_tooltip_consecutive-show.js
@@ -0,0 +1,70 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+/* import-globals-from helper_html_tooltip.js */
+
+"use strict";
+
+/**
+ * Test the HTMLTooltip show can be called several times. It should move according to the
+ * new anchor/options and should not leak event listeners.
+ */
+
+const HTML_NS = "http://www.w3.org/1999/xhtml";
+const TEST_URI = `data:text/xml;charset=UTF-8,<?xml version="1.0"?>
+  <?xml-stylesheet href="chrome://global/skin/global.css"?>
+  <?xml-stylesheet href="chrome://devtools/skin/tooltips.css"?>
+  <window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
+   title="Tooltip test">
+    <vbox flex="1">
+      <hbox id="box1" flex="1">test1</hbox>
+      <hbox id="box2" flex="1">test2</hbox>
+      <hbox id="box3" flex="1">test3</hbox>
+      <hbox id="box4" flex="1">test4</hbox>
+    </vbox>
+  </window>`;
+
+const {HTMLTooltip} = require("devtools/client/shared/widgets/HTMLTooltip");
+loadHelperScript("helper_html_tooltip.js");
+
+function getTooltipContent(doc) {
+  let div = doc.createElementNS(HTML_NS, "div");
+  div.style.height = "50px";
+  div.textContent = "tooltip";
+  return div;
+}
+
+add_task(function* () {
+  yield addTab("about:blank");
+  let [,, doc] = yield createHost("bottom", TEST_URI);
+
+  let box1 = doc.getElementById("box1");
+  let box2 = doc.getElementById("box2");
+  let box3 = doc.getElementById("box3");
+  let box4 = doc.getElementById("box4");
+
+  let width = 100, height = 50;
+
+  let tooltip = new HTMLTooltip({doc}, {});
+  tooltip.setContent(getTooltipContent(doc), {width, height});
+
+  info("Show the tooltip on each of the 4 hbox, without calling hide in between");
+
+  info("Show tooltip on box1");
+  tooltip.show(box1);
+  checkTooltipGeometry(tooltip, box1, {position: "bottom", width, height});
+
+  info("Show tooltip on box2");
+  tooltip.show(box2);
+  checkTooltipGeometry(tooltip, box2, {position: "bottom", width, height});
+
+  info("Show tooltip on box3");
+  tooltip.show(box3);
+  checkTooltipGeometry(tooltip, box3, {position: "top", width, height});
+
+  info("Show tooltip on box4");
+  tooltip.show(box4);
+  checkTooltipGeometry(tooltip, box4, {position: "top", width, height});
+
+  info("Hide tooltip before leaving test");
+  yield hideTooltip(tooltip);
+});
--- a/devtools/client/shared/widgets/HTMLTooltip.js
+++ b/devtools/client/shared/widgets/HTMLTooltip.js
@@ -153,16 +153,17 @@ HTMLTooltip.prototype = {
       this.arrow.style.left = computedPosition.arrowLeft + "px";
     }
 
     this.container.classList.add("tooltip-visible");
 
     // Keep a pointer on the focused element to refocus it when hiding the tooltip.
     this._focusedElement = this.doc.activeElement;
 
+    this.doc.defaultView.clearTimeout(this.attachEventsTimer);
     this.attachEventsTimer = this.doc.defaultView.setTimeout(() => {
       this._maybeFocusTooltip();
       this.topWindow.addEventListener("click", this._onClick, true);
       this.emit("shown");
     }, 0);
   },
 
   /**