The XHProf application looks pretty average on mobile. In particular, the sizing seems off.
Description
Description
Revisions and Commits
Revisions and Commits
Event Timeline
Comment Actions
I can't imagine someone using this on mobile, is there a need for it o can't think of?
Comment Actions
@joshuaspence my assumption is exactly 0 people use this tool on mobile, And maybe 5 use it on desktop.
Comment Actions
This stretches the bounds of imagination, but I can come up with a use: although we do only a tiny amount of this today, a possible use case is that we may take a different server-side rendering pathway for mobile agents than we do for desktop agents in the future. This does technically happen today, but the amount of code involved is tiny, and is unlikely to ever affect a profile.
However, you could imagine something more drastic in the future, where sections of the page get rendered differently (e.g., omitted on mobile) and the mobile and desktop profiles start to diverge.
More broadly, I think spending a couple of hours here and there to keep lesser-used applications modern is worthwhile. If we have an application which sees so little use that we can't justify bringing it up to spec as the UI and application infrastructure evolve, I think we should consider removing it. But XHProf is a cornerstone infrastructure application and definitely worth keeping modern to me.
Comment Actions
OIC, I am confusing XPHAST and XHPROF tasks, I'm a bit jetlagged it seems. Didn't notice two different tasks here.
Comment Actions
Oh, on an actual mobile device. We should remove that JS check now, I think it's safe globally.
Comment Actions
hmmm maybe this is buildStandardPage thing.
@epriestley is it safe to delete the $is_framed logic in this application? I don't know what it does or how to test, assume a page layout thing.
Comment Actions
Removing $is_framed will break the DarkConsole "XHProf" tab, which loads the profile in an <iframe />.
Comment Actions
I'm not sure how best to proceed here. What's the difference with buildApplicationPage and buildStandardPage when it comes to device?
Comment Actions
buildApplicationPage() calls setDeviceReady(true) on the $page, buildStandardPage() does not.
Comment Actions
I think we can put the frame logic in buildApplicationPage(), and then switch to it. I can tackle that if you aren't sure how to not break stuff.
Comment Actions
Typically I'd just switch this over to buildApplicationPage and call it a day, but I don't think that works with frame.
Comment Actions
There is a manageable number of buildStandardPage callsites, might be easy to remove if there is only 1-2 missing features.
Comment Actions
Definitely desirable, although some of them (releeph, oauth server, arcanist project edit) might be tricky to test.