Skip to content

Commit

Permalink
Merge "ui: de-controllify recording" into main
Browse files Browse the repository at this point in the history
  • Loading branch information
LalitMaganti authored and Gerrit Code Review committed Nov 8, 2024
2 parents a3e11f4 + a3c18df commit 5181eb8
Show file tree
Hide file tree
Showing 38 changed files with 785 additions and 1,281 deletions.
3 changes: 2 additions & 1 deletion python/tools/check_imports.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,8 @@
(['/public/lib/colorizer'], '/core/feature_flags'),

# TODO(primiano): Record page-related technical debt.
('/frontend/record_*', '/controller/*'),
('/frontend/record*', '/controller/*'),
('/frontend/permalink', '/controller/*'),
('/common/*', '/controller/record_config_types'),
('/controller/index', '/common/recordingV2/target_factories/index'),
('/common/recordingV2/*', '/controller/*'),
Expand Down
2 changes: 1 addition & 1 deletion python/tools/check_ratchet.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@

from dataclasses import dataclass

EXPECTED_ANY_COUNT = 57
EXPECTED_ANY_COUNT = 52
EXPECTED_RUN_METRIC_COUNT = 4

ROOT_DIR = os.path.dirname(
Expand Down
146 changes: 0 additions & 146 deletions ui/src/common/actions.ts

This file was deleted.

21 changes: 5 additions & 16 deletions ui/src/common/empty_state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,28 +12,15 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import {createEmptyRecordConfig} from '../controller/record_config_types';
import {featureFlags} from '../core/feature_flags';
import {
autosaveConfigStore,
recordTargetStore,
} from '../frontend/record_config';
import {State} from './state';
import {RecordingState} from './state';

const AUTOLOAD_STARTED_CONFIG_FLAG = featureFlags.register({
id: 'autoloadStartedConfig',
name: 'Auto-load last used recording config',
description:
'Starting a recording automatically saves its configuration. ' +
'This flag controls whether this config is automatically loaded.',
defaultValue: true,
});

export function createEmptyState(): State {
export function createEmptyState(): RecordingState {
return {
recordConfig: AUTOLOAD_STARTED_CONFIG_FLAG.get()
? autosaveConfigStore.get()
: createEmptyRecordConfig(),
recordConfig: autosaveConfigStore.get(),
lastLoadedConfig: {type: 'NONE'},

recordingInProgress: false,
Expand All @@ -44,5 +31,7 @@ export function createEmptyState(): State {

fetchChromeCategories: false,
chromeCategories: undefined,
bufferUsage: 0,
recordingLog: '',
};
}
22 changes: 15 additions & 7 deletions ui/src/common/recordingV2/recording_page_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,16 @@

import {assertExists, assertTrue} from '../../base/logging';
import {currentDateHourAndMinute} from '../../base/time';
import {RecordingManager} from '../../controller/recording_manager';
import {AppImpl} from '../../core/app_impl';
import {raf} from '../../core/raf_scheduler';
import {globals} from '../../frontend/globals';
import {autosaveConfigStore} from '../../frontend/record_config';
import {
DEFAULT_ADB_WEBSOCKET_URL,
DEFAULT_TRACED_WEBSOCKET_URL,
} from '../../frontend/recording/recording_ui_utils';
import {couldNotClaimInterface} from '../../frontend/recording/reset_interface_modal';
import {TraceConfig} from '../../protos';
import {Actions} from '../actions';
import {TRACE_SUFFIX} from '../constants';
import {genTraceConfig} from './recording_config_utils';
import {RecordingError, showRecordingModal} from './recording_error_handling';
Expand Down Expand Up @@ -251,6 +250,8 @@ class TracingSessionWrapper {
// Keeps track of the state the Ui is in. Has methods which are executed on
// user actions such as starting/stopping/cancelling a tracing session.
export class RecordingPageController {
private recMgr: RecordingManager;

// State of the recording page. This is set by user actions and/or automatic
// transitions. This is queried by the UI in order to
private state: RecordingState = RecordingState.NO_TARGET;
Expand All @@ -266,6 +267,10 @@ export class RecordingPageController {
// transitions don't override one another in async functions.
private stateGeneration = 0;

constructor(recMgr: RecordingManager) {
this.recMgr = recMgr;
}

getBufferUsagePercentage(): number {
return this.bufferUsagePercentage;
}
Expand All @@ -290,7 +295,7 @@ export class RecordingPageController {
throw new RecordingError('Recording page state transition out of order.');
}
this.setState(state);
globals.dispatch(Actions.setRecordingStatus({status: undefined}));
this.recMgr.setRecordingStatus(undefined);
raf.scheduleFullRedraw();
}

Expand Down Expand Up @@ -322,7 +327,7 @@ export class RecordingPageController {
// For the 'Recording in progress for 7000ms we don't show a
// modal.'
if (message.startsWith(RECORDING_IN_PROGRESS)) {
globals.dispatch(Actions.setRecordingStatus({status: message}));
this.recMgr.setRecordingStatus(message);
} else {
// For messages such as 'Please allow USB debugging on your
// device, which require a user action, we show a modal.
Expand Down Expand Up @@ -422,15 +427,18 @@ export class RecordingPageController {
onStartRecordingPressed(): void {
assertTrue(RecordingState.TARGET_INFO_DISPLAYED === this.state);
location.href = '#!/record/instructions';
autosaveConfigStore.save(globals.state.recordConfig);
autosaveConfigStore.save(this.recMgr.state.recordConfig);

const target = this.getTarget();
const targetInfo = target.getInfo();
AppImpl.instance.analytics.logEvent(
'Record Trace',
`Record trace (${targetInfo.targetType})`,
);
const traceConfig = genTraceConfig(globals.state.recordConfig, targetInfo);
const traceConfig = genTraceConfig(
this.recMgr.state.recordConfig,
targetInfo,
);

this.tracingSessionWrapper = this.createTracingSessionWrapper(target);
this.tracingSessionWrapper.start(traceConfig);
Expand Down Expand Up @@ -541,7 +549,7 @@ export class RecordingPageController {
this.bufferUsagePercentage = 0;
this.tracingSessionWrapper = undefined;
this.setState(RecordingState.TARGET_INFO_DISPLAYED);
globals.dispatch(Actions.setRecordingStatus({status: undefined}));
this.recMgr.setRecordingStatus(undefined);
// Redrawing because this method has changed the RecordingState, which will
// affect the display of the record_page.
raf.scheduleFullRedraw();
Expand Down
14 changes: 12 additions & 2 deletions ui/src/common/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,13 @@ export type LoadedConfig =
| LoadedConfigAutomatic
| LoadedConfigNamed;

export interface State {
export interface RecordCommand {
commandline: string;
pbtxt: string;
pbBase64: string;
}

export interface RecordingState {
/**
* State of the ConfigEditor.
*/
Expand All @@ -63,6 +69,10 @@ export interface State {

fetchChromeCategories: boolean;
chromeCategories: string[] | undefined;

bufferUsage: number;
recordingLog: string;
recordCmd?: RecordCommand;
}

export declare type RecordMode =
Expand All @@ -87,7 +97,7 @@ export function isAndroidP(target: RecordingTarget) {
}

export function isAndroidTarget(target: RecordingTarget) {
return ['Q', 'P', 'O'].includes(target.os);
return ['Q', 'P', 'O', 'S'].includes(target.os);
}

export function isChromeTarget(target: RecordingTarget) {
Expand Down
22 changes: 14 additions & 8 deletions ui/src/controller/adb_base_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,11 @@
// limitations under the License.

import {exists} from '../base/utils';
import {isAdbTarget} from '../common/state';
import {RecordingState, RecordingTarget, isAdbTarget} from '../common/state';
import {
extractDurationFromTraceConfig,
extractTraceConfig,
} from '../core/trace_config_utils';
import {globals} from '../frontend/globals';
import {Adb} from './adb_interfaces';
import {ReadBuffersResponse} from './consumer_port_types';
import {Consumer, RpcConsumerPort} from './record_controller_interfaces';
Expand All @@ -44,10 +43,16 @@ export abstract class AdbBaseConsumerPort extends RpcConsumerPort {
protected adb: Adb;
protected state = AdbConnectionState.READY_TO_CONNECT;
protected device?: USBDevice;
protected recState: RecordingState;

protected constructor(adb: Adb, consumer: Consumer) {
protected constructor(
adb: Adb,
consumer: Consumer,
recState: RecordingState,
) {
super(consumer);
this.adb = adb;
this.recState = recState;
}

async handleCommand(method: string, params: Uint8Array) {
Expand All @@ -74,10 +79,10 @@ export abstract class AdbBaseConsumerPort extends RpcConsumerPort {
this.deviceDisconnected()
) {
this.state = AdbConnectionState.AUTH_IN_PROGRESS;
this.device = await this.findDevice();
this.device = await this.findDevice(this.recState.recordingTarget);
if (!this.device) {
this.state = AdbConnectionState.READY_TO_CONNECT;
const target = globals.state.recordingTarget;
const target = this.recState.recordingTarget;
throw Error(
`Device with serial ${
isAdbTarget(target) ? target.serial : 'n/a'
Expand All @@ -91,7 +96,7 @@ export abstract class AdbBaseConsumerPort extends RpcConsumerPort {
await this.adb.connect(this.device);

// During the authentication the device may have been disconnected.
if (!globals.state.recordingInProgress || this.deviceDisconnected()) {
if (!this.recState.recordingInProgress || this.deviceDisconnected()) {
throw Error('Recording not in progress after adb authorization.');
}

Expand Down Expand Up @@ -140,9 +145,10 @@ export abstract class AdbBaseConsumerPort extends RpcConsumerPort {
};
}

async findDevice(): Promise<USBDevice | undefined> {
async findDevice(
connectedDevice: RecordingTarget,
): Promise<USBDevice | undefined> {
if (!('usb' in navigator)) return undefined;
const connectedDevice = globals.state.recordingTarget;
if (!isAdbTarget(connectedDevice)) return undefined;
const devices = await navigator.usb.getDevices();
return devices.find((d) => d.serialNumber === connectedDevice.serial);
Expand Down
Loading

0 comments on commit 5181eb8

Please sign in to comment.