[ui] Use mountStore() for using persistent state in plugins.
- Avoids having to template plugins everywhere with the store type.
- Similar interface to tracks.
- Less confusing, persistent state is totally opt-in.
Change-Id: Iebd8cbe1842c5cef7b2480b220e0129b0355ef0d
diff --git a/docs/contributing/ui-plugins.md b/docs/contributing/ui-plugins.md
index 241e0a7..23fbe25 100644
--- a/docs/contributing/ui-plugins.md
+++ b/docs/contributing/ui-plugins.md
@@ -176,58 +176,73 @@
}
```
-This interface will be used as type parameter to the `Plugin` and
-`TracePluginContext` interfaces.
+To access permalink state, call `mountStore()` on your `TracePluginContext`
+object, passing in a migration function.
```typescript
-class MyPlugin implements Plugin<MyState> {
-
- migrate(initialState: unknown): MyState {
- // ...
+class MyPlugin implements Plugin {
+ async onTraceLoad(ctx: TracePluginContext): Promise<void> {
+ const store = ctx.mountStore(migrate);
}
+}
- async onTraceLoad(ctx: TracePluginContext<MyState>): Promise<void> {
- // You can access the store on ctx.store
- }
-
- async onTraceUnload(ctx: TracePluginContext<MyState>): Promise<void> {
- // You can access the store on ctx.store
- }
-
+function migrate(initialState: unknown): MyState {
// ...
}
```
-`migrate()` is called after `onActivate()` just before `onTraceLoad()`. There
-are two cases to consider:
+When it comes to migration, there are two cases to consider:
- Loading a new trace
- Loading from a permalink
-In case of a new trace `migrate()` is called with `undefined`. In this
-case you should return a default version of `MyState`:
+In case of a new trace, your migration function is called with `undefined`. In
+this case you should return a default version of `MyState`:
```typescript
-class MyPlugin implements Plugin<MyState> {
+const DEFAULT = {favouriteSlices: []};
- migrate(initialState: unknown): MyState {
- if (initialState === undefined) {
- return {
- favouriteSlices: [];
- };
- }
- // ...
+function migrate(initialState: unknown): MyState {
+ if (initialState === undefined) {
+ // Return default version of MyState.
+ return DEFAULT;
+ } else {
+ // Migrate old version here.
}
-
- // ...
}
```
-In the permalink case `migrate()` is called with the state of the plugin
-store at the time the permalink was generated. This may be from a
-older or newer version of the plugin.
-**Plugin's must not make assumptions about the contents of `initialState`**.
+In the permalink case, your migration function is called with the state of the
+plugin store at the time the permalink was generated. This may be from an older
+or newer version of the plugin.
-In this case you need to carefully validate the state object.
+**Plugins must not make assumptions about the contents of `initialState`!**
-TODO: Add validation example.
+In this case you need to carefully validate the state object. This could be
+achieved in several ways, none of which are particularly straight forward. State
+migration is difficult!
+
+One brute force way would be to use a version number.
+
+```typescript
+interface MyState {
+ version: number;
+ favouriteSlices: MySliceInfo[];
+}
+
+const VERSION = 3;
+const DEFAULT = {favouriteSlices: []};
+
+function migrate(initialState: unknown): MyState {
+ if (initialState && (initialState as {version: any}).version === VERSION) {
+ // Version number checks out, assume the structure is correct.
+ return initialState as State;
+ } else {
+ // Null, undefined, or bad version number - return default value.
+ return DEFAULT;
+ }
+}
+```
+
+You'll need to remember to update your version number when making changes!
+Migration should be unit-tested to ensure compatibility.
Examples:
- [dev.perfetto.ExampleState](https://cs.android.com/android/platform/superproject/main/+/main:external/perfetto/ui/src/plugins/dev.perfetto.ExampleState/index.ts).
diff --git a/ui/src/common/plugins.ts b/ui/src/common/plugins.ts
index bca6b15..cb27e61 100644
--- a/ui/src/common/plugins.ts
+++ b/ui/src/common/plugins.ts
@@ -17,16 +17,15 @@
import {ViewerImpl, ViewerProxy} from '../common/viewer';
import {globals} from '../frontend/globals';
import {
- BasePlugin,
Command,
EngineProxy,
MetricVisualisation,
+ Migrate,
Plugin,
PluginClass,
PluginContext,
PluginContextTrace,
PluginDescriptor,
- StatefulPlugin,
Store,
Track,
TrackContext,
@@ -77,18 +76,16 @@
// related resources, such as the engine and the store.
// The TracePluginContext exists for the whole duration a plugin is active AND a
// trace is loaded.
-class TracePluginContextImpl<T> implements PluginContextTrace<T>, Disposable {
+class TracePluginContextImpl implements PluginContextTrace, Disposable {
private trash = new Trash();
private alive = true;
constructor(
- private ctx: PluginContext, readonly store: Store<T>,
- readonly engine: EngineProxy,
+ private ctx: PluginContext, readonly engine: EngineProxy,
readonly trackRegistry: Map<string, TrackDescriptor>,
private defaultTracks: Set<TrackRef>,
private commandRegistry: Map<string, Command>) {
this.trash.add(engine);
- this.trash.add(store);
}
addCommand(cmd: Command): void {
@@ -110,6 +107,13 @@
return this.ctx.viewer;
}
+ get pluginId(): string {
+ return this.ctx.pluginId;
+ }
+
+ // Register a new track in this context.
+ // All tracks registered through this method are removed when this context is
+ // destroyed, i.e. when the trace is unloaded.
registerTrack(trackDesc: TrackDescriptor): void {
// Silently ignore if context is dead.
if (!this.alive) return;
@@ -142,26 +146,42 @@
this.trash.dispose();
this.alive = false;
}
+
+ mountStore<T>(migrate: Migrate<T>): Store<T> {
+ const globalStore = globals.store;
+
+ // Migrate initial state
+ const initialState = globalStore.state.plugins[this.pluginId];
+ const migratedState = migrate(initialState);
+
+ // Update global store with migrated plugin state
+ globalStore.edit((draft) => {
+ draft.plugins[this.pluginId] = migratedState;
+ });
+
+ // Return proxy store for this plugin
+ return globalStore.createProxy<T>(['plugins', this.pluginId]);
+ }
}
// 'Static' registry of all known plugins.
-export class PluginRegistry extends Registry<PluginDescriptor<unknown>> {
+export class PluginRegistry extends Registry<PluginDescriptor> {
constructor() {
super((info) => info.pluginId);
}
}
-interface PluginDetails<T> {
- plugin: Plugin<T>;
+interface PluginDetails {
+ plugin: Plugin;
context: PluginContext&Disposable;
- traceContext?: TracePluginContextImpl<unknown>;
+ traceContext?: TracePluginContextImpl;
}
-function isPluginClass<T>(v: unknown): v is PluginClass<T> {
+function isPluginClass(v: unknown): v is PluginClass {
return typeof v === 'function' && !!(v.prototype.onActivate);
}
-function makePlugin<T>(info: PluginDescriptor<T>): Plugin<T> {
+function makePlugin(info: PluginDescriptor): Plugin {
const {plugin} = info;
if (typeof plugin === 'function') {
@@ -178,7 +198,7 @@
export class PluginManager {
private registry: PluginRegistry;
- private plugins: Map<string, PluginDetails<unknown>>;
+ private plugins: Map<string, PluginDetails>;
private engine?: Engine;
readonly trackRegistry = new Map<string, TrackDescriptor>();
readonly commandRegistry = new Map<string, Command>();
@@ -203,7 +223,7 @@
plugin.onActivate && plugin.onActivate(context);
- const pluginDetails: PluginDetails<unknown> = {
+ const pluginDetails: PluginDetails = {
plugin,
context,
};
@@ -236,7 +256,7 @@
return this.getPluginContext(pluginId) !== undefined;
}
- getPluginContext(pluginId: string): PluginDetails<unknown>|undefined {
+ getPluginContext(pluginId: string): PluginDetails|undefined {
return this.plugins.get(pluginId);
}
@@ -286,59 +306,26 @@
return trackInfo && trackInfo.track(trackCtx);
}
- private doPluginTraceLoad<T>(
- pluginDetails: PluginDetails<T>, engine: Engine, pluginId: string): void {
+ private doPluginTraceLoad(
+ pluginDetails: PluginDetails, engine: Engine, pluginId: string): void {
const {plugin, context} = pluginDetails;
const engineProxy = engine.getProxy(pluginId);
- // Migrate state & write back to store.
- if (isStatefulPlugin(plugin)) {
- const initialState = globals.store.state.plugins[pluginId];
- const migratedState = plugin.migrate(initialState);
- globals.store.edit((draft) => {
- draft.plugins[pluginId] = migratedState;
- });
+ const traceCtx = new TracePluginContextImpl(
+ context,
+ engineProxy,
+ this.trackRegistry,
+ this.defaultTracks,
+ this.commandRegistry);
+ pluginDetails.traceContext = traceCtx;
- const proxyStore = globals.store.createProxy<T>(['plugins', pluginId]);
- const traceCtx = new TracePluginContextImpl(
- context,
- proxyStore,
- engineProxy,
- this.trackRegistry,
- this.defaultTracks,
- this.commandRegistry);
- pluginDetails.traceContext = traceCtx;
-
- // TODO(stevegolton): Await onTraceLoad.
- plugin.onTraceLoad && plugin.onTraceLoad(traceCtx);
- } else {
- // Stateless plugin i.e. the plugin's state type is undefined.
- // Just provide a store proxy over this plugin's state, the plugin can
- // work the state out for itself if it wants to, but we're not going to
- // help it out by calling migrate().
- const proxyStore = globals.store.createProxy<T>(['plugins', pluginId]);
- const traceCtx = new TracePluginContextImpl(
- context,
- proxyStore,
- engineProxy,
- this.trackRegistry,
- this.defaultTracks,
- this.commandRegistry);
- pluginDetails.traceContext = traceCtx;
-
- // TODO(stevegolton): Await onTraceLoad.
- plugin.onTraceLoad && plugin.onTraceLoad(traceCtx);
- }
+ // TODO(stevegolton): Await onTraceLoad.
+ plugin.onTraceLoad && plugin.onTraceLoad(traceCtx);
}
}
-function isStatefulPlugin<T>(plugin: BasePlugin<T>|
- StatefulPlugin<T>): plugin is StatefulPlugin<T> {
- return 'migrate' in plugin;
-}
-
-function maybeDoPluginTraceUnload(pluginDetails: PluginDetails<unknown>): void {
+function maybeDoPluginTraceUnload(pluginDetails: PluginDetails): void {
const {traceContext, plugin} = pluginDetails;
if (traceContext) {
diff --git a/ui/src/common/plugins_unittest.ts b/ui/src/common/plugins_unittest.ts
index 5be1f4a..4d3d4c2 100644
--- a/ui/src/common/plugins_unittest.ts
+++ b/ui/src/common/plugins_unittest.ts
@@ -26,9 +26,8 @@
rpcSendRequestBytes(_data: Uint8Array) {}
}
-function makeMockPlugin(): Plugin<any> {
+function makeMockPlugin(): Plugin {
return {
- migrate: jest.fn(),
onActivate: jest.fn(),
onDeactivate: jest.fn(),
onTraceLoad: jest.fn(),
@@ -91,17 +90,4 @@
expect(mockPlugin.onTraceUnload).toHaveBeenCalledTimes(1);
});
-
- it('does not invoke migrate at activation time', () => {
- manager.activatePlugin('foo', viewer);
-
- expect(mockPlugin.migrate).not.toHaveBeenCalled();
- });
-
- it('invokes migrate when trace is loaded', () => {
- manager.activatePlugin('foo', viewer);
- manager.onTraceLoad(engine);
-
- expect(mockPlugin.migrate).toHaveBeenCalledTimes(1);
- });
});
diff --git a/ui/src/plugins/com.example.Skeleton/index.ts b/ui/src/plugins/com.example.Skeleton/index.ts
index 9a30c32..282d615 100644
--- a/ui/src/plugins/com.example.Skeleton/index.ts
+++ b/ui/src/plugins/com.example.Skeleton/index.ts
@@ -13,11 +13,13 @@
// limitations under the License.
import {
+ createStore,
MetricVisualisation,
Plugin,
PluginContext,
PluginContextTrace,
PluginDescriptor,
+ Store,
} from '../../public';
interface State {
@@ -25,20 +27,24 @@
}
// SKELETON: Rename this class to match your plugin.
-class Skeleton implements Plugin<State> {
+class Skeleton implements Plugin {
+ private store: Store<State> = createStore({foo: 'foo'});
+
onActivate(_: PluginContext): void {
//
}
- migrate(_initialState: unknown): State {
- return {foo: 'bar'};
+ async onTraceLoad(ctx: PluginContextTrace): Promise<void> {
+ this.store = ctx.mountStore((_: unknown): State => {
+ return {foo: 'bar'};
+ });
+
+ this.store.edit((state) => {
+ state.foo = 'baz';
+ });
}
- async onTraceLoad(_: PluginContextTrace<State>): Promise<void> {
- //
- }
-
- async onTraceUnload(_: PluginContextTrace<State>): Promise<void> {
+ async onTraceUnload(_: PluginContextTrace): Promise<void> {
//
}
@@ -46,12 +52,12 @@
//
}
- metricVisualisations(_: PluginContextTrace<State>): MetricVisualisation[] {
+ metricVisualisations(_: PluginContextTrace): MetricVisualisation[] {
return [];
}
}
-export const plugin: PluginDescriptor<State> = {
+export const plugin: PluginDescriptor = {
// SKELETON: Update pluginId to match the directory of the plugin.
pluginId: 'com.example.Skeleton',
plugin: Skeleton,
diff --git a/ui/src/plugins/dev.perfetto.ExampleState/index.ts b/ui/src/plugins/dev.perfetto.ExampleState/index.ts
index a18cfe2..9e4e31c 100644
--- a/ui/src/plugins/dev.perfetto.ExampleState/index.ts
+++ b/ui/src/plugins/dev.perfetto.ExampleState/index.ts
@@ -25,8 +25,8 @@
// This example plugin shows using state that is persisted in the
// permalink.
-class ExampleState implements Plugin<State> {
- migrate(initialState: unknown): State {
+class ExampleState implements Plugin {
+ private migrate(initialState: unknown): State {
if (initialState && typeof initialState === 'object' &&
'counter' in initialState && typeof initialState.counter === 'number') {
return {counter: initialState.counter};
@@ -39,8 +39,10 @@
//
}
- async onTraceLoad(ctx: PluginContextTrace<State>): Promise<void> {
- const {viewer, store} = ctx;
+ async onTraceLoad(ctx: PluginContextTrace): Promise<void> {
+ const {viewer} = ctx;
+ const store = ctx.mountStore((init: unknown) => this.migrate(init));
+
ctx.addCommand({
id: 'dev.perfetto.ExampleState#ShowCounter',
name: 'Show ExampleState counter',
@@ -54,7 +56,7 @@
}
}
-export const plugin: PluginDescriptor<State> = {
+export const plugin: PluginDescriptor = {
pluginId: 'dev.perfetto.ExampleState',
plugin: ExampleState,
};
diff --git a/ui/src/public/index.ts b/ui/src/public/index.ts
index d47e608..435f137 100644
--- a/ui/src/public/index.ts
+++ b/ui/src/public/index.ts
@@ -30,7 +30,7 @@
STR,
STR_NULL,
} from '../common/query_result';
-export {Store} from '../frontend/store';
+export {createStore, Store} from '../frontend/store';
// An imperative API for plugins to change the UI.
@@ -146,6 +146,10 @@
// This interface defines a context for a plugin, which is an object passed to
// most hooks within the plugin. It should be used to interact with Perfetto.
export interface PluginContext {
+ // The unique ID for this plugin.
+ readonly pluginId: string;
+
+ // The viewer API, used to interface with Perfetto.
readonly viewer: Viewer;
// Add a command.
@@ -254,9 +258,8 @@
// Similar to PluginContext but with additional methods to operate on the
// currently loaded trace. Passed to trace-relevant hooks on a plugin instead of
// PluginContext.
-export interface PluginContextTrace<T = undefined> extends PluginContext {
+export interface PluginContextTrace extends PluginContext {
readonly engine: EngineProxy;
- readonly store: Store<T>;
// Register a new track against a unique key known as a URI.
// Once a track is registered it can be referenced multiple times on the
@@ -275,30 +278,22 @@
// This is simply a helper which calls registerTrack() then addDefaultTrack()
// with the same URI.
registerStaticTrack(track: TrackDescriptor&TrackRef): void;
+
+ // Create a store mounted over the top of this plugin's persistent state.
+ mountStore<T>(migrate: Migrate<T>): Store<T>;
}
-export interface BasePlugin<State> {
+export interface Plugin {
// Lifecycle methods.
onActivate(ctx: PluginContext): void;
- onTraceLoad?(ctx: PluginContextTrace<State>): Promise<void>;
- onTraceUnload?(ctx: PluginContextTrace<State>): Promise<void>;
+ onTraceLoad?(ctx: PluginContextTrace): Promise<void>;
+ onTraceUnload?(ctx: PluginContextTrace): Promise<void>;
onDeactivate?(ctx: PluginContext): void;
// Extension points.
metricVisualisations?(ctx: PluginContext): MetricVisualisation[];
}
-export interface StatefulPlugin<State> extends BasePlugin<State> {
- // Function to migrate the persistent state.
- migrate(initialState: unknown): State;
-}
-
-// Generic interface all plugins must implement.
-// If a state type is passed, the plugin must implement migrate(). Otherwise if
-// the state type is omitted, migrate need not be defined.
-export type Plugin<State = undefined> =
- State extends undefined ? BasePlugin<State>: StatefulPlugin<State>;
-
// This interface defines what a plugin factory should look like.
// This can be defined in the plugin class definition by defining a constructor
// and the relevant static methods:
@@ -309,9 +304,9 @@
// ... methods from the TracePlugin interface go here ...
// }
// ... which can then be passed around by class i.e. MyPlugin
-export interface PluginClass<T> {
+export interface PluginClass {
// Instantiate the plugin.
- new(): Plugin<T>;
+ new(): Plugin;
}
// Describes a reference to a registered track.
@@ -356,9 +351,9 @@
// Plugins can be passed as class refs, factory functions, or concrete plugin
// implementations.
-export type PluginFactory<T> = PluginClass<T>|Plugin<T>|(() => Plugin<T>);
+export type PluginFactory = PluginClass|Plugin|(() => Plugin);
-export interface PluginDescriptor<T = undefined> {
+export interface PluginDescriptor {
// A unique string for your plugin. To ensure the name is unique you
// may wish to use a URL with reversed components in the manner of
// Java package names.
@@ -366,5 +361,5 @@
// The plugin factory used to instantiate the plugin object, or if this is
// an actual plugin implementation, it's just used as-is.
- plugin: PluginFactory<T>;
+ plugin: PluginFactory;
}
diff --git a/ui/src/tracks/actual_frames/index.ts b/ui/src/tracks/actual_frames/index.ts
index 4095041..e069236 100644
--- a/ui/src/tracks/actual_frames/index.ts
+++ b/ui/src/tracks/actual_frames/index.ts
@@ -155,7 +155,7 @@
class ActualFrames implements Plugin {
onActivate(_ctx: PluginContext): void {}
- async onTraceLoad(ctx: PluginContextTrace<undefined>): Promise<void> {
+ async onTraceLoad(ctx: PluginContextTrace): Promise<void> {
const {engine} = ctx;
const result = await engine.query(`
with process_async_tracks as materialized (
diff --git a/ui/src/tracks/annotation/index.ts b/ui/src/tracks/annotation/index.ts
index 5a0899d..7c05829 100644
--- a/ui/src/tracks/annotation/index.ts
+++ b/ui/src/tracks/annotation/index.ts
@@ -38,7 +38,7 @@
await this.addAnnotationCounterTracks(ctx);
}
- private async addAnnotationTracks(ctx: PluginContextTrace<undefined>) {
+ private async addAnnotationTracks(ctx: PluginContextTrace) {
const {engine} = ctx;
const result = await engine.query(`
diff --git a/ui/src/tracks/null_track/index.ts b/ui/src/tracks/null_track/index.ts
index 4040900..dff2b06 100644
--- a/ui/src/tracks/null_track/index.ts
+++ b/ui/src/tracks/null_track/index.ts
@@ -42,7 +42,7 @@
class NullTrackPlugin implements Plugin {
onActivate(_ctx: PluginContext): void {}
- async onTraceLoad(ctx: PluginContextTrace<undefined>): Promise<void> {
+ async onTraceLoad(ctx: PluginContextTrace): Promise<void> {
// TODO(stevegolton): This is not the right way to handle blank tracks,
// instead we should probably just render some blank element at render time
// if no track uri is supplied.
diff --git a/ui/src/tracks/screenshots/index.ts b/ui/src/tracks/screenshots/index.ts
index c354371..ca73ab5 100644
--- a/ui/src/tracks/screenshots/index.ts
+++ b/ui/src/tracks/screenshots/index.ts
@@ -83,7 +83,7 @@
class ScreenshotsPlugin implements Plugin {
onActivate(_ctx: PluginContext): void {}
- async onTraceLoad(ctx: PluginContextTrace<undefined>): Promise<void> {
+ async onTraceLoad(ctx: PluginContextTrace): Promise<void> {
ctx.registerStaticTrack({
uri: 'perfetto.Screenshots',
displayName: 'Screenshots',
diff --git a/ui/src/tracks/thread_state/index.ts b/ui/src/tracks/thread_state/index.ts
index d5944ca..62c79bb 100644
--- a/ui/src/tracks/thread_state/index.ts
+++ b/ui/src/tracks/thread_state/index.ts
@@ -297,7 +297,7 @@
class ThreadState implements Plugin {
onActivate(_ctx: PluginContext): void {}
- async onTraceLoad(ctx: PluginContextTrace<undefined>): Promise<void> {
+ async onTraceLoad(ctx: PluginContextTrace): Promise<void> {
const {engine} = ctx;
const result = await engine.query(`
select
diff --git a/ui/src/tracks/visualised_args/index.ts b/ui/src/tracks/visualised_args/index.ts
index e4bf07b..e2f92e6 100644
--- a/ui/src/tracks/visualised_args/index.ts
+++ b/ui/src/tracks/visualised_args/index.ts
@@ -108,7 +108,8 @@
class VisualisedArgsPlugin implements Plugin {
onActivate(_ctx: PluginContext): void {}
- async onTraceLoad(ctx: PluginContextTrace<undefined>): Promise<void> {
+
+ async onTraceLoad(ctx: PluginContextTrace): Promise<void> {
ctx.registerTrack({
uri: VISUALISED_ARGS_SLICE_TRACK_URI,
tags: {
