Closed Bug 1058898 Opened 9 years ago Closed 9 years ago

[e10s] Enable profiler tests for e10s

Categories

(DevTools :: Performance Tools (Profiler/Timeline), defect)

defect
Not set
normal

Tracking

(e10s+, firefox40 fixed, firefox41 fixed)

RESOLVED FIXED
Firefox 41
Tracking Status
e10s + ---
firefox40 --- fixed
firefox41 --- fixed

People

(Reporter: bgrins, Assigned: jsantell)

References

Details

(Whiteboard: [e10s-m7])

Attachments

(1 file, 5 obsolete files)

There are some failures in e10s mode, as can be seen in this log: https://tbpl.mozilla.org/php/getParsedLog.php?id=46490101&tree=Holly&full=1.

171 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_aaa_run_first_leaktest.js | Test timed out
173 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_aaa_run_first_leaktest.js | Found a tab after previous test timed out: http://example.com/browser/browser/devtools/profiler/test/doc_simple-test.html
295 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_jump-to-debugger-01.js | Test timed out
297 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_jump-to-debugger-01.js | Found a tab after previous test timed out: http://example.com/browser/browser/devtools/profiler/test/doc_simple-test.html
369 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_jump-to-debugger-02.js | Test timed out
371 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_jump-to-debugger-02.js | Found a tab after previous test timed out: http://example.com/browser/browser/devtools/profiler/test/doc_simple-test.html
436 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_profile-view-events.js | Test timed out
438 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_profile-view-events.js | Found a tab after previous test timed out: http://example.com/browser/browser/devtools/profiler/test/doc_simple-test.html
501 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_recordings-io-02.js | Test timed out
503 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_recordings-io-02.js | Found a tab after previous test timed out: http://example.com/browser/browser/devtools/profiler/test/doc_simple-test.html
566 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_recordings-io-03.js | Test timed out
568 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_recordings-io-03.js | Found a tab after previous test timed out: http://example.com/browser/browser/devtools/profiler/test/doc_simple-test.html
806 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_shared-connection-02.js | Test timed out
808 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_shared-connection-02.js | Found a tab after previous test timed out: http://example.com/browser/browser/devtools/profiler/test/doc_simple-test.html
I can confirm locally with: mach mochitest-devtools --e10s browser/devtools/profiler/test/browser_profiler_aaa_run_first_leaktest.js

10 INFO *************************
11 INFO A coding exception was thrown and uncaught in a Task.
12 INFO Full message: TypeError: target.window is null
13 INFO Full stack: initFrontend@chrome://mochitests/content/browser/browser/devtools/profiler/test/head.js:77:7
14 INFO Tester_execTest@chrome://mochikit/content/browser-test.js:651:9
15 INFO Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:548:7
16 INFO this.DOMApplicationRegistry.doGetList@resource://gre/modules/Webapps.jsm:1216:7
17 INFO this.DOMApplicationRegistry.receiveMessage@resource://gre/modules/Webapps.jsm:1036:1
18 INFO *************************
19 INFO *************************
20 INFO A coding exception was thrown and uncaught in a Task.
21 INFO Full message: TypeError: target.window is null
22 INFO Full stack: initFrontend@chrome://mochitests/content/browser/browser/devtools/profiler/test/head.js:77:7
23 INFO Tester_execTest@chrome://mochikit/content/browser-test.js:651:9
24 INFO Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:548:7
25 INFO this.DOMApplicationRegistry.doGetList@resource://gre/modules/Webapps.jsm:1216:7
26 INFO this.DOMApplicationRegistry.receiveMessage@resource://gre/modules/Webapps.jsm:1036:1
27 INFO Tester_execTest@chrome://mochikit/content/browser-test.js:651:9
28 INFO Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:548:7
29 INFO this.DOMApplicationRegistry.doGetList@resource://gre/modules/Webapps.jsm:1216:7
30 INFO this.DOMApplicationRegistry.receiveMessage@resource://gre/modules/Webapps.jsm:1036:1
31 INFO *************************
Attached patch profiler-e10s-skip.patch (obsolete) — Splinter Review
Skip these tests for now in e10s to help make the dt suite green.  Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=9a60ec87ef7d
Attachment #8485296 - Flags: review?(vporof)
Attachment #8485296 - Attachment description: profiler-e10s.patch → profiler-e10s-skip.patch
Attachment #8485296 - Attachment filename: profiler-e10s.patch → profiler-e10s-skip.patch
Attachment #8485296 - Flags: review?(vporof) → review+
Attachment #8485296 - Flags: checkin+
New performance front tests has some utilities for communicating with console methods across e10s, adding bug 1077442 as a dependency
Depends on: 1077442
Attached patch v1 (obsolete) — Splinter Review
Magic.
Try: https://tbpl.mozilla.org/?tree=Try&rev=7f3dd26d19f2
Assignee: nobody → vporof
Attachment #8485296 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8514252 - Flags: review?(ejpbruel)
Keeping leave-open because of bug 1047124; we'll have to verify the disabled tests again after that bug is fixed.
Attachment #8514252 - Flags: review?(ejpbruel) → review+
I applied this patch plus the patches from bug 1047124 and bug 1084902
and ran the profiler tests with --e10s.  I saw a number of failures:

2858 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_console-record-01.js | Test timed out - expected PASS
2859 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_console-record-01.js | Found a tab after previous test timed out: http://example.com/browser/browser/devtools/profiler/test/doc_simple-test.html - expected PASS
2860 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_console-record-02.js | Test timed out - expected PASS
2861 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_console-record-02.js | Found a tab after previous test timed out: http://example.com/browser/browser/devtools/profiler/test/doc_simple-test.html - expected PASS
2862 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_console-record-03.js | Test timed out - expected PASS
2863 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_console-record-03.js | Found a tab after previous test timed out: http://example.com/browser/browser/devtools/profiler/test/doc_simple-test.html - expected PASS
2864 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_console-record-04.js | Test timed out - expected PASS
2865 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_console-record-04.js | Found a tab after previous test timed out: http://example.com/browser/browser/devtools/profiler/test/doc_simple-test.html - expected PASS
2866 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_console-record-05.js | Test timed out - expected PASS
2867 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_console-record-05.js | Found a tab after previous test timed out: http://example.com/browser/browser/devtools/profiler/test/doc_simple-test.html - expected PASS
2868 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_console-record-06.js | Test timed out - expected PASS
2869 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_console-record-06.js | Found a tab after previous test timed out: http://example.com/browser/browser/devtools/profiler/test/doc_simple-test.html - expected PASS
2870 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_console-record-07.js | Test timed out - expected PASS
2871 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_console-record-07.js | Found a tab after previous test timed out: http://example.com/browser/browser/devtools/profiler/test/doc_simple-test.html - expected PASS
2872 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_console-record-08.js | Test timed out - expected PASS
2873 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_console-record-08.js | Found a tab after previous test timed out: http://example.com/browser/browser/devtools/profiler/test/doc_simple-test.html - expected PASS
2874 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_console-record-09.js | Test timed out - expected PASS
2875 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_console-record-09.js | Found a tab after previous test timed out: http://example.com/browser/browser/devtools/profiler/test/doc_simple-test.html - expected PASS
2876 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_recording-cancelled.js | Test timed out - expected PASS
2877 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_recording-cancelled.js | Found a tab after previous test timed out: http://example.com/browser/browser/devtools/profiler/test/doc_simple-test.html - expected PASS
2878 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_recordings-clear.js | Test timed out - expected PASS
2879 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_recordings-clear.js | Found a tab after previous test timed out: http://example.com/browser/browser/devtools/profiler/test/doc_simple-test.html - expected PASS
2880 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_shared-connection-03.js | The built-in profiler module should now be active. - 
Stack trace:
    chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_shared-connection-03.js:test<:18
    self-hosted:next:883
    Tester_execTest@chrome://mochikit/content/browser-test.js:674:9
    Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:571:7
    SimpleTest.waitForFocus/maybeRunTests/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:683:49
2881 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_shared-connection-04.js | The profiler should not be active yet. - 
Stack trace:
    chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_shared-connection-04.js:test<:24
    self-hosted:next:883
    Tester_execTest@chrome://mochikit/content/browser-test.js:674:9
    Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:571:7
    SimpleTest.waitForFocus/maybeRunTests/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:683:49
2882 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_shared-connection-04.js | Test timed out - expected PASS
2883 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_shared-connection-04.js | Found a tab after previous test timed out: http://example.com/browser/browser/devtools/profiler/test/doc_simple-test.html - expected PASS
2884 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_shared-front-01.js | Test timed out - expected PASS
2885 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_shared-front-01.js | Found a tab after previous test timed out: http://example.com/browser/browser/devtools/profiler/test/doc_simple-test.html - expected PASS
2886 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_shared-front-02.js | Test timed out - expected PASS
2887 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_shared-front-02.js | Found a tab after previous test timed out: http://example.com/browser/browser/devtools/profiler/test/doc_simple-test.html - expected PASS
2888 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_shared-front-03.js | Test timed out - expected PASS
2889 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_shared-front-03.js | Found a tab after previous test timed out: http://example.com/browser/browser/devtools/profiler/test/doc_simple-test.html - expected PASS
2890 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_shared-front-04.js | The built-in profiler module should have been automatically stoped. - 
Stack trace:
    chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_shared-front-04.js:test<:39
    self-hosted:next:883
    Tester_execTest@chrome://mochikit/content/browser-test.js:674:9
    Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:571:7
    SimpleTest.waitForFocus/maybeRunTests/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:683:49
2891 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_shared-front-05.js | Test timed out - expected PASS
2892 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_shared-front-05.js | Found a tab after previous test timed out: http://example.com/browser/browser/devtools/profiler/test/doc_simple-test.html - expected PASS
2893 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_shared-front-06.js | Test timed out - expected PASS
2894 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_shared-front-06.js | Found a tab after previous test timed out: http://example.com/browser/browser/devtools/profiler/test/doc_simple-test.html - expected PASS
2895 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_simple-record-02.js | Test timed out - expected PASS
2896 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_simple-record-02.js | Found a tab after previous test timed out: http://example.com/browser/browser/devtools/profiler/test/doc_simple-test.html - expected PASS
2897 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_simple-record-03.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:1113 - Error: Connection closed
Stack trace:
Front<.destroy@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:1113:23
ProfilerConnection.prototype._disconnectMiscActors@resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:150:5
ProfilerConnection.prototype.destroy@resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:119:5
ProfilerPanel.prototype.destroy<@resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/panel.js:64:5
RecordingsListView<._onRecordButtonClick<@chrome://browser/content/devtools/ui-recordings.js:234:33
oncommand@chrome://browser/content/devtools/profiler.xul:1:1
doApply@chrome://mochikit/content/tests/SimpleTest/specialpowersAPI.js:153:10
wrapPrivileged/callTrap@chrome://mochikit/content/tests/SimpleTest/specialpowersAPI.js:177:31
synthesizeMouseAtPoint@chrome://mochikit/content/tests/SimpleTest/EventUtils.js:269:7
synthesizeMouse@chrome://mochikit/content/tests/SimpleTest/EventUtils.js:221:1
synthesizeMouseAtCenter@chrome://mochikit/content/tests/SimpleTest/EventUtils.js:328:1
stopRecording@chrome://mochitests/content/browser/browser/devtools/profiler/test/head.js:143:3
TaskImpl_run@resource://gre/modules/Task.jsm:314:40
TaskImpl@resource://gre/modules/Task.jsm:275:3
createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:249:14
Task_spawn@resource://gre/modules/Task.jsm:164:12
TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:381:1
TaskImpl_run@resource://gre/modules/Task.jsm:322:13
Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:868:23
this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:747:7
2898 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_sudden-deactivation-01.js | Test timed out - expected PASS
2899 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_sudden-deactivation-01.js | Found a tab after previous test timed out: http://example.com/browser/browser/devtools/profiler/test/doc_simple-test.html - expected PASS
2900 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_sudden-deactivation-02.js | Test timed out - expected PASS
2901 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_sudden-deactivation-02.js | Found a tab after previous test timed out: http://example.com/browser/browser/devtools/profiler/test/doc_simple-test.html - expected PASS
2902 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_tabbed-browser-01.js | Test timed out - expected PASS
2903 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_tabbed-browser-01.js | Found a tab after previous test timed out: http://example.com/browser/browser/devtools/profiler/test/doc_simple-test.html - expected PASS
2904 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_tabbed-browser-02.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:1113 - Error: Connection closed
Stack trace:
Front<.destroy@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:1113:23
ProfilerConnection.prototype._disconnectMiscActors@resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:150:5
ProfilerConnection.prototype.destroy@resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:119:5
ProfilerPanel.prototype.destroy<@resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/panel.js:64:5
RecordingsListView<._onRecordButtonClick<@chrome://browser/content/devtools/ui-recordings.js:234:33
oncommand@chrome://browser/content/devtools/profiler.xul:1:1
doApply@chrome://mochikit/content/tests/SimpleTest/specialpowersAPI.js:153:10
wrapPrivileged/callTrap@chrome://mochikit/content/tests/SimpleTest/specialpowersAPI.js:177:31
synthesizeMouseAtPoint@chrome://mochikit/content/tests/SimpleTest/EventUtils.js:269:7
synthesizeMouse@chrome://mochikit/content/tests/SimpleTest/EventUtils.js:221:1
synthesizeMouseAtCenter@chrome://mochikit/content/tests/SimpleTest/EventUtils.js:328:1
stopRecording@chrome://mochitests/content/browser/browser/devtools/profiler/test/head.js:143:3
TaskImpl_run@resource://gre/modules/Task.jsm:314:40
TaskImpl@resource://gre/modules/Task.jsm:275:3
createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:249:14
Task_spawn@resource://gre/modules/Task.jsm:164:12
TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:381:1
TaskImpl_run@resource://gre/modules/Task.jsm:322:13
Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:868:23
this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:747:7
2905 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_tabbed-browser-02.js | The first displayed item in the tree has the expected function name. - Got Startup::XRE_InitChildProcess, expected Startup::XRE_Main
Stack trace:
    chrome://mochikit/content/browser-test.js:test_is:828
    chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_tabbed-browser-02.js:test<:35
    self-hosted:next:883
    Tester_execTest@chrome://mochikit/content/browser-test.js:674:9
    Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:571:7
    SimpleTest.waitForFocus/maybeRunTests/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:683:49
2906 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_tabbed-browser-02.js | The first displayed item in the tree has the expected function name. - Got Startup::XRE_InitChildProcess, expected Startup::XRE_Main
Stack trace:
    chrome://mochikit/content/browser-test.js:test_is:828
    chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_tabbed-browser-02.js:test<:78
    self-hosted:next:883
    Tester_execTest@chrome://mochikit/content/browser-test.js:674:9
    Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:571:7
    SimpleTest.waitForFocus/maybeRunTests/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:683:49
Comment on attachment 8514252 [details] [diff] [review]
v1

Removing patch since we should now focus on fixing the remaining tests.
Attachment #8514252 - Attachment is obsolete: true
Assignee: vporof → nobody
Status: ASSIGNED → NEW
Moving DevTools test bugs to e10s milestone M7 (i.e. not blocking e10s merge to Aurora).
Whiteboard: [e10s-m7]
No longer blocks: 1058875
Summary: Enable profiler tests for e10s → [e10s] Enable profiler tests for e10s
Attached patch 1058898-perf-e10s.patch (obsolete) — Splinter Review
This passes all tests with e10s on.

We have to directly access the nsIProfiler on the child process, so this uses a process MM to do this. Is there a better way? Can we inject arbitrary commands into the child process but with chrome privilege, so we don't permanently need this listener in the server actor?

Other thing that was weird was a test that forces GC (so we can measure markers being emitted) -- seems to take several seconds of spamming force GC (SpecialPowers.DOMWindowUtils.garbageCollect()). Is there anything I'm missing here regarding e10s and GC?

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4b8f824b63ad
Assignee: nobody → jsantell
Status: NEW → ASSIGNED
Attachment #8603805 - Flags: feedback?(vporof)
Attachment #8603805 - Flags: feedback?(ejpbruel)
Comment on attachment 8603805 [details] [diff] [review]
1058898-perf-e10s.patch

Review of attachment 8603805 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/devtools/performance/test/head.js
@@ +390,5 @@
> +  PMM.broadcastAsyncMessage("devtools-profiler-command:request", { method, args });
> +  return promise;
> +}
> +
> +function isProfilerActive () {

Maybe prefix these methods with PMM_, or rename them to isProfilerModuleActive and forceStopProfilerModule to avoid some confusion. When you have stopProfiling and stopProfile and other variations of that, things can get messy.
Attachment #8603805 - Flags: feedback?(vporof) → feedback+
Ah, good call making them explicitly bridges to the nsIProfiler.
Comment on attachment 8603805 [details] [diff] [review]
1058898-perf-e10s.patch

Dave, can you check this out and see if this is a good way to handle this in e10s? Mainly the listener on the profiler actor on the server.
Attachment #8603805 - Flags: feedback?(ejpbruel) → feedback?(dtownsend)
Comment on attachment 8603805 [details] [diff] [review]
1058898-perf-e10s.patch

Review of attachment 8603805 [details] [diff] [review]:
-----------------------------------------------------------------

My one concern here is around the number of child processes. Even in current e10s there are technically two processes your "devtools-profiler-command:request" message will get sent to, the child process and the chrome process. When we increase the process count in the future there will be more. Are you sure that the profiler actor is only running in one?

::: browser/devtools/performance/test/head.js
@@ +13,5 @@
>  let { DevToolsUtils } = Cu.import("resource://gre/modules/devtools/DevToolsUtils.jsm", {});
>  let { DebuggerServer } = Cu.import("resource://gre/modules/devtools/dbg-server.jsm", {});
>  let { merge } = devtools.require("sdk/util/object");
>  let { getPerformanceActorsConnection, PerformanceFront } = devtools.require("devtools/performance/front");
> +let PMM = Cc["@mozilla.org/parentprocessmessagemanager;1"].getService(Ci.nsIMessageBroadcaster);

Services.ppmm

@@ +388,5 @@
> +    }
> +  });
> +  PMM.broadcastAsyncMessage("devtools-profiler-command:request", { method, args });
> +  return promise;
> +}

Do you expect to ever send the same command quickly? Might want to throw in a unique ID to distinguish responses in that case.

::: toolkit/devtools/server/actors/profiler.js
@@ +5,5 @@
>  
>  const {Cc, Ci, Cu, Cr} = require("chrome");
>  const Services = require("Services");
>  const DevToolsUtils = require("devtools/toolkit/DevToolsUtils.js");
> +const PMM = Cc["@mozilla.org/childprocessmessagemanager;1"].getService(Ci.nsISyncMessageSender);

Services.cpmm

@@ +35,5 @@
> + * with chrome processes.
> + * Used mostly for tests in multiprocess Firefox without requiring an RDP connection.
> + */
> +PMM.addMessageListener("devtools-profiler-command:request", function ({ data: { method, args }}) {
> +  let result = nsIProfilerModule[method].apply(nsIProfilerModule, args || []);

nsIProfilerMethod[method](...args)
Attachment #8603805 - Flags: feedback?(dtownsend)
(In reply to Dave Townsend [:mossop] from comment #17)
> Comment on attachment 8603805 [details] [diff] [review]
> 1058898-perf-e10s.patch
> 
> Review of attachment 8603805 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> My one concern here is around the number of child processes. Even in current
> e10s there are technically two processes your
> "devtools-profiler-command:request" message will get sent to, the child
> process and the chrome process. When we increase the process count in the
> future there will be more. Are you sure that the profiler actor is only
> running in one?

In e10s, it's only running in the child process -- is there a more appropriate MM to scope communication just to a single child process with future process expansion?

> 
> @@ +388,5 @@
> > +    }
> > +  });
> > +  PMM.broadcastAsyncMessage("devtools-profiler-command:request", { method, args });
> > +  return promise;
> > +}
> 
> Do you expect to ever send the same command quickly? Might want to throw in
> a unique ID to distinguish responses in that case.

No, these should just be tests, but we can extend some other communication patterns we have that use incrementing IDs to cover our bases
(In reply to Jordan Santell [:jsantell] [@jsantell] from comment #18)
> (In reply to Dave Townsend [:mossop] from comment #17)
> > Comment on attachment 8603805 [details] [diff] [review]
> > 1058898-perf-e10s.patch
> > 
> > Review of attachment 8603805 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > My one concern here is around the number of child processes. Even in current
> > e10s there are technically two processes your
> > "devtools-profiler-command:request" message will get sent to, the child
> > process and the chrome process. When we increase the process count in the
> > future there will be more. Are you sure that the profiler actor is only
> > running in one?
> 
> In e10s, it's only running in the child process -- is there a more
> appropriate MM to scope communication just to a single child process with
> future process expansion?

The simplest way is to have the profiler send a message up the parent. A listener on Services.ppmm receives the message and the target it receives is the message manager for just that child.
Attached patch 1058898-perf-e10s.patch (obsolete) — Splinter Review
Addressed previous comments.

As of now, it will probably fail if we have more than 1 child process (or at the very least, be confused), because myself I'm not quite sure how multiple profiler's would work (one for each tab, possibly).

We need a nicer way to manage this interprocess communication with actors in these cases, but I don't want to not have e10s tests running on this while we wait to figure that out (and imagine all the different, horrible things that can go wrong when we have multiple SPS profiler instances running)
Attachment #8603805 - Attachment is obsolete: true
Attachment #8608184 - Flags: review?(vporof)
Comment on attachment 8608184 [details] [diff] [review]
1058898-perf-e10s.patch

Cancelling review -- does not work with new tests that run not needing the profiler (marker tests), since the profiler does not exist yet to listen to the responses. I think we'll need something a part of protocol.js to handle this.
Attachment #8608184 - Flags: review?(vporof)
Comment on attachment 8608415 [details] [diff] [review]
1058898-perf-e10s.patch

Review of attachment 8608415 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/devtools/performance/test/head.js
@@ +572,5 @@
>      markers: []
>    }, uniqueStacks);
>  }
> +
> +function isProfilerActive () {

From my previous review:

Maybe prefix these methods with PMM_, or rename them to isProfilerModuleActive and forceStopProfilerModule to avoid some confusion. When you have stopProfiling and stopProfile and other variations of that, things can get messy.

@@ +576,5 @@
> +function isProfilerActive () {
> +  return sendProfilerCommand("IsActive");
> +}
> +
> +function stopProfiler () {

Ditto.
Attachment #8608415 - Flags: review?(vporof) → review+
Oops, you are right -- renamed those methods
Attachment #8608415 - Attachment is obsolete: true
Attachment #8608836 - Flags: review+
https://hg.mozilla.org/integration/fx-team/rev/b14a2608032e
Keywords: checkin-needed
Whiteboard: [e10s-m7] → [e10s-m7][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/b14a2608032e
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [e10s-m7][fixed-in-fx-team] → [e10s-m7]
Target Milestone: --- → Firefox 41
Flags: qe-verify-
Comment on attachment 8608836 [details] [diff] [review]
1058898-perf-e10s.patch


Approval Request Comment
[Feature/regressing bug #]: 1167252, the new performance tool
[User impact if declined]: Won't ship the performance tool
[Describe test coverage new/current, TreeHerder]: There are try pushes in Bug 1167252 with all patches needing uplift
[Risks and why]: Requesting uplift for the accumulated changes in the performance tool since the 40 merge date, so these changes haven't had the full 6 weeks to bake.  Risks are generally contained within devtools, specifically within the performance panel.
[String/UUID change made/needed]: None
Attachment #8608836 - Flags: approval-mozilla-aurora?
Note: I had verbal confirmation for these uplifts from Sylvestre even before he's flagged them as a+.  See https://bugzilla.mozilla.org/show_bug.cgi?id=1167252#c26
Comment on attachment 8608836 [details] [diff] [review]
1058898-perf-e10s.patch

Change approved to skip one train as part of the spring campaign.
Attachment #8608836 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.