Closed Bug 1507709 Opened 2 years ago Closed 1 year ago

Move geckoProfiler API to toolkit

Categories

(WebExtensions :: Developer Tools, enhancement, P5)

enhancement

Tracking

(firefox68 verified)

VERIFIED FIXED
mozilla68
Tracking Status
firefox68 --- verified

People

(Reporter: Fallen, Assigned: Fallen)

References

(Depends on 1 open bug, Blocks 6 open bugs)

Details

Attachments

(1 file)

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.
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?
Priority: -- → P5
(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.
Depends on: 1510301
(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.
Blocks: 1512974
Blocks: 562977
Blocks: 760859
Blocks: 837680
Blocks: 1118278
Blocks: 1192827
Blocks: 683651
>>! 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?
Flags: needinfo?(kmaglione+bmo)

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.

Flags: needinfo?(kmaglione+bmo)

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

Blocks: 1538097
Pushed by mozilla@kewis.ch:
https://hg.mozilla.org/integration/autoland/rev/2a4bddaaf3dc
Move WebExtensions geckoProfiler API to toolkit. r=kmag
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Depends on: 1538592

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

Flags: needinfo?(philipp)

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.

Flags: needinfo?(philipp)

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.

Status: RESOLVED → VERIFIED

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

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?

You need to log in before you can comment on or make changes to this bug.