Merge "Fix the LIMIT that didn't take effect on RepeatField()"
diff --git a/src/traced/probes/statsd_client/statsd_binder_data_source.cc b/src/traced/probes/statsd_client/statsd_binder_data_source.cc
index 9d3d7f7..df38bda 100644
--- a/src/traced/probes/statsd_client/statsd_binder_data_source.cc
+++ b/src/traced/probes/statsd_client/statsd_binder_data_source.cc
@@ -202,7 +202,7 @@
// static
const ProbesDataSource::Descriptor StatsdBinderDataSource::descriptor = {
- /*name*/ "android.statsd_binder",
+ /*name*/ "android.statsd",
/*flags*/ Descriptor::kFlagsNone,
/*fill_descriptor_func*/ nullptr,
};
diff --git a/test/configs/statsd.cfg b/test/configs/statsd.cfg
index 10d5b0d..a384b57 100644
--- a/test/configs/statsd.cfg
+++ b/test/configs/statsd.cfg
@@ -5,7 +5,7 @@
data_sources {
config {
- name: "android.statsd_binder"
+ name: "android.statsd"
target_buffer: 0
statsd_tracing_config {
push_atom_id: ATOM_FLASHLIGHT_STATE_CHANGED
diff --git a/ui/src/common/actions.ts b/ui/src/common/actions.ts
index 15167eb..7b8f462 100644
--- a/ui/src/common/actions.ts
+++ b/ui/src/common/actions.ts
@@ -458,6 +458,15 @@
}
},
+ maybeSetPendingDeeplink(
+ state: StateDraft, args: {ts?: string, dur?: string, tid?: string}) {
+ state.pendingDeeplink = args;
+ },
+
+ clearPendingDeeplink(state: StateDraft, _: {}) {
+ state.pendingDeeplink = undefined;
+ },
+
// TODO(hjd): engine.ready should be a published thing. If it's part
// of the state it interacts badly with permalinks.
setEngineReady(
diff --git a/ui/src/common/state.ts b/ui/src/common/state.ts
index aa28c8e..52855b8 100644
--- a/ui/src/common/state.ts
+++ b/ui/src/common/state.ts
@@ -104,6 +104,7 @@
// 29. Add ftrace state. <-- Borked, state contains a non-serializable object.
// 30. Convert ftraceFilter.excludedNames from Set<string> to string[].
// 31. Convert all timestamps to bigints.
+// 32. Add pendingDeeplink.
export const STATE_VERSION = 31;
export const SCROLLING_TRACK_GROUP = 'ScrollingTracks';
@@ -515,6 +516,12 @@
excludedNames: string[];
}
+export interface PendingDeeplinkState {
+ ts?: string;
+ dur?: string;
+ tid?: string;
+}
+
export interface State {
version: number;
nextId: string;
@@ -609,6 +616,10 @@
// Omnibox info.
omniboxState: OmniboxState;
+
+ // Pending deeplink which will happen when we first finish opening a
+ // trace.
+ pendingDeeplink?: PendingDeeplinkState;
}
export const defaultTraceTime = {
diff --git a/ui/src/controller/trace_controller.ts b/ui/src/controller/trace_controller.ts
index f0e2d02..49301e3 100644
--- a/ui/src/controller/trace_controller.ts
+++ b/ui/src/controller/trace_controller.ts
@@ -39,7 +39,12 @@
STR_NULL,
} from '../common/query_result';
import {onSelectionChanged} from '../common/selection_observer';
-import {defaultTraceTime, EngineMode, ProfileType} from '../common/state';
+import {
+ defaultTraceTime,
+ EngineMode,
+ PendingDeeplinkState,
+ ProfileType,
+} from '../common/state';
import {Span} from '../common/time';
import {
TPTime,
@@ -523,6 +528,12 @@
await this.selectPerfSample();
}
+ const pendingDeeplink = globals.state.pendingDeeplink;
+ if (pendingDeeplink !== undefined) {
+ globals.dispatch(Actions.clearPendingDeeplink({}));
+ await this.selectPendingDeeplink(pendingDeeplink);
+ }
+
// If the trace was shared via a permalink, it might already have a
// selection. Emit onSelectionChanged to ensure that the components (like
// current selection details) react to it.
@@ -530,6 +541,8 @@
onSelectionChanged(globals.state.currentSelection, undefined);
}
+ globals.dispatch(Actions.maybeExpandOnlyTrackGroup({}));
+
// Trace Processor doesn't support the reliable range feature for JSON
// traces.
if (!isJsonTrace && ENABLE_CHROME_RELIABLE_RANGE_ANNOTATION_FLAG.get()) {
@@ -583,6 +596,52 @@
globals.dispatch(Actions.selectHeapProfile({id: 0, upid, ts, type}));
}
+ private async selectPendingDeeplink(link: PendingDeeplinkState) {
+ const conditions = [];
+ const {ts, dur} = link;
+
+ if (ts !== undefined) {
+ conditions.push(`ts = ${ts}`);
+ }
+ if (dur !== undefined) {
+ conditions.push(`dur = ${dur}`);
+ }
+
+ if (conditions.length === 0) {
+ return;
+ }
+
+ const query = `
+ select
+ id,
+ track_id as traceProcessorTrackId,
+ type
+ from slice
+ where ${conditions.join(' and ')}
+ ;`;
+
+ const result = await assertExists(this.engine).query(query);
+ if (result.numRows() > 0) {
+ const row = result.firstRow({
+ id: NUM,
+ traceProcessorTrackId: NUM,
+ type: STR,
+ });
+
+ const id = row.traceProcessorTrackId;
+ const trackId = globals.state.uiTrackIdByTraceTrackId[id];
+ if (trackId === undefined) {
+ return;
+ }
+ globals.makeSelection(Actions.selectChromeSlice({
+ id: row.id,
+ trackId,
+ table: '',
+ scroll: true,
+ }));
+ }
+ }
+
private async listTracks() {
this.updateStatus('Loading tracks');
const engine = assertExists<Engine>(this.engine);
diff --git a/ui/src/controller/validators.ts b/ui/src/controller/validators.ts
index c35c69d..cc072b7 100644
--- a/ui/src/controller/validators.ts
+++ b/ui/src/controller/validators.ts
@@ -85,6 +85,28 @@
}
}
+class OptionalValidator<T> implements Validator<T|undefined> {
+ private inner: Validator<T>;
+
+ constructor(inner: Validator<T>) {
+ this.inner = inner;
+ }
+
+ validate(input: unknown, context: ValidatorContext): T|undefined {
+ if (input === undefined) {
+ return undefined;
+ }
+ try {
+ return this.inner.validate(input, context);
+ } catch (e) {
+ if (e instanceof ValidationError) {
+ context.invalidKeys.push(renderPath(context.path));
+ return undefined;
+ }
+ throw e;
+ }
+ }
+}
class StringValidator extends PrimitiveValidator<string> {
predicate(input: unknown): input is string {
@@ -244,14 +266,25 @@
export const requiredStr = new StringValidator('', true);
+export const optStr = new OptionalValidator<string>(requiredStr);
+
export function num(defaultValue = 0): NumberValidator {
return new NumberValidator(defaultValue, false);
}
+export const requiredNum = new NumberValidator(0, true);
+
+export const optNum = new OptionalValidator<number>(requiredNum);
+
export function bool(defaultValue = false): BooleanValidator {
return new BooleanValidator(defaultValue, false);
}
+export const requiredBool = new BooleanValidator(false, true);
+
+export const optBool = new OptionalValidator<boolean>(requiredBool);
+
+
export function record<T extends Record<string, Validator<unknown>>>(
validators: T): RecordValidator<T> {
return new RecordValidator<T>(validators);
diff --git a/ui/src/controller/validators_unittest.ts b/ui/src/controller/validators_unittest.ts
index eec9aa5..fc0fe51 100644
--- a/ui/src/controller/validators_unittest.ts
+++ b/ui/src/controller/validators_unittest.ts
@@ -16,6 +16,7 @@
arrayOf,
num,
oneOf,
+ optStr,
record,
requiredStr,
runValidator,
@@ -107,3 +108,27 @@
expect(result.extraKeys).toContain('deeply.nested.array[1].extra3');
expect(result.invalidKeys).toContain('deeply.nested.array[0].x');
});
+
+
+describe('optStr', () => {
+ test('it validates undefined', () => {
+ const validation = runValidator(optStr, undefined);
+ expect(validation.result).toEqual(undefined);
+ expect(validation.invalidKeys).toEqual([]);
+ expect(validation.extraKeys).toEqual([]);
+ });
+
+ test('it validates string', () => {
+ const validation = runValidator(optStr, 'foo');
+ expect(validation.result).toEqual('foo');
+ expect(validation.invalidKeys).toEqual([]);
+ expect(validation.extraKeys).toEqual([]);
+ });
+
+ test('it reports numbers', () => {
+ const validation = runValidator(optStr, 42);
+ expect(validation.result).toEqual(undefined);
+ expect(validation.invalidKeys).toEqual(['']);
+ expect(validation.extraKeys).toEqual([]);
+ });
+});
diff --git a/ui/src/frontend/index.ts b/ui/src/frontend/index.ts
index cef61e8..053316f 100644
--- a/ui/src/frontend/index.ts
+++ b/ui/src/frontend/index.ts
@@ -142,12 +142,6 @@
}));
}
-function initGlobalsFromQueryString() {
- const queryString = window.location.search;
- globals.embeddedMode = queryString.includes('mode=embedded');
- globals.hideSidebar = queryString.includes('hideSidebar=true');
-}
-
function setupContentSecurityPolicy() {
// Note: self and sha-xxx must be quoted, urls data: and blob: must not.
const policy = {
@@ -252,9 +246,10 @@
maybeOpenTraceFromRoute(route);
};
- // This must be called before calling `globals.initialize` so that the
- // `embeddedMode` global is set.
- initGlobalsFromQueryString();
+ // These need to be set before globals.initialize.
+ const route = Router.parseUrl(window.location.href);
+ globals.embeddedMode = route.args.mode === 'embedded';
+ globals.hideSidebar = route.args.hideSidebar === true;
globals.initialize(dispatch, router);
globals.serviceWorkerController.install();
@@ -311,7 +306,6 @@
}
}
-
function onCssLoaded() {
initCssConstants();
// Clear all the contents of the initial page (e.g. the <pre> error message)
@@ -343,6 +337,14 @@
// accidentially clober the state of an open trace processor instance
// otherwise.
CheckHttpRpcConnection().then(() => {
+ const route = Router.parseUrl(window.location.href);
+
+ globals.dispatch(Actions.maybeSetPendingDeeplink({
+ ts: route.args.ts,
+ tid: route.args.tid,
+ dur: route.args.dur,
+ }));
+
if (!globals.embeddedMode) {
installFileDropHandler();
}
@@ -359,7 +361,7 @@
// Handles the initial ?local_cache_key=123 or ?s=permalink or ?url=...
// cases.
- maybeOpenTraceFromRoute(Router.parseUrl(window.location.href));
+ maybeOpenTraceFromRoute(route);
});
}
diff --git a/ui/src/frontend/router.ts b/ui/src/frontend/router.ts
index a693b2e..1057d80 100644
--- a/ui/src/frontend/router.ts
+++ b/ui/src/frontend/router.ts
@@ -13,20 +13,24 @@
// limitations under the License.
import m from 'mithril';
+
import {assertExists, assertTrue} from '../base/logging';
+import {
+ oneOf,
+ optBool,
+ optStr,
+ record,
+ runValidator,
+ ValidatedType,
+} from '../controller/validators';
+
import {PageAttrs} from './pages';
export const ROUTE_PREFIX = '#!';
const DEFAULT_ROUTE = '/';
-// A broken down representation of a route.
-// For instance: #!/record/gpu?local_cache_key=a0b1
-// becomes: {page: '/record', subpage: '/gpu', args: {local_cache_key: 'a0b1'}}
-export interface Route {
- page: string;
- subpage: string;
- args: RouteArgs;
-}
+const modes = ['embedded', undefined] as const;
+type Mode = typeof modes[number];
// The set of args that can be set on the route via #!/page?a=1&b2.
// Route args are orthogonal to pages (i.e. should NOT make sense only in a
@@ -44,18 +48,46 @@
// This is client-only. All the routing logic in the Perfetto UI uses only
// this.
-// This must be a type literial to avoid having to duplicate the
-// index type logic of Params.
-export type RouteArgs = {
+const routeArgs = record({
// The local_cache_key is special and is persisted across navigations.
- local_cache_key?: string;
+ local_cache_key: optStr,
// These are transient and are really set only on startup.
- openFromAndroidBugTool?: string;
- s?: string; // For permalinks.
- p?: string; // DEPRECATED: for #!/record?p=cpu subpages (b/191255021).
- url?: string; // For fetching traces from Cloud Storage.
-};
+
+ // Are we loading a trace via ABT.
+ openFromAndroidBugTool: optBool,
+
+ // For permalink hash.
+ s: optStr,
+
+ // DEPRECATED: for #!/record?p=cpu subpages (b/191255021).
+ p: optStr,
+
+ // For fetching traces from Cloud Storage.
+ url: optStr,
+
+ // For the 'mode' of the UI. For example when the mode is 'embedded'
+ // some features are disabled.
+ mode: oneOf<Mode>(modes, undefined),
+
+ // Should we hide the sidebar?
+ hideSidebar: optBool,
+
+ // Deep link support
+ ts: optStr,
+ dur: optStr,
+ tid: optStr,
+});
+type RouteArgs = ValidatedType<typeof routeArgs>;
+
+// A broken down representation of a route.
+// For instance: #!/record/gpu?local_cache_key=a0b1
+// becomes: {page: '/record', subpage: '/gpu', args: {local_cache_key: 'a0b1'}}
+export interface Route {
+ page: string;
+ subpage: string;
+ args: RouteArgs;
+}
export interface RoutesMap {
[key: string]: m.Component<PageAttrs>;
@@ -168,16 +200,50 @@
const argsStart = hash.indexOf('?');
const argsStr = argsStart < 0 ? '' : hash.substring(argsStart + 1);
- const args = argsStr ? m.parseQueryString(hash.substring(argsStart)) : {};
+ const rawArgs =
+ argsStr ? m.parseQueryString(hash.substring(argsStart)) : {};
+
+ const args = runValidator(routeArgs, rawArgs).result;
+
+ // Javascript sadly distinguishes between foo[bar] === undefined
+ // and foo[bar] is not set at all. Here we need the second case to
+ // avoid making the URL ugly.
+ for (const key of Object.keys(args)) {
+ if ((args as any)[key] === undefined) {
+ delete (args as any)[key];
+ }
+ }
return {page, subpage, args};
}
+ private static parseSearchParams(url: string): RouteArgs {
+ const query = (new URL(url)).search;
+ const rawArgs = m.parseQueryString(query);
+ const args = runValidator(routeArgs, rawArgs).result;
+
+ // Javascript sadly distinguishes between foo[bar] === undefined
+ // and foo[bar] is not set at all. Here we need the second case to
+ // avoid making the URL ugly.
+ for (const key of Object.keys(args)) {
+ if ((args as any)[key] === undefined) {
+ delete (args as any)[key];
+ }
+ }
+
+ return args;
+ }
+
// Like parseFragment() but takes a full URL.
static parseUrl(url: string): Route {
+ const searchArgs = Router.parseSearchParams(url);
+
const hashPos = url.indexOf('#');
const fragment = hashPos < 0 ? '' : url.substring(hashPos);
- return Router.parseFragment(fragment);
+ const route = Router.parseFragment(fragment);
+ route.args = Object.assign({}, searchArgs, route.args);
+
+ return route;
}
// Throws if EVENT_LIMIT onhashchange events occur within WINDOW_MS.
diff --git a/ui/src/frontend/router_unittest.ts b/ui/src/frontend/router_unittest.ts
index 612347c..19dac36 100644
--- a/ui/src/frontend/router_unittest.ts
+++ b/ui/src/frontend/router_unittest.ts
@@ -18,89 +18,138 @@
view() {},
};
-beforeEach(() => {
- window.location.hash = '';
-});
-
-test('Default route must be defined', () => {
- expect(() => new Router({'/a': mockComponent})).toThrow();
-});
-
-test('Resolves empty route to default component', () => {
- const router = new Router({'/': mockComponent});
- window.location.hash = '';
- expect(router.resolve().tag).toBe(mockComponent);
-});
-
-test('Resolves subpage route to component of main page', () => {
- const nonDefaultComponent = {view() {}};
- const router = new Router({
- '/': mockComponent,
- '/a': nonDefaultComponent,
+describe('Router#resolve', () => {
+ beforeEach(() => {
+ window.location.hash = '';
});
- window.location.hash = '#!/a/subpage';
- expect(router.resolve().tag).toBe(nonDefaultComponent);
- expect(router.resolve().attrs.subpage).toBe('/subpage');
-});
-test('Pass empty subpage if not found in URL', () => {
- const nonDefaultComponent = {view() {}};
- const router = new Router({
- '/': mockComponent,
- '/a': nonDefaultComponent,
+ test('Default route must be defined', () => {
+ expect(() => new Router({'/a': mockComponent})).toThrow();
});
- window.location.hash = '#!/a';
- expect(router.resolve().tag).toBe(nonDefaultComponent);
- expect(router.resolve().attrs.subpage).toBe('');
+
+ test('Resolves empty route to default component', () => {
+ const router = new Router({'/': mockComponent});
+ window.location.hash = '';
+ expect(router.resolve().tag).toBe(mockComponent);
+ });
+
+ test('Resolves subpage route to component of main page', () => {
+ const nonDefaultComponent = {view() {}};
+ const router = new Router({
+ '/': mockComponent,
+ '/a': nonDefaultComponent,
+ });
+ window.location.hash = '#!/a/subpage';
+ expect(router.resolve().tag).toBe(nonDefaultComponent);
+ expect(router.resolve().attrs.subpage).toBe('/subpage');
+ });
+
+ test('Pass empty subpage if not found in URL', () => {
+ const nonDefaultComponent = {view() {}};
+ const router = new Router({
+ '/': mockComponent,
+ '/a': nonDefaultComponent,
+ });
+ window.location.hash = '#!/a';
+ expect(router.resolve().tag).toBe(nonDefaultComponent);
+ expect(router.resolve().attrs.subpage).toBe('');
+ });
});
-test('Args parsing', () => {
- const url = 'http://localhost/#!/foo?p=123&s=42&url=a?b?c';
- const args = Router.parseUrl(url).args;
- expect(args.p).toBe('123');
- expect(args.s).toBe('42');
- expect(args.url).toBe('a?b?c');
+describe('Router.parseUrl', () => {
+ // Can parse arguments from the search string.
+ test('Search parsing', () => {
+ const url = 'http://localhost?p=123&s=42&url=a?b?c';
+ const args = Router.parseUrl(url).args;
+ expect(args.p).toBe('123');
+ expect(args.s).toBe('42');
+ expect(args.url).toBe('a?b?c');
+ });
+
+ // Or from the fragment string.
+ test('Fragment parsing', () => {
+ const url = 'http://localhost/#!/foo?p=123&s=42&url=a?b?c';
+ const args = Router.parseUrl(url).args;
+ expect(args.p).toBe('123');
+ expect(args.s).toBe('42');
+ expect(args.url).toBe('a?b?c');
+ });
+
+ // Or both in which case fragment overrides the search.
+ test('Fragment parsing', () => {
+ const url =
+ 'http://localhost/?p=1&s=2&hideSidebar=true#!/foo?s=3&url=4&hideSidebar=false';
+ const args = Router.parseUrl(url).args;
+ expect(args.p).toBe('1');
+ expect(args.s).toBe('3');
+ expect(args.url).toBe('4');
+ expect(args.hideSidebar).toBe(false);
+ });
});
-test('empty route broken into empty components', () => {
- const {page, subpage, args} = Router.parseFragment('');
- expect(page).toBe('');
- expect(subpage).toBe('');
- expect(args).toEqual({});
-});
+describe('Router.parseFragment', () => {
+ test('empty route broken into empty components', () => {
+ const {page, subpage, args} = Router.parseFragment('');
+ expect(page).toBe('');
+ expect(subpage).toBe('');
+ expect(args.mode).toBe(undefined);
+ });
-test('invalid route broken into empty components', () => {
- const {page, subpage, args} = Router.parseFragment('/bla');
- expect(page).toBe('');
- expect(subpage).toBe('');
- expect(args).toEqual({});
-});
+ test('by default args are undefined', () => {
+ // This prevents the url from becoming messy.
+ const {args} = Router.parseFragment('');
+ expect(args).toEqual({});
+ });
-test('simple route has page defined', () => {
- const {page, subpage, args} = Router.parseFragment('#!/record');
- expect(page).toBe('/record');
- expect(subpage).toBe('');
- expect(args).toEqual({});
-});
+ test('invalid route broken into empty components', () => {
+ const {page, subpage} = Router.parseFragment('/bla');
+ expect(page).toBe('');
+ expect(subpage).toBe('');
+ });
-test('simple route has both components defined', () => {
- const {page, subpage, args} = Router.parseFragment('#!/record/memory');
- expect(page).toBe('/record');
- expect(subpage).toBe('/memory');
- expect(args).toEqual({});
-});
+ test('simple route has page defined', () => {
+ const {page, subpage} = Router.parseFragment('#!/record');
+ expect(page).toBe('/record');
+ expect(subpage).toBe('');
+ });
-test('route broken at first slash', () => {
- const {page, subpage, args} = Router.parseFragment('#!/record/memory/stuff');
- expect(page).toBe('/record');
- expect(subpage).toBe('/memory/stuff');
- expect(args).toEqual({});
-});
+ test('simple route has both components defined', () => {
+ const {page, subpage} = Router.parseFragment('#!/record/memory');
+ expect(page).toBe('/record');
+ expect(subpage).toBe('/memory');
+ });
-test('parameters separated from route', () => {
- const {page, subpage, args} =
- Router.parseFragment('#!/record/memory?url=http://localhost:1234/aaaa');
- expect(page).toBe('/record');
- expect(subpage).toBe('/memory');
- expect(args).toEqual({url: 'http://localhost:1234/aaaa'});
+ test('route broken at first slash', () => {
+ const {page, subpage} = Router.parseFragment('#!/record/memory/stuff');
+ expect(page).toBe('/record');
+ expect(subpage).toBe('/memory/stuff');
+ });
+
+ test('parameters separated from route', () => {
+ const {page, subpage, args} =
+ Router.parseFragment('#!/record/memory?url=http://localhost:1234/aaaa');
+ expect(page).toBe('/record');
+ expect(subpage).toBe('/memory');
+ expect(args.url).toEqual('http://localhost:1234/aaaa');
+ });
+
+ test('openFromAndroidBugTool can be false', () => {
+ const {args} = Router.parseFragment('#!/?openFromAndroidBugTool=false');
+ expect(args.openFromAndroidBugTool).toEqual(false);
+ });
+
+ test('openFromAndroidBugTool can be true', () => {
+ const {args} = Router.parseFragment('#!/?openFromAndroidBugTool=true');
+ expect(args.openFromAndroidBugTool).toEqual(true);
+ });
+
+ test('bad modes are coerced to default', () => {
+ const {args} = Router.parseFragment('#!/?mode=1234');
+ expect(args.mode).toEqual(undefined);
+ });
+
+ test('bad hideSidebar is coerced to default', () => {
+ const {args} = Router.parseFragment('#!/?hideSidebar=helloworld!');
+ expect(args.hideSidebar).toEqual(undefined);
+ });
});
diff --git a/ui/src/frontend/tickmark_panel.ts b/ui/src/frontend/tickmark_panel.ts
index ccf4d4c..6076188 100644
--- a/ui/src/frontend/tickmark_panel.ts
+++ b/ui/src/frontend/tickmark_panel.ts
@@ -75,7 +75,7 @@
size.height);
}
const index = globals.state.searchIndex;
- if (index !== -1) {
+ if (index !== -1 && index <= globals.currentSearchResults.tsStarts.length) {
const start = globals.currentSearchResults.tsStarts[index];
const triangleStart =
Math.max(visibleTimeScale.tpTimeToPx(start), 0) + TRACK_SHELL_WIDTH;
diff --git a/ui/src/frontend/trace_url_handler.ts b/ui/src/frontend/trace_url_handler.ts
index 8b398fb..5e329f5 100644
--- a/ui/src/frontend/trace_url_handler.ts
+++ b/ui/src/frontend/trace_url_handler.ts
@@ -23,7 +23,6 @@
import {Route, Router} from './router';
import {taskTracker} from './task_tracker';
-
export function maybeOpenTraceFromRoute(route: Route) {
if (route.args.s) {
// /?s=xxxx for permalinks.
