Repository navigation
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6293 +/- ##
==========================================
+ Coverage 84.45% 84.46% +0.01%
==========================================
Files 357 357
Lines 38652 38653 +1
Branches 10948 10936 -12
==========================================
+ Hits 32643 32649 +6
+ Misses 5579 5574 -5
Partials 430 430 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Bug for the Gecko side - https://bugzilla.mozilla.org/show_bug.cgi?id=2068137. |
|
@mstange A kind reminder for review 😌 |
Marker styles use methods to resolve theme-dependent colors, which cannot be serialized as marker schema metadata. Store only color data in each style and resolve it when drawing timeline markers.
Timeline rendering selects styles from a hardcoded table keyed by marker `data.type`. Putting the style in each schema removes that type-specific lookup and lets producers define marker appearance.
mstange
left a comment
There was a problem hiding this comment.
Sorry about the somewhat non-actionable feedback here - I'm curious if you have ideas or opinions.
| top: CssPixels; | ||
| height: CssPixels; | ||
| background: MarkerSchemaStyleColor; | ||
| squareCorners: boolean; | ||
| borderLeft: MarkerSchemaStyleColor | null; | ||
| borderRight: MarkerSchemaStyleColor | null; |
There was a problem hiding this comment.
I think the color is worth having. I'm not sure what to do about the other properties. Putting them all in here feels like we're giving profiles too much control over our UI. Also, the existing code is a bit of a mess - nobody has looked at it in a long time and it hasn't been updated as the markers that Firefox emitted changed. For example, I think the RefreshDriverTick doesn't actually apply anymore - either it doesn't match the marker, or the markers ask to not be shown in the timeline (because they're Text markers? not sure) so I think nothing actually uses squareCorners:false.
Maybe as a first pass we should remove marker styles which aren't matching any markers in profiles from modern Firefox anymore? And then we can remove unused marker style properties. And then we can think about whether we want top and height in there.
Main | Deploy preview
Timeline marker styles are currently selected from a hardcoded table keyed by marker
data.type. The style objects also contain methods, so they cannot be stored as marker schema metadata.This PR makes marker styles serializable and stores them in marker schemas. Timeline rendering now reads styles from the schemas, while preserving marker-name overrides.
Profile processing and the processed-profile upgrader derive the existing styles for schemas without this metadata. A follow-up Gecko change will populate the style field in Gecko marker schemas.
Closes #6292
Profile