Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ctrl + C does nothing when using firefox-android without auto-reload or during activity start #2523

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 36 additions & 9 deletions src/extension-runners/firefox-android.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,13 @@ export class FirefoxAndroidExtensionRunner {
// push the profile preferences to the remote profile dir.
await this.adbPrepareProfileDir();

const stdin = this.params.stdin || process.stdin;

if (isTTY(stdin)) {
readline.emitKeypressEvents(stdin);
setRawMode(stdin, true);
}

// NOTE: running Firefox for Android on the Android Emulator can be
// pretty slow, we can run the following 3 steps in parallel to speed up
// it a bit.
Expand All @@ -157,6 +164,11 @@ export class FirefoxAndroidExtensionRunner {
this.adbDiscoveryAndForwardRDPUnixSocket(),
]);

if (isTTY(stdin)) {
// Restore mode so Ctrl-C can be used to kill the process.
setRawMode(stdin, false);
}

// Connect to RDP socket on the local tcp server, install all the pushed extension
// and keep track of the built and installed extension by extension sourceDir.
await this.rdpInstallExtensions();
Expand Down Expand Up @@ -488,12 +500,30 @@ export class FirefoxAndroidExtensionRunner {

log.debug(`Using profile ${deviceProfileDir} (ignored by Fenix)`);

await adbUtils.startFirefoxAPK(
selectedAdbDevice,
selectedFirefoxApk,
firefoxApkComponent,
deviceProfileDir
);
const stdin = this.params.stdin || process.stdin;

const handleCtrlC = (str, key) => {
if (key.ctrl && key.name === 'c') {
adbUtils.setUserAbortStartActivity(true);
}
};

if (isTTY(stdin)) {
stdin.on('keypress', handleCtrlC);
}

try {
await adbUtils.startFirefoxAPK(
selectedAdbDevice,
selectedFirefoxApk,
firefoxApkComponent,
deviceProfileDir
);
} finally {
if (isTTY(stdin)) {
stdin.removeListener('keypress', handleCtrlC);
}
}
}

async buildAndPushExtension(sourceDir: string) {
Expand Down Expand Up @@ -562,9 +592,6 @@ export class FirefoxAndroidExtensionRunner {
// TODO: use noInput property to decide if we should
// disable direct keypress handling.
if (isTTY(stdin)) {
readline.emitKeypressEvents(stdin);
setRawMode(stdin, true);

stdin.on('keypress', handleCtrlC);
}

Expand Down
32 changes: 30 additions & 2 deletions src/util/adb.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ export default class ADBUtils {
// while it is still executing.
userAbortDiscovery: boolean;

// Toggled when the user wants to abort the Start Activity loop
// while it is still executing.
userAbortStartActivity: boolean;

constructor(params: ADBUtilsParams) {
this.params = params;

Expand All @@ -75,6 +79,7 @@ export default class ADBUtils {
this.artifactsDirMap = new Map();

this.userAbortDiscovery = false;
this.userAbortStartActivity = false;
}

runShellCommand(
Expand Down Expand Up @@ -351,20 +356,43 @@ export default class ADBUtils {
// the following to: `${apk}/${apk}.${apkComponent}`
const component = `${apk}/${apkComponent}`;

await wrapADBCall(async () => {
let exception;
wrapADBCall(async () => {
await adbClient.getDevice(deviceId).startActivity({
wait: true,
action: 'android.activity.MAIN',
component,
extras,
});
});
})
.then(() => {
exception = null;
})
.catch((err) => {
exception = err;
});

// Wait for the activity to be started.
while (exception === undefined) {
if (this.userAbortStartActivity) {
throw new UsageError('Exiting Firefox Start Activity on user request');
}
await new Promise((resolve) => setTimeout(resolve, 1000));
}

if (exception) {
throw exception;
}
}

setUserAbortDiscovery(value: boolean) {
this.userAbortDiscovery = value;
}

setUserAbortStartActivity(value: boolean) {
this.userAbortStartActivity = value;
}

async discoverRDPUnixSocket(
deviceId: string,
apk: string,
Expand Down
16 changes: 16 additions & 0 deletions tests/unit/test-extension-runners/test.firefox-android.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ function prepareSelectedDeviceAndAPKParams(
clearArtifactsDir: sinon.spy(() => Promise.resolve()),
detectOrRemoveOldArtifacts: sinon.spy(() => Promise.resolve(true)),
setUserAbortDiscovery: sinon.spy(() => {}),
setUserAbortStartActivity: sinon.spy(() => {}),
ensureRequiredAPKRuntimePermissions: sinon.spy(() => Promise.resolve()),
...adbOverrides,
};
Expand Down Expand Up @@ -989,4 +990,19 @@ describe('util/extension-runners/firefox-android', () => {
consoleStream.stopCapturing();
});
});
it('Stdin raw mode is set to false before setupForward', async () => {
const { params, fakeADBUtils } = prepareSelectedDeviceAndAPKParams();

fakeADBUtils.setupForward = sinon.spy(async () => {
fakeStdin.emit('keypress', 'c', { name: 'c', ctrl: true });
});

const fakeStdin = sinon.spy(createFakeStdin());
params.stdin = fakeStdin;

const runnerInstance = new FirefoxAndroidExtensionRunner(params);
await runnerInstance.run();

assert.deepEqual(fakeStdin.setRawMode.lastCall.args, [false]);
});
});
27 changes: 27 additions & 0 deletions tests/unit/test-util/test.adb.js
Original file line number Diff line number Diff line change
Expand Up @@ -1136,6 +1136,33 @@ describe('utils/adb', () => {
wait: true,
});
});

it('rejects an UsageError on setUserAbortStartActivity call', async () => {
const adb = getFakeADBKit({
adbDevice: {
startActivity: sinon.spy(() => Promise.resolve()),
},
adbkitUtil: {
readAll: sinon.spy(() => Promise.resolve(Buffer.from('\n'))),
},
});
const adbUtils = new ADBUtils({ adb });

adbUtils.setUserAbortStartActivity(true);

const promise = adbUtils.startFirefoxAPK(
'device1',
'org.mozilla.firefox_mybuild',
undefined, // firefoxApkComponent/*
'/fake/custom/profile/path'
);

await assert.isRejected(promise, UsageError);
await assert.isRejected(
promise,
'Exiting Firefox Start Activity on user request'
);
});
});

describe('discoverRDPUnixSocket', () => {
Expand Down