1058898 - [e10s] Enable profiler tests for e10s

Status: RESOLVED FIXED

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

Description

[Brian Grinstead [:bgrins]]

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

Comment 1

[Brian Grinstead [:bgrins]]

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 *************************

Comment 2

[Brian Grinstead [:bgrins]]

Attachment: profiler-e10s-skip.patch

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

Comment 3

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/496721d62973

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/496721d62973

Comment 5

[Jordan Santell [:jsantell] [@jsantell]]

New performance front tests has some utilities for communicating with console methods across e10s, adding bug 1077442 as a dependency

Comment 6

[Victor Porof [:vporof][:vp]]

Attachment: v1

Magic.
Try: https://tbpl.mozilla.org/?tree=Try&rev=7f3dd26d19f2

Comment 7

[Victor Porof [:vporof][:vp]]

Keeping leave-open because of bug 1047124; we'll have to verify the disabled tests again after that bug is fixed.

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/922e304672bf

Comment 9

[Tom Tromey :tromey]

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 10

[Victor Porof [:vporof][:vp]]

Comment on attachment 8514252 [details] [diff] [review]
v1

Removing patch since we should now focus on fixing the remaining tests.

Comment 11

[Chris Peterson [:cpeterson]]

Moving DevTools test bugs to e10s milestone M7 (i.e. not blocking e10s merge to Aurora).

Comment 13

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1058898-perf-e10s.patch

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

Comment 14

[Victor Porof [:vporof][:vp]]

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.

Comment 15

[Jordan Santell [:jsantell] [@jsantell]]

Ah, good call making them explicitly bridges to the nsIProfiler.

Comment 16

[Jordan Santell [:jsantell] [@jsantell]]

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.

Comment 17

[Dave Townsend [:mossop]]

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)

Comment 18

[Jordan Santell [:jsantell] [@jsantell]]

(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

Comment 19

[Dave Townsend [:mossop]]

(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.

Comment 20

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1058898-perf-e10s.patch

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)

Comment 21

[Jordan Santell [:jsantell] [@jsantell]]

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.

Comment 22

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1058898-perf-e10s.patch

ughghghgh
https://treeherder.mozilla.org/#/jobs?repo=try&revision=495fb38ac44e

Comment 23

[Victor Porof [:vporof][:vp]]

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.

Comment 24

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1058898-perf-e10s.patch

Oops, you are right -- renamed those methods

Comment 25

[Jordan Santell [:jsantell] [@jsantell]]

https://hg.mozilla.org/integration/fx-team/rev/b14a2608032e

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b14a2608032e

Comment 27

[Brian Grinstead [:bgrins]]

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

Comment 28

[Brian Grinstead [:bgrins]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/c7d51f7a9a55

Comment 29

[Brian Grinstead [:bgrins]]

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 30

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8608836 [details] [diff] [review]
1058898-perf-e10s.patch

Change approved to skip one train as part of the spring campaign.