Merge "ui: Deprecate 'enableScrollJankPluginV2' flag" into main

google · Oct 8, 2024 · c5032d8 · c5032d8
2 parents 7a2fd2c + 49bfd0e
commit c5032d8
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 21 deletions.
diff --git a/ui/src/core/default_plugins.ts b/ui/src/core/default_plugins.ts
@@ -44,7 +44,6 @@ export const defaultPlugins = [
  'perfetto.AndroidLog',
  'perfetto.Annotation',
  'perfetto.AsyncSlices',
- 'perfetto.ChromeScrollJank',
  'perfetto.CoreCommands',
  'perfetto.Counter',
  'perfetto.CpuFreq',

diff --git a/ui/src/core/feature_flags.ts b/ui/src/core/feature_flags.ts
@@ -118,6 +118,21 @@ class Flags {
 
  this.store.save(this.overrides);
  }
+
+ /**
+ * Modify an override at runtime and save it back to local storage.
+ *
+ * This method operates on the raw JSON in local storage and doesn't require
+ * the presence of a flag to work. Thus, it may be called at any point in the
+ * lifecycle of the flags object.
+ *
+ * @param key - The key of the flag to modify.
+ * @param state - The desired state of the flag override.
+ */
+ patchOverride(key: string, state: OverrideState): void {
+ this.overrides[key] = state;
+ this.save();
+ }
 }
 
 export interface Flag {

diff --git a/ui/src/core_plugins/chrome_scroll_jank/index.ts b/ui/src/core_plugins/chrome_scroll_jank/index.ts
@@ -14,7 +14,6 @@
 
 import {uuidv4Sql} from '../../base/uuid';
 import {generateSqlWithInternalLayout} from '../../common/internal_layout_utils';
-import {featureFlags} from '../../core/feature_flags';
 import {Trace} from '../../public/trace';
 import {PerfettoPlugin, PluginDescriptor} from '../../public/plugin';
 import {EventLatencySliceDetailsPanel} from './event_latency_details_panel';
@@ -25,29 +24,60 @@ import {ScrollJankV3Track} from './scroll_jank_v3_track';
 import {TopLevelScrollTrack} from './scroll_track';
 import {ScrollJankCauseMap} from './scroll_jank_cause_map';
 import {TrackNode} from '../../public/workspace';
+import {featureFlags, OverrideState} from '../../core/feature_flags';
 
-const ENABLE_SCROLL_JANK_PLUGIN_V2 = featureFlags.register({
- id: 'enableScrollJankPluginV2',
- name: 'Enable Chrome Scroll Jank plugin V2',
- description: 'Adds new tracks and visualizations for scroll jank.',
- defaultValue: false,
-});
+// Before plugins were a thing, this plugin was enabled using a feature flag.
+// However, nowadays, plugins themselves can be selectively enabled and
+// disabled. This function inspects local storage to see whether the old feature
+// flag is enabled, and patches the flags settings to enable the chrome scroll
+// jank plugin, before deleting the old flag. This provides a seamless
+// experience for anyone who currently uses the chrome scroll jank plugin.
+//
+// TODO(stevegolton): Remove this code after 2025-01-01. This should give it
+// enough time on stable for most relevant users to have run it at least once.
+function patchChromeScrollJankFlag() {
+ try {
+ const flagsKey = 'perfettoFeatureFlags';
+ const enableScrollJankPluginV2FlagKey = 'enableScrollJankPluginV2';
+ const chromeScrollJankPuginFlagKey = 'plugin_perfetto.ChromeScrollJank';
+
+ const flagsRaw = localStorage.getItem(flagsKey);
+ if (flagsRaw) {
+ const flags = JSON.parse(flagsRaw);
+ if (flags[enableScrollJankPluginV2FlagKey] === 'OVERRIDE_TRUE') {
+ featureFlags.patchOverride(
+ chromeScrollJankPuginFlagKey,
+ OverrideState.TRUE,
+ );
+ console.log(
+ `Cleared deprecated 'enableScrollJankPluginV2' flag & enabled 'ChromeScrollJank' plugin.`,
+ );
+ }
+
+ // Just remove the original flag
+ delete flags[enableScrollJankPluginV2FlagKey];
+ localStorage.setItem(flagsKey, JSON.stringify(flags));
+ }
+ } catch {
+ // Ignore - this was very much best-effort.
+ }
+}
+
+patchChromeScrollJankFlag();
 
 class ChromeScrollJankPlugin implements PerfettoPlugin {
  async onTraceLoad(ctx: Trace): Promise<void> {
- if (ENABLE_SCROLL_JANK_PLUGIN_V2.get()) {
- const group = new TrackNode({
- title: 'Chrome Scroll Jank',
- sortOrder: -30,
- isSummary: true,
- });
- await this.addTopLevelScrollTrack(ctx, group);
- await this.addEventLatencyTrack(ctx, group);
- await this.addScrollJankV3ScrollTrack(ctx, group);
- await ScrollJankCauseMap.initialize(ctx.engine);
- ctx.workspace.addChildInOrder(group);
- group.expand();
- }
+ const group = new TrackNode({
+ title: 'Chrome Scroll Jank',
+ sortOrder: -30,
+ isSummary: true,
+ });
+ await this.addTopLevelScrollTrack(ctx, group);
+ await this.addEventLatencyTrack(ctx, group);
+ await this.addScrollJankV3ScrollTrack(ctx, group);
+ await ScrollJankCauseMap.initialize(ctx.engine);
+ ctx.workspace.addChildInOrder(group);
+ group.expand();
  }
 
  private async addTopLevelScrollTrack(