Bug 1160332 - Correctly check that the mouse is active in graphs to clean up rerender jank, a rebase error from 1157914. r=vp
authorJordan Santell <jsantell@gmail.com>
Thu, 30 Apr 2015 20:33:00 -0400
changeset 273370 14c8c3fd403cf01f23fef4cc255b22b06ae3155f
parent 273369 950723ac69a43385b993c997d5805528e2b5057e
child 273371 3e1cb78ad72beeab811453987537f1bf1e087da5
push id863
push userraliiev@mozilla.com
push dateMon, 03 Aug 2015 13:22:43 +0000
treeherdermozilla-release@f6321b14228d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersvp
bugs1160332, 1157914
milestone40.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1160332 - Correctly check that the mouse is active in graphs to clean up rerender jank, a rebase error from 1157914. r=vp
browser/devtools/performance/modules/graphs.js
browser/devtools/performance/test/browser.ini
browser/devtools/performance/test/browser_perf-details-06.js
browser/devtools/performance/views/overview.js
--- a/browser/devtools/performance/modules/graphs.js
+++ b/browser/devtools/performance/modules/graphs.js
@@ -261,30 +261,30 @@ GraphsController.prototype = {
     }
 
     // If there was rendering, wait until the most recent render cycle
     // has finished
     if (this._rendering) {
       yield this._rendering.promise;
     }
 
-    for (let graphName in this._graphs) {
-      yield this._graphs[graphName].destroy();
+    for (let graph of this.getWidgets()) {
+      yield graph.destroy();
     }
   }),
 
   /**
    * Applies the theme to the underlying graphs. Optionally takes
    * a `redraw` boolean in the options to force redraw.
    */
   setTheme: function (options={}) {
     let theme = options.theme || this._getTheme();
-    for (let graph in this._graphs) {
-      this._graphs[graph].setTheme(theme);
-      this._graphs[graph].refresh({ force: options.redraw });
+    for (let graph of this.getWidgets()) {
+      graph.setTheme(theme);
+      graph.refresh({ force: options.redraw });
     }
   },
 
   /**
    * Sets up the graph, if needed. Returns a promise resolving
    * to the graph if it is enabled once it's ready, or otherwise returns
    * null if disabled.
    */
@@ -344,16 +344,24 @@ GraphsController.prototype = {
     return this._getPrimaryLink().setMappedSelection(selection, { mapStart, mapEnd });
   },
 
   getMappedSelection: function ({ mapStart, mapEnd }) {
     return this._getPrimaryLink().getMappedSelection({ mapStart, mapEnd });
   },
 
   /**
+   * Returns an array of graphs that have been created, not necessarily
+   * enabled currently.
+   */
+  getWidgets: function () {
+    return Object.keys(this._graphs).map(name => this._graphs[name]);
+  },
+
+  /**
    * Drops the selection.
    */
   dropSelection: function () {
     if (this._getPrimaryLink()) {
       return this._getPrimaryLink().dropSelection();
     }
   },
 
--- a/browser/devtools/performance/test/browser.ini
+++ b/browser/devtools/performance/test/browser.ini
@@ -38,16 +38,17 @@ support-files =
 [browser_perf-details-memory-calltree-render.js]
 [browser_perf-details-memory-flamegraph-render.js]
 [browser_perf-details-waterfall-render.js]
 [browser_perf-details-01.js]
 [browser_perf-details-02.js]
 [browser_perf-details-03.js]
 [browser_perf-details-04.js]
 [browser_perf-details-05.js]
+[browser_perf-details-06.js]
 [browser_perf-events-calltree.js]
 [browser_perf-front-basic-profiler-01.js]
 [browser_perf-front-basic-timeline-01.js]
 #[browser_perf-front-profiler-01.js] bug 1077464
 [browser_perf-front-profiler-02.js]
 [browser_perf-front-profiler-03.js]
 [browser_perf-front-profiler-04.js]
 #[browser_perf-front-profiler-05.js] bug 1077464
new file mode 100644
--- /dev/null
+++ b/browser/devtools/performance/test/browser_perf-details-06.js
@@ -0,0 +1,61 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * Tests that the views with `shouldUpdateWhileMouseIsActive` works as intended.
+ */
+function spawnTest () {
+  let { panel } = yield initPerformance(SIMPLE_URL);
+  let { EVENTS, PerformanceController, OverviewView, DetailsView, WaterfallView, JsFlameGraphView } = panel.panelWin;
+
+  yield startRecording(panel);
+  yield stopRecording(panel);
+
+  // Set the debounce on WaterfallView and JsFlameGraphView to 0
+  WaterfallView.rangeChangeDebounceTime = 0;
+  JsFlameGraphView.rangeChangeDebounceTime = 0;
+
+  yield DetailsView.selectView("js-flamegraph");
+  let duration = PerformanceController.getCurrentRecording().getDuration();
+
+  // Fake an active mouse
+  Object.defineProperty(OverviewView, "isMouseActive", { value: true });
+
+  // Flame Graph should update on every selection, debounced, while mouse is down
+  let flamegraphRendered = once(JsFlameGraphView, EVENTS.JS_FLAMEGRAPH_RENDERED);
+  OverviewView.emit(EVENTS.OVERVIEW_RANGE_SELECTED, { startTime: 0, endTime: duration });
+  yield flamegraphRendered;
+  ok(true, "FlameGraph rerenders when mouse is active (1)");
+
+  flamegraphRendered = once(JsFlameGraphView, EVENTS.JS_FLAMEGRAPH_RENDERED);
+  OverviewView.emit(EVENTS.OVERVIEW_RANGE_SELECTED, { startTime: 0, endTime: duration });
+  yield flamegraphRendered;
+  ok(true, "FlameGraph rerenders when mouse is active (2)");
+
+  ok(OverviewView.isMouseActive, "Fake mouse is still active");
+
+  // Fake an inactive mouse for rerender
+  Object.defineProperty(OverviewView, "isMouseActive", { value: false });
+  yield DetailsView.selectView("waterfall");
+
+  // Fake an active mouse for rerender
+  Object.defineProperty(OverviewView, "isMouseActive", { value: true });
+
+  let oneSecondElapsed = false;
+  let waterfallRendered = false;
+
+  WaterfallView.on(EVENTS.WATERFALL_RENDERED, () => waterfallRendered = true);
+
+  // Keep firing range selection events for one second
+  idleWait(1000).then(() => oneSecondElapsed = true);
+  yield waitUntil(() => {
+    OverviewView.emit(EVENTS.OVERVIEW_RANGE_SELECTED, { startTime: 0, endTime: duration });
+    return oneSecondElapsed;
+  });
+
+  ok(OverviewView.isMouseActive, "Fake mouse is still active");
+  ok(!waterfallRendered, "the waterfall view should not have been rendered while mouse is active.");
+
+  yield teardown(panel);
+  finish();
+}
--- a/browser/devtools/performance/views/overview.js
+++ b/browser/devtools/performance/views/overview.js
@@ -92,19 +92,20 @@ let OverviewView = {
     yield this.graphs.destroy();
   }),
 
   /**
    * Returns true if any of the overview graphs have mouse dragging active,
    * false otherwise.
    */
   get isMouseActive() {
-    return (this.markersOverview && this.markersOverview.isMouseActive) ||
-           (this.memoryOverview && this.memoryOverview.isMouseActive) ||
-           (this.framerateGraph && this.framerateGraph.isMouseActive);
+    // Fetch all graphs currently stored in the GraphsController.
+    // These graphs are not necessarily active, but will not have
+    // an active mouse, in that case.
+    return !!this.graphs.getWidgets().some(e => e.isMouseActive);
   },
 
   /**
    * Disabled in the event we're using a Timeline mock, so we'll have no
    * timeline, ticks or memory data to show, so just block rendering and hide
    * the panel.
    */
   disable: function () {