1507709 - Move geckoProfiler API to toolkit

Status: VERIFIED FIXED

(WebExtensions :: Developer Tools, enhancement, P5)

Description

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

The browser.geckoProfiler API is in browser/components/extensions, but it seems to me none of the code there depends on a Firefox feature. I'd like to move this code to toolkit, so that Thunderbird can also profit from it.

Not quite done yet, but the try run seems to be passing:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0e4d489ba70d813d69918172cfaae7293f5ecead

I've also done a brief test of the gecko profiler add-on in the resulting build.

Comment 1

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: Bug 1507709 - Move WebExtensions geckoProfiler API to toolkit. r=kmag

Comment 2

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Hmm, a process crash on Android. Any ideas how to debug this without an Android device? Or does Android just not support the profiler and I should turn off those tests?

Comment 3

[Rob Wu [:robwu]]

You could use an emulator if you don't have a physical device. See https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_for_Android_build

Comment 4

[Markus Stange [:mstange]]

(In reply to Philipp Kewisch [:Fallen] [:📆] from comment #2)
> Hmm, a process crash on Android. Any ideas how to debug this without an
> Android device? Or does Android just not support the profiler and I should
> turn off those tests?

I don't think your patch changes anything that might trigger this crash, so it's probably an existing crash that would also happen without your patch. It only happens on the "Android 4.2 opt x86" platform, and looking at a mozilla-central job such as
https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&searchStr=android%2C4.2%2Copt&revision=24e87b02707bee36e1e98eb37c94fbaf3834e898
, it looks like none of the X jobs on that platform currently run the test_ext_geckoProfiler_control.js test. I don't know why that is.
So let's just disable the test on Android x86 for now.

Comment 5

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

(In reply to Markus Stange [:mstange] from comment #4)
> , it looks like none of the X jobs on that platform currently run the
> test_ext_geckoProfiler_control.js test. I don't know why that is.
> So let's just disable the test on Android x86 for now.

Thanks for digging into this Markus! test_ext_geckoProfiler_control.js was previously in browser/ so I am not surprised this test did not run previously. I moved all the tests over because the point of this bug is to make browser.geckoProfiler available to all gecko apps, which also includes Firefox for Android.

I'm happy to disable the test for now though, I'll add a comment referencing bug 1510301.

Comment 6

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

>>! In D12100#357139, @kmag wrote:
> I'm fine with moving this to toolkit, but we need to make sure we don't package it in places where it's not supported.

My understanding is that this is generally supported under Android, but bug 1510301 is causing a crash under certain circumstances. I'm not sure how to not package this in Android aside from moving the ext-toolkit.json to ext-browser.json/ext-mail.json but referencing the toolkit files. Is that what you were intending? Or can we leave it in ext-toolkit and assume that Android folks will fix the crash instead?

Comment 7

[llrraa]

(In reply to Philipp Kewisch [:Fallen] [:📆] from comment #6)

! In D12100#357139, @kmag wrote:
I'm fine with moving this to toolkit, but we need to make sure we don't package it in places where it's not supported.

My understanding is that this is generally supported under Android, but bug
1510301 is causing a crash under certain circumstances. I'm not sure how to
not package this in Android aside from moving the ext-toolkit.json to
ext-browser.json/ext-mail.json but referencing the toolkit files. Is that
what you were intending? Or can we leave it in ext-toolkit and assume that
Android folks will fix the crash instead?

Is it working for windows thunderbird? As @Wayne Mery (:wsmwk) said, I have a bug depending on your code? Thanks.

Comment 8

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

I'd assume yes, but let's keep the discussion here focused on moving the APIs. If it doesn't work for Thunderbird afterwards we can file a bug in the Thunderbird component (where the dependencies here should go instead btw)

Comment 9

[llrraa]

(In reply to Philipp Kewisch [:Fallen] [:📆] from comment #8)

I'd assume yes, but let's keep the discussion here focused on moving the APIs. If it doesn't work for Thunderbird afterwards we can file a bug in the Thunderbird component (where the dependencies here should go instead btw)

Then which branch should we checkout for bug 1512974? @Wayne Mery (:wsmwk) said a profiler will point us to the hot code which can identify the cause that makes thunderbird slow down.

Comment 10

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Kris, whats the status here? I'd like to move this forward. I'd appreciate a response and am happy to defer to e.g. zombie if you are low on time.

Comment 11

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Updated try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=fffdcf151b3e210fe87c57dcadb47d582f9df1b5

Comment 12

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Ok, I think I got it now. Sorry for missing some of the shipping bits, I thought it was ok to keep them in as part of the platform. Updated try:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=80b9feaf71c62f9b1285f07cc147134c675371a6

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2a4bddaaf3dc
Move WebExtensions geckoProfiler API to toolkit. r=kmag

Comment 14

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/2a4bddaaf3dc

Comment 15

[Madalin Cotetiu]

Can you please provide some STR for this issue so we can check it manually? If no manual testing is needed please mark it as "qe-verify- "

Comment 16

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

STR:

1. Go to https://profiler.firefox.com/ and install the gecko profiler add-on
2. Start a profile using the browser action toolbar button
3. Finish profiling by clicking capture profile and view the profile

If you can view the profile, then it is very likely these changes did not break anything.

Comment 17

[Madalin Cotetiu]

This issue is verified as fixed on Firefox 68.0a1 (20190508111321) under Win 7 64-bit and Mac OS X 10.14.1 using the steps from the previous comment.

Comment 18

[llrraa]

Yesterday I tried to start the performance profiler, but failed to Capture Profile, and ctrl+shift+2 is not working either. Please help.

Application Basics

Name: Thunderbird
Version: 72.0a1
Build ID: 20191104093451

Update Channel: nightly
User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:72.0) Gecko/20100101 Thunderbird/72.0a1
OS: Windows_NT 6.1

Launcher Process: Enabled
Multiprocess Windows: 0/0
Disabled
Remote Processes: 0
Enterprise Policies: Inactive
Google Location Service Key: Missing
Google Safebrowsing Key: Missing
Mozilla Location Service Key: Missing
Safe Mode: false

Comment 19

[llrraa]

I try to start the Gecko Profiler(buffer size: 90MB), and Capture Profile without doing anything, it does open a new tab and try to grab the profile, but it fails with the following hint:
We were unable to connect to the Gecko profiler add-on within thirty seconds. This might be because the profile is big or your machine is slower than usual. Still waiting...

Should I report a new bug for this?