Closed Bug 1602800 Opened 7 months ago Closed 7 months ago

Can't profile Fenix via `about:debugging`

Categories

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

defect

Tracking

(firefox-esr68 unaffected, firefox71 unaffected, firefox72 verified, firefox73 verified)

VERIFIED FIXED
Firefox 73
Tracking Status
firefox-esr68 --- unaffected
firefox71 --- unaffected
firefox72 --- verified
firefox73 --- verified

People

(Reporter: snorp, Assigned: canova)

References

(Regression)

Details

(Keywords: regression)

Attachments

(3 files)

When connecting to a remote Fenix and hitting the "Profile performance" button, I get a blank panel. The following error also occurs in the browser console:

uncaught exception: Protocol error (unknownError): preference is not of the right type: devtools.performance.recording.entries

:snorp Can you provide the Firefox version for the desktop Firefox, and the remote target? This sounds like a remote debugging protocol backwards compatibility error.

Component: about:debugging → Performance Tools (Profiler/Timeline)
Flags: needinfo?(snorp)
Priority: -- → P1

Or perhaps it's because the prefs are now defined in the Firefox preferences, and not in the DevTools anymore.

https://searchfox.org/mozilla-central/source/browser/app/profile/firefox.js#2097

I'm using NIghtly for both.

Flags: needinfo?(snorp)

Bug 1599745 definitely broke backward compatibility, remote debugging FF71 from FF72 or 73 leads to a white screen. And I guess browser/app/profile/firefox.js is not used for Fenix, so it probably also broke mobile remote debugging in general.

If this is correct moving the prefs to init/all.js would fix mobile remote debugging, but it won't fix backward compatibility issues.
The client code can't assume that the preferences exist / have the right type on the debuggee.

Regressed by: 1599745
Assignee: nobody → canaltinova
Status: NEW → ASSIGNED

We had to do it because firefox android doesn't use that file. There is a
mobile.js file for that purpose. We had to move all recording preferences to
all.js and add the android only preference to mobile.js to be able to handle
that better.

Pushed by canaltinova@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/16642f81cdc4
Move profiler recording preferences from firefox.js to all.js r=gregtatum
https://hg.mozilla.org/integration/autoland/rev/0694837833fe
Move the function that gets BrowsingContext ID to a new file inside shared folder r=gregtatum
https://hg.mozilla.org/integration/autoland/rev/3263f5baebf0
Backout some of the preferences handling to make sure we have defaults for older firefox versions r=gregtatum
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 73

Hey James, the fix is on mozilla-central now. It solved the issue on my end but could you also test and let me know if it works now?
We will also most likely need to uplift those patches to beta since it's also broken.

Flags: needinfo?(snorp)

Yup, works with the example app now in latest m-c.

Flags: needinfo?(snorp)

Comment on attachment 9115158 [details]
Bug 1602800 - Move profiler recording preferences from firefox.js to all.js r?gregtatum

Beta/Release Uplift Approval Request

  • User impact if declined: Users won't be able to profile Fenix/Firefox Android or older Firefox versions
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: STR:
  • Enable remote debugging in your android device(it can be an emulator too, it should already be checked in that case).
  • Download and open geckoview_example app(or Firefox Preview Nightly) in that device.
  • Check the "Remote Debugging" checkbox in the app settings.
  • Connect the Android device with a USB to your desktop.
  • Open Firefox in your desktop and navigate to "about:debugging".
  • Enable USB devices. Your android device should pop up on the left sidebar.
  • Click to "Connect" button right side of your device and go inside that device.
  • Click to "Profile Performance" button and see that the opened popup has content(previously it did not).
  • Click to "Start Recording" button, then click to "Capture recording".

ER:
It should capture a profile and navigate to "profiler.firefox.com" with that captured profile data.

  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): It's not risky, because first patch includes preference changes only, second patch moves a function to a new file and third patch mostly reverts the old change. Also, it's not risky because remote profiling is something gecko developers and power users do. There is no way to break end user workflow with those changes. But without that uplift, we won't be able to profile older Firefox versions.
  • String changes made/needed:
Attachment #9115158 - Flags: approval-mozilla-beta?
Attachment #9115159 - Flags: approval-mozilla-beta?
Attachment #9115160 - Flags: approval-mozilla-beta?
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

Comment on attachment 9115158 [details]
Bug 1602800 - Move profiler recording preferences from firefox.js to all.js r?gregtatum

profiler regression fix, approved for 72.0b8

Attachment #9115158 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9115159 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9115160 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Hello, I can confirm that this issue is fixed using the latest version of Firefox Preview Nightly 12/27 #13610605.

Devices:

  • LG G7 fit (Android 8.1);
  • OnePlus 5T (Android 9);
  • Google Pixel (Android Q).
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.