Backed out changeset 5b64b2e8e518 (bug 1300473) for failing devtools test browser_perf-recording-selected-03.js. r=backout
authorSebastian Hengst <archaeopteryx@coole-files.de>
Mon, 12 Sep 2016 20:03:26 +0200
changeset 313505 54cf088080cb11a9532f7b86ee1a77ed7e7bf89d
parent 313504 b36aa2b45076542947324127f604a1058cc66ecf
child 313506 b8cba97b26485efbeab6530cefa75bc0a3365c0f
push id20513
push userarchaeopteryx@coole-files.de
push dateMon, 12 Sep 2016 18:03:44 +0000
treeherderfx-team@54cf088080cb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1300473
milestone51.0a1
backs out5b64b2e8e518c80e39c53aa4872e80661bd8834d
Backed out changeset 5b64b2e8e518 (bug 1300473) for failing devtools test browser_perf-recording-selected-03.js. r=backout
devtools/client/performance/components/recording-button.js
devtools/client/performance/components/recording-controls.js
devtools/client/performance/test/browser_perf-button-states.js
devtools/client/themes/common.css
devtools/client/themes/performance.css
--- a/devtools/client/performance/components/recording-button.js
+++ b/devtools/client/performance/components/recording-button.js
@@ -8,30 +8,22 @@ const {DOM, createClass} = require("devt
 const {button} = DOM;
 
 module.exports = createClass({
   displayName: "Recording Button",
 
   render() {
     let {
       onRecordButtonClick,
-      isRecording,
-      isLocked
+      isRecording
     } = this.props;
 
-    let classList = ["devtools-button", "record-button"];
-
-    if (isRecording) {
-      classList.push("checked");
-    }
-
     return button(
       {
-        className: classList.join(" "),
+        className: "devtools-toolbarbutton record-button",
         onClick: onRecordButtonClick,
         "data-standalone": "true",
         "data-text-only": "true",
-        disabled: isLocked
       },
       isRecording ? L10N.getStr("recordings.stop") : L10N.getStr("recordings.start")
     );
   }
 });
--- a/devtools/client/performance/components/recording-controls.js
+++ b/devtools/client/performance/components/recording-controls.js
@@ -5,46 +5,62 @@
 
 const {L10N} = require("devtools/client/performance/modules/global");
 const {DOM, createClass} = require("devtools/client/shared/vendor/react");
 const {div, button} = DOM;
 
 module.exports = createClass({
   displayName: "Recording Controls",
 
+  /**
+   * Manually handle the "checked" and "locked" attributes, as the DOM element won't
+   * change by just by changing the checked attribute through React.
+   */
+  componentDidUpdate() {
+    if (this.props.isRecording) {
+      this._recordButton.setAttribute("checked", true);
+    } else {
+      this._recordButton.removeAttribute("checked");
+    }
+
+    if (this.props.isLocked) {
+      this._recordButton.setAttribute("locked", true);
+    } else {
+      this._recordButton.removeAttribute("locked");
+    }
+  },
+
   render() {
     let {
       onClearButtonClick,
       onRecordButtonClick,
       onImportButtonClick,
       isRecording,
       isLocked
     } = this.props;
 
-    let recordButtonClassList = ["devtools-button", "record-button"];
-
-    if (isRecording) {
-      recordButtonClassList.push("checked");
-    }
-
     return (
       div({ className: "devtools-toolbar" },
         div({ className: "toolbar-group" },
           button({
             id: "clear-button",
             className: "devtools-button",
             title: L10N.getStr("recordings.clear.tooltip"),
             onClick: onClearButtonClick
           }),
           button({
             id: "main-record-button",
-            className: recordButtonClassList.join(" "),
-            disabled: isLocked,
+            className: "devtools-button record-button",
             title: L10N.getStr("recordings.start.tooltip"),
-            onClick: onRecordButtonClick
+            onClick: onRecordButtonClick,
+            checked: isRecording,
+            ref: (el) => {
+              this._recordButton = el;
+            },
+            locked: isLocked
           }),
           button({
             id: "import-button",
             className: "devtools-button",
             title: L10N.getStr("recordings.import.tooltip"),
             onClick: onImportButtonClick
           })
         )
--- a/devtools/client/performance/test/browser_perf-button-states.js
+++ b/devtools/client/performance/test/browser_perf-button-states.js
@@ -11,40 +11,48 @@ const { initPerformanceInNewTab, teardow
 const { once } = require("devtools/client/performance/test/helpers/event-utils");
 
 add_task(function* () {
   let { panel } = yield initPerformanceInNewTab({
     url: SIMPLE_URL,
     win: window
   });
 
-  let { $, $$, EVENTS, PerformanceController, PerformanceView } = panel.panelWin;
-
+  let { $, EVENTS, PerformanceController, PerformanceView } = panel.panelWin;
   let recordButton = $("#main-record-button");
 
-  checkRecordButtonsStates(false, false);
+  ok(!recordButton.hasAttribute("checked"),
+    "The record button should not be checked yet.");
+  ok(!recordButton.hasAttribute("locked"),
+    "The record button should not be locked yet.");
 
   let uiStartClick = once(PerformanceView, EVENTS.UI_START_RECORDING);
   let recordingStarted = once(PerformanceController, EVENTS.RECORDING_STATE_CHANGE, {
     expectedArgs: { "1": "recording-started" }
   });
   let backendStartReady = once(PerformanceController,
                                EVENTS.BACKEND_READY_AFTER_RECORDING_START);
   let uiStateRecording = once(PerformanceView, EVENTS.UI_STATE_CHANGED, {
     expectedArgs: { "1": "recording" }
   });
 
   click(recordButton);
   yield uiStartClick;
 
-  checkRecordButtonsStates(true, true);
+  ok(recordButton.hasAttribute("checked"),
+    "The record button should now be checked.");
+  ok(recordButton.hasAttribute("locked"),
+    "The record button should be locked.");
 
   yield recordingStarted;
 
-  checkRecordButtonsStates(true, false);
+  ok(recordButton.hasAttribute("checked"),
+    "The record button should still be checked.");
+  ok(!recordButton.hasAttribute("locked"),
+    "The record button should not be locked.");
 
   yield backendStartReady;
   yield uiStateRecording;
 
   let uiStopClick = once(PerformanceView, EVENTS.UI_STOP_RECORDING);
   let recordingStopped = once(PerformanceController, EVENTS.RECORDING_STATE_CHANGE, {
     expectedArgs: { "1": "recording-stopped" }
   });
@@ -53,24 +61,18 @@ add_task(function* () {
   let uiStateRecorded = once(PerformanceView, EVENTS.UI_STATE_CHANGED, {
     expectedArgs: { "1": "recorded" }
   });
 
   click(recordButton);
   yield uiStopClick;
   yield recordingStopped;
 
-  checkRecordButtonsStates(false, false);
+  ok(!recordButton.hasAttribute("checked"),
+    "The record button should not be checked.");
+  ok(!recordButton.hasAttribute("locked"),
+    "The record button should not be locked.");
 
   yield backendStopReady;
   yield uiStateRecorded;
 
   yield teardownToolboxAndRemoveTab(panel);
-
-  function checkRecordButtonsStates(checked, locked) {
-    for (let button of $$(".record-button")) {
-      is(button.classList.contains("checked"), checked,
-         "The record button checked state should be " + checked);
-      is(button.disabled, locked,
-         "The record button locked state should be " + locked);
-    }
-  }
 });
--- a/devtools/client/themes/common.css
+++ b/devtools/client/themes/common.css
@@ -362,29 +362,27 @@ checkbox:-moz-focusring {
 
 /* Icon-only buttons */
 .devtools-button:empty::before,
 .devtools-toolbarbutton:not([label]):not([disabled]) > image {
   opacity: 0.8;
 }
 
 .devtools-button:hover:empty::before,
-.devtools-button.checked:empty::before,
 .devtools-button[checked]:empty::before,
 .devtools-button[open]:empty::before,
 .devtools-toolbarbutton:not([label]):hover > image,
 .devtools-toolbarbutton:not([label])[checked=true] > image,
 .devtools-toolbarbutton:not([label])[open=true] > image {
   opacity: 1;
 }
 
 .devtools-button:disabled,
 .devtools-button[disabled],
 .devtools-toolbarbutton[disabled] {
-  pointer-events: none;
   opacity: 0.5 !important;
 }
 
 .devtools-button[checked]:empty::before,
 .devtools-button[open]:empty::before,
 .devtools-button.checked::before,
 .devtools-toolbarbutton:not([label])[checked=true] > image,
 .devtools-toolbarbutton:not([label])[open=true] > image {
@@ -394,41 +392,39 @@ checkbox:-moz-focusring {
 /* Icon-and-text buttons */
 .devtools-toolbarbutton.icon-and-text .toolbarbutton-text {
   margin-inline-start: .5em !important;
   font-weight: 600;
 }
 
 /* Text-only buttons */
 .theme-light .devtools-toolbarbutton[label]:not([text-as-image]):not([type=menu-button]),
-.theme-light .devtools-toolbarbutton[data-text-only],
-.theme-light .devtools-button:not(:empty) {
+.theme-light .devtools-toolbarbutton[data-text-only] {
   background-color: var(--toolbar-tab-hover);
 }
 .theme-dark .devtools-toolbarbutton[label]:not([text-as-image]):not([type=menu-button]),
-.theme-dark .devtools-toolbarbutton[data-text-only],
-.theme-dark .devtools-button:not(:empty) {
+.theme-dark .devtools-toolbarbutton[data-text-only] {
   background-color: rgba(0, 0, 0, .2); /* Splitter */
 }
 
 /* Text-only button states */
-.theme-dark .devtools-button:not(:empty):not(:disabled):hover,
+.theme-dark .devtools-button:not(:empty):not([disabled]):hover,
 .theme-dark .devtools-toolbarbutton:not(:-moz-any([checked=true],[disabled],[text-as-image]))[label]:hover {
   background: rgba(0, 0, 0, .3); /* Splitters */
 }
-.theme-light .devtools-button:not(:empty):not(:disabled):hover,
+.theme-light .devtools-button:not(:empty):not([disabled]):hover,
 .theme-light .devtools-toolbarbutton:not(:-moz-any([checked=true],[disabled],[text-as-image]))[label]:hover {
   background: rgba(170, 170, 170, .3); /* Splitters */
 }
 
-.theme-dark .devtools-button:not(:empty):not(:disabled):hover:active,
+.theme-dark .devtools-button:not(:empty):not([disabled]):hover:active,
 .theme-dark .devtools-toolbarbutton:not(:-moz-any([checked=true],[disabled],[text-as-image]))[label]:hover:active {
   background: rgba(0, 0, 0, .4); /* Splitters */
 }
-.theme-light .devtools-button:not(:empty):not(:disabled):hover:active,
+.theme-light .devtools-button:not(:empty):not([disabled]):hover:active,
 .theme-light .devtools-toolbarbutton:not(:-moz-any([checked=true],[disabled],[text-as-image]))[label]:hover:active {
   background: var(--toolbar-tab-hover-active);
 }
 
 .theme-dark .devtools-toolbarbutton:not([disabled])[label][checked=true],
 .theme-dark .devtools-toolbarbutton:not([disabled])[label][open],
 .theme-dark .devtools-button:not(:empty)[checked=true] {
   background: var(--theme-selection-background-semitransparent);
--- a/devtools/client/themes/performance.css
+++ b/devtools/client/themes/performance.css
@@ -111,22 +111,27 @@
 #main-record-button .button-text, #import-button .button-text {
   display: none;
 }
 
 .notice-container .record-button {
   padding: 5px !important;
 }
 
-.notice-container .record-button.checked,
-.notice-container .record-button.checked {
+.notice-container .record-button[checked],
+.notice-container .record-button[checked] {
   color: var(--theme-selection-color) !important;
   background: var(--theme-selection-background) !important;
 }
 
+.record-button[locked] {
+  pointer-events: none;
+  opacity: 0.5;
+}
+
 /* Sidebar & recording items */
 
 #recordings-pane {
   border-inline-end: 1px solid var(--theme-splitter-color);
   width: var(--sidebar-width);
 }
 
 #recording-controls-mount {