1184486 - Let PerformanceStats.jsm play nicer with process-per-tab

Status: RESOLVED FIXED

(Toolkit :: Performance Monitoring, defect)

Comment 1

[David Teller [:Yoric] - still alive but not very active]

Attachment: MozReview Request: Bug 1184486 - Let PerformanceStats.jsm play nicer with process-per-tab;r=mconley

Bug 1184486 - Let PerformanceStats.jsm play nicer with process-per-tab;r?mconley

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8634665 [details]
MozReview Request: Bug 1184486 - Let PerformanceStats.jsm play nicer with process-per-tab;r=mconley

https://reviewboard.mozilla.org/r/13429/#review12085

One suggestion, one question.

::: toolkit/components/perfmonitoring/tests/browser/browser_compartments.js:37
(Diff revision 1)
> +let pid = getpid();

Because of the mixture of scopes in this file, pid is getting overloaded and it gets a bit confusing.

Maybe: `const PARENT_PID = getpid();`?

::: toolkit/components/perfmonitoring/tests/browser/browser_compartments.js:50
(Diff revision 1)
> +      let sysName = Services.sysinfo.getPropertyAsAString("name");
> +      if (sysName == "Windows_NT") {
> +        let libc = ctypes.open("kernel32.dll");
> +        return libc.declare("GetCurrentProcessId",
> +          ctypes.winapi_abi,
> +          ctypes.uint32_t
> +        );
> +      } else {
> +        let libc = ctypes.open(ctypes.libraryName("c"));
> +        return libc.declare("getpid",
> +          ctypes.default_abi,
> +          ctypes.int  // PID
> +        );
> +      }

Will this work with sandboxing?

Comment 3

[David Teller [:Yoric] - still alive but not very active]

https://reviewboard.mozilla.org/r/13429/#review12085

> Will this work with sandboxing?

Well, if `getpid`/`GetCurrentProcessId` is blocked by sandboxing, the C++ code will also break, so I'm going to assume that this should work.

Comment 4

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8634665 [details]
MozReview Request: Bug 1184486 - Let PerformanceStats.jsm play nicer with process-per-tab;r=mconley

Bug 1184486 - Let PerformanceStats.jsm play nicer with process-per-tab;r?mconley

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Can we run this through Try now so it can land ASAP once it has review? It's blocking a frequent crash in automation :(

Comment 6

[David Teller [:Yoric] - still alive but not very active]

For consistency, I have moved the fixup patch to bug 1175098.

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8634665 [details]
MozReview Request: Bug 1184486 - Let PerformanceStats.jsm play nicer with process-per-tab;r=mconley

https://reviewboard.mozilla.org/r/13429/#review12411

Ship It!

Comment 8

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8634665 [details]
MozReview Request: Bug 1184486 - Let PerformanceStats.jsm play nicer with process-per-tab;r=mconley

Bug 1184486 - Let PerformanceStats.jsm play nicer with process-per-tab;r=mconley

Comment 9

[David Teller [:Yoric] - still alive but not very active]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=07937c30c948

Comment 10

[David Teller [:Yoric] - still alive but not very active]

Mmmh...
Apparently, my libc call in the test doesn't work on Linux. I'll see what I can do about that.

Comment 11

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8634665 [details]
MozReview Request: Bug 1184486 - Let PerformanceStats.jsm play nicer with process-per-tab;r=mconley

Bug 1184486 - Let PerformanceStats.jsm play nicer with process-per-tab;r=mconley

Comment 12

[David Teller [:Yoric] - still alive but not very active]

Attachment: MozReview Request: Bug 1184486 - Publish pid in nsIAppStartup;r?bsmedberg

Bug 1184486 - Publish pid in nsIAppStartup;r?bsmedberg

Comment 13

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8638432 [details]
MozReview Request: Bug 1184486 - Publish pid in nsIAppStartup;r?bsmedberg

Bug 1184486 - Publish pid in nsIAppStartup;r?bsmedberg

Comment 14

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8634665 [details]
MozReview Request: Bug 1184486 - Let PerformanceStats.jsm play nicer with process-per-tab;r=mconley

Bug 1184486 - Let PerformanceStats.jsm play nicer with process-per-tab;r=mconley

Comment 15

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8634665 [details]
MozReview Request: Bug 1184486 - Let PerformanceStats.jsm play nicer with process-per-tab;r=mconley

Bug 1184486 - Let PerformanceStats.jsm play nicer with process-per-tab;r=mconley

Comment 16

[David Teller [:Yoric] - still alive but not very active]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ae22babdf347

Comment 17

[Pulsebot]

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

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Backed out for Static Analysis failures.
https://treeherder.mozilla.org/logviewer.html#?job_id=3974414&repo=fx-team

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

Comment 19

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8634665 [details]
MozReview Request: Bug 1184486 - Let PerformanceStats.jsm play nicer with process-per-tab;r=mconley

Bug 1184486 - Let PerformanceStats.jsm play nicer with process-per-tab;r=mconley

Comment 20

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8634665 [details]
MozReview Request: Bug 1184486 - Let PerformanceStats.jsm play nicer with process-per-tab;r=mconley

https://reviewboard.mozilla.org/r/13429/#review12961

Ship It!

Comment 21

[David Teller [:Yoric] - still alive but not very active]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4892f93c5e8e

Thanks for the help with mozreview, mconley.

Comment 22

[Pulsebot]

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

Comment 23

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/14ae070d4a41