From bf6ebe368ef6723842607030d463bbde6548d237 Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Thu, 22 Oct 2015 09:49:00 +0200 Subject: [PATCH 01/30] Bug 1003554 - make entry points correspond to entries in the line table; r=jimb,fitzgen --- devtools/server/actors/script.js | 11 ++ js/src/doc/Debugger/Debugger.Script.md | 3 + .../jit-test/tests/debug/Frame-onStep-11.js | 36 +++++ .../jit-test/tests/debug/Frame-onStep-12.js | 129 ++++++++++++++++++ .../debug/Script-getAllColumnOffsets-06.js | 3 +- .../tests/debug/Script-getOffsetLocation.js | 20 ++- js/src/vm/CommonPropertyNames.h | 1 + js/src/vm/Debugger.cpp | 44 +++++- 8 files changed, 238 insertions(+), 9 deletions(-) create mode 100644 js/src/jit-test/tests/debug/Frame-onStep-11.js create mode 100644 js/src/jit-test/tests/debug/Frame-onStep-12.js diff --git a/devtools/server/actors/script.js b/devtools/server/actors/script.js index 4bd8e0fc366..ba5e4d1f273 100644 --- a/devtools/server/actors/script.js +++ b/devtools/server/actors/script.js @@ -814,6 +814,17 @@ ThreadActor.prototype = { return function () { // onStep is called with 'this' set to the current frame. + // Only allow stepping stops at entry points for the line, when + // the stepping occurs in a single frame. The "same frame" + // check makes it so a sequence of steps can step out of a frame + // and into subsequent calls in the outer frame. E.g., if there + // is a call "a(b())" and the user steps into b, then this + // condition makes it possible to step out of b and into a. + if (this === startFrame && + !this.script.getOffsetLocation(this.offset).isEntryPoint) { + return undefined; + } + const generatedLocation = thread.sources.getFrameLocation(this); const newLocation = thread.unsafeSynchronize(thread.sources.getOriginalLocation( generatedLocation)); diff --git a/js/src/doc/Debugger/Debugger.Script.md b/js/src/doc/Debugger/Debugger.Script.md index 96649085345..d66b8adaef1 100644 --- a/js/src/doc/Debugger/Debugger.Script.md +++ b/js/src/doc/Debugger/Debugger.Script.md @@ -218,6 +218,9 @@ methods of other kinds of objects. * columnNumber: the column number for which offset is an entry point + * isEntryPoint: true if the offset is a column entry point, as + would be reported by getAllColumnOffsets(); otherwise false. + `getOffsetsCoverage()`: : Return `null` or an array which contains informations about the coverage of all opcodes. The elements of the array are objects, each of which describes diff --git a/js/src/jit-test/tests/debug/Frame-onStep-11.js b/js/src/jit-test/tests/debug/Frame-onStep-11.js new file mode 100644 index 00000000000..8b597cfe2bd --- /dev/null +++ b/js/src/jit-test/tests/debug/Frame-onStep-11.js @@ -0,0 +1,36 @@ +// Stepping out of a finally should not appear to +// step backward to some earlier statement. + +var g = newGlobal(); +g.eval(`function f() { + debugger; // +0 + var x = 0; // +1 + try { // +2 + x = 1; // +3 + throw 'something'; // +4 + } catch (e) { // +5 + x = 2; // +6 + } finally { // +7 + x = 3; // +8 + } // +9 + x = 4; // +10 + }`); // +11 + +var dbg = Debugger(g); + +let foundLines = ''; + +dbg.onDebuggerStatement = function(frame) { + let debugLine = frame.script.getOffsetLocation(frame.offset).lineNumber; + frame.onStep = function() { + // Only record a stop when the offset is an entry point. + let foundLine = this.script.getOffsetLocation(this.offset).lineNumber; + if (foundLine != debugLine && this.script.getLineOffsets(foundLine).indexOf(this.offset) >= 0) { + foundLines += "," + (foundLine - debugLine); + } + }; +}; + +g.f(); + +assertEq(foundLines, ",1,2,3,4,6,7,8,10"); diff --git a/js/src/jit-test/tests/debug/Frame-onStep-12.js b/js/src/jit-test/tests/debug/Frame-onStep-12.js new file mode 100644 index 00000000000..517ccbf1d97 --- /dev/null +++ b/js/src/jit-test/tests/debug/Frame-onStep-12.js @@ -0,0 +1,129 @@ +// Because our script source notes record only those bytecode offsets +// at which source positions change, the default behavior in the +// absence of a source note is to attribute a bytecode instruction to +// the same source location as the preceding instruction. When control +// flows from the preceding bytecode to the one we're emitting, that's +// usually plausible. But successors in the bytecode stream are not +// necessarily successors in the control flow graph. If the preceding +// bytecode was a back edge of a loop, or the jump at the end of a +// 'then' clause, its source position can be completely unrelated to +// that of its successor. + +// We try to avoid showing such nonsense offsets to the user by +// requiring breakpoints and single-stepping to stop only at a line's +// entry points, as reported by Debugger.Script.prototype.getLineOffsets; +// and then ensuring that those entry points are all offsets mentioned +// explicitly in the source notes, and hence deliberately attributed +// to the given bytecode. + +// This bit of JavaScript compiles to bytecode ending in a branch +// instruction whose source position is the body of an unreachable +// loop. The first instruction of the bytecode we emit following it +// will inherit this nonsense position, if we have not explicitly +// emitted a source note for said instruction. + +// This test steps across such code and verifies that control never +// appears to enter the unreachable loop. + +var bitOfCode = `debugger; // +0 + if(false) { // +1 + for(var b=0; b<0; b++) { // +2 + c = 2 // +3 + } // +4 + }`; // +5 + +var g = newGlobal(); +var dbg = Debugger(g); + +g.eval("function nothing() { }\n"); + +var log = ''; +dbg.onDebuggerStatement = function(frame) { + let debugLine = frame.script.getOffsetLocation(frame.offset).lineNumber; + frame.onStep = function() { + let foundLine = this.script.getOffsetLocation(this.offset).lineNumber; + if (this.script.getLineOffsets(foundLine).indexOf(this.offset) >= 0) { + log += (foundLine - debugLine).toString(16); + } + }; +}; + +function testOne(name, body, expected) { + print(name); + log = ''; + g.eval(`function ${name} () { ${body} }`); + g.eval(`${name}();`); + assertEq(log, expected); +} + + + +// Test the instructions at the end of a "try". +testOne("testTryFinally", + `try { + ${bitOfCode} + } finally { // +6 + } // +7 + nothing(); // +8 + `, "168"); + +// The same but without a finally clause. +testOne("testTryCatch", + `try { + ${bitOfCode} + } catch (e) { // +6 + } // +7 + nothing(); // +8 + `, "18"); + +// Test the instructions at the end of a "catch". +testOne("testCatchFinally", + `try { + throw new TypeError(); + } catch (e) { + ${bitOfCode} + } finally { // +6 + } // +7 + nothing(); // +8 + `, "168"); + +// The same but without a finally clause. This relies on a +// SpiderMonkey extension, because otherwise there's no way to see +// extra instructions at the end of a catch. +testOne("testCatch", + `try { + throw new TypeError(); + } catch (e if e instanceof TypeError) { + ${bitOfCode} + } catch (e) { // +6 + } // +7 + nothing(); // +8 + `, "18"); + +// Test the instruction at the end of a "finally" clause. +testOne("testFinally", + `try { + } finally { + ${bitOfCode} + } // +6 + nothing(); // +7 + `, "17"); + +// Test the instruction at the end of a "then" clause. +testOne("testThen", + `if (1 === 1) { + ${bitOfCode} + } else { // +6 + } // +7 + nothing(); // +8 + `, "18"); + +// Test the instructions leaving a switch block. +testOne("testSwitch", + `var x = 5; + switch (x) { + case 5: + ${bitOfCode} + } // +6 + nothing(); // +7 + `, "17"); diff --git a/js/src/jit-test/tests/debug/Script-getAllColumnOffsets-06.js b/js/src/jit-test/tests/debug/Script-getAllColumnOffsets-06.js index a5bb74c136f..fe9bdb62179 100644 --- a/js/src/jit-test/tests/debug/Script-getAllColumnOffsets-06.js +++ b/js/src/jit-test/tests/debug/Script-getAllColumnOffsets-06.js @@ -14,9 +14,10 @@ Debugger(global).onDebuggerStatement = function (frame) { }; global.log = ""; +global.eval("function ppppp() { return 1; }"); // 1 2 3 4 // 0123456789012345678901234567890123456789012345678 -global.eval("function f(){ 1 && print(print()) && new Error() } debugger;"); +global.eval("function f(){ 1 && ppppp(ppppp()) && new Error() } debugger;"); global.f(); // 14 - Enter the function body diff --git a/js/src/jit-test/tests/debug/Script-getOffsetLocation.js b/js/src/jit-test/tests/debug/Script-getOffsetLocation.js index cd714bebe5e..7cfc2e79064 100644 --- a/js/src/jit-test/tests/debug/Script-getOffsetLocation.js +++ b/js/src/jit-test/tests/debug/Script-getOffsetLocation.js @@ -2,18 +2,28 @@ var global = newGlobal(); Debugger(global).onDebuggerStatement = function (frame) { - var script = frame.eval("f").return.script; + var script = frame.script; + var byOffset = []; script.getAllColumnOffsets().forEach(function (entry) { var {lineNumber, columnNumber, offset} = entry; - var location = script.getOffsetLocation(offset); - assertEq(location.lineNumber, lineNumber); - assertEq(location.columnNumber, columnNumber); + byOffset[offset] = {lineNumber, columnNumber}; }); + + frame.onStep = function() { + var offset = frame.offset; + var location = script.getOffsetLocation(offset); + if (location.isEntryPoint) { + assertEq(location.lineNumber, byOffset[offset].lineNumber); + assertEq(location.columnNumber, byOffset[offset].columnNumber); + } else { + assertEq(byOffset[offset], undefined); + } + }; }; function test(body) { print("Test: " + body); - global.eval(`function f(n) { ${body} } debugger;`); + global.eval(`function f(n) { debugger; ${body} }`); global.f(3); } diff --git a/js/src/vm/CommonPropertyNames.h b/js/src/vm/CommonPropertyNames.h index f69fada2da3..9130068d4a7 100644 --- a/js/src/vm/CommonPropertyNames.h +++ b/js/src/vm/CommonPropertyNames.h @@ -124,6 +124,7 @@ macro(int8, int8, "int8") \ macro(int16, int16, "int16") \ macro(int32, int32, "int32") \ + macro(isEntryPoint, isEntryPoint, "isEntryPoint") \ macro(isExtensible, isExtensible, "isExtensible") \ macro(iteratorIntrinsic, iteratorIntrinsic, "__iterator__") \ macro(join, join, "join") \ diff --git a/js/src/vm/Debugger.cpp b/js/src/vm/Debugger.cpp index d54612913f5..5fc9d9e55c8 100644 --- a/js/src/vm/Debugger.cpp +++ b/js/src/vm/Debugger.cpp @@ -4856,53 +4856,71 @@ class BytecodeRangeWithPosition : private BytecodeRange BytecodeRangeWithPosition(JSContext* cx, JSScript* script) : BytecodeRange(cx, script), lineno(script->lineno()), column(0), - sn(script->notes()), snpc(script->code()) + sn(script->notes()), snpc(script->code()), isEntryPoint(false) { if (!SN_IS_TERMINATOR(sn)) snpc += SN_DELTA(sn); updatePosition(); while (frontPC() != script->main()) popFront(); + isEntryPoint = true; } void popFront() { BytecodeRange::popFront(); - if (!empty()) + if (empty()) + isEntryPoint = false; + else updatePosition(); } size_t frontLineNumber() const { return lineno; } size_t frontColumnNumber() const { return column; } + // Entry points are restricted to bytecode offsets that have an + // explicit mention in the line table. This restriction avoids a + // number of failing cases caused by some instructions not having + // sensible (to the user) line numbers, and it is one way to + // implement the idea that the bytecode emitter should tell the + // debugger exactly which offsets represent "interesting" (to the + // user) places to stop. + bool frontIsEntryPoint() const { return isEntryPoint; } + private: void updatePosition() { /* * Determine the current line number by reading all source notes up to * and including the current offset. */ + jsbytecode *lastLinePC = nullptr; while (!SN_IS_TERMINATOR(sn) && snpc <= frontPC()) { SrcNoteType type = (SrcNoteType) SN_TYPE(sn); if (type == SRC_COLSPAN) { ptrdiff_t colspan = SN_OFFSET_TO_COLSPAN(GetSrcNoteOffset(sn, 0)); MOZ_ASSERT(ptrdiff_t(column) + colspan >= 0); column += colspan; + lastLinePC = snpc; } else if (type == SRC_SETLINE) { lineno = size_t(GetSrcNoteOffset(sn, 0)); column = 0; + lastLinePC = snpc; } else if (type == SRC_NEWLINE) { lineno++; column = 0; + lastLinePC = snpc; } sn = SN_NEXT(sn); snpc += SN_DELTA(sn); } + isEntryPoint = lastLinePC == frontPC(); } size_t lineno; size_t column; jssrcnote* sn; jsbytecode* snpc; + bool isEntryPoint; }; /* @@ -5083,6 +5101,10 @@ DebuggerScript_getOffsetLocation(JSContext* cx, unsigned argc, Value* vp) if (!ScriptOffset(cx, script, args[0], &offset)) return false; + FlowGraphSummary flowData(cx); + if (!flowData.populate(cx, script)) + return false; + RootedPlainObject result(cx, NewBuiltinClassInstance(cx)); if (!result) return false; @@ -5100,6 +5122,15 @@ DebuggerScript_getOffsetLocation(JSContext* cx, unsigned argc, Value* vp) if (!DefineProperty(cx, result, cx->names().columnNumber, value)) return false; + // The same entry point test that is used by getAllColumnOffsets. + bool isEntryPoint = (r.frontIsEntryPoint() && + !flowData[offset].hasNoEdges() && + (flowData[offset].lineno() != r.frontLineNumber() || + flowData[offset].column() != r.frontColumnNumber())); + value.setBoolean(isEntryPoint); + if (!DefineProperty(cx, result, cx->names().isEntryPoint, value)) + return false; + args.rval().setObject(*result); return true; } @@ -5122,6 +5153,9 @@ DebuggerScript_getAllOffsets(JSContext* cx, unsigned argc, Value* vp) if (!result) return false; for (BytecodeRangeWithPosition r(cx, script); !r.empty(); r.popFront()) { + if (!r.frontIsEntryPoint()) + continue; + size_t offset = r.frontOffset(); size_t lineno = r.frontLineNumber(); @@ -5195,7 +5229,8 @@ DebuggerScript_getAllColumnOffsets(JSContext* cx, unsigned argc, Value* vp) size_t offset = r.frontOffset(); /* Make a note, if the current instruction is an entry point for the current position. */ - if (!flowData[offset].hasNoEdges() && + if (r.frontIsEntryPoint() && + !flowData[offset].hasNoEdges() && (flowData[offset].lineno() != lineno || flowData[offset].column() != column)) { RootedPlainObject entry(cx, NewBuiltinClassInstance(cx)); @@ -5259,6 +5294,9 @@ DebuggerScript_getLineOffsets(JSContext* cx, unsigned argc, Value* vp) if (!result) return false; for (BytecodeRangeWithPosition r(cx, script); !r.empty(); r.popFront()) { + if (!r.frontIsEntryPoint()) + continue; + size_t offset = r.frontOffset(); /* If the op at offset is an entry point, append offset to result. */ From ef69458153fa667fe8f7cd7d983c73215f2040e6 Mon Sep 17 00:00:00 2001 From: Patrick Brosset Date: Tue, 27 Oct 2015 10:55:00 +0100 Subject: [PATCH 02/30] Bug 1202179 - html/head/body not dragdrop-able and drag starts after move only; r=bgrins --- devtools/client/markupview/markup-view.css | 12 +- devtools/client/markupview/markup-view.js | 397 +++++++++--------- devtools/client/markupview/test/browser.ini | 3 +- .../browser_markupview_dragdrop_autoscroll.js | 86 ++-- .../browser_markupview_dragdrop_distance.js | 49 +++ ...rowser_markupview_dragdrop_dragRootNode.js | 24 +- ...wser_markupview_dragdrop_escapeKeyPress.js | 33 +- ...rowser_markupview_dragdrop_invalidNodes.js | 35 +- .../browser_markupview_dragdrop_isDragging.js | 65 --- .../browser_markupview_dragdrop_reorder.js | 137 +++--- ...owser_markupview_dragdrop_textSelection.js | 48 --- devtools/client/markupview/test/head.js | 70 +++ devtools/client/styleinspector/test/head.js | 4 +- 13 files changed, 450 insertions(+), 513 deletions(-) create mode 100644 devtools/client/markupview/test/browser_markupview_dragdrop_distance.js delete mode 100644 devtools/client/markupview/test/browser_markupview_dragdrop_isDragging.js delete mode 100644 devtools/client/markupview/test/browser_markupview_dragdrop_textSelection.js diff --git a/devtools/client/markupview/markup-view.css b/devtools/client/markupview/markup-view.css index 161eaaa839a..2c7e0b083eb 100644 --- a/devtools/client/markupview/markup-view.css +++ b/devtools/client/markupview/markup-view.css @@ -77,26 +77,30 @@ body.dragging .tag-line { position: relative; pointer-events: none; opacity: 0.7; + z-index: 1; height: 0; } /* Indicates a tag-line in the markup-view as being an active drop target by * drawing a horizontal line where the dragged element would be inserted if * dropped here */ -.tag-line.drop-target::before, .tag-line.drag-target::before { +.tag-line.drop-target::before, +.tag-line.drag-target::before { content: ''; position: absolute; - left: 0; top: 0; width: 100%; + /* Offset these by 1000px to make sure they cover the full width of the view */ + padding-left: 1000px; + left: -1000px; } .tag-line.drag-target::before { - border-top: 2px dashed var(--theme-contrast-background); + border-top: 2px solid var(--theme-content-color2); } .tag-line.drop-target::before { - border-top: 2px dashed var(--theme-content-color1); + border-top: 2px solid var(--theme-contrast-background); } /* In case the indicator is put on the closing .tag-line, the indentation level diff --git a/devtools/client/markupview/markup-view.js b/devtools/client/markupview/markup-view.js index 80b90fdf9cb..2e2855e8bf2 100644 --- a/devtools/client/markupview/markup-view.js +++ b/devtools/client/markupview/markup-view.js @@ -3,6 +3,7 @@ /* This Source Code Form is subject to the terms of the Mozilla Public * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +"use strict"; const {Cc, Cu, Ci} = require("chrome"); @@ -16,11 +17,11 @@ const NEW_SELECTION_HIGHLIGHTER_TIMER = 1000; const DRAG_DROP_AUTOSCROLL_EDGE_DISTANCE = 50; const DRAG_DROP_MIN_AUTOSCROLL_SPEED = 5; const DRAG_DROP_MAX_AUTOSCROLL_SPEED = 15; +const DRAG_DROP_MIN_INITIAL_DISTANCE = 10; const AUTOCOMPLETE_POPUP_PANEL_ID = "markupview_autoCompletePopup"; const {UndoStack} = require("devtools/client/shared/undo"); const {editableField, InplaceEditor} = require("devtools/client/shared/inplace-editor"); -const {gDevTools} = Cu.import("resource://devtools/client/framework/gDevTools.jsm", {}); const {HTMLEditor} = require("devtools/client/markupview/html-editor"); const promise = require("promise"); const {Tooltip} = require("devtools/client/shared/widgets/Tooltip"); @@ -37,7 +38,7 @@ Cu.import("resource://gre/modules/Services.jsm"); Cu.import("resource://gre/modules/XPCOMUtils.jsm"); loader.lazyGetter(this, "DOMParser", function() { - return Cc["@mozilla.org/xmlextras/domparser;1"].createInstance(Ci.nsIDOMParser); + return Cc["@mozilla.org/xmlextras/domparser;1"].createInstance(Ci.nsIDOMParser); }); loader.lazyGetter(this, "AutocompletePopup", () => { return require("devtools/client/shared/autocomplete-popup").AutocompletePopup; @@ -75,7 +76,7 @@ function MarkupView(aInspector, aFrame, aControllerWindow) { try { this.maxChildren = Services.prefs.getIntPref("devtools.markup.pagesize"); - } catch(ex) { + } catch (ex) { this.maxChildren = DEFAULT_MAX_CHILDREN; } @@ -94,34 +95,34 @@ function MarkupView(aInspector, aFrame, aControllerWindow) { this._containers = new Map(); - this._boundMutationObserver = this._mutationObserver.bind(this); - this.walker.on("mutations", this._boundMutationObserver); - - this._boundOnDisplayChange = this._onDisplayChange.bind(this); - this.walker.on("display-change", this._boundOnDisplayChange); - + // Binding functions that need to be called in scope. + this._mutationObserver = this._mutationObserver.bind(this); + this._onDisplayChange = this._onDisplayChange.bind(this); this._onMouseClick = this._onMouseClick.bind(this); - this._onMouseUp = this._onMouseUp.bind(this); - this.doc.body.addEventListener("mouseup", this._onMouseUp); - - this._boundOnNewSelection = this._onNewSelection.bind(this); - this._inspector.selection.on("new-node-front", this._boundOnNewSelection); - this._onNewSelection(); - - this._boundKeyDown = this._onKeyDown.bind(this); - this._frame.contentWindow.addEventListener("keydown", this._boundKeyDown, false); - + this._onNewSelection = this._onNewSelection.bind(this); + this._onKeyDown = this._onKeyDown.bind(this); this._onCopy = this._onCopy.bind(this); - this._frame.contentWindow.addEventListener("copy", this._onCopy); + this._onFocus = this._onFocus.bind(this); + this._onMouseMove = this._onMouseMove.bind(this); + this._onMouseLeave = this._onMouseLeave.bind(this); + this._onToolboxPickerHover = this._onToolboxPickerHover.bind(this); - this._boundFocus = this._onFocus.bind(this); - this._frame.addEventListener("focus", this._boundFocus, false); - - this._makeTooltipPersistent = this._makeTooltipPersistent.bind(this); + // Listening to various events. + this._elt.addEventListener("click", this._onMouseClick, false); + this._elt.addEventListener("mousemove", this._onMouseMove, false); + this._elt.addEventListener("mouseleave", this._onMouseLeave, false); + this.doc.body.addEventListener("mouseup", this._onMouseUp); + this.win.addEventListener("keydown", this._onKeyDown, false); + this.win.addEventListener("copy", this._onCopy); + this._frame.addEventListener("focus", this._onFocus, false); + this.walker.on("mutations", this._mutationObserver); + this.walker.on("display-change", this._onDisplayChange); + this._inspector.selection.on("new-node-front", this._onNewSelection); + this._inspector.toolbox.on("picker-node-hovered", this._onToolboxPickerHover); + this._onNewSelection(); this._initTooltips(); - this._initHighlighter(); EventEmitter.decorate(this); } @@ -133,36 +134,12 @@ MarkupView.prototype = { * How long does a node flash when it mutates (in ms). */ CONTAINER_FLASHING_DURATION: 500, - /** - * How long do you have to hold the mouse down before a drag - * starts (in ms). - */ - GRAB_DELAY: 400, _selectedContainer: null, _initTooltips: function() { this.tooltip = new Tooltip(this._inspector.panelDoc); this._makeTooltipPersistent(false); - - this._elt.addEventListener("click", this._onMouseClick, false); - }, - - _initHighlighter: function() { - // Show the box model on markup-view mousemove - this._onMouseMove = this._onMouseMove.bind(this); - this._elt.addEventListener("mousemove", this._onMouseMove, false); - this._onMouseLeave = this._onMouseLeave.bind(this); - this._elt.addEventListener("mouseleave", this._onMouseLeave, false); - - // Show markup-containers as hovered on toolbox "picker-node-hovered" event - // which happens when the "pick" button is pressed - this._onToolboxPickerHover = (event, nodeFront) => { - this.showNode(nodeFront).then(() => { - this._showContainerAsHovered(nodeFront); - }); - }; - this._inspector.toolbox.on("picker-node-hovered", this._onToolboxPickerHover); }, _makeTooltipPersistent: function(state) { @@ -174,52 +151,26 @@ MarkupView.prototype = { } }, + _onToolboxPickerHover: function(event, nodeFront) { + this.showNode(nodeFront).then(() => { + this._showContainerAsHovered(nodeFront); + }, e => console.error(e)); + }, + isDragging: false, _onMouseMove: function(event) { - if (this.isDragging) { - event.preventDefault(); - this._dragStartEl = event.target; - - let docEl = this.doc.documentElement; - - if (this._scrollInterval) { - clearInterval(this._scrollInterval); - } - - // Auto-scroll when the mouse approaches top/bottom edge - let distanceFromBottom = docEl.clientHeight - event.pageY + this.win.scrollY, - distanceFromTop = event.pageY - this.win.scrollY; - - if (distanceFromBottom <= DRAG_DROP_AUTOSCROLL_EDGE_DISTANCE) { - // Map our distance from 0-50 to 5-15 range so the speed is kept - // in a range not too fast, not too slow - let speed = map(distanceFromBottom, 0, DRAG_DROP_AUTOSCROLL_EDGE_DISTANCE, - DRAG_DROP_MIN_AUTOSCROLL_SPEED, DRAG_DROP_MAX_AUTOSCROLL_SPEED); - // Here, we use minus because the value of speed - 15 is always negative - // and it makes the speed relative to the distance between mouse and edge - // the closer to the edge, the faster - this._scrollInterval = setInterval(() => { - docEl.scrollTop -= speed - DRAG_DROP_MAX_AUTOSCROLL_SPEED; - }, 0); - } - - if (distanceFromTop <= DRAG_DROP_AUTOSCROLL_EDGE_DISTANCE) { - // refer to bottom edge's comments for more info - let speed = map(distanceFromTop, 0, DRAG_DROP_AUTOSCROLL_EDGE_DISTANCE, - DRAG_DROP_MIN_AUTOSCROLL_SPEED, DRAG_DROP_MAX_AUTOSCROLL_SPEED); - - this._scrollInterval = setInterval(() => { - docEl.scrollTop += speed - DRAG_DROP_MAX_AUTOSCROLL_SPEED; - }, 0); - } - - return; - }; - let target = event.target; - // Search target for a markupContainer reference, if not found, walk up + // Auto-scroll if we're dragging. + if (this.isDragging) { + event.preventDefault(); + this._autoScroll(event); + return; + } + + // Show the current container as hovered and highlight it. + // This requires finding the current MarkupContainer (walking up the DOM). while (!target.container) { if (target.tagName.toLowerCase() === "body") { return; @@ -238,6 +189,47 @@ MarkupView.prototype = { this._showContainerAsHovered(container.node); }, + /** + * Executed on each mouse-move while a node is being dragged in the view. + * Auto-scrolls the view to reveal nodes below the fold to drop the dragged + * node in. + */ + _autoScroll: function(event) { + let docEl = this.doc.documentElement; + + if (this._autoScrollInterval) { + clearInterval(this._autoScrollInterval); + } + + // Auto-scroll when the mouse approaches top/bottom edge. + let fromBottom = docEl.clientHeight - event.pageY + this.win.scrollY; + let fromTop = event.pageY - this.win.scrollY; + + if (fromBottom <= DRAG_DROP_AUTOSCROLL_EDGE_DISTANCE) { + // Map our distance from 0-50 to 5-15 range so the speed is kept in a + // range not too fast, not too slow. + let speed = map( + fromBottom, + 0, DRAG_DROP_AUTOSCROLL_EDGE_DISTANCE, + DRAG_DROP_MIN_AUTOSCROLL_SPEED, DRAG_DROP_MAX_AUTOSCROLL_SPEED); + + this._autoScrollInterval = setInterval(() => { + docEl.scrollTop -= speed - DRAG_DROP_MAX_AUTOSCROLL_SPEED; + }, 0); + } + + if (fromTop <= DRAG_DROP_AUTOSCROLL_EDGE_DISTANCE) { + let speed = map( + fromTop, + 0, DRAG_DROP_AUTOSCROLL_EDGE_DISTANCE, + DRAG_DROP_MIN_AUTOSCROLL_SPEED, DRAG_DROP_MAX_AUTOSCROLL_SPEED); + + this._autoScrollInterval = setInterval(() => { + docEl.scrollTop += speed - DRAG_DROP_MAX_AUTOSCROLL_SPEED; + }, 0); + } + }, + _onMouseClick: function(event) { // From the target passed here, let's find the parent MarkupContainer // and ask it if the tooltip should be shown @@ -261,8 +253,8 @@ MarkupView.prototype = { _onMouseUp: function() { this.indicateDropTarget(null); this.indicateDragTarget(null); - if (this._scrollInterval) { - clearInterval(this._scrollInterval); + if (this._autoScrollInterval) { + clearInterval(this._autoScrollInterval); } }, @@ -280,13 +272,11 @@ MarkupView.prototype = { this.indicateDropTarget(null); this.indicateDragTarget(null); - if (this._scrollInterval) { - clearInterval(this._scrollInterval); + if (this._autoScrollInterval) { + clearInterval(this._autoScrollInterval); } }, - - _hoveredNode: null, /** @@ -307,10 +297,12 @@ MarkupView.prototype = { }, _onMouseLeave: function() { - if (this._scrollInterval) { - clearInterval(this._scrollInterval); + if (this._autoScrollInterval) { + clearInterval(this._autoScrollInterval); + } + if (this.isDragging) { + return; } - if (this.isDragging) return; this._hideBoxModel(true); if (this._hoveredNode) { @@ -457,7 +449,6 @@ MarkupView.prototype = { */ _onNewSelection: function() { let selection = this._inspector.selection; - let reason = selection.reason; this.htmlEditor.hide(); if (this._hoveredNode && this._hoveredNode !== selection.nodeFront) { @@ -1237,7 +1228,7 @@ MarkupView.prototype = { this.htmlEditor.once("popuphidden", (e, aCommit, aValue) => { // Need to focus the element instead of the frame / window // in order to give keyboard focus back to doc (from editor). - this._frame.contentDocument.documentElement.focus(); + this.doc.documentElement.focus(); if (aCommit) { this.updateNodeOuterHTML(aNode, aValue, oldValue); @@ -1527,10 +1518,7 @@ MarkupView.prototype = { this._clearBriefBoxModelTimer(); - this._elt.removeEventListener("click", this._onMouseClick, false); - this._hoveredNode = null; - this._inspector.toolbox.off("picker-node-hovered", this._onToolboxPickerHover); this.htmlEditor.destroy(); this.htmlEditor = null; @@ -1541,46 +1529,21 @@ MarkupView.prototype = { this.popup.destroy(); this.popup = null; - this._frame.removeEventListener("focus", this._boundFocus, false); - this._boundFocus = null; - - if (this._boundUpdatePreview) { - this._frame.contentWindow.removeEventListener("scroll", - this._boundUpdatePreview, true); - this._boundUpdatePreview = null; - } - - if (this._boundResizePreview) { - this._frame.contentWindow.removeEventListener("resize", - this._boundResizePreview, true); - this._frame.contentWindow.removeEventListener("overflow", - this._boundResizePreview, true); - this._frame.contentWindow.removeEventListener("underflow", - this._boundResizePreview, true); - this._boundResizePreview = null; - } - - this._frame.contentWindow.removeEventListener("keydown", - this._boundKeyDown, false); - this._boundKeyDown = null; - - this._frame.contentWindow.removeEventListener("copy", this._onCopy); - this._onCopy = null; - - this._inspector.selection.off("new-node-front", this._boundOnNewSelection); - this._boundOnNewSelection = null; - - this.walker.off("mutations", this._boundMutationObserver); - this._boundMutationObserver = null; - - this.walker.off("display-change", this._boundOnDisplayChange); - this._boundOnDisplayChange = null; - + this._elt.removeEventListener("click", this._onMouseClick, false); this._elt.removeEventListener("mousemove", this._onMouseMove, false); this._elt.removeEventListener("mouseleave", this._onMouseLeave, false); + this.doc.body.removeEventListener("mouseup", this._onMouseUp); + this.win.removeEventListener("keydown", this._onKeyDown, false); + this.win.removeEventListener("copy", this._onCopy); + this._frame.removeEventListener("focus", this._onFocus, false); + this.walker.off("mutations", this._mutationObserver); + this.walker.off("display-change", this._onDisplayChange); + this._inspector.selection.off("new-node-front", this._onNewSelection); + this._inspector.toolbox.off("picker-node-hovered", this._onToolboxPickerHover); + this._elt = null; - for (let [key, container] of this._containers) { + for (let [, container] of this._containers) { container.destroy(); } this._containers = null; @@ -1597,6 +1560,18 @@ MarkupView.prototype = { return this._destroyer; }, + /** + * Find the closest element with class tag-line. These are used to indicate + * drag and drop targets. + * @param {DOMNode} el + * @return {DOMNode} + */ + findClosestDragDropTarget: function(el) { + return el.classList.contains("tag-line") + ? el + : el.querySelector(".tag-line") || el.closest(".tag-line"); + }, + /** * Takes an element as it's only argument and marks the element * as the drop target @@ -1606,14 +1581,15 @@ MarkupView.prototype = { this._lastDropTarget.classList.remove("drop-target"); } - if (!el) return; + if (!el) { + return; + } - let target = el.classList.contains("tag-line") ? - el : el.querySelector(".tag-line") || el.closest(".tag-line"); - if (!target) return; - - target.classList.add("drop-target"); - this._lastDropTarget = target; + let target = this.findClosestDragDropTarget(el); + if (target) { + target.classList.add("drop-target"); + this._lastDropTarget = target; + } }, /** @@ -1624,19 +1600,20 @@ MarkupView.prototype = { this._lastDragTarget.classList.remove("drag-target"); } - if (!el) return; + if (!el) { + return; + } - let target = el.classList.contains("tag-line") ? - el : el.querySelector(".tag-line") || el.closest(".tag-line"); - - if (!target) return; - - target.classList.add("drag-target"); - this._lastDragTarget = target; + let target = this.findClosestDragDropTarget(el); + if (target) { + target.classList.add("drag-target"); + this._lastDragTarget = target; + } }, /** - * Used to get the nodes required to modify the markup after dragging the element (parent/nextSibling) + * Used to get the nodes required to modify the markup after dragging the + * element (parent/nextSibling). */ get dropTargetNodes() { let target = this._lastDropTarget; @@ -1686,7 +1663,6 @@ MarkupView.prototype = { function MarkupContainer() { } MarkupContainer.prototype = { - /* * Initialize the MarkupContainer. Should be called while one * of the other contain classes is instantiated. @@ -1743,9 +1719,9 @@ MarkupContainer.prototype = { let isCanvas = tagName === "canvas"; return isImage || isCanvas; - } else { - return false; } + + return false; }, /** @@ -1868,7 +1844,6 @@ MarkupContainer.prototype = { return this.elt.parentNode ? this.elt.parentNode.container : null; }, - _isMouseDown: false, _isDragging: false, _dragStartY: 0, @@ -1892,25 +1867,30 @@ MarkupContainer.prototype = { /** * Check if element is draggable */ - isDraggable: function(target) { - return this._isMouseDown && - this.markup._dragStartEl === target && - !this.node.isPseudoElement && + isDraggable: function() { + let tagName = this.node.tagName.toLowerCase(); + + return !this.node.isPseudoElement && !this.node.isAnonymous && + !this.node.isDocumentElement && + tagName !== "body" && + tagName !== "head" && this.win.getSelection().isCollapsed && this.node.parentNode().tagName !== null; }, _onMouseDown: function(event) { - let target = event.target; + let {target, button, metaKey, ctrlKey} = event; + let isLeftClick = button === 0; + let isMiddleClick = button === 1; + let isMetaClick = isLeftClick && (metaKey || ctrlKey); - // The "show more nodes" button (already has its onclick). + // The "show more nodes" button already has its onclick, so early return. if (target.nodeName === "button") { return; } // target is the MarkupContainer itself. - this._isMouseDown = true; this.hovered = false; this.markup.navigate(this); event.stopPropagation(); @@ -1924,9 +1904,7 @@ MarkupContainer.prototype = { event.preventDefault(); } - let isMiddleClick = event.button === 1; - let isMetaClick = event.button === 0 && (event.metaKey || event.ctrlKey); - + // Follow attribute links if middle or meta click. if (isMiddleClick || isMetaClick) { let link = target.dataset.link; let type = target.dataset.type; @@ -1934,62 +1912,61 @@ MarkupContainer.prototype = { return; } - // Start dragging the container after a delay. - this.markup._dragStartEl = target; - setTimeout(() => { - // Make sure the mouse is still down and on target. - if (!this.isDraggable(target)) { - return; - } - this.isDragging = true; - + // Start node drag & drop (if the mouse moved, see _onMouseMove). + if (isLeftClick && this.isDraggable()) { + this._isPreDragging = true; this._dragStartY = event.pageY; - this.markup.indicateDropTarget(this.elt); - - // If this is the last child, use the closing of parent as indicator - this.markup.indicateDragTarget(this.elt.nextElementSibling || - this.markup.getContainer(this.node.parentNode()).closeTagLine); - }, this.markup.GRAB_DELAY); + } }, /** * On mouse up, stop dragging. */ _onMouseUp: Task.async(function*() { - this._isMouseDown = false; + this._isPreDragging = false; - if (!this.isDragging) { - return; + if (this.isDragging) { + this.cancelDragging(); + + let dropTargetNodes = this.markup.dropTargetNodes; + + if (!dropTargetNodes) { + return; + } + + yield this.markup.walker.insertBefore(this.node, dropTargetNodes.parent, + dropTargetNodes.nextSibling); + this.markup.emit("drop-completed"); } - - this.cancelDragging(); - - let dropTargetNodes = this.markup.dropTargetNodes; - - if (!dropTargetNodes) { - return; - } - - yield this.markup.walker.insertBefore(this.node, dropTargetNodes.parent, - dropTargetNodes.nextSibling); - this.markup.emit("drop-completed"); }), /** - * On mouse move, move the dragged element if any and indicate the drop target. + * On mouse move, move the dragged element and indicate the drop target. */ _onMouseMove: function(event) { - if (!this.isDragging) { - return; + // If this is the first move after mousedown, only start dragging after the + // mouse has travelled a few pixels and then indicate the start position. + let initialDiff = Math.abs(event.pageY - this._dragStartY); + if (this._isPreDragging && initialDiff >= DRAG_DROP_MIN_INITIAL_DISTANCE) { + this._isPreDragging = false; + this.isDragging = true; + + // If this is the last child, use the closing of parent as + // indicator. + let position = this.elt.nextElementSibling || + this.markup.getContainer(this.node.parentNode()) + .closeTagLine; + this.markup.indicateDragTarget(position); } - let diff = event.pageY - this._dragStartY; - this.elt.style.top = diff + "px"; + if (this.isDragging) { + let diff = event.pageY - this._dragStartY; + this.elt.style.top = diff + "px"; - let el = this.markup.doc.elementFromPoint(event.pageX - this.win.scrollX, - event.pageY - this.win.scrollY); - - this.markup.indicateDropTarget(el); + let el = this.markup.doc.elementFromPoint(event.pageX - this.win.scrollX, + event.pageY - this.win.scrollY); + this.markup.indicateDropTarget(el); + } }, cancelDragging: function() { @@ -1997,7 +1974,7 @@ MarkupContainer.prototype = { return; } - this._isMouseDown = false; + this._isPreDragging = false; this.isDragging = false; this.elt.style.removeProperty("top"); }, diff --git a/devtools/client/markupview/test/browser.ini b/devtools/client/markupview/test/browser.ini index 740522a1a0d..4bde459bf21 100644 --- a/devtools/client/markupview/test/browser.ini +++ b/devtools/client/markupview/test/browser.ini @@ -46,12 +46,11 @@ skip-if = e10s # scratchpad.xul is not loading in e10s window [browser_markupview_copy_image_data.js] [browser_markupview_css_completion_style_attribute.js] [browser_markupview_dragdrop_autoscroll.js] +[browser_markupview_dragdrop_distance.js] [browser_markupview_dragdrop_dragRootNode.js] [browser_markupview_dragdrop_escapeKeyPress.js] [browser_markupview_dragdrop_invalidNodes.js] -[browser_markupview_dragdrop_isDragging.js] [browser_markupview_dragdrop_reorder.js] -[browser_markupview_dragdrop_textSelection.js] [browser_markupview_events.js] skip-if = e10s # Bug 1040751 - CodeMirror editor.destroy() isn't e10s compatible [browser_markupview_events_form.js] diff --git a/devtools/client/markupview/test/browser_markupview_dragdrop_autoscroll.js b/devtools/client/markupview/test/browser_markupview_dragdrop_autoscroll.js index 21d7a428805..0799223b1fe 100644 --- a/devtools/client/markupview/test/browser_markupview_dragdrop_autoscroll.js +++ b/devtools/client/markupview/test/browser_markupview_dragdrop_autoscroll.js @@ -4,62 +4,64 @@ "use strict"; -// Test: Dragging nodes near top/bottom edges of inspector -// should auto-scroll +// Test that dragging a node near the top or bottom edge of the markup-view +// auto-scrolls the view. const TEST_URL = TEST_URL_ROOT + "doc_markup_dragdrop_autoscroll.html"; -const GRAB_DELAY = 400; add_task(function*() { let {inspector} = yield addTab(TEST_URL).then(openInspector); - let markup = inspector.markup; + let viewHeight = markup.doc.documentElement.clientHeight; - let container = yield getContainerForSelector("#first", inspector); - let rect = container.elt.getBoundingClientRect(); + info("Pretend the markup-view is dragging"); + markup.isDragging = true; - info("Simulating mouseDown on #first"); - container._onMouseDown({ - target: container.tagLine, - pageX: 10, - pageY: rect.top, - stopPropagation: function() {}, - preventDefault: function() {} + info("Simulate a mousemove on the view, at the bottom, and expect scrolling"); + let onScrolled = waitForViewScroll(markup); + + markup._onMouseMove({ + preventDefault: () => {}, + target: markup.doc.body, + pageY: viewHeight }); - yield wait(GRAB_DELAY + 1); + let bottomScrollPos = yield onScrolled; + ok(bottomScrollPos > 0, "The view was scrolled down"); - let clientHeight = markup.doc.documentElement.clientHeight; - info("Simulating mouseMove on #first with pageY: " + clientHeight); + info("Simulate a mousemove at the top and expect more scrolling"); + onScrolled = waitForViewScroll(markup); - let ev = { - target: container.tagLine, - pageX: 10, - pageY: clientHeight, - preventDefault: function() {} - }; + markup._onMouseMove({ + preventDefault: () => {}, + target: markup.doc.body, + pageY: 0 + }); - info("Listening on scroll event"); - let scroll = onScroll(markup.win); + let topScrollPos = yield onScrolled; + ok(topScrollPos < bottomScrollPos, "The view was scrolled up"); + is(topScrollPos, 0, "The view was scrolled up to the top"); - markup._onMouseMove(ev); - - yield scroll; - - let dropCompleted = once(markup, "drop-completed"); - - container._onMouseUp(ev); - markup._onMouseUp(ev); - - yield dropCompleted; - - ok("Scroll event fired"); + info("Simulate a mouseup to stop dragging"); + markup._onMouseUp(); }); -function onScroll(win) { - return new Promise((resolve, reject) => { - win.onscroll = function(e) { - resolve(e); - } +function waitForViewScroll(markup) { + let el = markup.doc.documentElement; + let startPos = el.scrollTop; + + return new Promise(resolve => { + let isDone = () => { + if (el.scrollTop === startPos) { + resolve(el.scrollTop); + } else { + startPos = el.scrollTop; + // Continue checking every 50ms. + setTimeout(isDone, 50); + } + }; + + // Start checking if the view scrolled after a while. + setTimeout(isDone, 50); }); -}; +} diff --git a/devtools/client/markupview/test/browser_markupview_dragdrop_distance.js b/devtools/client/markupview/test/browser_markupview_dragdrop_distance.js new file mode 100644 index 00000000000..9d1b989b68a --- /dev/null +++ b/devtools/client/markupview/test/browser_markupview_dragdrop_distance.js @@ -0,0 +1,49 @@ +/* vim: set ts=2 et sw=2 tw=80: */ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +// Test that nodes don't start dragging before the mouse has moved by at least +// the minimum vertical distance defined in markup-view.js by +// DRAG_DROP_MIN_INITIAL_DISTANCE. + +const TEST_URL = TEST_URL_ROOT + "doc_markup_dragdrop.html"; +const TEST_NODE = "#test"; + +// Keep this in sync with DRAG_DROP_MIN_INITIAL_DISTANCE in markup-view.js +const MIN_DISTANCE = 10; + +add_task(function*() { + let {inspector} = yield addTab(TEST_URL).then(openInspector); + + info("Drag the test node by half of the minimum distance"); + yield simulateNodeDrag(inspector, TEST_NODE, 0, MIN_DISTANCE / 2); + yield checkIsDragging(inspector, TEST_NODE, false); + + info("Drag the test node by exactly the minimum distance"); + yield simulateNodeDrag(inspector, TEST_NODE, 0, MIN_DISTANCE); + yield checkIsDragging(inspector, TEST_NODE, true); + inspector.markup.cancelDragging(); + + info("Drag the test node by more than the minimum distance"); + yield simulateNodeDrag(inspector, TEST_NODE, 0, MIN_DISTANCE * 2); + yield checkIsDragging(inspector, TEST_NODE, true); + inspector.markup.cancelDragging(); + + info("Drag the test node by minus the minimum distance"); + yield simulateNodeDrag(inspector, TEST_NODE, 0, MIN_DISTANCE * -1); + yield checkIsDragging(inspector, TEST_NODE, true); + inspector.markup.cancelDragging(); +}); + +function* checkIsDragging(inspector, selector, isDragging) { + let container = yield getContainerForSelector(selector, inspector); + if (isDragging) { + ok(container.isDragging, "The container is being dragged"); + ok(inspector.markup.isDragging, "And the markup-view knows it"); + } else { + ok(!container.isDragging, "The container hasn't been marked as dragging"); + ok(!inspector.markup.isDragging, "And the markup-view either"); + } +} diff --git a/devtools/client/markupview/test/browser_markupview_dragdrop_dragRootNode.js b/devtools/client/markupview/test/browser_markupview_dragdrop_dragRootNode.js index 8a758df449d..ac5f0853ab8 100644 --- a/devtools/client/markupview/test/browser_markupview_dragdrop_dragRootNode.js +++ b/devtools/client/markupview/test/browser_markupview_dragdrop_dragRootNode.js @@ -4,27 +4,19 @@ "use strict"; -// Test if html root node is draggable +// Test that the root node isn't draggable (as well as head and body). const TEST_URL = TEST_URL_ROOT + "doc_markup_dragdrop.html"; -const GRAB_DELAY = 400; +const TEST_DATA = ["html", "head", "body"]; add_task(function*() { let {inspector} = yield addTab(TEST_URL).then(openInspector); - let el = yield getContainerForSelector("html", inspector); - let rect = el.tagLine.getBoundingClientRect(); + for (let selector of TEST_DATA) { + info("Try to drag/drop node " + selector); + yield simulateNodeDrag(inspector, selector); - info("Simulating mouseDown on html root node"); - el._onMouseDown({ - target: el.tagLine, - pageX: rect.x, - pageY: rect.y, - stopPropagation: function() {}, - preventDefault: function() {} - }); - - info("Waiting for a little bit more than the markup-view grab delay"); - yield wait(GRAB_DELAY + 1); - is(el.isDragging, false, "isDragging is false"); + let container = yield getContainerForSelector(selector, inspector); + ok(!container.isDragging, "The container hasn't been marked as dragging"); + } }); diff --git a/devtools/client/markupview/test/browser_markupview_dragdrop_escapeKeyPress.js b/devtools/client/markupview/test/browser_markupview_dragdrop_escapeKeyPress.js index 0d9d31c5ccc..2e886b89bbc 100644 --- a/devtools/client/markupview/test/browser_markupview_dragdrop_escapeKeyPress.js +++ b/devtools/client/markupview/test/browser_markupview_dragdrop_escapeKeyPress.js @@ -4,31 +4,30 @@ "use strict"; -// Test whether ESCAPE keypress cancels dragging of an element +// Test whether ESCAPE keypress cancels dragging of an element. const TEST_URL = TEST_URL_ROOT + "doc_markup_dragdrop.html"; -const GRAB_DELAY = 400; add_task(function*() { let {inspector} = yield addTab(TEST_URL).then(openInspector); + let {markup} = inspector; - let el = yield getContainerForSelector("#test", inspector); - let rect = el.tagLine.getBoundingClientRect(); + info("Get a test container"); + let container = yield getContainerForSelector("#test", inspector); - info("Simulating mouseDown on #test"); - el._onMouseDown({ - target: el.tagLine, - pageX: rect.x, - pageY: rect.y, - stopPropagation: function() {}, - preventDefault: function() {} - }); + info("Simulate a drag/drop on this container"); + yield simulateNodeDrag(inspector, "#test"); - info("Waiting for a little bit more than the markup-view grab delay"); - yield wait(GRAB_DELAY + 1); - ok(el.isDragging, "isDragging true after mouseDown"); + ok(container.isDragging && markup.isDragging, + "The container is being dragged"); + ok(markup.doc.body.classList.contains("dragging"), + "The dragging css class was added"); - info("Simulating ESCAPE keypress"); + info("Simulate ESCAPE keypress"); EventUtils.sendKey("escape", inspector.panelWin); - is(el.isDragging, false, "isDragging false after ESCAPE keypress"); + + ok(!container.isDragging && !markup.isDragging, + "The dragging has stopped"); + ok(!markup.doc.body.classList.contains("dragging"), + "The dragging css class was removed"); }); diff --git a/devtools/client/markupview/test/browser_markupview_dragdrop_invalidNodes.js b/devtools/client/markupview/test/browser_markupview_dragdrop_invalidNodes.js index cd863342323..d273ee73481 100644 --- a/devtools/client/markupview/test/browser_markupview_dragdrop_invalidNodes.js +++ b/devtools/client/markupview/test/browser_markupview_dragdrop_invalidNodes.js @@ -4,55 +4,42 @@ "use strict"; -// Test: pseudo-elements and anonymous nodes should not be draggable +// Check that pseudo-elements and anonymous nodes are not draggable. const TEST_URL = TEST_URL_ROOT + "doc_markup_dragdrop.html"; -const GRAB_DELAY = 400; add_task(function*() { Services.prefs.setBoolPref("devtools.inspector.showAllAnonymousContent", true); let {inspector} = yield addTab(TEST_URL).then(openInspector); - info("Expanding #test"); + info("Expanding nodes below #test"); let parentFront = yield getNodeFront("#test", inspector); yield inspector.markup.expandNode(parentFront); yield waitForMultipleChildrenUpdates(inspector); + info("Getting the ::before pseudo element"); let parentContainer = yield getContainerForNodeFront(parentFront, inspector); let beforePseudo = parentContainer.elt.children[1].firstChild.container; - parentContainer.elt.scrollIntoView(true); - info("Simulating mouseDown on #test::before"); - beforePseudo._onMouseDown({ - target: beforePseudo.tagLine, - stopPropagation: function() {}, - preventDefault: function() {} - }); + info("Simulate dragging the ::before pseudo element"); + yield simulateNodeDrag(inspector, beforePseudo); - info("Waiting " + (GRAB_DELAY + 1) + "ms") - yield wait(GRAB_DELAY + 1); - is(beforePseudo.isDragging, false, "[pseudo-element] isDragging is false after GRAB_DELAY has passed"); + ok(!beforePseudo.isDragging, "::before pseudo element isn't dragging"); + info("Expanding nodes below #anonymousParent"); let inputFront = yield getNodeFront("#anonymousParent", inspector); - yield inspector.markup.expandNode(inputFront); yield waitForMultipleChildrenUpdates(inspector); + info("Getting the anonymous node"); let inputContainer = yield getContainerForNodeFront(inputFront, inspector); let anonymousDiv = inputContainer.elt.children[1].firstChild.container; - inputContainer.elt.scrollIntoView(true); - info("Simulating mouseDown on input#anonymousParent div"); - anonymousDiv._onMouseDown({ - target: anonymousDiv.tagLine, - stopPropagation: function() {}, - preventDefault: function() {} - }); + info("Simulate dragging the anonymous node"); + yield simulateNodeDrag(inspector, anonymousDiv); - info("Waiting " + (GRAB_DELAY + 1) + "ms") - yield wait(GRAB_DELAY + 1); - is(anonymousDiv.isDragging, false, "[anonymous element] isDragging is false after GRAB_DELAY has passed"); + ok(!anonymousDiv.isDragging, "anonymous node isn't dragging"); }); diff --git a/devtools/client/markupview/test/browser_markupview_dragdrop_isDragging.js b/devtools/client/markupview/test/browser_markupview_dragdrop_isDragging.js deleted file mode 100644 index c2d5be84bee..00000000000 --- a/devtools/client/markupview/test/browser_markupview_dragdrop_isDragging.js +++ /dev/null @@ -1,65 +0,0 @@ -/* vim: set ts=2 et sw=2 tw=80: */ -/* Any copyright is dedicated to the Public Domain. - http://creativecommons.org/publicdomain/zero/1.0/ */ - -"use strict"; - -// Test drag mode's delay, it shouldn't enable dragging before -// GRAB_DELAY = 400 has passed - -const TEST_URL = TEST_URL_ROOT + "doc_markup_dragdrop.html"; -const GRAB_DELAY = 400; - -add_task(function*() { - let {inspector} = yield addTab(TEST_URL).then(openInspector); - - let el = yield getContainerForSelector("#test", inspector); - let rect = el.tagLine.getBoundingClientRect(); - - info("Simulating mouseDown on #test"); - el._onMouseDown({ - target: el.tagLine, - pageX: rect.x, - pageY: rect.y, - stopPropagation: function() {}, - preventDefault: function() {} - }); - - ok(!el.isDragging, "isDragging should not be set to true immediately"); - - info("Waiting for 10ms"); - yield wait(10); - ok(!el.isDragging, "isDragging should not be set to true after a brief wait"); - - info("Waiting " + (GRAB_DELAY + 1) + "ms"); - yield wait(GRAB_DELAY + 1); - ok(el.isDragging, "isDragging true after GRAB_DELAY has passed"); - - let dropCompleted = once(inspector.markup, "drop-completed"); - - info("Simulating mouseUp on #test"); - el._onMouseUp({ - target: el.tagLine, - pageX: rect.x, - pageY: rect.y - }); - - yield dropCompleted; - - ok(!el.isDragging, "isDragging false after mouseUp"); - - info("Simulating middle click on #test"); - el._onMouseDown({ - target: el.tagLine, - button: 1, - pageX: rect.x, - pageY: rect.y, - stopPropagation: function() {}, - preventDefault: function() {} - }); - ok(!el.isDragging, "isDragging should not be set to true immediately"); - - info("Waiting " + (GRAB_DELAY + 1) + "ms"); - yield wait(GRAB_DELAY + 1); - ok(!el.isDragging, "isDragging never starts after middle click after mouseUp"); -}); diff --git a/devtools/client/markupview/test/browser_markupview_dragdrop_reorder.js b/devtools/client/markupview/test/browser_markupview_dragdrop_reorder.js index d36a46b1731..8fafcc7ef8d 100644 --- a/devtools/client/markupview/test/browser_markupview_dragdrop_reorder.js +++ b/devtools/client/markupview/test/browser_markupview_dragdrop_reorder.js @@ -4,135 +4,104 @@ "use strict"; -// Test different kinds of drag and drop node re-ordering +// Test different kinds of drag and drop node re-ordering. const TEST_URL = TEST_URL_ROOT + "doc_markup_dragdrop.html"; -const GRAB_DELAY = 5; add_task(function*() { let {inspector} = yield addTab(TEST_URL).then(openInspector); - inspector.markup.GRAB_DELAY = GRAB_DELAY; + let ids; - info("Expanding #test"); + info("Expand #test node"); let parentFront = yield getNodeFront("#test", inspector); - let parent = yield getNode("#test"); - let parentContainer = yield getContainerForNodeFront(parentFront, inspector); - yield inspector.markup.expandNode(parentFront); yield waitForMultipleChildrenUpdates(inspector); + info("Scroll #test into view"); + let parentContainer = yield getContainerForNodeFront(parentFront, inspector); parentContainer.elt.scrollIntoView(true); - info("Testing putting an element back in it's original place"); + info("Test putting an element back at its original place"); yield dragElementToOriginalLocation("#firstChild", inspector); - is(parent.children[0].id, "firstChild", "#firstChild is still the first child of #test"); - is(parent.children[1].id, "middleChild", "#middleChild is still the second child of #test"); + ids = yield getChildrenIDsOf(parentFront, inspector); + is(ids[0], "firstChild", + "#firstChild is still the first child of #test"); + is(ids[1], "middleChild", + "#middleChild is still the second child of #test"); info("Testing switching elements inside their parent"); yield moveElementDown("#firstChild", "#middleChild", inspector); - - is(parent.children[0].id, "middleChild", "#firstChild is now the second child of #test"); - is(parent.children[1].id, "firstChild", "#middleChild is now the first child of #test"); + ids = yield getChildrenIDsOf(parentFront, inspector); + is(ids[0], "middleChild", + "#firstChild is now the second child of #test"); + is(ids[1], "firstChild", + "#middleChild is now the first child of #test"); info("Testing switching elements with a last child"); yield moveElementDown("#firstChild", "#lastChild", inspector); - - is(parent.children[1].id, "lastChild", "#lastChild is now the second child of #test"); - is(parent.children[2].id, "firstChild", "#firstChild is now the last child of #test"); + ids = yield getChildrenIDsOf(parentFront, inspector); + is(ids[1], "lastChild", + "#lastChild is now the second child of #test"); + is(ids[2], "firstChild", + "#firstChild is now the last child of #test"); info("Testing appending element to a parent"); yield moveElementDown("#before", "#test", inspector); - - is(parent.children.length, 4, "New element appended to #test"); - is(parent.children[0].id, "before", "New element is appended at the right place (currently first child)"); + ids = yield getChildrenIDsOf(parentFront, inspector); + is(ids.length, 4, + "New element appended to #test"); + is(ids[0], "before", + "New element is appended at the right place (currently first child)"); info("Testing moving element to after it's parent"); yield moveElementDown("#firstChild", "#test", inspector); - - is(parent.children.length, 3, "#firstChild is no longer #test's child"); - is(parent.nextElementSibling.id, "firstChild", "#firstChild is now #test's nextElementSibling"); + ids = yield getChildrenIDsOf(parentFront, inspector); + is(ids.length, 3, + "#firstChild is no longer #test's child"); + let siblingFront = yield inspector.walker.nextSibling(parentFront); + is(siblingFront.id, "firstChild", + "#firstChild is now #test's nextElementSibling"); }); -function* dragContainer(selector, targetOffset, inspector) { - info("Dragging the markup-container for node " + selector); - - let container = yield getContainerForSelector(selector, inspector); - - let updated = inspector.once("inspector-updated"); - - let rect = { - x: container.tagLine.offsetLeft, - y: container.tagLine.offsetTop - }; - - info("Simulating mouseDown on " + selector); - container._onMouseDown({ - target: container.tagLine, - pageX: rect.x, - pageY: rect.y, - stopPropagation: function() {}, - preventDefault: function() {} - }); - - let targetX = rect.x + targetOffset.x, - targetY = rect.y + targetOffset.y; - - setTimeout(() => { - info("Simulating mouseMove on " + selector + - " with pageX: " + targetX + " pageY: " + targetY); - container._onMouseMove({ - target: container.tagLine, - pageX: targetX, - pageY: targetY - }); - - info("Simulating mouseUp on " + selector + - " with pageX: " + targetX + " pageY: " + targetY); - container._onMouseUp({ - target: container.tagLine, - pageX: targetX, - pageY: targetY - }); - - container.markup._onMouseUp(); - }, GRAB_DELAY+1); - - return updated; -}; - function* dragElementToOriginalLocation(selector, inspector) { - let el = yield getContainerForSelector(selector, inspector); - let height = el.tagLine.getBoundingClientRect().height; - info("Picking up and putting back down " + selector); function onMutation() { ok(false, "Mutation received from dragging a node back to its location"); } inspector.on("markupmutation", onMutation); - yield dragContainer(selector, {x: 0, y: 0}, inspector); + yield simulateNodeDragAndDrop(inspector, selector, 0, 0); // Wait a bit to make sure the event never fires. // This doesn't need to catch *all* cases, since the mutation // will cause failure later in the test when it checks element ordering. - yield new Promise(resolve => { - setTimeout(resolve, 500); - }); + yield wait(500); inspector.off("markupmutation", onMutation); } function* moveElementDown(selector, next, inspector) { + info("Switching " + selector + " with " + next); + + let container = yield getContainerForSelector(next, inspector); + let height = container.tagLine.getBoundingClientRect().height; + let onMutated = inspector.once("markupmutation"); let uiUpdate = inspector.once("inspector-updated"); - let el = yield getContainerForSelector(next, inspector); - let height = el.tagLine.getBoundingClientRect().height; - - info("Switching " + selector + ' with ' + next); - - yield dragContainer(selector, {x: 0, y: Math.round(height) + 2}, inspector); + yield simulateNodeDragAndDrop(inspector, selector, 0, Math.round(height) + 2); let mutations = yield onMutated; - is(mutations.length, 2, "2 mutations"); yield uiUpdate; -}; \ No newline at end of file + + is(mutations.length, 2, "2 mutations were received"); +} + +function* getChildrenIDsOf(parentFront, {walker}) { + let {nodes} = yield walker.children(parentFront); + // Filter out non-element nodes since children also returns pseudo-elements. + return nodes.filter(node => { + return !node.isPseudoElement; + }).map(node => { + return node.id; + }); +} diff --git a/devtools/client/markupview/test/browser_markupview_dragdrop_textSelection.js b/devtools/client/markupview/test/browser_markupview_dragdrop_textSelection.js deleted file mode 100644 index 4c4221b4039..00000000000 --- a/devtools/client/markupview/test/browser_markupview_dragdrop_textSelection.js +++ /dev/null @@ -1,48 +0,0 @@ -/* vim: set ts=2 et sw=2 tw=80: */ -/* Any copyright is dedicated to the Public Domain. - http://creativecommons.org/publicdomain/zero/1.0/ */ - -"use strict"; - -// Test: Nodes should not be draggable if there is a text selected -// (trying to move selected text around shouldn't trigger node drag and drop) - -const TEST_URL = TEST_URL_ROOT + "doc_markup_dragdrop.html"; -const GRAB_DELAY = 400; - -add_task(function*() { - let {inspector} = yield addTab(TEST_URL).then(openInspector); - let markup = inspector.markup; - - info("Expanding span#before"); - let spanFront = yield getNodeFront("#before", inspector); - let spanContainer = yield getContainerForNodeFront(spanFront, inspector); - let span = yield getNode("#before"); - - yield inspector.markup.expandNode(spanFront); - yield waitForMultipleChildrenUpdates(inspector); - - spanContainer.elt.scrollIntoView(true); - - info("Selecting #before's text content"); - - let textContent = spanContainer.elt.children[1].firstChild.container; - - let selectRange = markup.doc.createRange(); - selectRange.selectNode(textContent.editor.elt.querySelector('[tabindex]')); - markup.doc.getSelection().addRange(selectRange); - - info("Simulating mouseDown on #before"); - - spanContainer._onMouseDown({ - pageX: 0, - pageY: 0, - target: spanContainer.tagLine, - stopPropagation: function() {}, - preventDefault: function() {} - }); - - yield wait(GRAB_DELAY + 1); - - is(spanContainer.isDragging, false, "isDragging should be false if there is a text selected"); -}); \ No newline at end of file diff --git a/devtools/client/markupview/test/head.js b/devtools/client/markupview/test/head.js index 13bb8a78261..d01f95f6e8e 100644 --- a/devtools/client/markupview/test/head.js +++ b/devtools/client/markupview/test/head.js @@ -776,3 +776,73 @@ function unregisterActor(registrar, front) { return registrar.unregister(); }); } + +/** + * Simulate dragging a MarkupContainer by calling its mousedown and mousemove + * handlers. + * @param {InspectorPanel} inspector The current inspector-panel instance. + * @param {String|MarkupContainer} selector The selector to identify the node or + * the MarkupContainer for this node. + * @param {Number} xOffset Optional x offset to drag by. + * @param {Number} yOffset Optional y offset to drag by. + */ +function* simulateNodeDrag(inspector, selector, xOffset = 10, yOffset = 10) { + let container = typeof selector === "string" + ? yield getContainerForSelector(selector, inspector) + : selector; + let rect = container.tagLine.getBoundingClientRect(); + let scrollX = inspector.markup.doc.documentElement.scrollLeft; + let scrollY = inspector.markup.doc.documentElement.scrollTop; + + info("Simulate mouseDown on element " + selector); + container._onMouseDown({ + target: container.tagLine, + button: 0, + pageX: scrollX + rect.x, + pageY: scrollY + rect.y, + stopPropagation: () => {}, + preventDefault: () => {} + }); + + // _onMouseDown selects the node, so make sure to wait for the + // inspector-updated event if the current selection was different. + if (inspector.selection.nodeFront !== container.node) { + yield inspector.once("inspector-updated"); + } + + info("Simulate mouseMove on element " + selector); + container._onMouseMove({ + pageX: scrollX + rect.x + xOffset, + pageY: scrollY + rect.y + yOffset + }); +} + +/** + * Simulate dropping a MarkupContainer by calling its mouseup handler. This is + * meant to be called after simulateNodeDrag has been called. + * @param {InspectorPanel} inspector The current inspector-panel instance. + * @param {String|MarkupContainer} selector The selector to identify the node or + * the MarkupContainer for this node. + */ +function* simulateNodeDrop(inspector, selector) { + info("Simulate mouseUp on element " + selector); + let container = typeof selector === "string" + ? yield getContainerForSelector(selector, inspector) + : selector; + container._onMouseUp(); + inspector.markup._onMouseUp(); +} + +/** + * Simulate drag'n'dropping a MarkupContainer by calling its mousedown, + * mousemove and mouseup handlers. + * @param {InspectorPanel} inspector The current inspector-panel instance. + * @param {String|MarkupContainer} selector The selector to identify the node or + * the MarkupContainer for this node. + * @param {Number} xOffset Optional x offset to drag by. + * @param {Number} yOffset Optional y offset to drag by. + */ +function* simulateNodeDragAndDrop(inspector, selector, xOffset, yOffset) { + yield simulateNodeDrag(inspector, selector, xOffset, yOffset); + yield simulateNodeDrop(inspector, selector); +} diff --git a/devtools/client/styleinspector/test/head.js b/devtools/client/styleinspector/test/head.js index 00f46087e73..9331a2e9d3c 100644 --- a/devtools/client/styleinspector/test/head.js +++ b/devtools/client/styleinspector/test/head.js @@ -1158,5 +1158,7 @@ function waitForStyleEditor(toolbox, href) { function reloadPage(inspector) { let onNewRoot = inspector.once("new-root"); content.location.reload(); - return onNewRoot.then(inspector.markup._waitForChildren); + return onNewRoot.then(() => { + inspector.markup._waitForChildren(); + }); } From ab9bbe63f92214519e37b5b77895b93feac07bc2 Mon Sep 17 00:00:00 2001 From: Jordan Santell Date: Wed, 28 Oct 2015 02:11:06 -0700 Subject: [PATCH 03/30] Bug 1218679 - Add integration/smoke-screen tests for memory tool. r=fitzgen --- devtools/client/memory/constants.js | 3 + devtools/client/memory/initializer.js | 18 ++++-- .../client/memory/test/browser/browser.ini | 2 + .../browser/browser_memory-breakdowns-01.js | 33 ++++++++++ .../test/browser/browser_memory-simple-01.js | 35 +++++++++++ .../test/browser/doc_steady_allocation.html | 16 +++++ devtools/client/memory/test/browser/head.js | 60 +++++++++++++++++++ 7 files changed, 162 insertions(+), 5 deletions(-) create mode 100644 devtools/client/memory/test/browser/browser_memory-breakdowns-01.js create mode 100644 devtools/client/memory/test/browser/browser_memory-simple-01.js create mode 100644 devtools/client/memory/test/browser/doc_steady_allocation.html diff --git a/devtools/client/memory/constants.js b/devtools/client/memory/constants.js index e5ae8b86a16..06b7b9dc280 100644 --- a/devtools/client/memory/constants.js +++ b/devtools/client/memory/constants.js @@ -29,6 +29,9 @@ actions.SELECT_SNAPSHOT = "select-snapshot"; // Fired to toggle tree inversion on or off. actions.TOGGLE_INVERTED = "toggle-inverted"; +// Fired to set a new breakdown. +actions.SET_BREAKDOWN = "set-breakdown"; + // Fired when there is an error processing a snapshot or taking a census. actions.SNAPSHOT_ERROR = "snapshot-error"; diff --git a/devtools/client/memory/initializer.js b/devtools/client/memory/initializer.js index 8b624556513..143c0647d9b 100644 --- a/devtools/client/memory/initializer.js +++ b/devtools/client/memory/initializer.js @@ -19,13 +19,21 @@ const Store = require("devtools/client/memory/store"); */ var gToolbox, gTarget, gFront, gHeapAnalysesClient; +/** + * Globals set by initialize() + */ +var gRoot, gStore, gApp, gProvider; + function initialize () { return Task.spawn(function*() { - let root = document.querySelector("#app"); - let store = Store(); - let app = createElement(App, { front: gFront, heapWorker: gHeapAnalysesClient }); - let provider = createElement(Provider, { store }, app); - render(provider, root); + gRoot = document.querySelector("#app"); + gStore = Store(); + gApp = createElement(App, { + front: gFront, + heapWorker: gHeapAnalysesClient + }); + gProvider = createElement(Provider, { store: gStore }, gApp); + render(gProvider, gRoot); }); } diff --git a/devtools/client/memory/test/browser/browser.ini b/devtools/client/memory/test/browser/browser.ini index 48a2e9e0c60..79c70b80e08 100644 --- a/devtools/client/memory/test/browser/browser.ini +++ b/devtools/client/memory/test/browser/browser.ini @@ -4,4 +4,6 @@ subsuite = devtools support-files = head.js +[browser_memory-breakdowns-01.js] +[browser_memory-simple-01.js] [browser_memory_transferHeapSnapshot_e10s_01.js] diff --git a/devtools/client/memory/test/browser/browser_memory-breakdowns-01.js b/devtools/client/memory/test/browser/browser_memory-breakdowns-01.js new file mode 100644 index 00000000000..8701ef867e3 --- /dev/null +++ b/devtools/client/memory/test/browser/browser_memory-breakdowns-01.js @@ -0,0 +1,33 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +/** + * Tests that the heap tree renders rows based on the breakdown + */ + +const TEST_URL = "http://example.com/browser/devtools/client/memory/test/browser/doc_steady_allocation.html"; + +this.test = makeMemoryTest(TEST_URL, function* ({ tab, panel }) { + const { gStore, document } = panel.panelWin; + const $$ = document.querySelectorAll.bind(document); + + yield takeSnapshot(panel.panelWin); + + yield waitUntilSnapshotState(gStore, [states.SAVED_CENSUS]); + + info("Check coarse type heap view"); + ["objects", "other", "scripts", "strings"].forEach(findNameCell); + + yield setBreakdown(panel.panelWin, "objectClass"); + info("Check object class heap view"); + ["Function", "Object"].forEach(findNameCell); + + yield setBreakdown(panel.panelWin, "internalType"); + info("Check internal type heap view"); + ["JSObject"].forEach(findNameCell); + + function findNameCell (name) { + let el = Array.prototype.find.call($$(".tree .heap-tree-item-name span"), el => el.textContent === name); + ok(el, `Found heap tree item cell for ${name}.`); + } +}); diff --git a/devtools/client/memory/test/browser/browser_memory-simple-01.js b/devtools/client/memory/test/browser/browser_memory-simple-01.js new file mode 100644 index 00000000000..7a52c313964 --- /dev/null +++ b/devtools/client/memory/test/browser/browser_memory-simple-01.js @@ -0,0 +1,35 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +/** + * Tests taking snapshots and default states. + */ + +const TEST_URL = "http://example.com/browser/devtools/client/memory/test/browser/doc_steady_allocation.html"; + +this.test = makeMemoryTest(TEST_URL, function* ({ tab, panel }) { + const { gStore, document } = panel.panelWin; + const { getState, dispatch } = gStore; + + let snapshotEls = document.querySelectorAll("#memory-tool-container .list li"); + is(getState().snapshots.length, 0, "Starts with no snapshots in store"); + is(snapshotEls.length, 0, "No snapshots rendered"); + + yield takeSnapshot(panel.panelWin); + snapshotEls = document.querySelectorAll("#memory-tool-container .list li"); + is(getState().snapshots.length, 1, "One snapshot was created in store"); + is(snapshotEls.length, 1, "One snapshot was rendered"); + ok(snapshotEls[0].classList.contains("selected"), "Only snapshot has `selected` class"); + + yield takeSnapshot(panel.panelWin); + snapshotEls = document.querySelectorAll("#memory-tool-container .list li"); + is(getState().snapshots.length, 2, "Two snapshots created in store"); + is(snapshotEls.length, 2, "Two snapshots rendered"); + ok(!snapshotEls[0].classList.contains("selected"), "First snapshot no longer has `selected` class"); + ok(snapshotEls[1].classList.contains("selected"), "Second snapshot has `selected` class"); + + yield waitUntilSnapshotState(gStore, [states.SAVED_CENSUS, states.SAVED_CENSUS]); + + ok(document.querySelector(".heap-tree-item-name"), + "Should have rendered some tree items"); +}); diff --git a/devtools/client/memory/test/browser/doc_steady_allocation.html b/devtools/client/memory/test/browser/doc_steady_allocation.html new file mode 100644 index 00000000000..65703c878f8 --- /dev/null +++ b/devtools/client/memory/test/browser/doc_steady_allocation.html @@ -0,0 +1,16 @@ + + + + + + diff --git a/devtools/client/memory/test/browser/head.js b/devtools/client/memory/test/browser/head.js index 6b6d5267d1b..77579c92f8d 100644 --- a/devtools/client/memory/test/browser/head.js +++ b/devtools/client/memory/test/browser/head.js @@ -8,6 +8,9 @@ Services.scriptloader.loadSubScript( "chrome://mochitests/content/browser/devtools/client/framework/test/shared-head.js", this); +var { snapshotState: states } = require("devtools/client/memory/constants"); +var { breakdownEquals, breakdownNameToSpec } = require("devtools/client/memory/utils"); + Services.prefs.setBoolPref("devtools.memory.enabled", true); /** @@ -63,3 +66,60 @@ function makeMemoryTest(url, generator) { finish(); }); } + + +function waitUntilState (store, predicate) { + let deferred = promise.defer(); + let unsubscribe = store.subscribe(check); + + function check () { + if (predicate(store.getState())) { + unsubscribe(); + deferred.resolve() + } + } + + // Fire the check immediately incase the action has already occurred + check(); + + return deferred.promise; +} + +function waitUntilSnapshotState (store, expected) { + let predicate = () => { + let snapshots = store.getState().snapshots; + info(snapshots.map(x => x.state)); + return snapshots.length === expected.length && + expected.every((state, i) => state === "*" || snapshots[i].state === state); + }; + info(`Waiting for snapshots to be of state: ${expected}`); + return waitUntilState(store, predicate); +} + +function takeSnapshot (window) { + let { gStore, document } = window; + let snapshotCount = gStore.getState().snapshots.length; + info(`Taking snapshot...`); + document.querySelector(".devtools-toolbar .take-snapshot").click(); + return waitUntilState(gStore, () => gStore.getState().snapshots.length === snapshotCount + 1); +} + +/** + * Sets breakdown and waits for currently selected breakdown to use it + * and be completed the census. + */ +function setBreakdown (window, type) { + info(`Setting breakdown to ${type}...`); + let { gStore, gHeapAnalysesClient } = window; + // XXX: Should handle this via clicking the DOM, but React doesn't + // fire the onChange event, so just change it in the store. + // window.document.querySelector(`.select-breakdown`).value = type; + gStore.dispatch(require("devtools/client/memory/actions/breakdown") + .setBreakdownAndRefresh(gHeapAnalysesClient, breakdownNameToSpec(type))); + + return waitUntilState(window.gStore, () => { + let selected = window.gStore.getState().snapshots.find(s => s.selected); + return selected.state === states.SAVED_CENSUS && + breakdownEquals(breakdownNameToSpec(type), selected.breakdown); + }); +} From 2e6b8687d399fd908e68bca41beae1bde6c82cf4 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Wed, 28 Oct 2015 02:11:06 -0700 Subject: [PATCH 04/30] Bug 1219066 - Make sure to traverse and unlink HeapSnapshot::mParent in cycle collection; r=mccr8 --- devtools/shared/heapsnapshot/HeapSnapshot.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/devtools/shared/heapsnapshot/HeapSnapshot.cpp b/devtools/shared/heapsnapshot/HeapSnapshot.cpp index 52efff82bb6..af1b74085dc 100644 --- a/devtools/shared/heapsnapshot/HeapSnapshot.cpp +++ b/devtools/shared/heapsnapshot/HeapSnapshot.cpp @@ -55,9 +55,11 @@ using JS::ubi::AtomOrTwoByteChars; NS_IMPL_CYCLE_COLLECTION_CLASS(HeapSnapshot) NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(HeapSnapshot) + NS_IMPL_CYCLE_COLLECTION_UNLINK(mParent) NS_IMPL_CYCLE_COLLECTION_UNLINK_END NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(HeapSnapshot) + NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mParent) NS_IMPL_CYCLE_COLLECTION_TRAVERSE_SCRIPT_OBJECTS NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END From 0b9221219b75c7ea477870236a1e094718122a82 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Wed, 28 Oct 2015 02:11:06 -0700 Subject: [PATCH 05/30] Bug 1218560 - Part 2: Add an integration test for allocation stack breakdowns; r=jsantell --- devtools/client/memory/initializer.js | 1 + .../client/memory/test/browser/browser.ini | 2 + ...wser_memory_allocationStackBreakdown_01.js | 39 +++++++++++++++++++ 3 files changed, 42 insertions(+) create mode 100644 devtools/client/memory/test/browser/browser_memory_allocationStackBreakdown_01.js diff --git a/devtools/client/memory/initializer.js b/devtools/client/memory/initializer.js index 143c0647d9b..0d2f63beb31 100644 --- a/devtools/client/memory/initializer.js +++ b/devtools/client/memory/initializer.js @@ -38,5 +38,6 @@ function initialize () { } function destroy () { + gRoot = gStore = gApp = gProvider = null; return Task.spawn(function*(){}); } diff --git a/devtools/client/memory/test/browser/browser.ini b/devtools/client/memory/test/browser/browser.ini index 79c70b80e08..ba4db15d199 100644 --- a/devtools/client/memory/test/browser/browser.ini +++ b/devtools/client/memory/test/browser/browser.ini @@ -3,7 +3,9 @@ tags = devtools subsuite = devtools support-files = head.js + doc_steady_allocation.html +[browser_memory_allocationStackBreakdown_01.js] [browser_memory-breakdowns-01.js] [browser_memory-simple-01.js] [browser_memory_transferHeapSnapshot_e10s_01.js] diff --git a/devtools/client/memory/test/browser/browser_memory_allocationStackBreakdown_01.js b/devtools/client/memory/test/browser/browser_memory_allocationStackBreakdown_01.js new file mode 100644 index 00000000000..1b8f6d25271 --- /dev/null +++ b/devtools/client/memory/test/browser/browser_memory_allocationStackBreakdown_01.js @@ -0,0 +1,39 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +// Sanity test that we can show allocation stack breakdowns in the tree. + +"use strict"; + +const { waitForTime } = require("devtools/shared/DevToolsUtils"); +const { breakdowns } = require("devtools/client/memory/constants"); +const { toggleRecordingAllocationStacks } = require("devtools/client/memory/actions/allocations"); +const { takeSnapshotAndCensus } = require("devtools/client/memory/actions/snapshot"); +const breakdownActions = require("devtools/client/memory/actions/breakdown"); +const { toggleInverted } = require("devtools/client/memory/actions/inverted"); + +const TEST_URL = "http://example.com/browser/devtools/client/memory/test/browser/doc_steady_allocation.html"; + +this.test = makeMemoryTest(TEST_URL, function* ({ tab, panel }) { + const heapWorker = panel.panelWin.gHeapAnalysesClient; + const front = panel.panelWin.gFront; + const { getState, dispatch } = panel.panelWin.gStore; + + dispatch(toggleInverted()); + ok(getState().inverted, true); + + dispatch(breakdownActions.setBreakdown(breakdowns.allocationStack.breakdown)); + is(getState().breakdown.by, "allocationStack"); + + yield dispatch(toggleRecordingAllocationStacks(front)); + ok(getState().allocations.recording); + + // Let some allocations build up. + yield waitForTime(500); + + yield dispatch(takeSnapshotAndCensus(front, heapWorker)); + + const doc = panel.panelWin.document; + ok(doc.querySelector(".heap-tree-item-function-display-name"), + "Should have rendered some allocation stack tree items"); +}); From db5fe14433b1b0858eb11b15dcbf2b2bded75fff Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Wed, 28 Oct 2015 02:11:06 -0700 Subject: [PATCH 06/30] Bug 1219079 - Small breakdown-related fixes for the memory tool; r=jsantell * Add "Breakdown by" in front of dropdown selector for the selected breakdown. * "Allocation Site" => "Allocation Stack" for breakdown's label. * Make the coarse type breakdown bucket strings by count. --- devtools/client/memory/components/toolbar.js | 11 +++++++---- devtools/client/memory/constants.js | 4 ++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/devtools/client/memory/components/toolbar.js b/devtools/client/memory/components/toolbar.js index 5fd6a7e2274..a7ad64d4847 100644 --- a/devtools/client/memory/components/toolbar.js +++ b/devtools/client/memory/components/toolbar.js @@ -36,10 +36,13 @@ const Toolbar = module.exports = createClass({ DOM.div({ className: "devtools-toolbar" }, [ DOM.button({ className: `take-snapshot devtools-button`, onClick: onTakeSnapshotClick }), - DOM.select({ - className: `select-breakdown`, - onChange: e => onBreakdownChange(e.target.value), - }, breakdowns.map(({ name, displayName }) => DOM.option({ value: name }, displayName))), + DOM.label({}, + "Breakdown by ", + DOM.select({ + className: `select-breakdown`, + onChange: e => onBreakdownChange(e.target.value), + }, breakdowns.map(({ name, displayName }) => DOM.option({ value: name }, displayName))) + ), DOM.label({}, [ DOM.input({ diff --git a/devtools/client/memory/constants.js b/devtools/client/memory/constants.js index 06b7b9dc280..6d8b763874b 100644 --- a/devtools/client/memory/constants.js +++ b/devtools/client/memory/constants.js @@ -52,14 +52,14 @@ const breakdowns = exports.breakdowns = { breakdown: { by: "coarseType", objects: ALLOCATION_STACK, - strings: ALLOCATION_STACK, + strings: COUNT, scripts: INTERNAL_TYPE, other: INTERNAL_TYPE, } }, allocationStack: { - displayName: "Allocation Site", + displayName: "Allocation Stack", breakdown: ALLOCATION_STACK, }, From 0fc411a4228f7e7055eb15b9fbaeb9cdbdc39e02 Mon Sep 17 00:00:00 2001 From: Patrick Brosset Date: Tue, 27 Oct 2015 11:21:46 +0100 Subject: [PATCH 07/30] Bug 1218409 - Eslint rule that checks for balanced listeners. r=miker --- devtools/.eslintrc | 1 + .../docs/balanced-listeners.rst | 11 ++ testing/eslint-plugin-mozilla/docs/index.rst | 5 + testing/eslint-plugin-mozilla/lib/index.js | 2 + .../lib/rules/balanced-listeners.js | 107 ++++++++++++++++++ 5 files changed, 126 insertions(+) create mode 100644 testing/eslint-plugin-mozilla/docs/balanced-listeners.rst create mode 100644 testing/eslint-plugin-mozilla/lib/rules/balanced-listeners.js diff --git a/devtools/.eslintrc b/devtools/.eslintrc index 90db6d13397..c88664c6972 100644 --- a/devtools/.eslintrc +++ b/devtools/.eslintrc @@ -26,6 +26,7 @@ // devtools coding style. // Rules from the mozilla plugin + "mozilla/balanced-listeners": 2, "mozilla/components-imports": 1, "mozilla/import-headjs-globals": 1, "mozilla/mark-test-function-used": 1, diff --git a/testing/eslint-plugin-mozilla/docs/balanced-listeners.rst b/testing/eslint-plugin-mozilla/docs/balanced-listeners.rst new file mode 100644 index 00000000000..d153edf94f5 --- /dev/null +++ b/testing/eslint-plugin-mozilla/docs/balanced-listeners.rst @@ -0,0 +1,11 @@ +.. _balanced-listeners: + +================== +balanced-listeners +================== + +Rule Details +------------ + +Checks that for every occurences of 'addEventListener' or 'on' there is an +occurence of 'removeEventListener' or 'off' with the same event name. diff --git a/testing/eslint-plugin-mozilla/docs/index.rst b/testing/eslint-plugin-mozilla/docs/index.rst index 48eb62b1e12..af1f7a17b2c 100644 --- a/testing/eslint-plugin-mozilla/docs/index.rst +++ b/testing/eslint-plugin-mozilla/docs/index.rst @@ -4,6 +4,9 @@ Mozilla ESLint Plugin ===================== +``balanced-listeners`` checks that every addEventListener has a +removeEventListener (and does the same for on/off). + ``components-imports`` adds the filename of imported files e.g. ``Cu.import("some/path/Blah.jsm")`` adds Blah to the global scope. @@ -31,6 +34,7 @@ level invalid. Example configuration:: "rules": { + "mozilla/balanced-listeners": 2, "mozilla/components-imports": 1, "mozilla/import-headjs-globals": 1, "mozilla/mark-test-function-used": 1, @@ -40,6 +44,7 @@ Example configuration:: .. toctree:: :maxdepth: 1 + balanced-listeners components-imports import-headjs-globals mark-test-function-used diff --git a/testing/eslint-plugin-mozilla/lib/index.js b/testing/eslint-plugin-mozilla/lib/index.js index 072289a179b..169f72dcbe3 100644 --- a/testing/eslint-plugin-mozilla/lib/index.js +++ b/testing/eslint-plugin-mozilla/lib/index.js @@ -13,12 +13,14 @@ module.exports = { rules: { + "balanced-listeners": require("../lib/rules/balanced-listeners"), "components-imports": require("../lib/rules/components-imports"), "import-headjs-globals": require("../lib/rules/import-headjs-globals"), "mark-test-function-used": require("../lib/rules/mark-test-function-used"), "var-only-at-top-level": require("../lib/rules/var-only-at-top-level") }, rulesConfig: { + "balanced-listeners": 0, "components-imports": 0, "import-headjs-globals": 0, "mark-test-function-used": 0, diff --git a/testing/eslint-plugin-mozilla/lib/rules/balanced-listeners.js b/testing/eslint-plugin-mozilla/lib/rules/balanced-listeners.js new file mode 100644 index 00000000000..9ee2f9f8d3d --- /dev/null +++ b/testing/eslint-plugin-mozilla/lib/rules/balanced-listeners.js @@ -0,0 +1,107 @@ +/** + * @fileoverview Check that there's a removeEventListener for each + * addEventListener and an off for each on. + * Note that for now, this rule is rather simple in that it only checks that + * for each event name there is both an add and remove listener. It doesn't + * check that these are called on the right objects or with the same callback. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ +"use strict"; + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +module.exports = function(context) { + //-------------------------------------------------------------------------- + // Helpers + //-------------------------------------------------------------------------- + + var DICTIONARY = { + "addEventListener": "removeEventListener", + "on": "off" + }; + // Invert this dictionary to make it easy later. + var INVERTED_DICTIONARY = {}; + for (var i in DICTIONARY) { + INVERTED_DICTIONARY[DICTIONARY[i]] = i; + } + + // Collect the add/remove listeners in these 2 arrays. + var addedListeners = []; + var removedListeners = []; + + function addAddedListener(node) { + addedListeners.push({ + functionName: node.callee.property.name, + type: node.arguments[0].value, + node: node.callee.property, + useCapture: node.arguments[2] ? node.arguments[2].value : null + }); + } + + function addRemovedListener(node) { + removedListeners.push({ + functionName: node.callee.property.name, + type: node.arguments[0].value, + useCapture: node.arguments[2] ? node.arguments[2].value : null + }); + } + + function getUnbalancedListeners() { + var unbalanced = []; + + for (var i = 0; i < addedListeners.length; i ++) { + if (!hasRemovedListener(addedListeners[i])) { + unbalanced.push(addedListeners[i]); + } + } + addedListeners = removedListeners = []; + + return unbalanced; + } + + function hasRemovedListener(addedListener) { + for (var i = 0; i < removedListeners.length; i ++) { + var listener = removedListeners[i]; + if (DICTIONARY[addedListener.functionName] === listener.functionName && + addedListener.type === listener.type && + addedListener.useCapture === listener.useCapture) { + return true; + } + } + + return false; + } + + //-------------------------------------------------------------------------- + // Public + //-------------------------------------------------------------------------- + + return { + CallExpression: function(node) { + if (node.callee.type === "MemberExpression") { + var listenerMethodName = node.callee.property.name; + + if (DICTIONARY.hasOwnProperty(listenerMethodName)) { + addAddedListener(node); + } else if (INVERTED_DICTIONARY.hasOwnProperty(listenerMethodName)) { + addRemovedListener(node); + } + } + }, + + "Program:exit": function() { + getUnbalancedListeners().forEach(function(listener) { + context.report(listener.node, + "No corresponding '{{functionName}}({{type}})' was found.", { + functionName: DICTIONARY[listener.functionName], + type: listener.type + }); + }); + } + }; +}; From a94615fe2e3472e173ac6d1f763edb382455d71b Mon Sep 17 00:00:00 2001 From: Patrick Brosset Date: Wed, 28 Oct 2015 08:32:02 +0100 Subject: [PATCH 08/30] Bug 1218425 - ESLint rule that warns against aArg notation in function params; r=miker --- devtools/.eslintrc | 1 + testing/eslint-plugin-mozilla/docs/index.rst | 3 ++ .../eslint-plugin-mozilla/docs/no-aArgs.rst | 12 +++++ testing/eslint-plugin-mozilla/lib/index.js | 2 + .../lib/rules/no-aArgs.js | 50 +++++++++++++++++++ 5 files changed, 68 insertions(+) create mode 100644 testing/eslint-plugin-mozilla/docs/no-aArgs.rst create mode 100644 testing/eslint-plugin-mozilla/lib/rules/no-aArgs.js diff --git a/devtools/.eslintrc b/devtools/.eslintrc index c88664c6972..7d87683422b 100644 --- a/devtools/.eslintrc +++ b/devtools/.eslintrc @@ -30,6 +30,7 @@ "mozilla/components-imports": 1, "mozilla/import-headjs-globals": 1, "mozilla/mark-test-function-used": 1, + "mozilla/no-aArgs": 1, "mozilla/var-only-at-top-level": 1, // Disallow using variables outside the blocks they are defined (especially diff --git a/testing/eslint-plugin-mozilla/docs/index.rst b/testing/eslint-plugin-mozilla/docs/index.rst index af1f7a17b2c..d2c0f8d818e 100644 --- a/testing/eslint-plugin-mozilla/docs/index.rst +++ b/testing/eslint-plugin-mozilla/docs/index.rst @@ -16,6 +16,8 @@ should be imported by head.js (as far as we can correctly resolve the path). ``mark-test-function-used`` simply marks test (the test method) as used. This avoids ESLint telling us that the function is never called. +``no-aArgs`` prevents using the hungarian notation in function arguments. + ``var-only-at-top-level`` Marks all var declarations that are not at the top level invalid. @@ -48,4 +50,5 @@ Example configuration:: components-imports import-headjs-globals mark-test-function-used + no-aArgs var-only-at-top-level diff --git a/testing/eslint-plugin-mozilla/docs/no-aArgs.rst b/testing/eslint-plugin-mozilla/docs/no-aArgs.rst new file mode 100644 index 00000000000..b5da083c5f5 --- /dev/null +++ b/testing/eslint-plugin-mozilla/docs/no-aArgs.rst @@ -0,0 +1,12 @@ +.. _no-aArgs: + +======== +no-aArgs +======== + +Rule Details +------------ + +Checks that function argument names don't start with lowercase 'a' followed by a +capital letter. This is to prevent the use of Hungarian notation whereby the +first letter is a prefix that indicates the type or intended use of a variable. diff --git a/testing/eslint-plugin-mozilla/lib/index.js b/testing/eslint-plugin-mozilla/lib/index.js index 169f72dcbe3..3b9642fb41d 100644 --- a/testing/eslint-plugin-mozilla/lib/index.js +++ b/testing/eslint-plugin-mozilla/lib/index.js @@ -17,6 +17,7 @@ module.exports = { "components-imports": require("../lib/rules/components-imports"), "import-headjs-globals": require("../lib/rules/import-headjs-globals"), "mark-test-function-used": require("../lib/rules/mark-test-function-used"), + "no-aArgs": require("../lib/rules/no-aArgs"), "var-only-at-top-level": require("../lib/rules/var-only-at-top-level") }, rulesConfig: { @@ -24,6 +25,7 @@ module.exports = { "components-imports": 0, "import-headjs-globals": 0, "mark-test-function-used": 0, + "no-aArgs": 0, "var-only-at-top-level": 0 } }; diff --git a/testing/eslint-plugin-mozilla/lib/rules/no-aArgs.js b/testing/eslint-plugin-mozilla/lib/rules/no-aArgs.js new file mode 100644 index 00000000000..b40a5a51be8 --- /dev/null +++ b/testing/eslint-plugin-mozilla/lib/rules/no-aArgs.js @@ -0,0 +1,50 @@ +/** + * @fileoverview warns against using hungarian notation in function arguments + * (i.e. aArg). + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ +"use strict"; + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +module.exports = function(context) { + //-------------------------------------------------------------------------- + // Helpers + //-------------------------------------------------------------------------- + + function isPrefixed(name) { + return name.length >= 2 && /^a[A-Z]/.test(name); + } + + function deHungarianize(name) { + return name.substring(1, 2).toLowerCase() + + name.substring(2, name.length); + } + + function checkFunction(node) { + for (var i = 0; i < node.params.length; i ++) { + var param = node.params[i]; + if (param.name && isPrefixed(param.name)) { + context.report(param, "Parameter '{{name}}' uses Hungarian Notation, consider using '{{suggestion}}' instead.", { + name: param.name, + suggestion: deHungarianize(param.name) + }); + } + } + } + + //-------------------------------------------------------------------------- + // Public + //-------------------------------------------------------------------------- + + return { + "FunctionDeclaration": checkFunction, + "ArrowFunctionExpression": checkFunction, + "FunctionExpression": checkFunction + }; +}; From 41f34ad26a7b5f87ee2aec7a31dffb6e78d05ee9 Mon Sep 17 00:00:00 2001 From: Victor Porof Date: Sat, 24 Oct 2015 17:10:22 +0200 Subject: [PATCH 09/30] Bug 1211839 - Don't allow off the main thread markers to nest under main thread markers, r=smaug, jsantell --- devtools/client/performance/docs/markers.md | 9 + .../modules/logic/waterfall-utils.js | 36 ++-- .../unit/test_waterfall-utils-collapse-05.js | 163 ++++++++++++++++++ .../client/performance/test/unit/xpcshell.ini | 1 + .../base/timeline/AbstractTimelineMarker.cpp | 7 + .../base/timeline/AbstractTimelineMarker.h | 6 + .../base/timeline/ConsoleTimelineMarker.h | 2 + docshell/base/timeline/EventTimelineMarker.h | 2 + .../base/timeline/JavascriptTimelineMarker.h | 2 + .../base/timeline/RestyleTimelineMarker.h | 2 + docshell/base/timeline/TimelineMarker.cpp | 5 +- .../base/timeline/TimestampTimelineMarker.h | 2 + docshell/base/timeline/WorkerTimelineMarker.h | 2 + dom/webidl/ProfileTimelineMarker.webidl | 3 + 14 files changed, 231 insertions(+), 11 deletions(-) create mode 100644 devtools/client/performance/test/unit/test_waterfall-utils-collapse-05.js diff --git a/devtools/client/performance/docs/markers.md b/devtools/client/performance/docs/markers.md index 3906408b42a..e743f7fcd63 100644 --- a/devtools/client/performance/docs/markers.md +++ b/devtools/client/performance/docs/markers.md @@ -7,6 +7,15 @@ * DOMString name * object? stack * object? endStack +* unsigned short processType; +* boolean isOffMainThread; + +The `processType` a GeckoProcessType enum listed in xpcom/build/nsXULAppAPI.h, +specifying if this marker originates in a content, chrome, plugin etc. process. +Additionally, markers may be created from any thread on those processes, and +`isOffMainThread` highights whether or not they're from the main thread. The most +common type of marker is probably going to be from a GeckoProcessType_Content's +main thread when debugging content. ## DOMEvent diff --git a/devtools/client/performance/modules/logic/waterfall-utils.js b/devtools/client/performance/modules/logic/waterfall-utils.js index a3a434dbd37..77393e525ec 100644 --- a/devtools/client/performance/modules/logic/waterfall-utils.js +++ b/devtools/client/performance/modules/logic/waterfall-utils.js @@ -32,7 +32,11 @@ function createParentNode (marker) { * @param array filter */ function collapseMarkersIntoNode({ rootNode, markersList, filter }) { - let { getCurrentParentNode, pushNode, popParentNode } = createParentNodeFactory(rootNode); + let { + getCurrentParentNode, + pushNode, + popParentNode + } = createParentNodeFactory(rootNode); for (let i = 0, len = markersList.length; i < len; i++) { let curr = markersList[i]; @@ -48,7 +52,7 @@ function collapseMarkersIntoNode({ rootNode, markersList, filter }) { let nestable = "nestable" in blueprint ? blueprint.nestable : true; let collapsible = "collapsible" in blueprint ? blueprint.collapsible : true; - let finalized = null; + let finalized = false; // If this marker is collapsible, turn it into a parent marker. // If there are no children within it later, it will be turned @@ -57,9 +61,14 @@ function collapseMarkersIntoNode({ rootNode, markersList, filter }) { curr = createParentNode(curr); } - // If not nestible, just push it inside the root node, - // like console.time/timeEnd. - if (!nestable) { + // If not nestible, just push it inside the root node. Additionally, + // markers originating outside the main thread are considered to be + // "never collapsible", to avoid confusion. + // A beter solution would be to collapse every marker with its siblings + // from the same thread, but that would require a thread id attached + // to all markers, which is potentially expensive and rather useless at + // the moment, since we don't really have that many OTMT markers. + if (!nestable || curr.isOffMainThread) { pushNode(rootNode, curr); continue; } @@ -68,9 +77,13 @@ function collapseMarkersIntoNode({ rootNode, markersList, filter }) { // recursively upwards if this marker is outside their ranges and nestable. while (!finalized && parentNode) { // If this marker is eclipsed by the current parent marker, - // make it a child of the current parent and stop - // going upwards. - if (nestable && curr.end <= parentNode.end) { + // make it a child of the current parent and stop going upwards. + // If the markers aren't from the same process, attach them to the root + // node as well. Every process has its own main thread. + if (nestable && + curr.start >= parentNode.start && + curr.end <= parentNode.end && + curr.processType == parentNode.processType) { pushNode(parentNode, curr); finalized = true; break; @@ -112,6 +125,7 @@ function createParentNodeFactory (root) { } let lastParent = parentMarkers.pop(); + // If this finished parent marker doesn't have an end time, // so probably a synthesized marker, use the last marker's end time. if (lastParent.end == void 0) { @@ -119,7 +133,7 @@ function createParentNodeFactory (root) { } // If no children were ever pushed into this parent node, - // remove it's submarkers so it behaves like a non collapsible + // remove its submarkers so it behaves like a non collapsible // node. if (!lastParent.submarkers.length) { delete lastParent.submarkers; @@ -131,7 +145,9 @@ function createParentNodeFactory (root) { /** * Returns the most recent parent node. */ - getCurrentParentNode: () => parentMarkers.length ? parentMarkers[parentMarkers.length - 1] : null, + getCurrentParentNode: () => parentMarkers.length + ? parentMarkers[parentMarkers.length - 1] + : null, /** * Push this marker into the most recent parent node. diff --git a/devtools/client/performance/test/unit/test_waterfall-utils-collapse-05.js b/devtools/client/performance/test/unit/test_waterfall-utils-collapse-05.js new file mode 100644 index 00000000000..a8a32eb453d --- /dev/null +++ b/devtools/client/performance/test/unit/test_waterfall-utils-collapse-05.js @@ -0,0 +1,163 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +/** + * Tests if the waterfall collapsing logic works properly + * when dealing with OTMT markers. + */ + +function run_test() { + run_next_test(); +} + +add_task(function test() { + const WaterfallUtils = require("devtools/client/performance/modules/logic/waterfall-utils"); + + let rootMarkerNode = WaterfallUtils.createParentNode({ name: "(root)" }); + + WaterfallUtils.collapseMarkersIntoNode({ + rootNode: rootMarkerNode, + markersList: gTestMarkers + }); + + compare(rootMarkerNode, gExpectedOutput); + + function compare (marker, expected) { + for (let prop in expected) { + if (prop === "submarkers") { + for (let i = 0; i < expected.submarkers.length; i++) { + compare(marker.submarkers[i], expected.submarkers[i]); + } + } else if (prop !== "uid") { + equal(marker[prop], expected[prop], `${expected.name} matches ${prop}`); + } + } + } +}); + +const gTestMarkers = [ + { start: 1, end: 4, name: "A1-mt", processType: 1, isOffMainThread: false }, + // This should collapse only under A1-mt + { start: 2, end: 3, name: "B1", processType: 1, isOffMainThread: false }, + // This should never collapse. + { start: 2, end: 3, name: "C1", processType: 1, isOffMainThread: true }, + + { start: 5, end: 8, name: "A1-otmt", processType: 1, isOffMainThread: true }, + // This should collapse only under A1-mt + { start: 6, end: 7, name: "B2", processType: 1, isOffMainThread: false }, + // This should never collapse. + { start: 6, end: 7, name: "C2", processType: 1, isOffMainThread: true }, + + { start: 9, end: 12, name: "A2-mt", processType: 2, isOffMainThread: false }, + // This should collapse only under A2-mt + { start: 10, end: 11, name: "D1", processType: 2, isOffMainThread: false }, + // This should never collapse. + { start: 10, end: 11, name: "E1", processType: 2, isOffMainThread: true }, + + { start: 13, end: 16, name: "A2-otmt", processType: 2, isOffMainThread: true }, + // This should collapse only under A2-mt + { start: 14, end: 15, name: "D2", processType: 2, isOffMainThread: false }, + // This should never collapse. + { start: 14, end: 15, name: "E2", processType: 2, isOffMainThread: true }, + + // This should not collapse, because there's no parent in this process. + { start: 14, end: 15, name: "F", processType: 3, isOffMainThread: false }, + + // This should never collapse. + { start: 14, end: 15, name: "G", processType: 3, isOffMainThread: true }, +]; + +const gExpectedOutput = { + name: "(root)", + submarkers: [{ + start: 1, + end: 4, + name: "A1-mt", + processType: 1, + isOffMainThread: false, + submarkers: [{ + start: 2, + end: 3, + name: "B1", + processType: 1, + isOffMainThread: false + }] + }, { + start: 2, + end: 3, + name: "C1", + processType: 1, + isOffMainThread: true + }, { + start: 5, + end: 8, + name: "A1-otmt", + processType: 1, + isOffMainThread: true, + submarkers: [{ + start: 6, + end: 7, + name: "B2", + processType: 1, + isOffMainThread: false + }] + }, { + start: 6, + end: 7, + name: "C2", + processType: 1, + isOffMainThread: true + }, { + start: 9, + end: 12, + name: "A2-mt", + processType: 2, + isOffMainThread: false, + submarkers: [{ + start: 10, + end: 11, + name: "D1", + processType: 2, + isOffMainThread: false + }] + }, { + start: 10, + end: 11, + name: "E1", + processType: 2, + isOffMainThread: true + }, { + start: 13, + end: 16, + name: "A2-otmt", + processType: 2, + isOffMainThread: true, + submarkers: [{ + start: 14, + end: 15, + name: "D2", + processType: 2, + isOffMainThread: false + }] + }, { + start: 14, + end: 15, + name: "E2", + processType: 2, + isOffMainThread: true + }, { + start: 14, + end: 15, + name: "F", + processType: 3, + isOffMainThread: false, + submarkers: [] + }, { + start: 14, + end: 15, + name: "G", + processType: 3, + isOffMainThread: true, + submarkers: [] + }] +}; diff --git a/devtools/client/performance/test/unit/xpcshell.ini b/devtools/client/performance/test/unit/xpcshell.ini index 80dbec6afbb..594624e10c5 100644 --- a/devtools/client/performance/test/unit/xpcshell.ini +++ b/devtools/client/performance/test/unit/xpcshell.ini @@ -33,3 +33,4 @@ skip-if = toolkit == 'android' || toolkit == 'gonk' [test_waterfall-utils-collapse-02.js] [test_waterfall-utils-collapse-03.js] [test_waterfall-utils-collapse-04.js] +[test_waterfall-utils-collapse-05.js] diff --git a/docshell/base/timeline/AbstractTimelineMarker.cpp b/docshell/base/timeline/AbstractTimelineMarker.cpp index b4cc4f578b0..aadaaed17fa 100644 --- a/docshell/base/timeline/AbstractTimelineMarker.cpp +++ b/docshell/base/timeline/AbstractTimelineMarker.cpp @@ -5,7 +5,10 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ #include "AbstractTimelineMarker.h" + #include "mozilla/TimeStamp.h" +#include "MainThreadUtils.h" +#include "nsAppRunner.h" namespace mozilla { @@ -13,6 +16,8 @@ AbstractTimelineMarker::AbstractTimelineMarker(const char* aName, MarkerTracingType aTracingType) : mName(aName) , mTracingType(aTracingType) + , mProcessType(XRE_GetProcessType()) + , mIsOffMainThread(!NS_IsMainThread()) { MOZ_COUNT_CTOR(AbstractTimelineMarker); SetCurrentTime(); @@ -23,6 +28,8 @@ AbstractTimelineMarker::AbstractTimelineMarker(const char* aName, MarkerTracingType aTracingType) : mName(aName) , mTracingType(aTracingType) + , mProcessType(XRE_GetProcessType()) + , mIsOffMainThread(!NS_IsMainThread()) { MOZ_COUNT_CTOR(AbstractTimelineMarker); SetCustomTime(aTime); diff --git a/docshell/base/timeline/AbstractTimelineMarker.h b/docshell/base/timeline/AbstractTimelineMarker.h index e6a1bca9969..659bfa60670 100644 --- a/docshell/base/timeline/AbstractTimelineMarker.h +++ b/docshell/base/timeline/AbstractTimelineMarker.h @@ -48,11 +48,17 @@ public: DOMHighResTimeStamp GetTime() const { return mTime; } MarkerTracingType GetTracingType() const { return mTracingType; } + const uint8_t GetProcessType() const { return mProcessType; }; + const bool IsOffMainThread() const { return mIsOffMainThread; }; + private: const char* mName; DOMHighResTimeStamp mTime; MarkerTracingType mTracingType; + uint8_t mProcessType; // @see `enum GeckoProcessType`. + bool mIsOffMainThread; + protected: void SetCurrentTime(); void SetCustomTime(const TimeStamp& aTime); diff --git a/docshell/base/timeline/ConsoleTimelineMarker.h b/docshell/base/timeline/ConsoleTimelineMarker.h index ecf32545601..39036494957 100644 --- a/docshell/base/timeline/ConsoleTimelineMarker.h +++ b/docshell/base/timeline/ConsoleTimelineMarker.h @@ -40,6 +40,8 @@ public: virtual void AddDetails(JSContext* aCx, dom::ProfileTimelineMarker& aMarker) override { + TimelineMarker::AddDetails(aCx, aMarker); + if (GetTracingType() == MarkerTracingType::START) { aMarker.mCauseName.Construct(mCause); } else { diff --git a/docshell/base/timeline/EventTimelineMarker.h b/docshell/base/timeline/EventTimelineMarker.h index 2779f2037a1..74dfb5e5c07 100644 --- a/docshell/base/timeline/EventTimelineMarker.h +++ b/docshell/base/timeline/EventTimelineMarker.h @@ -25,6 +25,8 @@ public: virtual void AddDetails(JSContext* aCx, dom::ProfileTimelineMarker& aMarker) override { + TimelineMarker::AddDetails(aCx, aMarker); + if (GetTracingType() == MarkerTracingType::START) { aMarker.mType.Construct(mType); aMarker.mEventPhase.Construct(mPhase); diff --git a/docshell/base/timeline/JavascriptTimelineMarker.h b/docshell/base/timeline/JavascriptTimelineMarker.h index e3a60054e19..eaacea838d3 100644 --- a/docshell/base/timeline/JavascriptTimelineMarker.h +++ b/docshell/base/timeline/JavascriptTimelineMarker.h @@ -31,6 +31,8 @@ public: virtual void AddDetails(JSContext* aCx, dom::ProfileTimelineMarker& aMarker) override { + TimelineMarker::AddDetails(aCx, aMarker); + aMarker.mCauseName.Construct(mCause); if (!mFunctionName.IsEmpty() || !mFileName.IsEmpty()) { diff --git a/docshell/base/timeline/RestyleTimelineMarker.h b/docshell/base/timeline/RestyleTimelineMarker.h index 87207f9251c..b5d5a02ef21 100644 --- a/docshell/base/timeline/RestyleTimelineMarker.h +++ b/docshell/base/timeline/RestyleTimelineMarker.h @@ -26,6 +26,8 @@ public: virtual void AddDetails(JSContext* aCx, dom::ProfileTimelineMarker& aMarker) override { + TimelineMarker::AddDetails(aCx, aMarker); + if (GetTracingType() == MarkerTracingType::START) { aMarker.mRestyleHint.Construct(mRestyleHint); } diff --git a/docshell/base/timeline/TimelineMarker.cpp b/docshell/base/timeline/TimelineMarker.cpp index bf5dd799335..b83e9ceb42c 100644 --- a/docshell/base/timeline/TimelineMarker.cpp +++ b/docshell/base/timeline/TimelineMarker.cpp @@ -28,7 +28,10 @@ TimelineMarker::TimelineMarker(const char* aName, void TimelineMarker::AddDetails(JSContext* aCx, dom::ProfileTimelineMarker& aMarker) { - // Nothing to do here for plain markers. + if (GetTracingType() == MarkerTracingType::START) { + aMarker.mProcessType.Construct(GetProcessType()); + aMarker.mIsOffMainThread.Construct(IsOffMainThread()); + } } JSObject* diff --git a/docshell/base/timeline/TimestampTimelineMarker.h b/docshell/base/timeline/TimestampTimelineMarker.h index 73ed91800ab..4e588b5769f 100644 --- a/docshell/base/timeline/TimestampTimelineMarker.h +++ b/docshell/base/timeline/TimestampTimelineMarker.h @@ -22,6 +22,8 @@ public: virtual void AddDetails(JSContext* aCx, dom::ProfileTimelineMarker& aMarker) override { + TimelineMarker::AddDetails(aCx, aMarker); + if (!mCause.IsEmpty()) { aMarker.mCauseName.Construct(mCause); } diff --git a/docshell/base/timeline/WorkerTimelineMarker.h b/docshell/base/timeline/WorkerTimelineMarker.h index f6ef9e361c1..2b2b6950030 100644 --- a/docshell/base/timeline/WorkerTimelineMarker.h +++ b/docshell/base/timeline/WorkerTimelineMarker.h @@ -30,6 +30,8 @@ public: virtual void AddDetails(JSContext* aCx, dom::ProfileTimelineMarker& aMarker) override { + TimelineMarker::AddDetails(aCx, aMarker); + if (GetTracingType() == MarkerTracingType::START) { aMarker.mWorkerOperation.Construct(mOperationType); } diff --git a/dom/webidl/ProfileTimelineMarker.webidl b/dom/webidl/ProfileTimelineMarker.webidl index 384c2cd1e2f..7b5b75adc3e 100644 --- a/dom/webidl/ProfileTimelineMarker.webidl +++ b/dom/webidl/ProfileTimelineMarker.webidl @@ -38,6 +38,9 @@ dictionary ProfileTimelineMarker { DOMHighResTimeStamp end = 0; object? stack = null; + unsigned short processType; + boolean isOffMainThread; + /* For ConsoleTime, Timestamp and Javascript markers. */ DOMString causeName; From f79d1f92ef9301b412da87f43a731f07ef1362bb Mon Sep 17 00:00:00 2001 From: Victor Porof Date: Wed, 28 Oct 2015 11:00:52 +0100 Subject: [PATCH 10/30] Bug 1211841 - Style off the main thread markers differently, r=jsantell --- .../modules/widgets/marker-view.js | 1 + .../browser_timeline-waterfall-workers.js | 23 +++++++++++-- devtools/client/themes/performance.css | 21 ++++++++++++ .../base/timeline/AbstractTimelineMarker.cpp | 12 +++++++ .../base/timeline/AbstractTimelineMarker.h | 3 ++ .../base/timeline/CompositeTimelineMarker.h | 33 +++++++++++++++++++ docshell/base/timeline/moz.build | 1 + view/nsView.cpp | 5 +-- 8 files changed, 94 insertions(+), 5 deletions(-) create mode 100644 docshell/base/timeline/CompositeTimelineMarker.h diff --git a/devtools/client/performance/modules/widgets/marker-view.js b/devtools/client/performance/modules/widgets/marker-view.js index 3969395c3f4..c17404f5dba 100644 --- a/devtools/client/performance/modules/widgets/marker-view.js +++ b/devtools/client/performance/modules/widgets/marker-view.js @@ -110,6 +110,7 @@ MarkerView.prototype = Heritage.extend(AbstractTreeItem.prototype, { _displaySelf: function(document, arrowNode) { let targetNode = document.createElement("hbox"); targetNode.className = "waterfall-tree-item"; + targetNode.setAttribute("otmt", this.marker.isOffMainThread); if (this == this.root) { // Bounds are needed for properly positioning and scaling markers in diff --git a/devtools/client/performance/test/browser_timeline-waterfall-workers.js b/devtools/client/performance/test/browser_timeline-waterfall-workers.js index d3e78ca2994..f3becad38cd 100644 --- a/devtools/client/performance/test/browser_timeline-waterfall-workers.js +++ b/devtools/client/performance/test/browser_timeline-waterfall-workers.js @@ -7,7 +7,7 @@ function* spawnTest() { let { panel } = yield initPerformance(WORKER_URL); - let { PerformanceController } = panel.panelWin; + let { $$, $, PerformanceController } = panel.panelWin; loadFrameScripts(); @@ -27,26 +27,43 @@ function* spawnTest() { return false; } - testWorkerMarker(markers.find(m => m.name == "Worker")); + testWorkerMarkerData(markers.find(m => m.name == "Worker")); return true; }); yield stopRecording(panel); ok(true, "Recording has ended."); + for (let node of $$(".waterfall-marker-name[value=Worker")) { + testWorkerMarkerUI(node.parentNode.parentNode); + } + yield teardown(panel); finish(); } -function testWorkerMarker(marker) { +function testWorkerMarkerData(marker) { ok(true, "Found a worker marker."); ok("start" in marker, "The start time is specified in the worker marker."); ok("end" in marker, "The end time is specified in the worker marker."); + ok("workerOperation" in marker, "The worker operation is specified in the worker marker."); + + ok("processType" in marker, + "The process type is specified in the worker marker."); + ok("isOffMainThread" in marker, + "The thread origin is specified in the worker marker."); +} + +function testWorkerMarkerUI(node) { + is(node.className, "waterfall-tree-item", + "The marker node has the correct class name."); + ok(node.hasAttribute("otmt"), + "The marker node specifies if it is off the main thread or not."); } /** diff --git a/devtools/client/themes/performance.css b/devtools/client/themes/performance.css index fbb71a835e0..432b3f91446 100644 --- a/devtools/client/themes/performance.css +++ b/devtools/client/themes/performance.css @@ -505,6 +505,17 @@ -moz-margin-end: -14px; } +/** + * OTMT markers + */ + +.waterfall-tree-item[otmt=true] .waterfall-marker-bullet, +.waterfall-tree-item[otmt=true] .waterfall-marker-bar { + background-color: transparent; + border-width: 1px; + border-style: solid; +} + /** * Marker details view */ @@ -552,43 +563,53 @@ menuitem.marker-color-graphs-full-red:before, .marker-color-graphs-full-red { background-color: var(--theme-graphs-full-red); + border-color: var(--theme-graphs-full-red); } menuitem.marker-color-graphs-full-blue:before, .marker-color-graphs-full-blue { background-color: var(--theme-graphs-full-blue); + border-color: var(--theme-graphs-full-blue); } menuitem.marker-color-graphs-green:before, .marker-color-graphs-green { background-color: var(--theme-graphs-green); + border-color: var(--theme-graphs-green); } menuitem.marker-color-graphs-blue:before, .marker-color-graphs-blue { background-color: var(--theme-graphs-blue); + border-color: var(--theme-graphs-blue); } menuitem.marker-color-graphs-bluegrey:before, .marker-color-graphs-bluegrey { background-color: var(--theme-graphs-bluegrey); + border-color: var(--theme-graphs-bluegrey); } menuitem.marker-color-graphs-purple:before, .marker-color-graphs-purple { background-color: var(--theme-graphs-purple); + border-color: var(--theme-graphs-purple); } menuitem.marker-color-graphs-yellow:before, .marker-color-graphs-yellow { background-color: var(--theme-graphs-yellow); + border-color: var(--theme-graphs-yellow); } menuitem.marker-color-graphs-orange:before, .marker-color-graphs-orange { background-color: var(--theme-graphs-orange); + border-color: var(--theme-graphs-orange); } menuitem.marker-color-graphs-red:before, .marker-color-graphs-red { background-color: var(--theme-graphs-red); + border-color: var(--theme-graphs-red); } menuitem.marker-color-graphs-grey:before, .marker-color-graphs-grey{ background-color: var(--theme-graphs-grey); + border-color: var(--theme-graphs-grey); } /** diff --git a/docshell/base/timeline/AbstractTimelineMarker.cpp b/docshell/base/timeline/AbstractTimelineMarker.cpp index aadaaed17fa..aeeab820783 100644 --- a/docshell/base/timeline/AbstractTimelineMarker.cpp +++ b/docshell/base/timeline/AbstractTimelineMarker.cpp @@ -75,4 +75,16 @@ AbstractTimelineMarker::SetCustomTime(DOMHighResTimeStamp aTime) mTime = aTime; } +void +AbstractTimelineMarker::SetProcessType(GeckoProcessType aProcessType) +{ + mProcessType = aProcessType; +} + +void +AbstractTimelineMarker::SetOffMainThread(bool aIsOffMainThread) +{ + mIsOffMainThread = aIsOffMainThread; +} + } // namespace mozilla diff --git a/docshell/base/timeline/AbstractTimelineMarker.h b/docshell/base/timeline/AbstractTimelineMarker.h index 659bfa60670..16c953c52fb 100644 --- a/docshell/base/timeline/AbstractTimelineMarker.h +++ b/docshell/base/timeline/AbstractTimelineMarker.h @@ -9,6 +9,7 @@ #include "TimelineMarkerEnums.h" // for MarkerTracingType #include "nsDOMNavigationTiming.h" // for DOMHighResTimeStamp +#include "nsXULAppAPI.h" // for GeckoProcessType #include "mozilla/UniquePtr.h" struct JSContext; @@ -63,6 +64,8 @@ protected: void SetCurrentTime(); void SetCustomTime(const TimeStamp& aTime); void SetCustomTime(DOMHighResTimeStamp aTime); + void SetProcessType(GeckoProcessType aProcessType); + void SetOffMainThread(bool aIsOffMainThread); }; } // namespace mozilla diff --git a/docshell/base/timeline/CompositeTimelineMarker.h b/docshell/base/timeline/CompositeTimelineMarker.h new file mode 100644 index 00000000000..571f5d7a30e --- /dev/null +++ b/docshell/base/timeline/CompositeTimelineMarker.h @@ -0,0 +1,33 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim: set ts=8 sts=2 et sw=2 tw=80: */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#ifndef mozilla_CompositeTimelineMarker_h_ +#define mozilla_CompositeTimelineMarker_h_ + +#include "TimelineMarker.h" +#include "mozilla/dom/ProfileTimelineMarkerBinding.h" + +namespace mozilla { + +class CompositeTimelineMarker : public TimelineMarker +{ +public: + explicit CompositeTimelineMarker(const TimeStamp& aTime, + MarkerTracingType aTracingType) + : TimelineMarker("Composite", aTime, aTracingType) + { + // Even though these markers end up being created on the main thread in the + // content or chrome processes, they actually trace down code in the + // compositor parent process. All the information for creating these markers + // is sent along via IPC to an nsView when a composite finishes. + // Mark this as 'off the main thread' to style it differently in frontends. + SetOffMainThread(true); + } +}; + +} // namespace mozilla + +#endif // mozilla_CompositeTimelineMarker_h_ diff --git a/docshell/base/timeline/moz.build b/docshell/base/timeline/moz.build index 869f30dd119..69f126596e6 100644 --- a/docshell/base/timeline/moz.build +++ b/docshell/base/timeline/moz.build @@ -8,6 +8,7 @@ EXPORTS.mozilla += [ 'AbstractTimelineMarker.h', 'AutoGlobalTimelineMarker.h', 'AutoTimelineMarker.h', + 'CompositeTimelineMarker.h', 'ConsoleTimelineMarker.h', 'EventTimelineMarker.h', 'JavascriptTimelineMarker.h', diff --git a/view/nsView.cpp b/view/nsView.cpp index 8661219954f..c5f72f21f50 100644 --- a/view/nsView.cpp +++ b/view/nsView.cpp @@ -19,6 +19,7 @@ #include "nsIWidgetListener.h" #include "nsContentUtils.h" // for nsAutoScriptBlocker #include "mozilla/TimelineConsumers.h" +#include "mozilla/CompositeTimelineMarker.h" using namespace mozilla; @@ -1098,9 +1099,9 @@ nsView::DidCompositeWindow(const TimeStamp& aCompositeStart, if (timelines && timelines->HasConsumer(docShell)) { timelines->AddMarkerForDocShell(docShell, - "Composite", aCompositeStart, MarkerTracingType::START); + MakeUnique(aCompositeStart, MarkerTracingType::START)); timelines->AddMarkerForDocShell(docShell, - "Composite", aCompositeEnd, MarkerTracingType::END); + MakeUnique(aCompositeEnd, MarkerTracingType::END)); } } } From bf76d19fb349fb9adcb3f23f1b642b90dfa8d53c Mon Sep 17 00:00:00 2001 From: Victor Porof Date: Wed, 28 Oct 2015 11:08:13 +0100 Subject: [PATCH 11/30] Bug 1218468 - Remove worker markers on each check in browser_timelineMarkers-02.js, r=me --- .../test/browser/browser_timelineMarkers-frame-02.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/docshell/test/browser/browser_timelineMarkers-frame-02.js b/docshell/test/browser/browser_timelineMarkers-frame-02.js index 25d52933ff2..14528cf6e11 100644 --- a/docshell/test/browser/browser_timelineMarkers-frame-02.js +++ b/docshell/test/browser/browser_timelineMarkers-frame-02.js @@ -11,6 +11,12 @@ function rectangleContains(rect, x, y, width, height) { rect.height >= height; } +function sanitizeMarkers(list) { + // Worker markers are currently gathered from all docshells, which may + // interfere with this test. + return list.filter(e => e.name != "Worker") +} + var TESTS = [{ desc: "Changing the width of the test element", searchFor: "Paint", @@ -19,6 +25,7 @@ var TESTS = [{ div.setAttribute("class", "resize-change-color"); }, check: function(markers) { + markers = sanitizeMarkers(markers); ok(markers.length > 0, "markers were returned"); console.log(markers); info(JSON.stringify(markers.filter(m => m.name == "Paint"))); @@ -40,6 +47,7 @@ var TESTS = [{ div.setAttribute("class", "change-color"); }, check: function(markers) { + markers = sanitizeMarkers(markers); ok(markers.length > 0, "markers were returned"); ok(!markers.some(m => m.name == "Reflow"), "markers doesn't include Reflow"); ok(markers.some(m => m.name == "Paint"), "markers includes Paint"); @@ -59,6 +67,7 @@ var TESTS = [{ div.setAttribute("class", "change-color add-class"); }, check: function(markers) { + markers = sanitizeMarkers(markers); ok(markers.length > 0, "markers were returned"); ok(!markers.some(m => m.name == "Reflow"), "markers doesn't include Reflow"); ok(!markers.some(m => m.name == "Paint"), "markers doesn't include Paint"); @@ -84,6 +93,7 @@ var TESTS = [{ }, 100); }, check: function(markers) { + markers = sanitizeMarkers(markers); is(markers.length, 2, "Got 2 markers"); is(markers[0].name, "ConsoleTime", "Got first ConsoleTime marker"); is(markers[0].causeName, "FOO", "Got ConsoleTime FOO detail"); @@ -105,7 +115,7 @@ var TESTS = [{ content.console.timeStamp(undefined); }, check: function (markers) { - markers = markers.filter(e => e.name != "Worker"); + markers = sanitizeMarkers(markers); is(markers.length, 4, "Got 4 markers"); is(markers[0].name, "TimeStamp", "Got Timestamp marker"); is(markers[0].causeName, "paper", "Got Timestamp label value"); From 95ade4e2f2f3dd21743bf49e642d48a1b505e811 Mon Sep 17 00:00:00 2001 From: Gijs Kruitbosch Date: Mon, 26 Oct 2015 16:29:22 +0100 Subject: [PATCH 12/30] Bug 1214330 - never hide spacer in customize mode to avoid making the footer 'jump' up to the top, r=jaws --- browser/components/customizableui/CustomizeMode.jsm | 3 --- 1 file changed, 3 deletions(-) diff --git a/browser/components/customizableui/CustomizeMode.jsm b/browser/components/customizableui/CustomizeMode.jsm index 0ca513c980c..f9d0b9f94e0 100644 --- a/browser/components/customizableui/CustomizeMode.jsm +++ b/browser/components/customizableui/CustomizeMode.jsm @@ -60,7 +60,6 @@ function CustomizeMode(aWindow) { // to the user when in customizing mode. this.visiblePalette = this.document.getElementById(kPaletteId); this.paletteEmptyNotice = this.document.getElementById("customization-empty"); - this.paletteSpacer = this.document.getElementById("customization-spacer"); this.tipPanel = this.document.getElementById("customization-tipPanel"); if (Services.prefs.getCharPref("general.skins.selectedSkin") != "classic/1.0") { let lwthemeButton = this.document.getElementById("customization-lwtheme-button"); @@ -287,7 +286,6 @@ CustomizeMode.prototype = { this.visiblePalette.clientTop; this.visiblePalette.setAttribute("showing", "true"); }, 0); - this.paletteSpacer.hidden = true; this._updateEmptyPaletteNotice(); this.swatchForTheme(document); @@ -366,7 +364,6 @@ CustomizeMode.prototype = { let documentElement = document.documentElement; // Hide the palette before starting the transition for increased perf. - this.paletteSpacer.hidden = false; this.visiblePalette.hidden = true; this.visiblePalette.removeAttribute("showing"); this.paletteEmptyNotice.hidden = true; From 7d9b520c3aa3d764579f04b94d03307cc384ac73 Mon Sep 17 00:00:00 2001 From: Gijs Kruitbosch Date: Tue, 20 Oct 2015 16:34:30 +0100 Subject: [PATCH 13/30] Bug 1216227 - do bucketed page-load-per-window counts to assess tablet mode usage, r=MattN,p=vladan --- browser/base/content/browser.js | 26 ++++++++++++++++++++ browser/components/nsBrowserGlue.js | 15 ----------- toolkit/components/telemetry/Histograms.json | 11 ++++++--- 3 files changed, 33 insertions(+), 19 deletions(-) diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js index 0911f762e9e..b64f53d20ca 100644 --- a/browser/base/content/browser.js +++ b/browser/base/content/browser.js @@ -1566,6 +1566,8 @@ var gBrowserInit = { TabletModeUpdater.uninit(); + gTabletModePageCounter.finish(); + BrowserOnClick.uninit(); DevEdition.uninit(); @@ -4443,6 +4445,7 @@ var XULBrowserWindow = { BookmarkingUI.onLocationChange(); SocialUI.updateState(location); UITour.onLocationChange(location); + gTabletModePageCounter.inc(); } // Utility functions for disabling find @@ -5440,6 +5443,29 @@ var TabletModeUpdater = { }, }; +var gTabletModePageCounter = { + inc() { + if (!AppConstants.isPlatformAndVersionAtLeast("win", "10.0")) { + this.inc = () => {}; + return; + } + this.inc = this._realInc; + this.inc(); + }, + + _desktopCount: 0, + _tabletCount: 0, + _realInc() { + let inTabletMode = document.documentElement.hasAttribute("tabletmode"); + this[inTabletMode ? "_tabletCount" : "_desktopCount"]++; + }, + + finish() { + Services.telemetry.getKeyedHistogramById("FX_TABLETMODE_PAGE_LOAD").add("tablet", this._tabletCount); + Services.telemetry.getKeyedHistogramById("FX_TABLETMODE_PAGE_LOAD").add("desktop", this._desktopCount); + }, +}; + #ifdef CAN_DRAW_IN_TITLEBAR function updateTitlebarDisplay() { diff --git a/browser/components/nsBrowserGlue.js b/browser/components/nsBrowserGlue.js index a030e421616..08baae6566f 100644 --- a/browser/components/nsBrowserGlue.js +++ b/browser/components/nsBrowserGlue.js @@ -524,12 +524,6 @@ BrowserGlue.prototype = { case "autocomplete-did-enter-text": this._handleURLBarTelemetry(subject.QueryInterface(Ci.nsIAutoCompleteInput)); break; - case "tablet-mode-change": - if (data == "tablet-mode") { - Services.telemetry.getHistogramById("FX_TABLET_MODE_USED_DURING_SESSION") - .add(1); - } - break; case "test-initialize-sanitizer": this._sanitizer.onStartup(); break; @@ -640,7 +634,6 @@ BrowserGlue.prototype = { os.addObserver(this, "flash-plugin-hang", false); os.addObserver(this, "xpi-signature-changed", false); os.addObserver(this, "autocomplete-did-enter-text", false); - os.addObserver(this, "tablet-mode-change", false); ExtensionManagement.registerScript("chrome://browser/content/ext-utils.js"); ExtensionManagement.registerScript("chrome://browser/content/ext-browserAction.js"); @@ -699,7 +692,6 @@ BrowserGlue.prototype = { os.removeObserver(this, "flash-plugin-hang"); os.removeObserver(this, "xpi-signature-changed"); os.removeObserver(this, "autocomplete-did-enter-text"); - os.removeObserver(this, "tablet-mode-change"); }, _onAppDefaults: function BG__onAppDefaults() { @@ -1080,13 +1072,6 @@ BrowserGlue.prototype = { let scaling = aWindow.devicePixelRatio * 100; Services.telemetry.getHistogramById(SCALING_PROBE_NAME).add(scaling); } - -#ifdef XP_WIN - if (WindowsUIUtils.inTabletMode) { - Services.telemetry.getHistogramById("FX_TABLET_MODE_USED_DURING_SESSION") - .add(1); - } -#endif }, // the first browser window has finished initializing diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json index ebc6ef8a37e..345a8e13600 100644 --- a/toolkit/components/telemetry/Histograms.json +++ b/toolkit/components/telemetry/Histograms.json @@ -4352,10 +4352,13 @@ "extended_statistics_ok": true, "description": "Session restore: Time spent blocking the main thread while restoring a window state (ms)" }, - "FX_TABLET_MODE_USED_DURING_SESSION": { - "expires_in_version": "46", - "kind": "count", - "description": "Windows 10+ only: The number of times tablet-mode is used during a session" + "FX_TABLETMODE_PAGE_LOAD": { + "expires_in_version": "47", + "kind": "exponential", + "high": 100000, + "n_buckets": 30, + "keyed": true, + "description": "Number of toplevel location changes in tablet and desktop mode (only used on win10 where tablet mode is available)" }, "FX_TOUCH_USED": { "expires_in_version": "46", From 2a35fb528fe2321cd20625f1e1e72727c9b47eeb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20Qu=C3=A8ze?= Date: Wed, 28 Oct 2015 12:35:05 +0100 Subject: [PATCH 14/30] Bug 1218882 - lz4.js should be usable outside of workers, r=Yoric. --- toolkit/components/{workerlz4 => lz4}/lz4.cpp | 0 toolkit/components/{workerlz4 => lz4}/lz4.js | 41 ++++++++++----- .../{workerlz4 => lz4}/lz4_internal.js | 47 +++++++++++------- .../components/{workerlz4 => lz4}/moz.build | 2 +- .../tests/xpcshell/data/chrome.manifest | 0 .../tests/xpcshell/data/compression.lz | Bin .../tests/xpcshell/data/worker_lz4.js | 4 +- .../tests/xpcshell/test_lz4.js | 0 .../lz4/tests/xpcshell/test_lz4_sync.js | 41 +++++++++++++++ .../tests/xpcshell/xpcshell.ini | 1 + toolkit/components/moz.build | 2 +- .../osfile/modules/osfile_shared_front.jsm | 2 +- 12 files changed, 105 insertions(+), 35 deletions(-) rename toolkit/components/{workerlz4 => lz4}/lz4.cpp (100%) rename toolkit/components/{workerlz4 => lz4}/lz4.js (80%) rename toolkit/components/{workerlz4 => lz4}/lz4_internal.js (57%) rename toolkit/components/{workerlz4 => lz4}/moz.build (94%) rename toolkit/components/{workerlz4 => lz4}/tests/xpcshell/data/chrome.manifest (100%) rename toolkit/components/{workerlz4 => lz4}/tests/xpcshell/data/compression.lz (100%) rename toolkit/components/{workerlz4 => lz4}/tests/xpcshell/data/worker_lz4.js (97%) rename toolkit/components/{workerlz4 => lz4}/tests/xpcshell/test_lz4.js (100%) create mode 100644 toolkit/components/lz4/tests/xpcshell/test_lz4_sync.js rename toolkit/components/{workerlz4 => lz4}/tests/xpcshell/xpcshell.ini (90%) diff --git a/toolkit/components/workerlz4/lz4.cpp b/toolkit/components/lz4/lz4.cpp similarity index 100% rename from toolkit/components/workerlz4/lz4.cpp rename to toolkit/components/lz4/lz4.cpp diff --git a/toolkit/components/workerlz4/lz4.js b/toolkit/components/lz4/lz4.js similarity index 80% rename from toolkit/components/workerlz4/lz4.js rename to toolkit/components/lz4/lz4.js index a1b49f473c4..de77d396201 100644 --- a/toolkit/components/workerlz4/lz4.js +++ b/toolkit/components/lz4/lz4.js @@ -4,15 +4,25 @@ "use strict"; +var SharedAll; +var Primitives; if (typeof Components != "undefined") { - throw new Error("This file is meant to be loaded in a worker"); -} -if (!module || !exports) { - throw new Error("Please load this module with require()"); -} + let Cu = Components.utils; + SharedAll = {}; + Cu.import("resource://gre/modules/osfile/osfile_shared_allthreads.jsm", SharedAll); + Cu.import("resource://gre/modules/lz4_internal.js"); + Cu.import("resource://gre/modules/ctypes.jsm"); -const SharedAll = require("resource://gre/modules/osfile/osfile_shared_allthreads.jsm"); -const Internals = require("resource://gre/modules/workers/lz4_internal.js"); + this.EXPORTED_SYMBOLS = [ + "Lz4" + ]; + this.exports = {}; +} else if (typeof module != "undefined" && typeof require != "undefined") { + SharedAll = require("resource://gre/modules/osfile/osfile_shared_allthreads.jsm"); + Primitives = require("resource://gre/modules/lz4_internal.js"); +} else { + throw new Error("Please load this module with Component.utils.import or with require()"); +} const MAGIC_NUMBER = new Uint8Array([109, 111, 122, 76, 122, 52, 48, 0]); // "mozLz4a\0" @@ -76,12 +86,12 @@ function compressFileContent(array, options = {}) { } else { throw new TypeError("compressFileContent requires a size"); } - let maxCompressedSize = Internals.maxCompressedSize(inputBytes); + let maxCompressedSize = Primitives.maxCompressedSize(inputBytes); let outputArray = new Uint8Array(HEADER_SIZE + maxCompressedSize); // Compress to output array let payload = new Uint8Array(outputArray.buffer, outputArray.byteOffset + HEADER_SIZE); - let compressedSize = Internals.compress(array, inputBytes, payload); + let compressedSize = Primitives.compress(array, inputBytes, payload); // Add headers outputArray.set(MAGIC_NUMBER); @@ -125,12 +135,19 @@ function decompressFileContent(array, options = {}) { let decompressedBytes = (new SharedAll.Type.size_t.implementation(0)); // Decompress - let success = Internals.decompress(inputData, bytes - HEADER_SIZE, - outputBuffer, outputBuffer.byteLength, - decompressedBytes.address()); + let success = Primitives.decompress(inputData, bytes - HEADER_SIZE, + outputBuffer, outputBuffer.byteLength, + decompressedBytes.address()); if (!success) { throw new LZError("decompress", "becauseLZInvalidContent", "Invalid content:Decompression stopped at " + decompressedBytes.value); } return new Uint8Array(outputBuffer.buffer, outputBuffer.byteOffset, decompressedBytes.value); } exports.decompressFileContent = decompressFileContent; + +if (typeof Components != "undefined") { + this.Lz4 = { + compressFileContent: compressFileContent, + decompressFileContent: decompressFileContent + }; +} diff --git a/toolkit/components/workerlz4/lz4_internal.js b/toolkit/components/lz4/lz4_internal.js similarity index 57% rename from toolkit/components/workerlz4/lz4_internal.js rename to toolkit/components/lz4/lz4_internal.js index ff0f5f4e2af..6d3ac357c70 100644 --- a/toolkit/components/workerlz4/lz4_internal.js +++ b/toolkit/components/lz4/lz4_internal.js @@ -4,19 +4,28 @@ "use strict"; +var Primitives = {}; + +var SharedAll; if (typeof Components != "undefined") { - throw new Error("This file is meant to be loaded in a worker"); -} -if (!module || !exports) { - throw new Error("Please load this module with require()"); + let Cu = Components.utils; + SharedAll = {}; + Cu.import("resource://gre/modules/osfile/osfile_shared_allthreads.jsm", SharedAll); + + this.EXPORTED_SYMBOLS = [ + "Primitives" + ]; + this.Primitives = Primitives; + this.exports = {}; +} else if (typeof module != "undefined" && typeof require != "undefined") { + SharedAll = require("resource://gre/modules/osfile/osfile_shared_allthreads.jsm"); +} else { + throw new Error("Please load this module with Component.utils.import or with require()"); } -var SharedAll = require("resource://gre/modules/osfile/osfile_shared_allthreads.jsm"); var libxul = new SharedAll.Library("libxul", SharedAll.Constants.Path.libxul); var Type = SharedAll.Type; -var Primitives = {}; - libxul.declareLazyFFI(Primitives, "compress", "workerlz4_compress", null, @@ -44,14 +53,16 @@ libxul.declareLazyFFI(Primitives, "maxCompressedSize", /*inputSize*/ Type.size_t ); -module.exports = { - get compress() { - return Primitives.compress; - }, - get decompress() { - return Primitives.decompress; - }, - get maxCompressedSize() { - return Primitives.maxCompressedSize; - } -}; +if (typeof module != "undefined") { + module.exports = { + get compress() { + return Primitives.compress; + }, + get decompress() { + return Primitives.decompress; + }, + get maxCompressedSize() { + return Primitives.maxCompressedSize; + } + }; +} diff --git a/toolkit/components/workerlz4/moz.build b/toolkit/components/lz4/moz.build similarity index 94% rename from toolkit/components/workerlz4/moz.build rename to toolkit/components/lz4/moz.build index bae3b55e940..8789681e1cf 100644 --- a/toolkit/components/workerlz4/moz.build +++ b/toolkit/components/lz4/moz.build @@ -6,7 +6,7 @@ XPCSHELL_TESTS_MANIFESTS += ['tests/xpcshell/xpcshell.ini'] -EXTRA_JS_MODULES.workers += [ +EXTRA_JS_MODULES += [ 'lz4.js', 'lz4_internal.js', ] diff --git a/toolkit/components/workerlz4/tests/xpcshell/data/chrome.manifest b/toolkit/components/lz4/tests/xpcshell/data/chrome.manifest similarity index 100% rename from toolkit/components/workerlz4/tests/xpcshell/data/chrome.manifest rename to toolkit/components/lz4/tests/xpcshell/data/chrome.manifest diff --git a/toolkit/components/workerlz4/tests/xpcshell/data/compression.lz b/toolkit/components/lz4/tests/xpcshell/data/compression.lz similarity index 100% rename from toolkit/components/workerlz4/tests/xpcshell/data/compression.lz rename to toolkit/components/lz4/tests/xpcshell/data/compression.lz diff --git a/toolkit/components/workerlz4/tests/xpcshell/data/worker_lz4.js b/toolkit/components/lz4/tests/xpcshell/data/worker_lz4.js similarity index 97% rename from toolkit/components/workerlz4/tests/xpcshell/data/worker_lz4.js rename to toolkit/components/lz4/tests/xpcshell/data/worker_lz4.js index 7ccb76d7b94..312cd69b3f4 100644 --- a/toolkit/components/workerlz4/tests/xpcshell/data/worker_lz4.js +++ b/toolkit/components/lz4/tests/xpcshell/data/worker_lz4.js @@ -44,8 +44,8 @@ self.onmessage = function() { var Lz4; var Internals; function test_import() { - Lz4 = require("resource://gre/modules/workers/lz4.js"); - Internals = require("resource://gre/modules/workers/lz4_internal.js"); + Lz4 = require("resource://gre/modules/lz4.js"); + Internals = require("resource://gre/modules/lz4_internal.js"); } function test_bound() { diff --git a/toolkit/components/workerlz4/tests/xpcshell/test_lz4.js b/toolkit/components/lz4/tests/xpcshell/test_lz4.js similarity index 100% rename from toolkit/components/workerlz4/tests/xpcshell/test_lz4.js rename to toolkit/components/lz4/tests/xpcshell/test_lz4.js diff --git a/toolkit/components/lz4/tests/xpcshell/test_lz4_sync.js b/toolkit/components/lz4/tests/xpcshell/test_lz4_sync.js new file mode 100644 index 00000000000..7f2f21271cc --- /dev/null +++ b/toolkit/components/lz4/tests/xpcshell/test_lz4_sync.js @@ -0,0 +1,41 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +const Cu = Components.utils; +Cu.import("resource://gre/modules/lz4.js"); +Cu.import("resource://gre/modules/osfile.jsm"); + +function run_test() { + run_next_test(); +} + +function compare_arrays(a, b) { + return Array.prototype.join.call(a) == Array.prototype.join.call(a); +} + +add_task(function() { + let path = OS.Path.join("data", "compression.lz"); + let data = yield OS.File.read(path); + let decompressed = Lz4.decompressFileContent(data); + let text = (new TextDecoder()).decode(decompressed); + do_check_eq(text, "Hello, lz4"); +}); + +add_task(function() { + for (let length of [0, 1, 1024]) { + let array = new Uint8Array(length); + for (let i = 0; i < length; ++i) { + array[i] = i % 256; + } + + let compressed = Lz4.compressFileContent(array); + do_print("Compressed " + array.byteLength + " bytes into " + + compressed.byteLength); + + let decompressed = Lz4.decompressFileContent(compressed); + do_print("Decompressed " + compressed.byteLength + " bytes into " + + decompressed.byteLength); + + do_check_true(compare_arrays(array, decompressed)); + } +}); diff --git a/toolkit/components/workerlz4/tests/xpcshell/xpcshell.ini b/toolkit/components/lz4/tests/xpcshell/xpcshell.ini similarity index 90% rename from toolkit/components/workerlz4/tests/xpcshell/xpcshell.ini rename to toolkit/components/lz4/tests/xpcshell/xpcshell.ini index 9e29bf66fd0..9c254155775 100644 --- a/toolkit/components/workerlz4/tests/xpcshell/xpcshell.ini +++ b/toolkit/components/lz4/tests/xpcshell/xpcshell.ini @@ -8,3 +8,4 @@ support-files = data/compression.lz [test_lz4.js] +[test_lz4_sync.js] diff --git a/toolkit/components/moz.build b/toolkit/components/moz.build index c3cf8c5afd8..9efaf006ffd 100644 --- a/toolkit/components/moz.build +++ b/toolkit/components/moz.build @@ -31,6 +31,7 @@ DIRS += [ 'find', 'gfx', 'jsdownloads', + 'lz4', 'mediasniffer', 'microformats', 'osfile', @@ -56,7 +57,6 @@ DIRS += [ 'urlformatter', 'viewconfig', 'workerloader', - 'workerlz4', 'xulstore' ] diff --git a/toolkit/components/osfile/modules/osfile_shared_front.jsm b/toolkit/components/osfile/modules/osfile_shared_front.jsm index e2ab42f689b..4a5f897b9d5 100644 --- a/toolkit/components/osfile/modules/osfile_shared_front.jsm +++ b/toolkit/components/osfile/modules/osfile_shared_front.jsm @@ -18,7 +18,7 @@ var SharedAll = require("resource://gre/modules/osfile/osfile_shared_allthreads.jsm"); var Path = require("resource://gre/modules/osfile/ospath.jsm"); var Lz4 = - require("resource://gre/modules/workers/lz4.js"); + require("resource://gre/modules/lz4.js"); var LOG = SharedAll.LOG.bind(SharedAll, "Shared front-end"); var clone = SharedAll.clone; From 0c6c249d8b3cb51562e7d5de3daa2c1f2be431b0 Mon Sep 17 00:00:00 2001 From: Jeremy Francispillai Date: Wed, 28 Oct 2015 12:38:27 +0100 Subject: [PATCH 15/30] Bug 1173715 - Removed the bottom border off of the search-panel-current-engine element and removed the check to see if the search suggestions is collapsed, r=florian. --- browser/themes/linux/searchbar.css | 10 +++++----- browser/themes/osx/searchbar.css | 10 +++++----- browser/themes/windows/searchbar.css | 10 +++++----- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/browser/themes/linux/searchbar.css b/browser/themes/linux/searchbar.css index a4703ca18a4..67eaa5ca3f1 100644 --- a/browser/themes/linux/searchbar.css +++ b/browser/themes/linux/searchbar.css @@ -85,7 +85,11 @@ menuitem[cmd="cmd_clearhistory"][disabled] { } .search-panel-current-engine { - border-bottom: 1px solid rgba(0, 0, 0, 0.2); + border-bottom: none; +} + +.search-panel-tree { + border-top: 1px solid #ccc !important; } .search-panel-header { @@ -95,10 +99,6 @@ menuitem[cmd="cmd_clearhistory"][disabled] { color: MenuText; } -.search-panel-tree[collapsed=true] + .search-panel-header { - border-top: none; -} - .search-panel-header > label { margin-top: 2px !important; margin-bottom: 1px !important; diff --git a/browser/themes/osx/searchbar.css b/browser/themes/osx/searchbar.css index 92166cb15b6..e374b9a5149 100644 --- a/browser/themes/osx/searchbar.css +++ b/browser/themes/osx/searchbar.css @@ -110,7 +110,11 @@ } .search-panel-current-engine { - border-bottom: 1px solid #ccc; + border-bottom: none; +} + +.search-panel-tree { + border-top: 1px solid #ccc !important; } .search-panel-header { @@ -123,10 +127,6 @@ color: #666; } -.search-panel-tree[collapsed=true] + .search-panel-header { - border-top: none; -} - .search-panel-header > label { margin-top: 2px !important; margin-bottom: 2px !important; diff --git a/browser/themes/windows/searchbar.css b/browser/themes/windows/searchbar.css index acecb3895a2..8adfdaa342c 100644 --- a/browser/themes/windows/searchbar.css +++ b/browser/themes/windows/searchbar.css @@ -118,7 +118,11 @@ } .search-panel-current-engine { - border-bottom: 1px solid #ccc; + border-bottom: none; +} + +.search-panel-tree { + border-top: 1px solid #ccc !important; } .search-panel-header { @@ -130,10 +134,6 @@ color: #666; } -.search-panel-tree[collapsed=true] + .search-panel-header { - border-top: none; -} - .search-panel-header > label { margin-top: 2px !important; margin-bottom: 1px !important; From 7159fdc63d8e2c3b4709b0c32fbb4e7d2912ced6 Mon Sep 17 00:00:00 2001 From: "Carsten \"Tomcat\" Book" Date: Wed, 28 Oct 2015 13:09:36 +0100 Subject: [PATCH 16/30] Backed out changeset 167cd2f19d93 (bug 1219079) --- devtools/client/memory/components/toolbar.js | 11 ++++------- devtools/client/memory/constants.js | 4 ++-- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/devtools/client/memory/components/toolbar.js b/devtools/client/memory/components/toolbar.js index a7ad64d4847..5fd6a7e2274 100644 --- a/devtools/client/memory/components/toolbar.js +++ b/devtools/client/memory/components/toolbar.js @@ -36,13 +36,10 @@ const Toolbar = module.exports = createClass({ DOM.div({ className: "devtools-toolbar" }, [ DOM.button({ className: `take-snapshot devtools-button`, onClick: onTakeSnapshotClick }), - DOM.label({}, - "Breakdown by ", - DOM.select({ - className: `select-breakdown`, - onChange: e => onBreakdownChange(e.target.value), - }, breakdowns.map(({ name, displayName }) => DOM.option({ value: name }, displayName))) - ), + DOM.select({ + className: `select-breakdown`, + onChange: e => onBreakdownChange(e.target.value), + }, breakdowns.map(({ name, displayName }) => DOM.option({ value: name }, displayName))), DOM.label({}, [ DOM.input({ diff --git a/devtools/client/memory/constants.js b/devtools/client/memory/constants.js index 6d8b763874b..06b7b9dc280 100644 --- a/devtools/client/memory/constants.js +++ b/devtools/client/memory/constants.js @@ -52,14 +52,14 @@ const breakdowns = exports.breakdowns = { breakdown: { by: "coarseType", objects: ALLOCATION_STACK, - strings: COUNT, + strings: ALLOCATION_STACK, scripts: INTERNAL_TYPE, other: INTERNAL_TYPE, } }, allocationStack: { - displayName: "Allocation Stack", + displayName: "Allocation Site", breakdown: ALLOCATION_STACK, }, From ed54abad6ebb2309d7eb5d5435d013e75e2bb020 Mon Sep 17 00:00:00 2001 From: "Carsten \"Tomcat\" Book" Date: Wed, 28 Oct 2015 13:09:38 +0100 Subject: [PATCH 17/30] Backed out changeset 2e2f56672850 (bug 1218560) --- devtools/client/memory/initializer.js | 1 - .../client/memory/test/browser/browser.ini | 2 - ...wser_memory_allocationStackBreakdown_01.js | 39 ------------------- 3 files changed, 42 deletions(-) delete mode 100644 devtools/client/memory/test/browser/browser_memory_allocationStackBreakdown_01.js diff --git a/devtools/client/memory/initializer.js b/devtools/client/memory/initializer.js index 0d2f63beb31..143c0647d9b 100644 --- a/devtools/client/memory/initializer.js +++ b/devtools/client/memory/initializer.js @@ -38,6 +38,5 @@ function initialize () { } function destroy () { - gRoot = gStore = gApp = gProvider = null; return Task.spawn(function*(){}); } diff --git a/devtools/client/memory/test/browser/browser.ini b/devtools/client/memory/test/browser/browser.ini index ba4db15d199..79c70b80e08 100644 --- a/devtools/client/memory/test/browser/browser.ini +++ b/devtools/client/memory/test/browser/browser.ini @@ -3,9 +3,7 @@ tags = devtools subsuite = devtools support-files = head.js - doc_steady_allocation.html -[browser_memory_allocationStackBreakdown_01.js] [browser_memory-breakdowns-01.js] [browser_memory-simple-01.js] [browser_memory_transferHeapSnapshot_e10s_01.js] diff --git a/devtools/client/memory/test/browser/browser_memory_allocationStackBreakdown_01.js b/devtools/client/memory/test/browser/browser_memory_allocationStackBreakdown_01.js deleted file mode 100644 index 1b8f6d25271..00000000000 --- a/devtools/client/memory/test/browser/browser_memory_allocationStackBreakdown_01.js +++ /dev/null @@ -1,39 +0,0 @@ -/* Any copyright is dedicated to the Public Domain. - http://creativecommons.org/publicdomain/zero/1.0/ */ - -// Sanity test that we can show allocation stack breakdowns in the tree. - -"use strict"; - -const { waitForTime } = require("devtools/shared/DevToolsUtils"); -const { breakdowns } = require("devtools/client/memory/constants"); -const { toggleRecordingAllocationStacks } = require("devtools/client/memory/actions/allocations"); -const { takeSnapshotAndCensus } = require("devtools/client/memory/actions/snapshot"); -const breakdownActions = require("devtools/client/memory/actions/breakdown"); -const { toggleInverted } = require("devtools/client/memory/actions/inverted"); - -const TEST_URL = "http://example.com/browser/devtools/client/memory/test/browser/doc_steady_allocation.html"; - -this.test = makeMemoryTest(TEST_URL, function* ({ tab, panel }) { - const heapWorker = panel.panelWin.gHeapAnalysesClient; - const front = panel.panelWin.gFront; - const { getState, dispatch } = panel.panelWin.gStore; - - dispatch(toggleInverted()); - ok(getState().inverted, true); - - dispatch(breakdownActions.setBreakdown(breakdowns.allocationStack.breakdown)); - is(getState().breakdown.by, "allocationStack"); - - yield dispatch(toggleRecordingAllocationStacks(front)); - ok(getState().allocations.recording); - - // Let some allocations build up. - yield waitForTime(500); - - yield dispatch(takeSnapshotAndCensus(front, heapWorker)); - - const doc = panel.panelWin.document; - ok(doc.querySelector(".heap-tree-item-function-display-name"), - "Should have rendered some allocation stack tree items"); -}); From 5b7afdaa384e62e256e1d4f0713db25523054f2e Mon Sep 17 00:00:00 2001 From: "Carsten \"Tomcat\" Book" Date: Wed, 28 Oct 2015 13:09:40 +0100 Subject: [PATCH 18/30] Backed out changeset 2e4f2de05673 (bug 1219066) --- devtools/shared/heapsnapshot/HeapSnapshot.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/devtools/shared/heapsnapshot/HeapSnapshot.cpp b/devtools/shared/heapsnapshot/HeapSnapshot.cpp index af1b74085dc..52efff82bb6 100644 --- a/devtools/shared/heapsnapshot/HeapSnapshot.cpp +++ b/devtools/shared/heapsnapshot/HeapSnapshot.cpp @@ -55,11 +55,9 @@ using JS::ubi::AtomOrTwoByteChars; NS_IMPL_CYCLE_COLLECTION_CLASS(HeapSnapshot) NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(HeapSnapshot) - NS_IMPL_CYCLE_COLLECTION_UNLINK(mParent) NS_IMPL_CYCLE_COLLECTION_UNLINK_END NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(HeapSnapshot) - NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mParent) NS_IMPL_CYCLE_COLLECTION_TRAVERSE_SCRIPT_OBJECTS NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END From 7eb09f8840e5695628e54516a9f8d4d510a865df Mon Sep 17 00:00:00 2001 From: "Carsten \"Tomcat\" Book" Date: Wed, 28 Oct 2015 13:09:57 +0100 Subject: [PATCH 19/30] Backed out changeset 5f17f4325f31 (bug 1218679) for memory leaks on a CLOSED TREE --- devtools/client/memory/constants.js | 3 - devtools/client/memory/initializer.js | 18 ++---- .../client/memory/test/browser/browser.ini | 2 - .../browser/browser_memory-breakdowns-01.js | 33 ---------- .../test/browser/browser_memory-simple-01.js | 35 ----------- .../test/browser/doc_steady_allocation.html | 16 ----- devtools/client/memory/test/browser/head.js | 60 ------------------- 7 files changed, 5 insertions(+), 162 deletions(-) delete mode 100644 devtools/client/memory/test/browser/browser_memory-breakdowns-01.js delete mode 100644 devtools/client/memory/test/browser/browser_memory-simple-01.js delete mode 100644 devtools/client/memory/test/browser/doc_steady_allocation.html diff --git a/devtools/client/memory/constants.js b/devtools/client/memory/constants.js index 06b7b9dc280..e5ae8b86a16 100644 --- a/devtools/client/memory/constants.js +++ b/devtools/client/memory/constants.js @@ -29,9 +29,6 @@ actions.SELECT_SNAPSHOT = "select-snapshot"; // Fired to toggle tree inversion on or off. actions.TOGGLE_INVERTED = "toggle-inverted"; -// Fired to set a new breakdown. -actions.SET_BREAKDOWN = "set-breakdown"; - // Fired when there is an error processing a snapshot or taking a census. actions.SNAPSHOT_ERROR = "snapshot-error"; diff --git a/devtools/client/memory/initializer.js b/devtools/client/memory/initializer.js index 143c0647d9b..8b624556513 100644 --- a/devtools/client/memory/initializer.js +++ b/devtools/client/memory/initializer.js @@ -19,21 +19,13 @@ const Store = require("devtools/client/memory/store"); */ var gToolbox, gTarget, gFront, gHeapAnalysesClient; -/** - * Globals set by initialize() - */ -var gRoot, gStore, gApp, gProvider; - function initialize () { return Task.spawn(function*() { - gRoot = document.querySelector("#app"); - gStore = Store(); - gApp = createElement(App, { - front: gFront, - heapWorker: gHeapAnalysesClient - }); - gProvider = createElement(Provider, { store: gStore }, gApp); - render(gProvider, gRoot); + let root = document.querySelector("#app"); + let store = Store(); + let app = createElement(App, { front: gFront, heapWorker: gHeapAnalysesClient }); + let provider = createElement(Provider, { store }, app); + render(provider, root); }); } diff --git a/devtools/client/memory/test/browser/browser.ini b/devtools/client/memory/test/browser/browser.ini index 79c70b80e08..48a2e9e0c60 100644 --- a/devtools/client/memory/test/browser/browser.ini +++ b/devtools/client/memory/test/browser/browser.ini @@ -4,6 +4,4 @@ subsuite = devtools support-files = head.js -[browser_memory-breakdowns-01.js] -[browser_memory-simple-01.js] [browser_memory_transferHeapSnapshot_e10s_01.js] diff --git a/devtools/client/memory/test/browser/browser_memory-breakdowns-01.js b/devtools/client/memory/test/browser/browser_memory-breakdowns-01.js deleted file mode 100644 index 8701ef867e3..00000000000 --- a/devtools/client/memory/test/browser/browser_memory-breakdowns-01.js +++ /dev/null @@ -1,33 +0,0 @@ -/* Any copyright is dedicated to the Public Domain. - http://creativecommons.org/publicdomain/zero/1.0/ */ - -/** - * Tests that the heap tree renders rows based on the breakdown - */ - -const TEST_URL = "http://example.com/browser/devtools/client/memory/test/browser/doc_steady_allocation.html"; - -this.test = makeMemoryTest(TEST_URL, function* ({ tab, panel }) { - const { gStore, document } = panel.panelWin; - const $$ = document.querySelectorAll.bind(document); - - yield takeSnapshot(panel.panelWin); - - yield waitUntilSnapshotState(gStore, [states.SAVED_CENSUS]); - - info("Check coarse type heap view"); - ["objects", "other", "scripts", "strings"].forEach(findNameCell); - - yield setBreakdown(panel.panelWin, "objectClass"); - info("Check object class heap view"); - ["Function", "Object"].forEach(findNameCell); - - yield setBreakdown(panel.panelWin, "internalType"); - info("Check internal type heap view"); - ["JSObject"].forEach(findNameCell); - - function findNameCell (name) { - let el = Array.prototype.find.call($$(".tree .heap-tree-item-name span"), el => el.textContent === name); - ok(el, `Found heap tree item cell for ${name}.`); - } -}); diff --git a/devtools/client/memory/test/browser/browser_memory-simple-01.js b/devtools/client/memory/test/browser/browser_memory-simple-01.js deleted file mode 100644 index 7a52c313964..00000000000 --- a/devtools/client/memory/test/browser/browser_memory-simple-01.js +++ /dev/null @@ -1,35 +0,0 @@ -/* Any copyright is dedicated to the Public Domain. - http://creativecommons.org/publicdomain/zero/1.0/ */ - -/** - * Tests taking snapshots and default states. - */ - -const TEST_URL = "http://example.com/browser/devtools/client/memory/test/browser/doc_steady_allocation.html"; - -this.test = makeMemoryTest(TEST_URL, function* ({ tab, panel }) { - const { gStore, document } = panel.panelWin; - const { getState, dispatch } = gStore; - - let snapshotEls = document.querySelectorAll("#memory-tool-container .list li"); - is(getState().snapshots.length, 0, "Starts with no snapshots in store"); - is(snapshotEls.length, 0, "No snapshots rendered"); - - yield takeSnapshot(panel.panelWin); - snapshotEls = document.querySelectorAll("#memory-tool-container .list li"); - is(getState().snapshots.length, 1, "One snapshot was created in store"); - is(snapshotEls.length, 1, "One snapshot was rendered"); - ok(snapshotEls[0].classList.contains("selected"), "Only snapshot has `selected` class"); - - yield takeSnapshot(panel.panelWin); - snapshotEls = document.querySelectorAll("#memory-tool-container .list li"); - is(getState().snapshots.length, 2, "Two snapshots created in store"); - is(snapshotEls.length, 2, "Two snapshots rendered"); - ok(!snapshotEls[0].classList.contains("selected"), "First snapshot no longer has `selected` class"); - ok(snapshotEls[1].classList.contains("selected"), "Second snapshot has `selected` class"); - - yield waitUntilSnapshotState(gStore, [states.SAVED_CENSUS, states.SAVED_CENSUS]); - - ok(document.querySelector(".heap-tree-item-name"), - "Should have rendered some tree items"); -}); diff --git a/devtools/client/memory/test/browser/doc_steady_allocation.html b/devtools/client/memory/test/browser/doc_steady_allocation.html deleted file mode 100644 index 65703c878f8..00000000000 --- a/devtools/client/memory/test/browser/doc_steady_allocation.html +++ /dev/null @@ -1,16 +0,0 @@ - - - - - - diff --git a/devtools/client/memory/test/browser/head.js b/devtools/client/memory/test/browser/head.js index 77579c92f8d..6b6d5267d1b 100644 --- a/devtools/client/memory/test/browser/head.js +++ b/devtools/client/memory/test/browser/head.js @@ -8,9 +8,6 @@ Services.scriptloader.loadSubScript( "chrome://mochitests/content/browser/devtools/client/framework/test/shared-head.js", this); -var { snapshotState: states } = require("devtools/client/memory/constants"); -var { breakdownEquals, breakdownNameToSpec } = require("devtools/client/memory/utils"); - Services.prefs.setBoolPref("devtools.memory.enabled", true); /** @@ -66,60 +63,3 @@ function makeMemoryTest(url, generator) { finish(); }); } - - -function waitUntilState (store, predicate) { - let deferred = promise.defer(); - let unsubscribe = store.subscribe(check); - - function check () { - if (predicate(store.getState())) { - unsubscribe(); - deferred.resolve() - } - } - - // Fire the check immediately incase the action has already occurred - check(); - - return deferred.promise; -} - -function waitUntilSnapshotState (store, expected) { - let predicate = () => { - let snapshots = store.getState().snapshots; - info(snapshots.map(x => x.state)); - return snapshots.length === expected.length && - expected.every((state, i) => state === "*" || snapshots[i].state === state); - }; - info(`Waiting for snapshots to be of state: ${expected}`); - return waitUntilState(store, predicate); -} - -function takeSnapshot (window) { - let { gStore, document } = window; - let snapshotCount = gStore.getState().snapshots.length; - info(`Taking snapshot...`); - document.querySelector(".devtools-toolbar .take-snapshot").click(); - return waitUntilState(gStore, () => gStore.getState().snapshots.length === snapshotCount + 1); -} - -/** - * Sets breakdown and waits for currently selected breakdown to use it - * and be completed the census. - */ -function setBreakdown (window, type) { - info(`Setting breakdown to ${type}...`); - let { gStore, gHeapAnalysesClient } = window; - // XXX: Should handle this via clicking the DOM, but React doesn't - // fire the onChange event, so just change it in the store. - // window.document.querySelector(`.select-breakdown`).value = type; - gStore.dispatch(require("devtools/client/memory/actions/breakdown") - .setBreakdownAndRefresh(gHeapAnalysesClient, breakdownNameToSpec(type))); - - return waitUntilState(window.gStore, () => { - let selected = window.gStore.getState().snapshots.find(s => s.selected); - return selected.state === states.SAVED_CENSUS && - breakdownEquals(breakdownNameToSpec(type), selected.breakdown); - }); -} From e644940cbc7003cfabb956cc910a5b0cfb9a2171 Mon Sep 17 00:00:00 2001 From: Paolo Amadini Date: Tue, 27 Oct 2015 14:24:51 +0000 Subject: [PATCH 20/30] Bug 1207089 - Telemetry for permission notifications. r=MattN,vladan --- toolkit/components/telemetry/Histograms.json | 34 +++- toolkit/content/widgets/notification.xml | 4 +- toolkit/modules/PopupNotifications.jsm | 165 +++++++++++++++++-- 3 files changed, 179 insertions(+), 24 deletions(-) diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json index 345a8e13600..fc3c79c042b 100644 --- a/toolkit/components/telemetry/Histograms.json +++ b/toolkit/components/telemetry/Histograms.json @@ -5708,13 +5708,33 @@ "kind": "boolean", "description": "Count the number of times the user clicked 'allow' on the hidden-plugin infobar." }, - "POPUP_NOTIFICATION_MAINACTION_TRIGGERED_MS": { - "expires_in_version": "40", - "kind": "linear", - "low": 25, - "high": "80 * 25", - "n_buckets": "80 + 1", - "description": "The time (in milliseconds) after showing a PopupNotification that the mainAction was first triggered" + "POPUP_NOTIFICATION_STATS": { + "alert_emails": ["firefox-dev@mozilla.org"], + "expires_in_version": "48", + "kind": "enumerated", + "keyed": true, + "n_values": 40, + "description": "(Bug 1207089) Usage of popup notifications, keyed by ID (0 = Offered, 1..4 = Action, 5 = Click outside, 6 = Leave page, 7 = Use 'X', 8 = Not now, 10 = Open submenu, 11 = Learn more. Add 20 if happened after reopen.)" + }, + "POPUP_NOTIFICATION_MAIN_ACTION_MS": { + "alert_emails": ["firefox-dev@mozilla.org"], + "expires_in_version": "48", + "kind": "exponential", + "keyed": true, + "low": 100, + "high": 600000, + "n_buckets": 40, + "description": "(Bug 1207089) Time in ms between initially requesting a popup notification and triggering the main action, keyed by ID" + }, + "POPUP_NOTIFICATION_DISMISSAL_MS": { + "alert_emails": ["firefox-dev@mozilla.org"], + "expires_in_version": "48", + "kind": "exponential", + "keyed": true, + "low": 200, + "high": 20000, + "n_buckets": 50, + "description": "(Bug 1207089) Time in ms between displaying a popup notification and dismissing it without an action the first time, keyed by ID" }, "DEVTOOLS_DEBUGGER_RDP_LOCAL_RELOAD_MS": { "expires_in_version": "never", diff --git a/toolkit/content/widgets/notification.xml b/toolkit/content/widgets/notification.xml index d6766a76a6c..d5519e94bcf 100644 --- a/toolkit/content/widgets/notification.xml +++ b/toolkit/content/widgets/notification.xml @@ -492,7 +492,7 @@ &learnMore; + xbl:inherits="onclick=learnmoreclick,href=learnmoreurl">&learnMore; @@ -500,7 +500,7 @@ + xbl:inherits="oncommand=buttoncommand,onpopupshown=buttonpopupshown,label=buttonlabel,accesskey=buttonaccesskey"> diff --git a/toolkit/modules/PopupNotifications.jsm b/toolkit/modules/PopupNotifications.jsm index 729cf09d305..d71588b6f7f 100644 --- a/toolkit/modules/PopupNotifications.jsm +++ b/toolkit/modules/PopupNotifications.jsm @@ -7,6 +7,7 @@ this.EXPORTED_SYMBOLS = ["PopupNotifications"]; var Cc = Components.classes, Ci = Components.interfaces, Cu = Components.utils; Cu.import("resource://gre/modules/Services.jsm"); +Cu.import("resource://gre/modules/PrivateBrowsingUtils.jsm"); Cu.import("resource://gre/modules/Promise.jsm"); const NOTIFICATION_EVENT_DISMISSED = "dismissed"; @@ -21,6 +22,21 @@ const ICON_ANCHOR_ATTRIBUTE = "popupnotificationanchor"; const PREF_SECURITY_DELAY = "security.notification_enable_delay"; +// Enumerated values for the POPUP_NOTIFICATION_STATS telemetry histogram. +const TELEMETRY_STAT_OFFERED = 0; +const TELEMETRY_STAT_ACTION_1 = 1; +const TELEMETRY_STAT_ACTION_2 = 2; +const TELEMETRY_STAT_ACTION_3 = 3; +const TELEMETRY_STAT_ACTION_LAST = 4; +const TELEMETRY_STAT_DISMISSAL_CLICK_ELSEWHERE = 5; +const TELEMETRY_STAT_DISMISSAL_LEAVE_PAGE = 6; +const TELEMETRY_STAT_DISMISSAL_CLOSE_BUTTON = 7; +const TELEMETRY_STAT_DISMISSAL_NOT_NOW = 8; +const TELEMETRY_STAT_OPEN_SUBMENU = 10; +const TELEMETRY_STAT_LEARN_MORE = 11; + +const TELEMETRY_STAT_REOPENED_OFFSET = 20; + var popupNotificationsMap = new WeakMap(); var gNotificationParents = new WeakMap; @@ -54,6 +70,13 @@ function Notification(id, message, anchorID, mainAction, secondaryActions, this.browser = browser; this.owner = owner; this.options = options || {}; + + this._dismissed = false; + this.wasDismissed = false; + this.recordedTelemetryStats = new Set(); + this.isPrivate = PrivateBrowsingUtils.isWindowPrivate( + this.browser.ownerDocument.defaultView); + this.timeCreated = this.owner.window.performance.now(); } Notification.prototype = { @@ -68,6 +91,20 @@ Notification.prototype = { options: null, timeShown: null, + /** + * Indicates whether the notification is currently dismissed. + */ + set dismissed(value) { + this._dismissed = value; + if (value) { + // Keep the dismissal into account when recording telemetry. + this.wasDismissed = true; + } + }, + get dismissed() { + return this._dismissed; + }, + /** * Removes the notification and updates the popup accordingly if needed. */ @@ -95,7 +132,45 @@ Notification.prototype = { reshow: function() { this.owner._reshowNotifications(this.anchorElement, this.browser); - } + }, + + /** + * Adds a value to the specified histogram, that must be keyed by ID. + */ + _recordTelemetry(histogramId, value) { + if (this.isPrivate) { + // The reason why we don't record telemetry in private windows is because + // the available actions can be different from regular mode. The main + // difference is that all of the persistent permission options like + // "Always remember" aren't there, so they really need to be handled + // separately to avoid skewing results. For notifications with the same + // choices, there would be no reason not to record in private windows as + // well, but it's just simpler to use the same check for everything. + return; + } + let histogram = Services.telemetry.getKeyedHistogramById(histogramId); + histogram.add("(all)", value); + histogram.add(this.id, value); + }, + + /** + * Adds an enumerated value to the POPUP_NOTIFICATION_STATS histogram, + * ensuring that it is recorded at most once for each distinct Notification. + * + * Statistics for reopened notifications are recorded in separate buckets. + * + * @param value + * One of the TELEMETRY_STAT_ constants. + */ + _recordTelemetryStat(value) { + if (this.wasDismissed) { + value += TELEMETRY_STAT_REOPENED_OFFSET; + } + if (!this.recordedTelemetryStats.has(value)) { + this.recordedTelemetryStats.add(value); + this._recordTelemetry("POPUP_NOTIFICATION_STATS", value); + } + }, }; /** @@ -416,6 +491,12 @@ PopupNotifications.prototype = { case "activate": case "TabSelect": let self = this; + // This is where we could detect if the panel is dismissed if the page + // was switched. Unfortunately, the user usually has clicked elsewhere + // at this point so this value only gets recorded for programmatic + // reasons, like the "Learn More" link being clicked and resulting in a + // tab switch. + this.nextDismissReason = TELEMETRY_STAT_DISMISSAL_LEAVE_PAGE; // setTimeout(..., 0) needed, otherwise openPopup from "activate" event // handler results in the popup being hidden again for some reason... this.window.setTimeout(function () { @@ -465,7 +546,11 @@ PopupNotifications.prototype = { /** * Dismisses the notification without removing it. */ - _dismiss: function PopupNotifications_dismiss() { + _dismiss: function PopupNotifications_dismiss(telemetryReason) { + if (telemetryReason) { + this.nextDismissReason = telemetryReason; + } + let browser = this.panel.firstChild && this.panel.firstChild.notification.browser; this.panel.hidePopup(); @@ -546,17 +631,21 @@ PopupNotifications.prototype = { popupnotification.setAttribute("label", n.message); popupnotification.setAttribute("id", popupnotificationID); popupnotification.setAttribute("popupid", n.id); - popupnotification.setAttribute("closebuttoncommand", "PopupNotifications._dismiss();"); + popupnotification.setAttribute("closebuttoncommand", `PopupNotifications._dismiss(${TELEMETRY_STAT_DISMISSAL_CLOSE_BUTTON});`); if (n.mainAction) { popupnotification.setAttribute("buttonlabel", n.mainAction.label); popupnotification.setAttribute("buttonaccesskey", n.mainAction.accessKey); - popupnotification.setAttribute("buttoncommand", "PopupNotifications._onButtonCommand(event);"); + popupnotification.setAttribute("buttoncommand", "PopupNotifications._onButtonEvent(event, 'buttoncommand');"); + popupnotification.setAttribute("buttonpopupshown", "PopupNotifications._onButtonEvent(event, 'buttonpopupshown');"); + popupnotification.setAttribute("learnmoreclick", "PopupNotifications._onButtonEvent(event, 'learnmoreclick');"); popupnotification.setAttribute("menucommand", "PopupNotifications._onMenuCommand(event);"); - popupnotification.setAttribute("closeitemcommand", "PopupNotifications._dismiss();event.stopPropagation();"); + popupnotification.setAttribute("closeitemcommand", `PopupNotifications._dismiss(${TELEMETRY_STAT_DISMISSAL_NOT_NOW});event.stopPropagation();`); } else { popupnotification.removeAttribute("buttonlabel"); popupnotification.removeAttribute("buttonaccesskey"); popupnotification.removeAttribute("buttoncommand"); + popupnotification.removeAttribute("buttonpopupshown"); + popupnotification.removeAttribute("learnmoreclick"); popupnotification.removeAttribute("menucommand"); popupnotification.removeAttribute("closeitemcommand"); } @@ -588,6 +677,8 @@ PopupNotifications.prototype = { popupnotification.notification = n; if (n.secondaryActions) { + let telemetryStatId = TELEMETRY_STAT_ACTION_2; + n.secondaryActions.forEach(function (a) { let item = doc.createElementNS(XUL_NS, "menuitem"); item.setAttribute("label", a.label); @@ -596,6 +687,13 @@ PopupNotifications.prototype = { item.action = a; popupnotification.appendChild(item); + + // We can only record a limited number of actions in telemetry. If + // there are more, the latest are all recorded in the last bucket. + item.action.telemetryStatId = telemetryStatId; + if (telemetryStatId < TELEMETRY_STAT_ACTION_LAST) { + telemetryStatId++; + } }, this); if (n.options.hideNotNow) { @@ -658,9 +756,18 @@ PopupNotifications.prototype = { // click-to-play plugins, so copy the popupid and use css. this.panel.setAttribute("popupid", this.panel.firstChild.getAttribute("popupid")); notificationsToShow.forEach(function (n) { + // Record that the notification was actually displayed on screen. + // Notifications that were opened a second time or that were originally + // shown with "options.dismissed" will be recorded in a separate bucket. + n._recordTelemetryStat(TELEMETRY_STAT_OFFERED); // Remember the time the notification was shown for the security delay. n.timeShown = this.window.performance.now(); }, this); + + // Unless the panel closing is triggered by a specific known code path, + // the next reason will be that the user clicked elsewhere. + this.nextDismissReason = TELEMETRY_STAT_DISMISSAL_CLICK_ELSEWHERE; + this.panel.openPopup(anchorElement, "bottomcenter topleft"); notificationsToShow.forEach(function (n) { this._fireCallback(n, NOTIFICATION_EVENT_SHOWN); @@ -979,6 +1086,16 @@ PopupNotifications.prototype = { if (notifications.indexOf(notificationObj) == -1) return; + // Record the time of the first notification dismissal if the main action + // was not triggered in the meantime. + let timeSinceShown = this.window.performance.now() - notificationObj.timeShown; + if (!notificationObj.wasDismissed && + !notificationObj.recordedTelemetryMainAction) { + notificationObj._recordTelemetry("POPUP_NOTIFICATION_DISMISSAL_MS", + timeSinceShown); + } + notificationObj._recordTelemetryStat(this.nextDismissReason); + // Do not mark the notification as dismissed or fire NOTIFICATION_EVENT_DISMISSED // if the notification is removed. if (notificationObj.options.removeOnDismissal) { @@ -990,7 +1107,7 @@ PopupNotifications.prototype = { }, this); }, - _onButtonCommand: function PopupNotifications_onButtonCommand(event) { + _onButtonEvent(event, type) { // Need to find the associated notification object, which is a bit tricky // since it isn't associated with the button directly - this is kind of // gross and very dependent on the structure of the popupnotification @@ -1002,27 +1119,42 @@ PopupNotifications.prototype = { notificationEl = parent; if (!notificationEl) - throw "PopupNotifications_onButtonCommand: couldn't find notification element"; + throw "PopupNotifications._onButtonEvent: couldn't find notification element"; if (!notificationEl.notification) - throw "PopupNotifications_onButtonCommand: couldn't find notification"; + throw "PopupNotifications._onButtonEvent: couldn't find notification"; let notification = notificationEl.notification; - let timeSinceShown = this.window.performance.now() - notification.timeShown; - // Only report the first time mainAction is triggered and remember that this occurred. - if (!notification.timeMainActionFirstTriggered) { - notification.timeMainActionFirstTriggered = timeSinceShown; - Services.telemetry.getHistogramById("POPUP_NOTIFICATION_MAINACTION_TRIGGERED_MS"). - add(timeSinceShown); + if (type == "buttonpopupshown") { + notification._recordTelemetryStat(TELEMETRY_STAT_OPEN_SUBMENU); + return; } + if (type == "learnmoreclick") { + notification._recordTelemetryStat(TELEMETRY_STAT_LEARN_MORE); + return; + } + + // Record the total timing of the main action since the notification was + // created, even if the notification was dismissed in the meantime. + let timeSinceCreated = this.window.performance.now() - notification.timeCreated; + if (!notification.recordedTelemetryMainAction) { + notification.recordedTelemetryMainAction = true; + notification._recordTelemetry("POPUP_NOTIFICATION_MAIN_ACTION_MS", + timeSinceCreated); + } + + let timeSinceShown = this.window.performance.now() - notification.timeShown; if (timeSinceShown < this.buttonDelay) { - Services.console.logStringMessage("PopupNotifications_onButtonCommand: " + + Services.console.logStringMessage("PopupNotifications._onButtonEvent: " + "Button click happened before the security delay: " + timeSinceShown + "ms"); return; } + + notification._recordTelemetryStat(TELEMETRY_STAT_ACTION_1); + try { notification.mainAction.callback.call(); } catch(error) { @@ -1044,6 +1176,9 @@ PopupNotifications.prototype = { throw "menucommand target has no associated action/notification"; event.stopPropagation(); + + target.notification._recordTelemetryStat(target.action.telemetryStatId); + try { target.action.callback.call(); } catch(error) { From 9f2c1329432fc106d63174685021d6d5cf2900f0 Mon Sep 17 00:00:00 2001 From: Ed Lee Date: Thu, 22 Oct 2015 23:28:44 -0700 Subject: [PATCH 21/30] Bug 1216918 - Style the conversation window title bar in blue [r=mikedeboer] --- browser/themes/shared/social/chat-icons.svg | 2 ++ browser/themes/shared/social/chat.inc.css | 17 +++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/browser/themes/shared/social/chat-icons.svg b/browser/themes/shared/social/chat-icons.svg index 766b974224a..c9b3ebf1ec9 100644 --- a/browser/themes/shared/social/chat-icons.svg +++ b/browser/themes/shared/social/chat-icons.svg @@ -42,8 +42,10 @@ + + diff --git a/browser/themes/shared/social/chat.inc.css b/browser/themes/shared/social/chat.inc.css index 67db2955647..2d32b8ed54a 100644 --- a/browser/themes/shared/social/chat.inc.css +++ b/browser/themes/shared/social/chat.inc.css @@ -105,6 +105,14 @@ chatbar > chatbox > .chat-titlebar > .chat-swap-button { transform: none; } +chatbox[src^="about:loopconversation#"] .chat-minimize-button { + list-style-image: url("chrome://browser/skin/social/chat-icons.svg#minimize-white"); +} + +chatbox[src^="about:loopconversation#"] .chat-swap-button { + list-style-image: url("chrome://browser/skin/social/chat-icons.svg#expand-white"); +} + .chat-loop-hangup { list-style-image: url("chrome://browser/skin/social/chat-icons.svg#exit-white"); background-color: #d13f1a; @@ -129,6 +137,10 @@ chatbar > chatbox > .chat-titlebar > .chat-swap-button { cursor: inherit; } +chatbox[src^="about:loopconversation#"] .chat-title { + color: white; +} + .chat-titlebar { height: 26px; min-height: 26px; @@ -147,6 +159,11 @@ chatbar > chatbox > .chat-titlebar > .chat-swap-button { background-color: #f0f0f0; } +chatbox[src^="about:loopconversation#"] > .chat-titlebar { + background-color: #00a9dc; + border-color: #00a9dc; +} + .chat-titlebar > .notification-anchor-icon { margin-left: 2px; margin-right: 2px; From bf105cbd5be7e2ebaf9dfa2f2d920448da54d516 Mon Sep 17 00:00:00 2001 From: Philip Chee Date: Wed, 28 Oct 2015 09:16:44 -0700 Subject: [PATCH 22/30] Bug 1217985 Follow up fix test bustage --- toolkit/mozapps/update/tests/TestAUSHelper.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/toolkit/mozapps/update/tests/TestAUSHelper.cpp b/toolkit/mozapps/update/tests/TestAUSHelper.cpp index a13ce3b16fe..8d207a74259 100644 --- a/toolkit/mozapps/update/tests/TestAUSHelper.cpp +++ b/toolkit/mozapps/update/tests/TestAUSHelper.cpp @@ -217,7 +217,7 @@ int NS_main(int argc, NS_tchar **argv) } if (!NS_tstrcmp(argv[1], NS_T("check-signature"))) { -#ifdef XP_WIN +#if defined(XP_WIN) && defined(MOZ_MAINTENANCE_SERVICE) if (ERROR_SUCCESS == VerifyCertificateTrustForFile(argv[2])) { return 0; } else { From ef7167d045fad4484b9696dcfb9e63651ac893d2 Mon Sep 17 00:00:00 2001 From: David Critchley Date: Wed, 28 Oct 2015 09:35:56 -0700 Subject: [PATCH 23/30] Bug 1213851 - Display only active room when user enters room [r=Mardak] --- browser/components/loop/content/css/panel.css | 8 +- browser/components/loop/content/js/panel.js | 45 ++++-- browser/components/loop/content/js/panel.jsx | 45 ++++-- .../loop/test/desktop-local/panel_test.js | 140 +++++++++++------- .../en-US/chrome/browser/loop/loop.properties | 7 +- 5 files changed, 153 insertions(+), 92 deletions(-) diff --git a/browser/components/loop/content/css/panel.css b/browser/components/loop/content/css/panel.css index 74ea6dc3cc6..23290a6fc6b 100644 --- a/browser/components/loop/content/css/panel.css +++ b/browser/components/loop/content/css/panel.css @@ -254,15 +254,13 @@ body { /* See .room-entry-context-item for the margin/size reductions. * An extra 40px to make space for the call button and chevron. */ width: calc(100% - 1rem - 56px); - } -.room-list > .room-entry.room-active > h2 { +.room-list > .room-entry.room-active:not(.room-opened) > h2 { font-weight: bold; - color: #000; } -.room-list > .room-entry:hover { +.room-list > .room-entry:not(.room-opened):hover { background: #dbf7ff; } @@ -417,7 +415,7 @@ html[dir="rtl"] .room-entry-context-actions > .dropdown-menu { height: 16px; } -.room-entry:hover .room-entry-context-item { +.room-entry:not(.room-opened):hover .room-entry-context-item { display: none; } diff --git a/browser/components/loop/content/js/panel.js b/browser/components/loop/content/js/panel.js index a7074f76fa0..642a1677354 100644 --- a/browser/components/loop/content/js/panel.js +++ b/browser/components/loop/content/js/panel.js @@ -362,10 +362,14 @@ loop.panel = (function(_, mozL10n) { /** * Room list entry. + * + * Active Room means there are participants in the room. + * Opened Room means the user is in the room. */ var RoomEntry = React.createClass({displayName: "RoomEntry", propTypes: { dispatcher: React.PropTypes.instanceOf(loop.Dispatcher).isRequired, + isOpenedRoom: React.PropTypes.bool.isRequired, mozLoop: React.PropTypes.object.isRequired, room: React.PropTypes.instanceOf(loop.store.Room).isRequired }, @@ -418,7 +422,8 @@ loop.panel = (function(_, mozL10n) { render: function() { var roomClasses = React.addons.classSet({ "room-entry": true, - "room-active": this._isActive() + "room-active": this._isActive(), + "room-opened": this.props.isOpenedRoom }); var roomTitle = this.props.room.decryptedContext.roomName || @@ -427,8 +432,8 @@ loop.panel = (function(_, mozL10n) { return ( React.createElement("div", {className: roomClasses, - onClick: this.handleClickEntry, - onMouseLeave: this._handleMouseOut, + onClick: this.props.isOpenedRoom ? null : this.handleClickEntry, + onMouseLeave: this.props.isOpenedRoom ? null : this._handleMouseOut, ref: "roomEntry"}, React.createElement("h2", null, roomTitle @@ -436,15 +441,17 @@ loop.panel = (function(_, mozL10n) { React.createElement(RoomEntryContextItem, { mozLoop: this.props.mozLoop, roomUrls: this.props.room.decryptedContext.urls}), - React.createElement(RoomEntryContextButtons, { - dispatcher: this.props.dispatcher, - eventPosY: this.state.eventPosY, - handleClickEntry: this.handleClickEntry, - handleContextChevronClick: this.handleContextChevronClick, - ref: "contextActions", - room: this.props.room, - showMenu: this.state.showMenu, - toggleDropdownMenu: this.toggleDropdownMenu}) + this.props.isOpenedRoom ? null : + React.createElement(RoomEntryContextButtons, { + dispatcher: this.props.dispatcher, + eventPosY: this.state.eventPosY, + handleClickEntry: this.handleClickEntry, + handleContextChevronClick: this.handleContextChevronClick, + ref: "contextActions", + room: this.props.room, + showMenu: this.state.showMenu, + toggleDropdownMenu: this.toggleDropdownMenu}) + ) ); } @@ -719,12 +726,20 @@ loop.panel = (function(_, mozL10n) { return ( React.createElement("div", {className: "rooms"}, this._renderNewRoomButton(), - React.createElement("h1", null, mozL10n.get("rooms_list_recent_conversations")), + React.createElement("h1", null, mozL10n.get(this.state.openedRoom === null ? + "rooms_list_recently_browsed" : + "rooms_list_currently_browsing")), React.createElement("div", {className: "room-list"}, this.state.rooms.map(function(room, i) { + if (this.state.openedRoom !== null && + room.roomToken !== this.state.openedRoom) { + return null; + } + return ( React.createElement(RoomEntry, { dispatcher: this.props.dispatcher, + isOpenedRoom: room.roomToken === this.state.openedRoom, key: room.roomToken, mozLoop: this.props.mozLoop, room: room}) @@ -927,8 +942,8 @@ loop.panel = (function(_, mozL10n) { clearOnDocumentHidden: true, notifications: this.props.notifications}), React.createElement(RoomList, {dispatcher: this.props.dispatcher, - mozLoop: this.props.mozLoop, - store: this.props.roomStore}), + mozLoop: this.props.mozLoop, + store: this.props.roomStore}), React.createElement("div", {className: "footer"}, React.createElement("div", {className: "user-details"}, React.createElement(AccountLink, {fxAEnabled: this.props.mozLoop.fxAEnabled, diff --git a/browser/components/loop/content/js/panel.jsx b/browser/components/loop/content/js/panel.jsx index 773577609ba..d700144dc00 100644 --- a/browser/components/loop/content/js/panel.jsx +++ b/browser/components/loop/content/js/panel.jsx @@ -362,10 +362,14 @@ loop.panel = (function(_, mozL10n) { /** * Room list entry. + * + * Active Room means there are participants in the room. + * Opened Room means the user is in the room. */ var RoomEntry = React.createClass({ propTypes: { dispatcher: React.PropTypes.instanceOf(loop.Dispatcher).isRequired, + isOpenedRoom: React.PropTypes.bool.isRequired, mozLoop: React.PropTypes.object.isRequired, room: React.PropTypes.instanceOf(loop.store.Room).isRequired }, @@ -418,7 +422,8 @@ loop.panel = (function(_, mozL10n) { render: function() { var roomClasses = React.addons.classSet({ "room-entry": true, - "room-active": this._isActive() + "room-active": this._isActive(), + "room-opened": this.props.isOpenedRoom }); var roomTitle = this.props.room.decryptedContext.roomName || @@ -427,8 +432,8 @@ loop.panel = (function(_, mozL10n) { return (

{roomTitle} @@ -436,15 +441,17 @@ loop.panel = (function(_, mozL10n) { - + {this.props.isOpenedRoom ? null : + + }

); } @@ -719,12 +726,20 @@ loop.panel = (function(_, mozL10n) { return (
{this._renderNewRoomButton()} -

{mozL10n.get("rooms_list_recent_conversations")}

+

{mozL10n.get(this.state.openedRoom === null ? + "rooms_list_recently_browsed" : + "rooms_list_currently_browsing")}

{ this.state.rooms.map(function(room, i) { + if (this.state.openedRoom !== null && + room.roomToken !== this.state.openedRoom) { + return null; + } + return ( @@ -927,8 +942,8 @@ loop.panel = (function(_, mozL10n) { clearOnDocumentHidden={true} notifications={this.props.notifications} /> + mozLoop={this.props.mozLoop} + store={this.props.roomStore} />
Date: Wed, 28 Oct 2015 09:56:17 -0700 Subject: [PATCH 24/30] Bug 1213011 - Allow post-build tasks to use build_{name,type,product} variables. r=garndt --- testing/taskcluster/mach_commands.py | 21 +++++++++++---------- testing/taskcluster/tasks/build.yml | 2 ++ testing/taskcluster/tasks/phone_build.yml | 2 ++ 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/testing/taskcluster/mach_commands.py b/testing/taskcluster/mach_commands.py index 563934d1c91..a6b0090f92d 100644 --- a/testing/taskcluster/mach_commands.py +++ b/testing/taskcluster/mach_commands.py @@ -110,7 +110,7 @@ def decorate_task_treeherder_routes(task, suffix): for env in treeheder_env: task['routes'].append('{}.{}'.format(TREEHERDER_ROUTES[env], suffix)) -def decorate_task_json_routes(build, task, json_routes, parameters): +def decorate_task_json_routes(task, json_routes, parameters): """ Decorate the given task with routes.json routes. @@ -118,15 +118,9 @@ def decorate_task_json_routes(build, task, json_routes, parameters): :param json_routes: the list of routes to use from routes.json :param parameters: dictionary of parameters to use in route templates """ - fmt = parameters.copy() - fmt.update({ - 'build_product': task['extra']['build_product'], - 'build_name': build['build_name'], - 'build_type': build['build_type'], - }) routes = task.get('routes', []) for route in json_routes: - routes.append(route.format(**fmt)) + routes.append(route.format(**parameters)) task['routes'] = routes @@ -436,6 +430,14 @@ class Graph(object): build_parameters = dict(parameters) build_parameters['build_slugid'] = slugid() build_task = templates.load(build['task'], build_parameters) + + # Copy build_* attributes to expose them to post-build tasks + # as well as json routes and tests + task_extra = build_task['task']['extra'] + build_parameters['build_name'] = task_extra['build_name'] + build_parameters['build_type'] = task_extra['build_type'] + build_parameters['build_product'] = task_extra['build_product'] + set_interactive_task(build_task, interactive) # try builds don't use cache @@ -445,8 +447,7 @@ class Graph(object): if params['revision_hash']: decorate_task_treeherder_routes(build_task['task'], treeherder_route) - decorate_task_json_routes(build, - build_task['task'], + decorate_task_json_routes(build_task['task'], json_routes, build_parameters) diff --git a/testing/taskcluster/tasks/build.yml b/testing/taskcluster/tasks/build.yml index a97247e43d3..8ddaf77a58b 100644 --- a/testing/taskcluster/tasks/build.yml +++ b/testing/taskcluster/tasks/build.yml @@ -57,6 +57,8 @@ task: extra: build_product: '{{build_product}}' + build_name: '{{build_name}}' + build_type: '{{build_type}}' index: rank: {{pushlog_id}} treeherder: diff --git a/testing/taskcluster/tasks/phone_build.yml b/testing/taskcluster/tasks/phone_build.yml index 9ef39d34611..beaa16b50cf 100644 --- a/testing/taskcluster/tasks/phone_build.yml +++ b/testing/taskcluster/tasks/phone_build.yml @@ -57,6 +57,8 @@ task: extra: build_product: 'b2g' + build_name: '{{build_name}}' + build_type: '{{build_type}}' index: rank: {{pushlog_id}} treeherder: From 83c475b01ab9b9711fe1fee0c6f9c3880b9faf39 Mon Sep 17 00:00:00 2001 From: Alexandre Poirot Date: Wed, 28 Oct 2015 09:56:17 -0700 Subject: [PATCH 25/30] Bug 1213011 - Add a route to post-build simulator task to easily download it. r=garndt --- testing/taskcluster/tasks/post-builds/mulet_simulator.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/testing/taskcluster/tasks/post-builds/mulet_simulator.yml b/testing/taskcluster/tasks/post-builds/mulet_simulator.yml index 272bd8173c0..736232d1c8a 100644 --- a/testing/taskcluster/tasks/post-builds/mulet_simulator.yml +++ b/testing/taskcluster/tasks/post-builds/mulet_simulator.yml @@ -17,6 +17,9 @@ task: provisionerId: aws-provisioner-v1 schedulerId: task-graph-scheduler + routes: + - 'index.gecko.v1.{{project}}.latest.simulator.{{build_type}}' + scopes: - 'docker-worker:cache:tc-vcs' - 'docker-worker:image:{{#docker_image}}builder{{/docker_image}}' From 424ba265a1c0760c7193209e00fce40a3189326e Mon Sep 17 00:00:00 2001 From: Alexandre Poirot Date: Wed, 28 Oct 2015 09:56:17 -0700 Subject: [PATCH 26/30] Bug 1217559 - Fix chrome overrides after new devtools files layout. r=jryans --- devtools/shared/Loader.jsm | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/devtools/shared/Loader.jsm b/devtools/shared/Loader.jsm index e92111f1afe..907aa45479b 100644 --- a/devtools/shared/Loader.jsm +++ b/devtools/shared/Loader.jsm @@ -211,8 +211,7 @@ SrcdirProvider.prototype = { let entries = []; let lines = data.split(/\n/); let preprocessed = /^\s*\*/; - let contentEntry = - new RegExp("^\\s+content/(\\w+)/(\\S+)\\s+\\((\\S+)\\)"); + let contentEntry = /^\s+content\/(\S+)\s+\((\S+)\)/; for (let line of lines) { if (preprocessed.test(line)) { dump("Unable to override preprocessed file: " + line + "\n"); @@ -220,12 +219,12 @@ SrcdirProvider.prototype = { } let match = contentEntry.exec(line); if (match) { - let pathComponents = match[3].split("/"); + let pathComponents = match[2].split("/"); pathComponents.unshift(clientDir); let path = OS.Path.join.apply(OS.Path, pathComponents); let uri = this.fileURI(path); - let entry = "override chrome://" + match[1] + - "/content/" + match[2] + "\t" + uri; + let chromeURI = "chrome://devtools/content/" + match[1]; + let entry = "override " + chromeURI + "\t" + uri; entries.push(entry); } } From 6861d675a69a2a67e63e970e49be9712ab31391f Mon Sep 17 00:00:00 2001 From: Alexandre Poirot Date: Wed, 28 Oct 2015 09:56:17 -0700 Subject: [PATCH 27/30] Bug 1217867 - Prevent actor id clash when debugging the same e10s tab with multiple clients. r=jryans --- devtools/server/actors/webbrowser.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/devtools/server/actors/webbrowser.js b/devtools/server/actors/webbrowser.js index d21ef94ba7d..f3c9dde3b9e 100644 --- a/devtools/server/actors/webbrowser.js +++ b/devtools/server/actors/webbrowser.js @@ -1975,6 +1975,10 @@ RemoteBrowserTabActor.prototype = { if (this._form) { let deferred = promise.defer(); let onFormUpdate = msg => { + // There may be more than just one childtab.js up and running + if (this._form.actor != msg.json.actor) { + return; + } this._mm.removeMessageListener("debug:form", onFormUpdate); this._form = msg.json; deferred.resolve(this); From 296d69ddb0c3f429507c9cc4bf6b390a6366f699 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Wed, 28 Oct 2015 10:20:32 -0700 Subject: [PATCH 28/30] Bug 1219071 - Cache the results of the dfs when rendering the tree widget; r=jsantell --- devtools/client/memory/components/heap.js | 3 +++ devtools/client/memory/components/tree.js | 33 ++++++++++++++++++----- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/devtools/client/memory/components/heap.js b/devtools/client/memory/components/heap.js index 6bf3421ba85..f4b0dfc6aa3 100644 --- a/devtools/client/memory/components/heap.js +++ b/devtools/client/memory/components/heap.js @@ -47,6 +47,9 @@ function createTreeProperties (census) { getRoots: () => census.children, getKey: node => node.id, itemHeight: HEAP_TREE_ROW_HEIGHT, + // Because we never add or remove children when viewing the same census, we + // can always reuse a cached traversal if one is available. + reuseCachedTraversal: traversal => true, }; } diff --git a/devtools/client/memory/components/tree.js b/devtools/client/memory/components/tree.js index cd6ec00b078..bafce4e7127 100644 --- a/devtools/client/memory/components/tree.js +++ b/devtools/client/memory/components/tree.js @@ -100,7 +100,7 @@ const TreeNode = createFactory(createClass({ /** * A generic tree component. See propTypes for the public API. - * + * * @see `devtools/client/memory/components/test/mochitest/head.js` for usage * @see `devtools/client/memory/components/heap.js` for usage */ @@ -130,7 +130,11 @@ const Tree = module.exports = createClass({ // A predicate function to filter out unwanted items from the tree. filter: PropTypes.func, // The depth to which we should automatically expand new items. - autoExpandDepth: PropTypes.number + autoExpandDepth: PropTypes.number, + // A predicate that returns true if the last DFS traversal that was cached + // can be reused, false otherwise. The predicate function is passed the + // cached traversal as an array of nodes. + reuseCachedTraversal: PropTypes.func, }, getDefaultProps() { @@ -139,7 +143,8 @@ const Tree = module.exports = createClass({ expanded: new Set(), seen: new Set(), focused: undefined, - autoExpandDepth: AUTO_EXPAND_DEPTH + autoExpandDepth: AUTO_EXPAND_DEPTH, + reuseCachedTraversal: null, }; }, @@ -149,7 +154,8 @@ const Tree = module.exports = createClass({ height: window.innerHeight, expanded: new Set(), seen: new Set(), - focused: undefined + focused: undefined, + cachedTraversal: undefined, }; }, @@ -273,10 +279,23 @@ const Tree = module.exports = createClass({ * Perform a pre-order depth-first search over the whole forest. */ _dfsFromRoots(maxDepth = Infinity) { + const cached = this.state.cachedTraversal; + if (cached + && maxDepth === Infinity + && this.props.reuseCachedTraversal + && this.props.reuseCachedTraversal(cached)) { + return cached; + } + const traversal = []; for (let root of this.props.getRoots()) { this._dfs(root, maxDepth, traversal); } + + if (this.props.reuseCachedTraversal) { + this.state.cachedTraversal = traversal; + } + return traversal; }, @@ -296,7 +315,8 @@ const Tree = module.exports = createClass({ } this.setState({ - expanded: this.state.expanded + expanded: this.state.expanded, + cachedTraversal: null, }); }, @@ -308,7 +328,8 @@ const Tree = module.exports = createClass({ _onCollapse(item) { this.state.expanded.delete(item); this.setState({ - expanded: this.state.expanded + expanded: this.state.expanded, + cachedTraversal: null, }); }, From f6659829bbd04a3e64f1b7701d93a9590ad9706e Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Wed, 28 Oct 2015 10:59:25 -0700 Subject: [PATCH 29/30] Bug 1219079 - Small breakdown-related fixes for the memory tool; r=jsantell * Add "Breakdown by" in front of dropdown selector for the selected breakdown. * "Allocation Site" => "Allocation Stack" for breakdown's label. * Make the coarse type breakdown bucket strings by count. --- devtools/client/memory/components/toolbar.js | 11 +++++++---- devtools/client/memory/constants.js | 4 ++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/devtools/client/memory/components/toolbar.js b/devtools/client/memory/components/toolbar.js index 5fd6a7e2274..a7ad64d4847 100644 --- a/devtools/client/memory/components/toolbar.js +++ b/devtools/client/memory/components/toolbar.js @@ -36,10 +36,13 @@ const Toolbar = module.exports = createClass({ DOM.div({ className: "devtools-toolbar" }, [ DOM.button({ className: `take-snapshot devtools-button`, onClick: onTakeSnapshotClick }), - DOM.select({ - className: `select-breakdown`, - onChange: e => onBreakdownChange(e.target.value), - }, breakdowns.map(({ name, displayName }) => DOM.option({ value: name }, displayName))), + DOM.label({}, + "Breakdown by ", + DOM.select({ + className: `select-breakdown`, + onChange: e => onBreakdownChange(e.target.value), + }, breakdowns.map(({ name, displayName }) => DOM.option({ value: name }, displayName))) + ), DOM.label({}, [ DOM.input({ diff --git a/devtools/client/memory/constants.js b/devtools/client/memory/constants.js index e5ae8b86a16..4b9b7ac1f2e 100644 --- a/devtools/client/memory/constants.js +++ b/devtools/client/memory/constants.js @@ -49,14 +49,14 @@ const breakdowns = exports.breakdowns = { breakdown: { by: "coarseType", objects: ALLOCATION_STACK, - strings: ALLOCATION_STACK, + strings: COUNT, scripts: INTERNAL_TYPE, other: INTERNAL_TYPE, } }, allocationStack: { - displayName: "Allocation Site", + displayName: "Allocation Stack", breakdown: ALLOCATION_STACK, }, From 920dd8dad7593a331224e25be06dc738206f41f5 Mon Sep 17 00:00:00 2001 From: David Critchley Date: Wed, 28 Oct 2015 10:10:57 -0700 Subject: [PATCH 30/30] Bug 1213851 - test bustage fix when rebasing on bug 1214582 [r=Mardak] --- browser/components/loop/test/desktop-local/panel_test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/browser/components/loop/test/desktop-local/panel_test.js b/browser/components/loop/test/desktop-local/panel_test.js index 9e984f07c27..b06b9beab70 100644 --- a/browser/components/loop/test/desktop-local/panel_test.js +++ b/browser/components/loop/test/desktop-local/panel_test.js @@ -793,6 +793,7 @@ describe("loop.panel", function() { beforeEach(function() { roomEntry = mountRoomEntry({ dispatcher: dispatcher, + isOpenedRoom: false, room: new loop.store.Room(roomData) }); });