Move geckoProfiler API to toolkit

VERIFIED FIXED in Firefox 68

Status

enhancement
P5
normal
VERIFIED FIXED
7 months ago
Last month

People

(Reporter: Fallen, Assigned: Fallen)

Tracking

(Depends on 1 bug, Blocks 8 bugs)

unspecified
mozilla68
Dependency tree / graph

Firefox Tracking Flags

(firefox68 verified)

Details

Attachments

(1 attachment)

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.

Updated

6 months ago
Blocks: 1512974

Updated

6 months ago
Blocks: 562977

Updated

6 months ago
Blocks: 760859

Updated

6 months ago
Blocks: 837680

Updated

6 months ago
Blocks: 1118278

Updated

6 months ago
Blocks: 1192827

Updated

6 months ago
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)
Comment hidden (offtopic)
Comment hidden (offtopic)
Comment hidden (offtopic)

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

Updated

3 months ago
Blocks: 1538097

Comment 13

3 months ago
Pushed by mozilla@kewis.ch:
https://hg.mozilla.org/integration/autoland/rev/2a4bddaaf3dc
Move WebExtensions geckoProfiler API to toolkit. r=kmag

Comment 14

3 months ago
bugherder
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

Updated

3 months ago
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
You need to log in before you can comment on or make changes to this bug.