From dbdcf1753518f1756f5f51599398d102de2bed7d Mon Sep 17 00:00:00 2001 From: Kannan Vijayan Date: Fri, 20 Dec 2013 18:11:21 -0500 Subject: [PATCH 01/12] Bug 951528. r=jandem --- js/src/jit/IonBuilder.cpp | 1 + js/src/jit/LinearScan.cpp | 8 +++++++- js/src/jit/MIRGenerator.h | 9 +++++++++ js/src/jit/MIRGraph.cpp | 3 ++- 4 files changed, 19 insertions(+), 2 deletions(-) diff --git a/js/src/jit/IonBuilder.cpp b/js/src/jit/IonBuilder.cpp index 392a89ce3f3b..0fa3c27bd289 100644 --- a/js/src/jit/IonBuilder.cpp +++ b/js/src/jit/IonBuilder.cpp @@ -9235,6 +9235,7 @@ IonBuilder::jsop_setarg(uint32_t arg) JS_ASSERT(script()->uninlineable() && !isInlineBuilder()); MSetFrameArgument *store = MSetFrameArgument::New(alloc(), arg, val); + modifiesFrameArguments_ = true; current->add(store); current->setArg(arg); return true; diff --git a/js/src/jit/LinearScan.cpp b/js/src/jit/LinearScan.cpp index bab16a47e58b..7b8d2cc8da95 100644 --- a/js/src/jit/LinearScan.cpp +++ b/js/src/jit/LinearScan.cpp @@ -767,7 +767,13 @@ LinearScanAllocator::assign(LAllocation allocation) } } - if (reg && allocation.isMemory()) { + bool useAsCanonicalSpillSlot = allocation.isMemory(); + // Only canonically spill argument values when frame arguments are not + // modified in the body. + if (mir->modifiesFrameArguments()) + useAsCanonicalSpillSlot = allocation.isStackSlot(); + + if (reg && useAsCanonicalSpillSlot) { if (reg->canonicalSpill()) { JS_ASSERT(allocation == *reg->canonicalSpill()); diff --git a/js/src/jit/MIRGenerator.h b/js/src/jit/MIRGenerator.h index bce7c0c963e0..9584c65371e7 100644 --- a/js/src/jit/MIRGenerator.h +++ b/js/src/jit/MIRGenerator.h @@ -127,6 +127,10 @@ class MIRGenerator return asmJSGlobalAccesses_; } + bool modifiesFrameArguments() const { + return modifiesFrameArguments_; + } + public: CompileCompartment *compartment; @@ -146,6 +150,11 @@ class MIRGenerator AsmJSGlobalAccessVector asmJSGlobalAccesses_; uint32_t minAsmJSHeapLength_; + // Keep track of whether frame arguments are modified during execution. + // RegAlloc needs to know this as spilling values back to their register + // slots is not compatible with that. + bool modifiesFrameArguments_; + #if defined(JS_ION_PERF) AsmJSPerfSpewer asmJSPerfSpewer_; diff --git a/js/src/jit/MIRGraph.cpp b/js/src/jit/MIRGraph.cpp index 4d0679b36229..76222f54c68b 100644 --- a/js/src/jit/MIRGraph.cpp +++ b/js/src/jit/MIRGraph.cpp @@ -30,7 +30,8 @@ MIRGenerator::MIRGenerator(CompileCompartment *compartment, performsAsmJSCall_(false), asmJSHeapAccesses_(*alloc), asmJSGlobalAccesses_(*alloc), - minAsmJSHeapLength_(AsmJSAllocationGranularity) + minAsmJSHeapLength_(AsmJSAllocationGranularity), + modifiesFrameArguments_(false) { } bool From 3c045a93e89424fc39bc058c618ac1105aba2d46 Mon Sep 17 00:00:00 2001 From: "Nils Ohlmeier [:drno]" Date: Fri, 20 Dec 2013 14:00:00 -0600 Subject: [PATCH 02/12] Bug 951892 - Adding ICE Connection state checks to PeerConnection Mochitests r=abr --- dom/media/tests/mochitest/pc.js | 79 +++++++++++++++++++++++++- dom/media/tests/mochitest/templates.js | 46 +++++++++++++++ 2 files changed, 124 insertions(+), 1 deletion(-) diff --git a/dom/media/tests/mochitest/pc.js b/dom/media/tests/mochitest/pc.js index c2165721b25f..7862e73d5984 100644 --- a/dom/media/tests/mochitest/pc.js +++ b/dom/media/tests/mochitest/pc.js @@ -976,8 +976,20 @@ function PeerConnectionWrapper(label, configuration) { var self = this; // This enables tests to validate that the next ice state is the one they expect to happen this.next_ice_state = ""; // in most cases, the next state will be "checking", but in some tests "closed" + // This allows test to register their own callbacks for ICE connection state changes + this.ice_connection_callbacks = [ ]; + this._pc.oniceconnectionstatechange = function() { ok(self._pc.iceConnectionState != undefined, "iceConnectionState should not be undefined"); + info(self + ": oniceconnectionstatechange fired, new state is: " + self._pc.iceConnectionState); + if (Object.keys(self.ice_connection_callbacks).length >= 1) { + var it = Iterator(self.ice_connection_callbacks); + var name = ""; + var callback = ""; + for ([name, callback] in it) { + callback(); + } + } if (self.next_ice_state != "") { is(self._pc.iceConnectionState, self.next_ice_state, "iceConnectionState changed to '" + self.next_ice_state + "'"); @@ -1081,13 +1093,21 @@ PeerConnectionWrapper.prototype = { }, /** - * Returns the remote signaling state. + * Returns the signaling state. * * @returns {object} The local description */ get signalingState() { return this._pc.signalingState; }, + /** + * Returns the ICE connection state. + * + * @returns {object} The local description + */ + get iceConnectionState() { + return this._pc.iceConnectionState; + }, /** * Callback when we get media from either side. Also an appropriate @@ -1316,6 +1336,63 @@ PeerConnectionWrapper.prototype = { }) ; }, + /** + * Returns if the ICE the connection state is "connected". + * + * @returns {boolean} True is the connection state is "connected", otherwise false. + */ + isIceConnected : function PCW_isIceConnected() { + info("iceConnectionState: " + this.iceConnectionState); + return this.iceConnectionState === "connected"; + }, + + /** + * Returns if the ICE the connection state is "checking". + * + * @returns {boolean} True is the connection state is "checking", otherwise false. + */ + isIceChecking : function PCW_isIceChecking() { + return this.iceConnectionState === "checking"; + }, + + /** + * Returns if the ICE the connection state is "new". + * + * @returns {boolean} True is the connection state is "new", otherwise false. + */ + isIceNew : function PCW_isIceNew() { + return this.iceConnectionState === "new"; + }, + + /** + * Registers a callback for the ICE connection state change and + * reports success (=connected) or failure via the callbacks. + * States "new" and "checking" are ignored. + * + * @param {function} onSuccess + * Callback if ICE connection status is "connected". + * @param {function} onFailure + * Callback if ICE connection reaches a different state than + * "new", "checking" or "connected". + */ + waitForIceConnected : function PCW_waitForIceConnected(onSuccess, onFailure) { + var self = this; + var mySuccess = onSuccess; + var myFailure = onFailure; + + function iceConnectedChanged () { + if (self.isIceConnected()) { + delete self.ice_connection_callbacks["waitForIceConnected"]; + mySuccess(); + } else if (! (self.isIceChecking() || self.isIceNew())) { + delete self.ice_connection_callbacks["waitForIceConnected"]; + myFailure(); + } + }; + + self.ice_connection_callbacks["waitForIceConnected"] = (function() {iceConnectedChanged()}); + }, + /** * Checks that we are getting the media streams we expect. * diff --git a/dom/media/tests/mochitest/templates.js b/dom/media/tests/mochitest/templates.js index ccd9927c76ea..1382fad7b86e 100644 --- a/dom/media/tests/mochitest/templates.js +++ b/dom/media/tests/mochitest/templates.js @@ -136,6 +136,52 @@ var commandsPeerConnection = [ }); } ], + [ + 'PC_LOCAL_WAIT_FOR_ICE_CONNECTED', + function (test) { + var myTest = test; + var myPc = myTest.pcLocal; + + function onIceConnectedSuccess () { + ok(true, "pc_local: ICE switched to connected state"); + myTest.next(); + }; + function onIceConnectedFailed () { + ok(false, "pc_local: ICE failed to switch to connected"); + myTest.next(); + }; + + if (myPc.isIceConnected()) { + ok(true, "pc_local: ICE is in connected state"); + myTest.next(); + } else { + myPc.waitForIceConnected(onIceConnectedSuccess, onIceConnectedFailed); + } + } + ], + [ + 'PC_REMOTE_WAIT_FOR_ICE_CONNECTED', + function (test) { + var myTest = test; + var myPc = myTest.pcRemote; + + function onIceConnectedSuccess () { + ok(true, "pc_remote: ICE switched to connected state"); + myTest.next(); + }; + function onIceConnectedFailed () { + ok(false, "pc_remote: ICE failed to switch to connected"); + myTest.next(); + }; + + if (myPc.isIceConnected()) { + ok(true, "pc_remote: ICE is in connected state"); + myTest.next(); + } else { + myPc.waitForIceConnected(onIceConnectedSuccess, onIceConnectedFailed); + } + } + ], [ 'PC_LOCAL_CHECK_MEDIA_STREAMS', function (test) { From e60dface48f3547e6e1674682af29beef5956e67 Mon Sep 17 00:00:00 2001 From: Steve Fink Date: Tue, 17 Dec 2013 11:21:41 -0800 Subject: [PATCH 03/12] Bug 950176 - Use mangled names to identify nodes in callgraph, r=bhackett --- js/src/devtools/rootAnalysis/CFG.js | 4 +- js/src/devtools/rootAnalysis/analyze.py | 8 ++ js/src/devtools/rootAnalysis/analyzeRoots.js | 25 +++--- js/src/devtools/rootAnalysis/annotations.js | 33 ++++---- .../devtools/rootAnalysis/computeCallgraph.js | 7 +- .../rootAnalysis/computeGCFunctions.js | 10 ++- js/src/devtools/rootAnalysis/loadCallgraph.js | 82 ++++++++++++++----- js/src/devtools/rootAnalysis/run_complete | 6 +- js/src/devtools/rootAnalysis/utility.js | 35 +++++--- 9 files changed, 137 insertions(+), 73 deletions(-) diff --git a/js/src/devtools/rootAnalysis/CFG.js b/js/src/devtools/rootAnalysis/CFG.js index 616e0e13d19f..4b9b3181aee2 100644 --- a/js/src/devtools/rootAnalysis/CFG.js +++ b/js/src/devtools/rootAnalysis/CFG.js @@ -90,8 +90,8 @@ function isMatchingConstructor(destructor, edge) var variable = callee.Variable; if (variable.Kind != "Func") return false; - var name = variable.Name[0]; - var destructorName = destructor.Exp[0].Variable.Name[0]; + var name = readable(variable.Name[0]); + var destructorName = readable(destructor.Exp[0].Variable.Name[0]); var match = destructorName.match(/^(.*?::)~(\w+)\(/); if (!match) { printErr("Unhandled destructor syntax: " + destructorName); diff --git a/js/src/devtools/rootAnalysis/analyze.py b/js/src/devtools/rootAnalysis/analyze.py index 8a6b207eb673..b82a09b380b6 100755 --- a/js/src/devtools/rootAnalysis/analyze.py +++ b/js/src/devtools/rootAnalysis/analyze.py @@ -220,6 +220,11 @@ for k,v in vars(args).items(): if args.tag and not args.buildcommand: args.buildcommand="build.%s" % args.tag +if args.jobs is not None: + data['jobs'] = args.jobs +if not data.get('jobs'): + data['jobs'] = subprocess.check_output(['nproc', '--ignore=1']) + if args.buildcommand: data['buildcommand'] = args.buildcommand elif 'BUILD' in os.environ: @@ -232,6 +237,9 @@ if 'ANALYZED_OBJDIR' in os.environ: if 'SOURCE' in os.environ: data['source'] = os.environ['SOURCE'] +if not data.get('source') and data.get('sixgill_bin'): + path = subprocess.check_output(['sh', '-c', data['sixgill_bin'] + '/xdbkeys file_source.xdb | grep jsapi.cpp']) + data['source'] = path.replace("/js/src/jsapi.cpp", "") steps = [ 'dbs', 'callgraph', diff --git a/js/src/devtools/rootAnalysis/analyzeRoots.js b/js/src/devtools/rootAnalysis/analyzeRoots.js index 607905944c5d..4da950c831e5 100644 --- a/js/src/devtools/rootAnalysis/analyzeRoots.js +++ b/js/src/devtools/rootAnalysis/analyzeRoots.js @@ -24,9 +24,8 @@ var tmpfile = scriptArgs[6] || "tmp.txt"; var gcFunctions = {}; var text = snarf("gcFunctions.lst").split("\n"); assert(text.pop().length == 0); -for (var line of text) { - gcFunctions[line] = true; -} +for (var line of text) + gcFunctions[mangled(line)] = true; var suppressedFunctions = {}; var text = snarf(suppressedFunctionsFile).split("\n"); @@ -190,7 +189,7 @@ function edgeKillsVariable(edge, variable) break; assert(callee.Variable.Kind == "Func"); - var calleeName = callee.Variable.Name[0]; + var calleeName = readable(callee.Variable.Name[0]); // Constructor calls include the text 'Name::Name(' or 'Name<...>::Name('. var openParen = calleeName.indexOf('('); @@ -224,11 +223,9 @@ function edgeCanGC(edge) if (callee.Kind == "Var") { var variable = callee.Variable; assert(variable.Kind == "Func"); - if (variable.Name[0] in gcFunctions) + var callee = mangled(variable.Name[0]); + if (callee in gcFunctions) return "'" + variable.Name[0] + "'"; - var otherName = otherDestructorName(variable.Name[0]); - if (otherName in gcFunctions) - return "'" + otherName + "'"; return null; } assert(callee.Kind == "Drf"); @@ -241,8 +238,8 @@ function edgeCanGC(edge) return (fullFieldName in suppressedFunctions) ? null : fullFieldName; } assert(callee.Exp[0].Kind == "Var"); - var calleeName = callee.Exp[0].Variable.Name[0]; - return indirectCallCannotGC(functionName, calleeName) ? null : "*" + calleeName; + var varName = callee.Exp[0].Variable.Name[0]; + return indirectCallCannotGC(functionName, varName) ? null : "*" + varName; } function variableUseFollowsGC(suppressed, variable, worklist) @@ -364,6 +361,12 @@ function variableLiveAcrossGC(suppressed, variable) return null; } +// An unrooted variable has its address stored in another variable via +// assignment, or passed into a function that can GC. If the address is +// assigned into some other variable, we can't track it to see if it is held +// live across a GC. If it is passed into a function that can GC, then it's +// sort of like a Handle to an unrooted location, and the callee could GC +// before overwriting it or rooting it. function unsafeVariableAddressTaken(suppressed, variable) { for (var body of functionBodies) { @@ -494,7 +497,7 @@ function processBodies(functionName) { if (!("DefineVariable" in functionBodies[0])) return; - var suppressed = (functionName in suppressedFunctions); + var suppressed = (mangled(functionName) in suppressedFunctions); for (var variable of functionBodies[0].DefineVariable) { if (variable.Variable.Kind == "Return") continue; diff --git a/js/src/devtools/rootAnalysis/annotations.js b/js/src/devtools/rootAnalysis/annotations.js index d2e479eb018c..ebaf8885f25c 100644 --- a/js/src/devtools/rootAnalysis/annotations.js +++ b/js/src/devtools/rootAnalysis/annotations.js @@ -19,8 +19,16 @@ var ignoreIndirectCalls = { "nsTraceRefcntImpl.cpp:void (* leakyLogRelease)(void*, int, int)": true, }; -function indirectCallCannotGC(caller, name) +function indirectCallCannotGC(fullCaller, fullVariable) { + var caller = readable(fullCaller); + + // This is usually a simple variable name, but sometimes a full name gets + // passed through. And sometimes that name is truncated. Examples: + // _ZL13gAbortHandler|mozalloc_oom.cpp:void (* gAbortHandler)(size_t) + // _ZL14pMutexUnlockFn|umutex.cpp:void (* pMutexUnlockFn)(const void* + var name = readable(fullVariable); + if (name in ignoreIndirectCalls) return true; @@ -42,8 +50,7 @@ function indirectCallCannotGC(caller, name) return true; // template method called during marking and hence cannot GC - if (name == "op" && - /^bool js::WeakMap::keyNeedsMark\(JSObject\*\)/.test(caller)) + if (name == "op" && caller.indexOf("bool js::WeakMap::keyNeedsMark(JSObject*)") != -1) { return true; } @@ -80,6 +87,7 @@ var ignoreCallees = { "mozilla::CycleCollectedJSRuntime.NoteCustomGCThingXPCOMChildren" : true, // During tracing, cannot GC. "nsIThreadManager.GetIsMainThread" : true, "PLDHashTableOps.hashKey" : true, + "z_stream_s.zfree" : true, }; function fieldCallCannotGC(csu, fullfield) @@ -91,16 +99,6 @@ function fieldCallCannotGC(csu, fullfield) return false; } -function shouldSuppressGC(name) -{ - // Various dead code that should only be called inside AutoEnterAnalysis. - // Functions with no known caller are by default treated as not suppressing GC. - return /TypeScript::Purge/.test(name) - || /StackTypeSet::addPropagateThis/.test(name) - || /ScriptAnalysis::addPushedType/.test(name) - || /IonBuilder/.test(name); -} - function ignoreEdgeUse(edge, variable) { // Functions which should not be treated as using variable. @@ -177,8 +175,11 @@ var ignoreFunctions = { "void js::AutoCompartment::AutoCompartment(js::ExclusiveContext*, JSCompartment*)": true, }; -function ignoreGCFunction(fun) +function ignoreGCFunction(mangled) { + assert(mangled in readableNames); + var fun = readableNames[mangled][0]; + if (fun in ignoreFunctions) return true; @@ -247,5 +248,9 @@ function isOverridableField(csu, field) return false; if (field == 'IsOnCurrentThread') return false; + if (field == 'GetNativeContext') + return false; + if (field == 'GetThreadFromPRThread') + return false; return true; } diff --git a/js/src/devtools/rootAnalysis/computeCallgraph.js b/js/src/devtools/rootAnalysis/computeCallgraph.js index 493cb54e173a..74f436adfec6 100644 --- a/js/src/devtools/rootAnalysis/computeCallgraph.js +++ b/js/src/devtools/rootAnalysis/computeCallgraph.js @@ -123,12 +123,7 @@ function getCallees(edge) var callees = []; if (callee.Kind == "Var") { assert(callee.Variable.Kind == "Func"); - var origName = callee.Variable.Name[0]; - var names = [ origName, otherDestructorName(origName) ]; - for (var name of names) { - if (name) - callees.push({'kind': 'direct', 'name': name}); - } + callees.push({'kind': 'direct', 'name': callee.Variable.Name[0]}); } else { assert(callee.Kind == "Drf"); if (callee.Exp[0].Kind == "Fld") { diff --git a/js/src/devtools/rootAnalysis/computeGCFunctions.js b/js/src/devtools/rootAnalysis/computeGCFunctions.js index a8c448b6d038..06885e3995a6 100644 --- a/js/src/devtools/rootAnalysis/computeGCFunctions.js +++ b/js/src/devtools/rootAnalysis/computeGCFunctions.js @@ -23,17 +23,21 @@ printErr("Writing " + gcFunctions_filename); redirect(gcFunctions_filename); for (var name in gcFunctions) { print(""); - print("GC Function: " + name); + print("GC Function: " + name + "|" + readableNames[name][0]); do { name = gcFunctions[name]; - print(" " + name); + if (name in readableNames) + print(" " + readableNames[name][0]); + else + print(" " + name); } while (name in gcFunctions); } printErr("Writing " + gcFunctionsList_filename); redirect(gcFunctionsList_filename); for (var name in gcFunctions) { - print(name); + for (var readable of readableNames[name]) + print(name + "|" + readable); } // gcEdges is a list of edges that can GC for more specific reasons than just diff --git a/js/src/devtools/rootAnalysis/loadCallgraph.js b/js/src/devtools/rootAnalysis/loadCallgraph.js index 8964ca64e4a9..f94517c1402b 100644 --- a/js/src/devtools/rootAnalysis/loadCallgraph.js +++ b/js/src/devtools/rootAnalysis/loadCallgraph.js @@ -2,11 +2,38 @@ "use strict"; -var calleeGraph = {}; -var callerGraph = {}; -var gcFunctions = {}; +loadRelativeToScript('utility.js'); + +// Functions come out of sixgill in the form "mangled|readable". The mangled +// name is Truth. One mangled name might correspond to multiple readable names, +// for multiple reasons, including (1) sixgill/gcc doesn't always qualify types +// the same way or de-typedef the same amount; (2) sixgill's output treats +// references and pointers the same, and so doesn't distinguish them, but C++ +// treats them as separate for overloading and linking; (3) (identical) +// destructors sometimes have an int32 parameter, sometimes not. +// +// The readable names are useful because they're far more meaningful to the +// user, and are what should show up in reports and questions to mrgiggles. At +// least in most cases, it's fine to have the extra mangled name tacked onto +// the beginning for these. +// +// The strategy used is to separate out the pieces whenever they are read in, +// create a table mapping mangled names to (one of the) readable names, and +// use the mangled names in all computation. +// +// Note that callgraph.txt uses a compressed representation -- each name is +// mapped to an integer, and those integers are what is recorded in the edges. +// But the integers depend on the full name, whereas the true edge should only +// consider the mangled name. And some of the names encoded in callgraph.txt +// are FieldCalls, not just function names. + +var readableNames = {}; // map from mangled name => list of readable names +var mangledName = {}; // map from demangled names => mangled names. Could be eliminated. +var calleeGraph = {}; // map from mangled => list of tuples of {'callee':mangled, 'suppressed':bool} +var callerGraph = {}; // map from mangled => list of tuples of {'caller':mangled, 'suppressed':bool} +var gcFunctions = {}; // map from mangled callee => reason +var suppressedFunctions = {}; // set of mangled names (map from mangled name => true) var gcEdges = {}; -var suppressedFunctions = {}; function addGCFunction(caller, reason) { @@ -35,8 +62,13 @@ function addCallEdge(caller, callee, suppressed) callerGraph[callee].push({caller:caller, suppressed:suppressed}); } +// Map from identifier to full "mangled|readable" name. Or sometimes to a +// Class.Field name. var functionNames = [""]; +// Map from identifier to mangled name (or to a Class.Field) +var idToMangled = [""]; + function loadCallgraph(file) { var suppressedFieldCalls = {}; @@ -45,37 +77,45 @@ function loadCallgraph(file) var textLines = snarf(file).split('\n'); for (var line of textLines) { var match; - if (match = /^\#(\d+) (.*)/.exec(line)) { + if (match = line.charAt(0) == "#" && /^\#(\d+) (.*)/.exec(line)) { assert(functionNames.length == match[1]); functionNames.push(match[2]); + var [ mangled, readable ] = splitFunction(match[2]); + if (mangled in readableNames) + readableNames[mangled].push(readable); + else + readableNames[mangled] = [ readable ]; + mangledName[readable] = mangled; + idToMangled.push(mangled); continue; } var suppressed = false; - if (/SUPPRESS_GC/.test(line)) { + if (line.indexOf("SUPPRESS_GC") != -1) { match = /^(..)SUPPRESS_GC (.*)/.exec(line); line = match[1] + match[2]; suppressed = true; } - if (match = /^I (\d+) VARIABLE ([^\,]*)/.exec(line)) { - var caller = functionNames[match[1]]; + var tag = line.charAt(0); + if (match = tag == 'I' && /^I (\d+) VARIABLE ([^\,]*)/.exec(line)) { + var mangledCaller = idToMangled[match[1]]; var name = match[2]; - if (!indirectCallCannotGC(caller, name) && !suppressed) - addGCFunction(caller, "IndirectCall: " + name); - } else if (match = /^F (\d+) CLASS (.*?) FIELD (.*)/.exec(line)) { - var caller = functionNames[match[1]]; + if (!indirectCallCannotGC(functionNames[match[1]], name) && !suppressed) + addGCFunction(mangledCaller, "IndirectCall: " + name); + } else if (match = tag == 'F' && /^F (\d+) CLASS (.*?) FIELD (.*)/.exec(line)) { + var caller = idToMangled[match[1]]; var csu = match[2]; var fullfield = csu + "." + match[3]; if (suppressed) suppressedFieldCalls[fullfield] = true; else if (!fieldCallCannotGC(csu, fullfield)) addGCFunction(caller, "FieldCall: " + fullfield); - } else if (match = /^D (\d+) (\d+)/.exec(line)) { - var caller = functionNames[match[1]]; - var callee = functionNames[match[2]]; + } else if (match = tag == 'D' && /^D (\d+) (\d+)/.exec(line)) { + var caller = idToMangled[match[1]]; + var callee = idToMangled[match[2]]; addCallEdge(caller, callee, suppressed); - } else if (match = /^R (\d+) (\d+)/.exec(line)) { - var callerField = functionNames[match[1]]; - var callee = functionNames[match[2]]; + } else if (match = tag == 'R' && /^R (\d+) (\d+)/.exec(line)) { + var callerField = idToMangled[match[1]]; + var callee = idToMangled[match[2]]; addCallEdge(callerField, callee, false); resolvedFunctions[callerField] = true; } @@ -99,8 +139,6 @@ function loadCallgraph(file) var top = worklist.length; while (top > 0) { name = worklist[--top]; - if (shouldSuppressGC(name)) - continue; if (!(name in suppressedFunctions)) continue; delete suppressedFunctions[name]; @@ -125,8 +163,8 @@ function loadCallgraph(file) for (var gcName of [ 'jsgc.cpp:void Collect(JSRuntime*, uint8, int64, uint32, uint32)', 'void js::MinorGC(JSRuntime*, uint32)' ]) { - assert(gcName in callerGraph); - addGCFunction(gcName, "GC"); + assert(gcName in mangledName); + addGCFunction(mangledName[gcName], "GC"); } // Initialize the worklist to all known gcFunctions. diff --git a/js/src/devtools/rootAnalysis/run_complete b/js/src/devtools/rootAnalysis/run_complete index 5120fcb3e1e1..a3a7c7875aec 100755 --- a/js/src/devtools/rootAnalysis/run_complete +++ b/js/src/devtools/rootAnalysis/run_complete @@ -233,8 +233,10 @@ sub run_build print CONFIG "$prefix_dir\n"; print CONFIG Cwd::abs_path("$result_dir/build_xgill.log")."\n"; print CONFIG "$address\n"; - print CONFIG "-fplugin-arg-xgill-annfile=$ann_file\n" - if ($ann_file ne "" && -e $ann_file); + my @extra = ("-fplugin-arg-xgill-mangle=1"); + push(@extra, "-fplugin-arg-xgill-annfile=$ann_file") + if ($ann_file ne "" && -e $ann_file); + print CONFIG join(" ", @extra) . "\n"; close(CONFIG); # Tell the wrapper where to find the config diff --git a/js/src/devtools/rootAnalysis/utility.js b/js/src/devtools/rootAnalysis/utility.js index 9b7487355ee7..c6f2117feb14 100644 --- a/js/src/devtools/rootAnalysis/utility.js +++ b/js/src/devtools/rootAnalysis/utility.js @@ -103,21 +103,30 @@ function getSuccessors(body) return body.successors; } -function otherDestructorName(name) +// Split apart a function from sixgill into its mangled and unmangled name. If +// no mangled name was given, use the unmangled name as its mangled name +function splitFunction(func) { - // gcc's information for destructors can be pretty messed up. Some functions - // have destructors with no arguments, some have destructors with an int32 - // argument, some have both, and which one matches what the programmer wrote - // is anyone's guess. Work around this by treating calls to one destructor - // form as a call to both destructor forms. - if (!/::~/.test(name)) - return null; + var split = func.indexOf("|"); + if (split == -1) + return [ func, func ]; + return [ func.substr(0, split), func.substr(split+1) ]; +} - if (/\(int32\)/.test(name)) - return name.replace("(int32)","()"); - if (/\(\)/.test(name)) - return name.replace("()","(int32)"); - return null; +function mangled(fullname) +{ + var split = fullname.indexOf("|"); + if (split == -1) + return fullname; + return fullname.substr(0, split); +} + +function readable(fullname) +{ + var split = fullname.indexOf("|"); + if (split == -1) + return fullname; + return fullname.substr(split+1); } function xdbLibrary() From 6fd0c4d072cc10d25d4f545b80a0b11fcca41e2e Mon Sep 17 00:00:00 2001 From: Steve Fink Date: Fri, 20 Dec 2013 15:58:36 -0800 Subject: [PATCH 04/12] Bug 952682 - New hazards in storage code, r=terrence --- storage/src/mozStorageAsyncStatementParams.cpp | 8 +++++--- storage/src/mozStorageStatementRow.cpp | 6 ++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/storage/src/mozStorageAsyncStatementParams.cpp b/storage/src/mozStorageAsyncStatementParams.cpp index c69ab239ae20..b9d802df89af 100644 --- a/storage/src/mozStorageAsyncStatementParams.cpp +++ b/storage/src/mozStorageAsyncStatementParams.cpp @@ -93,6 +93,8 @@ AsyncStatementParams::NewResolve( bool *_retval ) { + JS::Rooted scopeObj(aCtx, aScopeObj); + NS_ENSURE_TRUE(mStatement, NS_ERROR_NOT_INITIALIZED); // We do not throw at any point after this because we want to allow the // prototype chain to be checked for the property. @@ -103,7 +105,7 @@ AsyncStatementParams::NewResolve( uint32_t idx = JSID_TO_INT(aId); // All indexes are good because we don't know how many parameters there // really are. - ok = ::JS_DefineElement(aCtx, aScopeObj, idx, JSVAL_VOID, nullptr, + ok = ::JS_DefineElement(aCtx, scopeObj, idx, JSVAL_VOID, nullptr, nullptr, 0); resolved = true; } @@ -111,13 +113,13 @@ AsyncStatementParams::NewResolve( // We are unable to tell if there's a parameter with this name and so // we must assume that there is. This screws the rest of the prototype // chain, but people really shouldn't be depending on this anyways. - ok = ::JS_DefinePropertyById(aCtx, aScopeObj, aId, JSVAL_VOID, nullptr, + ok = ::JS_DefinePropertyById(aCtx, scopeObj, aId, JSVAL_VOID, nullptr, nullptr, 0); resolved = true; } *_retval = ok; - *_objp = resolved && ok ? aScopeObj : nullptr; + *_objp = resolved && ok ? scopeObj.get() : nullptr; return NS_OK; } diff --git a/storage/src/mozStorageStatementRow.cpp b/storage/src/mozStorageStatementRow.cpp index 5ebd6b161ac8..a5a87b46d72c 100644 --- a/storage/src/mozStorageStatementRow.cpp +++ b/storage/src/mozStorageStatementRow.cpp @@ -123,6 +123,8 @@ StatementRow::NewResolve(nsIXPConnectWrappedNative *aWrapper, JSObject **_objp, bool *_retval) { + JS::Rooted scopeObj(aCtx, aScopeObj); + NS_ENSURE_TRUE(mStatement, NS_ERROR_NOT_INITIALIZED); // We do not throw at any point after this because we want to allow the // prototype chain to be checked for the property. @@ -142,9 +144,9 @@ StatementRow::NewResolve(nsIXPConnectWrappedNative *aWrapper, return NS_OK; } - *_retval = ::JS_DefinePropertyById(aCtx, aScopeObj, aId, JSVAL_VOID, + *_retval = ::JS_DefinePropertyById(aCtx, scopeObj, aId, JSVAL_VOID, nullptr, nullptr, 0); - *_objp = aScopeObj; + *_objp = scopeObj; return NS_OK; } From abb96de92a676a0d25e5e10529aae79d0a62418e Mon Sep 17 00:00:00 2001 From: Steve Fink Date: Fri, 20 Dec 2013 16:07:00 -0800 Subject: [PATCH 05/12] Bug 952688 - Root CallbackObject's CallSetup around GlobalScope() call, r=terrence --- dom/bindings/CallbackObject.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dom/bindings/CallbackObject.cpp b/dom/bindings/CallbackObject.cpp index 4035c06b0cbd..6cd960694990 100644 --- a/dom/bindings/CallbackObject.cpp +++ b/dom/bindings/CallbackObject.cpp @@ -69,6 +69,7 @@ CallbackObject::CallSetup::CallSetup(CallbackObject* aCallback, JSObject* realCallback = js::UncheckedUnwrap(aCallback->CallbackPreserveColor()); JSContext* cx = nullptr; nsIGlobalObject* globalObject = nullptr; + Maybe< JS::Rooted > callbackRooter; if (mIsMainThread) { // Now get the global and JSContext for this callback. @@ -89,6 +90,7 @@ CallbackObject::CallSetup::CallSetup(CallbackObject* aCallback, // This happens - Removing it causes // test_bug293235.xul to go orange. : nsContentUtils::GetSafeJSContext(); + callbackRooter.construct(cx, realCallback); globalObject = win; } else { // No DOM Window. Store the global and use the SafeJSContext. @@ -96,9 +98,11 @@ CallbackObject::CallSetup::CallSetup(CallbackObject* aCallback, globalObject = xpc::GetNativeForGlobal(glob); MOZ_ASSERT(globalObject); cx = nsContentUtils::GetSafeJSContext(); + callbackRooter.construct(cx, realCallback); } } else { cx = workers::GetCurrentThreadJSContext(); + callbackRooter.construct(cx, realCallback); globalObject = workers::GetCurrentThreadWorkerPrivate()->GlobalScope(); } From e979b883fc28c3bc50694fdbda948adb6110b37f Mon Sep 17 00:00:00 2001 From: Olli Pettay Date: Sat, 21 Dec 2013 02:22:13 +0200 Subject: [PATCH 06/12] Bug 949722 - Assigning to '.onerror' of XHR appends an event listener, rather than overwriting it (only in workers), r=khuey --- content/events/src/nsEventListenerManager.cpp | 8 +------- content/events/src/nsEventListenerManager.h | 2 +- dom/src/events/nsJSEventListener.cpp | 4 +++- dom/workers/WorkerPrivate.cpp | 5 ++++- dom/workers/test/errorwarning_worker.js | 4 ++++ dom/workers/test/xhr_worker.js | 8 ++++++-- 6 files changed, 19 insertions(+), 12 deletions(-) diff --git a/content/events/src/nsEventListenerManager.cpp b/content/events/src/nsEventListenerManager.cpp index e5a95256a11c..7b380459af04 100644 --- a/content/events/src/nsEventListenerManager.cpp +++ b/content/events/src/nsEventListenerManager.cpp @@ -525,8 +525,7 @@ nsEventListenerManager::ListenerCanHandle(nsListenerStruct* aLs, } return aLs->mTypeString.Equals(aEvent->typeString); } - MOZ_ASSERT_IF(aEvent->eventStructType != NS_SCRIPT_ERROR_EVENT, - mIsMainThreadELM); + MOZ_ASSERT(mIsMainThreadELM); return aLs->mEventType == aEvent->message; } @@ -595,11 +594,6 @@ nsEventListenerManager::SetEventHandlerInternal(JS::Handle aScopeObje nsCOMPtr scriptListener; NS_NewJSEventListener(aScopeObject, mTarget, aName, aHandler, getter_AddRefs(scriptListener)); - - if (!aName && aTypeString.EqualsLiteral("error")) { - eventType = NS_LOAD_ERROR; - } - EventListenerHolder holder(scriptListener); AddEventListenerInternal(holder, eventType, aName, aTypeString, flags, true); diff --git a/content/events/src/nsEventListenerManager.h b/content/events/src/nsEventListenerManager.h index 70ea66ca8369..fe4bea4ccb50 100644 --- a/content/events/src/nsEventListenerManager.h +++ b/content/events/src/nsEventListenerManager.h @@ -482,7 +482,7 @@ public: if (mIsMainThreadELM) { handler = GetEventHandlerInternal(nsGkAtoms::onerror, EmptyString()); } else { - handler = GetEventHandlerInternal(nullptr, NS_LITERAL_STRING("onerror")); + handler = GetEventHandlerInternal(nullptr, NS_LITERAL_STRING("error")); } return handler ? handler->OnErrorEventHandler() : nullptr; } diff --git a/dom/src/events/nsJSEventListener.cpp b/dom/src/events/nsJSEventListener.cpp index b8075c56e79d..b68b312e47ca 100644 --- a/dom/src/events/nsJSEventListener.cpp +++ b/dom/src/events/nsJSEventListener.cpp @@ -169,7 +169,9 @@ nsJSEventListener::HandleEvent(nsIDOMEvent* aEvent) NS_ENSURE_TRUE(aEvent, NS_ERROR_UNEXPECTED); InternalScriptErrorEvent* scriptEvent = aEvent->GetInternalNSEvent()->AsScriptErrorEvent(); - if (scriptEvent && scriptEvent->message == NS_LOAD_ERROR) { + if (scriptEvent && + (scriptEvent->message == NS_LOAD_ERROR || + scriptEvent->typeString.EqualsLiteral("error"))) { errorMsg = scriptEvent->errorMsg; msgOrEvent.SetAsString() = static_cast(&errorMsg); diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp index 095085078b5e..200ea537d23d 100644 --- a/dom/workers/WorkerPrivate.cpp +++ b/dom/workers/WorkerPrivate.cpp @@ -1223,7 +1223,8 @@ public: MOZ_ASSERT(target == globalTarget->GetWrapperPreserveColor()); // Icky, we have to fire an InternalScriptErrorEvent... - InternalScriptErrorEvent event(true, NS_LOAD_ERROR); + MOZ_ASSERT(!NS_IsMainThread()); + InternalScriptErrorEvent event(true, NS_USER_DEFINED_EVENT); event.lineNr = aLineNumber; event.errorMsg = aMessage.get(); event.fileName = aFilename.get(); @@ -1238,6 +1239,7 @@ public: } else if ((sgo = nsJSUtils::GetStaticScriptGlobal(target))) { // Icky, we have to fire an InternalScriptErrorEvent... + MOZ_ASSERT(NS_IsMainThread()); InternalScriptErrorEvent event(true, NS_LOAD_ERROR); event.lineNr = aLineNumber; event.errorMsg = aMessage.get(); @@ -3213,6 +3215,7 @@ WorkerPrivateParent::BroadcastErrorToSharedWorkers( do_QueryInterface(windowAction.mWindow); MOZ_ASSERT(sgo); + MOZ_ASSERT(NS_IsMainThread()); InternalScriptErrorEvent event(true, NS_LOAD_ERROR); event.lineNr = aLineNumber; event.errorMsg = aMessage.BeginReading(); diff --git a/dom/workers/test/errorwarning_worker.js b/dom/workers/test/errorwarning_worker.js index 1b612635c81f..1c372104c8e9 100644 --- a/dom/workers/test/errorwarning_worker.js +++ b/dom/workers/test/errorwarning_worker.js @@ -36,3 +36,7 @@ onmessage = function(event) { } onerror = errorHandler; +onerror = onerror; +if (!onerror || onerror != onerror) { + throw "onerror wasn't set properly"; +} diff --git a/dom/workers/test/xhr_worker.js b/dom/workers/test/xhr_worker.js index 5e3e74a3ec8c..178c391b63fc 100644 --- a/dom/workers/test/xhr_worker.js +++ b/dom/workers/test/xhr_worker.js @@ -29,14 +29,18 @@ if (!xhr.onload) { postMessage(message); } -xhr.addEventListener("error", function(event) { +xhr.onerror = function(event) { if (event.target != xhr) { throw "onerror event.target != xhr"; } var message = { type: "error", error: event.target.status }; postMessage(message); -}, false); +}; +xhr.onerror = xhr.onerror; +if (!xhr.onerror || xhr.onerror != xhr.onerror) { + throw "onerror wasn't set properly"; +} function onprogress(event) { if (event.target != xhr) { From a710319696338be207c0222cb5e893cf8f131a35 Mon Sep 17 00:00:00 2001 From: Steve Fink Date: Fri, 20 Dec 2013 16:46:31 -0800 Subject: [PATCH 07/12] Backed out changeset f71e6905567f (bug 952688) --HG-- extra : rebase_source : 8d66cb74d228fd4e34c72afab98132bf1d2ba549 --- dom/bindings/CallbackObject.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/dom/bindings/CallbackObject.cpp b/dom/bindings/CallbackObject.cpp index 6cd960694990..4035c06b0cbd 100644 --- a/dom/bindings/CallbackObject.cpp +++ b/dom/bindings/CallbackObject.cpp @@ -69,7 +69,6 @@ CallbackObject::CallSetup::CallSetup(CallbackObject* aCallback, JSObject* realCallback = js::UncheckedUnwrap(aCallback->CallbackPreserveColor()); JSContext* cx = nullptr; nsIGlobalObject* globalObject = nullptr; - Maybe< JS::Rooted > callbackRooter; if (mIsMainThread) { // Now get the global and JSContext for this callback. @@ -90,7 +89,6 @@ CallbackObject::CallSetup::CallSetup(CallbackObject* aCallback, // This happens - Removing it causes // test_bug293235.xul to go orange. : nsContentUtils::GetSafeJSContext(); - callbackRooter.construct(cx, realCallback); globalObject = win; } else { // No DOM Window. Store the global and use the SafeJSContext. @@ -98,11 +96,9 @@ CallbackObject::CallSetup::CallSetup(CallbackObject* aCallback, globalObject = xpc::GetNativeForGlobal(glob); MOZ_ASSERT(globalObject); cx = nsContentUtils::GetSafeJSContext(); - callbackRooter.construct(cx, realCallback); } } else { cx = workers::GetCurrentThreadJSContext(); - callbackRooter.construct(cx, realCallback); globalObject = workers::GetCurrentThreadWorkerPrivate()->GlobalScope(); } From 9994fec4ef6dee0dc779eab59ed1960e035ccc42 Mon Sep 17 00:00:00 2001 From: Steve Fink Date: Fri, 20 Dec 2013 18:00:51 -0800 Subject: [PATCH 08/12] No bug. Hold the line at 3 hazards! r=terrence --- js/src/devtools/rootAnalysis/expect.browser.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/devtools/rootAnalysis/expect.browser.json b/js/src/devtools/rootAnalysis/expect.browser.json index d55d083101e9..1246ef9ef22e 100644 --- a/js/src/devtools/rootAnalysis/expect.browser.json +++ b/js/src/devtools/rootAnalysis/expect.browser.json @@ -1,3 +1,3 @@ { - "expect-hazards": 15 + "expect-hazards": 3 } From 7f84d88bf6f90cf1c2dc295c464f1f8e1474051a Mon Sep 17 00:00:00 2001 From: Wes Kocher Date: Fri, 20 Dec 2013 18:32:20 -0800 Subject: [PATCH 09/12] Backed out changeset 00aa14a1f461 (bug 950830) --- content/media/TextTrackCue.h | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/content/media/TextTrackCue.h b/content/media/TextTrackCue.h index 188ff84bce95..633316b0b9e7 100644 --- a/content/media/TextTrackCue.h +++ b/content/media/TextTrackCue.h @@ -83,9 +83,8 @@ public: void SetStartTime(double aStartTime) { - if (mStartTime == aStartTime) { + if (mStartTime == aStartTime) return; - } mStartTime = aStartTime; CueChanged(); @@ -98,9 +97,8 @@ public: void SetEndTime(double aEndTime) { - if (mEndTime == aEndTime) { + if (mEndTime == aEndTime) return; - } mEndTime = aEndTime; CueChanged(); @@ -113,9 +111,8 @@ public: void SetPauseOnExit(bool aPauseOnExit) { - if (mPauseOnExit == aPauseOnExit) { + if (mPauseOnExit == aPauseOnExit) return; - } mPauseOnExit = aPauseOnExit; CueChanged(); @@ -143,9 +140,8 @@ public: void SetVertical(const DirectionSetting& aVertical) { - if (mVertical == aVertical) { + if (mVertical == aVertical) return; - } mReset = true; mVertical = aVertical; @@ -159,9 +155,8 @@ public: void SetSnapToLines(bool aSnapToLines) { - if (mSnapToLines == aSnapToLines) { + if (mSnapToLines == aSnapToLines) return; - } mReset = true; mSnapToLines = aSnapToLines; @@ -220,9 +215,9 @@ public: void SetPosition(int32_t aPosition, ErrorResult& aRv) { - if (mPosition == aPosition) { + // XXXhumph: validate? bug 868519. + if (mPosition == aPosition) return; - } if (aPosition > 100 || aPosition < 0){ aRv.Throw(NS_ERROR_DOM_INDEX_SIZE_ERR); @@ -262,9 +257,8 @@ public: void SetAlign(AlignSetting& aAlign) { - if (mAlign == aAlign) { + if (mAlign == aAlign) return; - } mReset = true; mAlign = aAlign; @@ -278,9 +272,8 @@ public: void SetText(const nsAString& aText) { - if (mText == aText) { + if (mText == aText) return; - } mReset = true; mText = aText; From 2f2426fd1c1f084b08b45412021dbd5fc9baea48 Mon Sep 17 00:00:00 2001 From: Wes Kocher Date: Fri, 20 Dec 2013 18:32:44 -0800 Subject: [PATCH 10/12] Backed out changeset 9ca7e64ef0d0 (bug 921484) --- content/media/TextTrack.cpp | 23 ++++---- content/media/test/basic.vtt | 12 +--- content/media/test/test_texttrackcue.html | 70 +++++------------------ 3 files changed, 29 insertions(+), 76 deletions(-) diff --git a/content/media/TextTrack.cpp b/content/media/TextTrack.cpp index 683290d2f6f7..49dc9ca5481d 100644 --- a/content/media/TextTrack.cpp +++ b/content/media/TextTrack.cpp @@ -148,27 +148,28 @@ TextTrack::GetActiveCues() // the active cue list from scratch. if (mDirty) { mCuePos = 0; - mDirty = false; + mDirty = true; mActiveCueList->RemoveAll(); } double playbackTime = mMediaElement->CurrentTime(); // Remove all the cues from the active cue list whose end times now occur - // earlier then the current playback time. - for (uint32_t i = mActiveCueList->Length(); i > 0; i--) { - if ((*mActiveCueList)[i - 1]->EndTime() < playbackTime) { - mActiveCueList->RemoveCueAt(i - 1); - } + // earlier then the current playback time. When we reach a cue whose end time + // is valid we can safely stop iterating as the list is sorted. + for (uint32_t i = 0; i < mActiveCueList->Length() && + (*mActiveCueList)[i]->EndTime() < playbackTime; i++) { + mActiveCueList->RemoveCueAt(i); } // Add all the cues, starting from the position of the last cue that was // added, that have valid start and end times for the current playback time. // We can stop iterating safely once we encounter a cue that does not have - // a valid start time as the cue list is sorted. - for (; mCuePos < mCueList->Length() && - (*mCueList)[mCuePos]->StartTime() <= playbackTime; mCuePos++) { - if ((*mCueList)[mCuePos]->EndTime() >= playbackTime) { - mActiveCueList->AddCue(*(*mCueList)[mCuePos]); + // valid times for the current playback time as the cue list is sorted. + for (; mCuePos < mCueList->Length(); mCuePos++) { + TextTrackCue* cue = (*mCueList)[mCuePos]; + if (cue->StartTime() > playbackTime || cue->EndTime() < playbackTime) { + break; } + mActiveCueList->AddCue(*cue); } return mActiveCueList; } diff --git a/content/media/test/basic.vtt b/content/media/test/basic.vtt index 8859d056595b..89e5f3271287 100644 --- a/content/media/test/basic.vtt +++ b/content/media/test/basic.vtt @@ -8,18 +8,10 @@ This 00:01.200 --> 00:02.400 Is -2.5 -00:02.000 --> 00:03.500 -(Over here?!) - 3 00:02.710 --> 00:02.910 A -4 +3 00:03.217 --> 00:03.989 -Test - -5 -00:03.217 --> 00:03.989 -And more! +Test \ No newline at end of file diff --git a/content/media/test/test_texttrackcue.html b/content/media/test/test_texttrackcue.html index 95fa34d35d0a..1231d6bad6fd 100644 --- a/content/media/test/test_texttrackcue.html +++ b/content/media/test/test_texttrackcue.html @@ -37,7 +37,7 @@ SpecialPowers.pushPrefEnv({"set": [["media.webvtt.enabled", true]]}, is(trackElement.readyState, 2, "Track::ReadyState should be set to LOADED."); var cueList = trackElement.track.cues; - is(cueList.length, 6, "Cue list length should be 6."); + is(cueList.length, 4, "Cue list length should be 4."); // Check that the typedef of TextTrackCue works in Gecko. is(window.TextTrackCue, undefined, "TextTrackCue should be undefined."); @@ -57,22 +57,18 @@ SpecialPowers.pushPrefEnv({"set": [["media.webvtt.enabled", true]]}, // Check that all cue times were not rounded is(cueList[1].startTime, 1.2, "Second cue's start time should be 1.2."); is(cueList[1].endTime, 2.4, "Second cue's end time should be 2.4."); - is(cueList[2].startTime, 2, "Third cue's start time should be 2."); - is(cueList[2].endTime, 3.5, "Third cue's end time should be 3.5."); - is(cueList[3].startTime, 2.71, "Fourth cue's start time should be 2.71."); - is(cueList[3].endTime, 2.91, "Fourth cue's end time should be 2.91."); - is(cueList[4].startTime, 3.217, "Fifth cue's start time should be 3.217."); - is(cueList[4].endTime, 3.989, "Fifth cue's end time should be 3.989."); - is(cueList[5].startTime, 3.217, "Sixth cue's start time should be 3.217."); - is(cueList[5].endTime, 3.989, "Sixth cue's end time should be 3.989."); + is(cueList[2].startTime, 2.71, "Third cue's start time should be 2.71."); + is(cueList[2].endTime, 2.91, "Third cue's end time should be 2.91."); + is(cueList[3].startTime, 3.217, "Fourth cue's start time should be 3.217."); + is(cueList[3].endTime, 3.989, "Fourth cue's end time should be 3.989."); // Check that Cue setters are working correctly. cue.id = "Cue 01"; is(cue.id, "Cue 01", "Cue's ID should be 'Cue 01'."); - cue.startTime = 0.51; - is(cue.startTime, 0.51, "Cue's start time should be 0.51."); - cue.endTime = 0.71; - is(cue.endTime, 0.71, "Cue's end time should be 0.71."); + cue.startTime = 1.5; + is(cue.startTime, 1.5, "Cue's start time should be 1.5."); + cue.endTime = 2.5; + is(cue.endTime, 2.5, "Cue's end time should be 2.5."); cue.pauseOnExit = true; is(cue.pauseOnExit, true, "Cue's pause on exit flag should be true."); @@ -114,21 +110,21 @@ SpecialPowers.pushPrefEnv({"set": [["media.webvtt.enabled", true]]}, // Check that we can create and add new VTTCues var vttCue = new VTTCue(3.999, 4, "foo"); trackElement.track.addCue(vttCue); - is(cueList.length, 7, "Cue list length should now be 7."); + is(cueList.length, 5, "Cue list length should now be 5."); // Check that new VTTCue was added correctly - cue = cueList[6]; + cue = cueList[4]; is(cue.startTime, 3.999, "Cue's start time should be 3.999."); is(cue.endTime, 4, "Cue's end time should be 4."); is(cue.text, "foo", "Cue's text should be foo."); // Adding the same cue again should not increase the cue count. trackElement.track.addCue(vttCue); - is(cueList.length, 7, "Cue list length should be 7."); + is(cueList.length, 5, "Cue list length should be 5."); // Check that we are able to remove cues. trackElement.track.removeCue(cue); - is(cueList.length, 6, "Cue list length should be 6."); + is(cueList.length, 4, "Cue list length should be 4."); exceptionHappened = false; try { @@ -145,45 +141,9 @@ SpecialPowers.pushPrefEnv({"set": [["media.webvtt.enabled", true]]}, // when we shouln't have. ok(exceptionHappened, "Exception should have happened."); - is(cueList.length, 6, "Cue list length should be 6."); + is(cueList.length, 4, "Cue list length should be 4."); - // Test TextTrack::ActiveCues. - var cueInfo = [ - { startTime: 0.51, endTime: 0.71, ids: ["Cue 01"] }, - { startTime: 1.2, endTime: 2, ids: [2] }, - { startTime: 2, endTime: 2.4, ids: [2, 2.5] }, - { startTime: 2.4, endTime: 2.71, ids: [2.5] }, - { startTime: 2.71, endTime: 2.91, ids: [2.5, 3] }, - { startTime: 2.91, endTime: 3.217, ids: [2.5] }, - { startTime: 3.217, endTime: 3.5, ids: [2.5, 4, 5] }, - { startTime: 3.50, endTime: 3.989, ids: [4, 5] } - ]; - - video.addEventListener("timeupdate", function() { - var activeCues = trackElement.track.activeCues, - found = false, - playbackTime = video.currentTime; - - for (var i = 0; i < cueInfo.length && !found; i++) { - var cue = cueInfo[i]; - if (playbackTime >= cue.startTime && playbackTime <= cue.endTime) { - is(activeCues.length, cue.ids.length, "There should be " + cue.ids.length + " currently active cue(s)."); - for (var j = 0; j < cue.ids.length; j++) { - isnot(activeCues.getCueById(cue.ids[j]), undefined, "The cue with ID " + cue.ids[j] + " should be active."); - } - found = true; - } - } - if (!found) { - is(activeCues.length, 0, "There should be 0 currently active cues."); - } - }); - - video.addEventListener("ended", function(){ - SimpleTest.finish(); - }); - - video.play(); + SimpleTest.finish(); }); } ); From 900bc805d543c082fd8fdff8334b8c543e0c2361 Mon Sep 17 00:00:00 2001 From: Wes Kocher Date: Fri, 20 Dec 2013 18:33:04 -0800 Subject: [PATCH 11/12] Backed out changeset 6532bf066bb4 (bug 882299) --- content/media/TextTrackCue.cpp | 5 +++- content/media/TextTrackCue.h | 28 ++++++----------------- content/media/test/test_texttrackcue.html | 7 ------ dom/bindings/Codegen.py | 4 +--- dom/webidl/VTTCue.webidl | 3 ++- 5 files changed, 14 insertions(+), 33 deletions(-) diff --git a/content/media/TextTrackCue.cpp b/content/media/TextTrackCue.cpp index ab108e2694b0..ca06c4d60501 100644 --- a/content/media/TextTrackCue.cpp +++ b/content/media/TextTrackCue.cpp @@ -10,6 +10,9 @@ #include "nsComponentManagerUtils.h" #include "mozilla/ClearOnShutdown.h" +// Alternate value for the 'auto' keyword. +#define WEBVTT_AUTO -1 + namespace mozilla { namespace dom { @@ -36,7 +39,7 @@ TextTrackCue::SetDefaultCueSettings() mSize = 100; mPauseOnExit = false; mSnapToLines = true; - mLine.SetAsAutoKeyword() = AutoKeyword::Auto; + mLine = WEBVTT_AUTO; mAlign = AlignSetting::Middle; mLineAlign = AlignSetting::Start; mVertical = DirectionSetting::_empty; diff --git a/content/media/TextTrackCue.h b/content/media/TextTrackCue.h index 633316b0b9e7..545f1c5c8207 100644 --- a/content/media/TextTrackCue.h +++ b/content/media/TextTrackCue.h @@ -14,7 +14,6 @@ #include "nsIWebVTTParserWrapper.h" #include "mozilla/StaticPtr.h" #include "nsIDocument.h" -#include "mozilla/dom/UnionTypes.h" namespace mozilla { namespace dom { @@ -163,29 +162,16 @@ public: CueChanged(); } - void GetLine(OwningLongOrAutoKeyword& aLine) const + double Line() const { - if (mLine.IsLong()) { - aLine.SetAsLong() = mLine.GetAsLong(); - return; - } - aLine.SetAsAutoKeyword() = mLine.GetAsAutoKeyword(); + return mLine; } - void SetLine(const LongOrAutoKeyword& aLine) + void SetLine(double aLine) { - if (aLine.IsLong() && - (mLine.IsAutoKeyword() || (aLine.GetAsLong() != mLine.GetAsLong()))) { - mLine.SetAsLong() = aLine.GetAsLong(); - CueChanged(); - mReset = true; - return; - } - if (mLine.IsLong()) { - mLine.SetAsAutoKeyword() = aLine.GetAsAutoKeyword(); - CueChanged(); - mReset = true; - } + //XXX: TODO Line position can be a keyword auto. bug882299 + mReset = true; + mLine = aLine; } AlignSetting LineAlign() const @@ -356,7 +342,7 @@ private: bool mSnapToLines; nsString mRegionId; DirectionSetting mVertical; - LongOrAutoKeyword mLine; + int mLine; AlignSetting mAlign; AlignSetting mLineAlign; diff --git a/content/media/test/test_texttrackcue.html b/content/media/test/test_texttrackcue.html index 1231d6bad6fd..81a259f6d932 100644 --- a/content/media/test/test_texttrackcue.html +++ b/content/media/test/test_texttrackcue.html @@ -100,13 +100,6 @@ SpecialPowers.pushPrefEnv({"set": [["media.webvtt.enabled", true]]}, cue.lineAlign = "end"; is(cue.lineAlign, "end", "Cue's line align should be end."); - // Check cue.line - is(cue.line, "auto", "Cue's line value should initially be auto."); - cue.line = 12410 - is(cue.line, 12410, "Cue's line value should now be 12410."); - cue.line = "auto"; - is(cue.line, "auto", "Cue's line value should now be auto."); - // Check that we can create and add new VTTCues var vttCue = new VTTCue(3.999, 4, "foo"); trackElement.track.addCue(vttCue); diff --git a/dom/bindings/Codegen.py b/dom/bindings/Codegen.py index 32e59c1ebcdd..8c41a24a70d0 100644 --- a/dom/bindings/Codegen.py +++ b/dom/bindings/Codegen.py @@ -840,9 +840,7 @@ def UnionTypes(descriptors, dictionaries, callbacks, config): if typeNeedsRooting(f): headers.add("mozilla/dom/RootedDictionary.h") elif f.isEnum(): - # Need to see the actual definition of the enum, - # unfortunately. - headers.add(CGHeaders.getDeclarationFilename(f.inner)) + headers.add(CGHeaders.getDeclarationFilename(f)) map(addInfoForType, getAllTypes(descriptors, dictionaries, callbacks)) diff --git a/dom/webidl/VTTCue.webidl b/dom/webidl/VTTCue.webidl index 33b926c6d63a..5030ffaf2045 100644 --- a/dom/webidl/VTTCue.webidl +++ b/dom/webidl/VTTCue.webidl @@ -35,7 +35,8 @@ interface VTTCue : EventTarget { attribute DOMString regionId; attribute DirectionSetting vertical; attribute boolean snapToLines; - attribute (long or AutoKeyword) line; + // XXXhumph: https://www.w3.org/Bugs/Public/show_bug.cgi?id=20651 + // attribute (long or AutoKeyword) line; [SetterThrows] attribute AlignSetting lineAlign; [SetterThrows] From 84b5646ffbd00fef6f6b20ffe16884c506edbfd8 Mon Sep 17 00:00:00 2001 From: Wes Kocher Date: Fri, 20 Dec 2013 18:33:55 -0800 Subject: [PATCH 12/12] Backed out changeset df0855f26e4d (bug 949642) for introducing a new intermittent failure --- content/media/TextTrackCue.cpp | 1 - content/media/TextTrackCue.h | 21 ---------------- content/media/test/test_texttrackcue.html | 30 +---------------------- dom/webidl/VTTCue.webidl | 2 -- 4 files changed, 1 insertion(+), 53 deletions(-) diff --git a/content/media/TextTrackCue.cpp b/content/media/TextTrackCue.cpp index ca06c4d60501..d80920b84838 100644 --- a/content/media/TextTrackCue.cpp +++ b/content/media/TextTrackCue.cpp @@ -41,7 +41,6 @@ TextTrackCue::SetDefaultCueSettings() mSnapToLines = true; mLine = WEBVTT_AUTO; mAlign = AlignSetting::Middle; - mLineAlign = AlignSetting::Start; mVertical = DirectionSetting::_empty; } diff --git a/content/media/TextTrackCue.h b/content/media/TextTrackCue.h index 545f1c5c8207..70f345491313 100644 --- a/content/media/TextTrackCue.h +++ b/content/media/TextTrackCue.h @@ -174,26 +174,6 @@ public: mLine = aLine; } - AlignSetting LineAlign() const - { - return mLineAlign; - } - - void SetLineAlign(AlignSetting& aLineAlign, ErrorResult& aRv) - { - if (mLineAlign == aLineAlign) - return; - - if (aLineAlign == AlignSetting::Left || - aLineAlign == AlignSetting::Right) { - return aRv.Throw(NS_ERROR_DOM_SYNTAX_ERR); - } - - mReset = true; - mLineAlign = aLineAlign; - CueChanged(); - } - int32_t Position() const { return mPosition; @@ -344,7 +324,6 @@ private: DirectionSetting mVertical; int mLine; AlignSetting mAlign; - AlignSetting mLineAlign; // Holds the computed DOM elements that represent the parsed cue text. // http://www.whatwg.org/specs/web-apps/current-work/#text-track-cue-display-state diff --git a/content/media/test/test_texttrackcue.html b/content/media/test/test_texttrackcue.html index 81a259f6d932..f19cc76a1f71 100644 --- a/content/media/test/test_texttrackcue.html +++ b/content/media/test/test_texttrackcue.html @@ -72,34 +72,6 @@ SpecialPowers.pushPrefEnv({"set": [["media.webvtt.enabled", true]]}, cue.pauseOnExit = true; is(cue.pauseOnExit, true, "Cue's pause on exit flag should be true."); - // Check that cue line align works properly - is(cue.lineAlign, "start", "Cue's default line alignment should be start."); - - var exceptionHappened = false; - try { - cue.lineAlign = "left"; - } catch(e) { - exceptionHappened = true; - is(e.name, "SyntaxError", "Should have thrown SyntaxError."); - } - ok(exceptionHappened, "Exception should have happened."); - - exceptionHappened = false; - try { - cue.lineAlign = "right"; - } catch(e) { - exceptionHappened = true; - is(e.name, "SyntaxError", "Should have thrown SyntaxError."); - } - ok(exceptionHappened, "Exception should have happened."); - - cue.lineAlign = "middle"; - is(cue.lineAlign, "middle", "Cue's line align should be middle."); - cue.lineAlign = "START"; - is(cue.lineAlign, "middle", "Cue's line align should be middle."); - cue.lineAlign = "end"; - is(cue.lineAlign, "end", "Cue's line align should be end."); - // Check that we can create and add new VTTCues var vttCue = new VTTCue(3.999, 4, "foo"); trackElement.track.addCue(vttCue); @@ -119,7 +91,7 @@ SpecialPowers.pushPrefEnv({"set": [["media.webvtt.enabled", true]]}, trackElement.track.removeCue(cue); is(cueList.length, 4, "Cue list length should be 4."); - exceptionHappened = false; + var exceptionHappened = false; try { // We should not be able to remove a cue that is not in the list. cue = new VTTCue(1, 2, "foo"); diff --git a/dom/webidl/VTTCue.webidl b/dom/webidl/VTTCue.webidl index 5030ffaf2045..7bab05095fb1 100644 --- a/dom/webidl/VTTCue.webidl +++ b/dom/webidl/VTTCue.webidl @@ -38,8 +38,6 @@ interface VTTCue : EventTarget { // XXXhumph: https://www.w3.org/Bugs/Public/show_bug.cgi?id=20651 // attribute (long or AutoKeyword) line; [SetterThrows] - attribute AlignSetting lineAlign; - [SetterThrows] attribute long position; [SetterThrows] attribute long size;