Skip to content

Commit

Permalink
Extract chrome-launcher to a standalone thing. (#2245)
Browse files Browse the repository at this point in the history
* Also extract default flags to standalone entity.
  • Loading branch information
samccone authored and paulirish committed May 13, 2017
1 parent d99b5ad commit 9c06fc1
Show file tree
Hide file tree
Showing 19 changed files with 356 additions and 50 deletions.
3 changes: 3 additions & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ coverage/**
lighthouse-cli/*.js
lighthouse-cli/commands/*.js
lighthouse-cli/types/*.js
!chrome-launcher/manual-chrome-launcher.js
!chrome-launcher/compiled-check.js
chrome-launcher/*.js

# Handlebar-templates
lighthouse-core/report/templates/report-templates.js
Expand Down
5 changes: 5 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ closure-error.log
lighthouse-cli/*.map
lighthouse-cli/*.js

!chrome-launcher/manual-chrome-launcher.js
!chrome-launcher/compiled-check.js
chrome-launcher/*.map
chrome-launcher/*.js

lighthouse-cli/commands/*.map
lighthouse-cli/commands/*.js

Expand Down
3 changes: 2 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ before_script:
- ./lighthouse-core/scripts/download-chrome.sh
- yarn build-all
script:
- yarn test-formatting
- yarn test-cli-formatting
- yarn test-launcher-formatting
- yarn lint
- yarn unit
- yarn closure
Expand Down
6 changes: 6 additions & 0 deletions chrome-launcher/.clang-format
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
BasedOnStyle: Google
Language: JavaScript
ColumnLimit: 100
ReflowComments: false
SpacesBeforeTrailingComments: 1
File renamed without changes.
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import * as fs from 'fs';
import * as path from 'path';
import * as chromeFinder from './chrome-finder';
import {ask} from './ask';
import {DEFAULT_FLAGS} from './flags';

const mkdirp = require('mkdirp');
import * as net from 'net';
Expand Down Expand Up @@ -57,29 +58,12 @@ export class ChromeLauncher {
this.port = defaults(opts.port, 9222);
}

flags() {
const flags = [
get flags() {
const flags = DEFAULT_FLAGS.concat([
`--remote-debugging-port=${this.port}`,
// Disable built-in Google Translate service
'--disable-translate',
// Disable all chrome extensions entirely
'--disable-extensions',
// Disable various background network services, including extension updating,
// safe browsing service, upgrade detector, translate, UMA
'--disable-background-networking',
// Disable fetching safebrowsing lists, likely redundant due to disable-background-networking
'--safebrowsing-disable-auto-update',
// Disable syncing to a Google account
'--disable-sync',
// Disable reporting to UMA, but allows for collection
'--metrics-recording-only',
// Disable installation of default apps on first run
'--disable-default-apps',
// Skip first run wizards
'--no-first-run',
// Place Chrome profile in a custom location we'll rm -rf later
`--user-data-dir=${this.TMP_PROFILE_DIR}`
];
]);

if (process.platform === 'linux') {
flags.push('--disable-setuid-sandbox');
Expand Down Expand Up @@ -146,7 +130,7 @@ export class ChromeLauncher {
}

const chrome = spawn(
execPath, this.flags(), {detached: true, stdio: ['ignore', this.outFile, this.errFile]});
execPath, this.flags, {detached: true, stdio: ['ignore', this.outFile, this.errFile]});
this.chrome = chrome;

fs.writeFileSync(this.pidFile, chrome.pid.toString());
Expand Down
14 changes: 14 additions & 0 deletions chrome-launcher/compiled-check.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
'use strict'

const fs = require('fs');
const path = require('path');

module.exports = function(filename) {
if (!fs.existsSync(path.join(__dirname, filename))) {
console.log(
'Oops! Looks like the chrome-launcher files needs to be compiled. Please run:');
console.log(' yarn install; yarn build;');
console.log('More at: https://github.com/GoogleChrome/lighthouse#develop');
process.exit(1);
}
}
19 changes: 19 additions & 0 deletions chrome-launcher/flags.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
export const DEFAULT_FLAGS = [
// Disable built-in Google Translate service
'--disable-translate',
// Disable all chrome extensions entirely
'--disable-extensions',
// Disable various background network services, including extension updating,
// safe browsing service, upgrade detector, translate, UMA
'--disable-background-networking',
// Disable fetching safebrowsing lists, likely redundant due to disable-background-networking
'--safebrowsing-disable-auto-update',
// Disable syncing to a Google account
'--disable-sync',
// Disable reporting to UMA, but allows for collection
'--metrics-recording-only',
// Disable installation of default apps on first run
'--disable-default-apps',
// Skip first run wizards
'--no-first-run',
];
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,3 @@ const chromeInstance = new ChromeLauncher({

chromeInstance.prepare();
chromeInstance.run();

20 changes: 20 additions & 0 deletions chrome-launcher/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"name": "chrome-launcher",
"private": true,
"main": "chrome-launcher.js",
"scripts": {
"build": "tsc",
"dev": "tsc -w",
"test": "mocha --reporter dot test/**/*-test.js",
"test-formatting": "test/check-formatting.sh",
"format": "clang-format -i -style=file *.ts"
},
"devDependencies": {
"mocha": "^3.2.0",
"typescript": "2.2.1",
"clang-format": "^1.0.50"
},
"dependencies": {
"@types/node": "6.0.66"
}
}
15 changes: 15 additions & 0 deletions chrome-launcher/test/check-formatting.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#!/usr/bin/env bash

check_formatting ()
{
diff -u <(cat $1) <(./node_modules/.bin/clang-format -style=file $1) &>/dev/null
if [ $? -eq 1 ]
then
echo "Error: formatting is required for *.ts files:"
echo " cd chrome-launcher"
echo " yarn format"
exit 1
fi
}

check_formatting "*.ts"
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@

'use strict';

require('../../compiled-check.js')('chrome-launcher.js');
require('../compiled-check.js')('chrome-launcher.js');

const ChromeLauncher = require('../../chrome-launcher.js').ChromeLauncher;
const log = require('../../../lighthouse-core/lib/log');
const ChromeLauncher = require('../chrome-launcher.js').ChromeLauncher;
const log = require('../../lighthouse-core/lib/log');
const assert = require('assert');

/* eslint-env mocha */
Expand All @@ -28,29 +28,25 @@ describe('ChromeLauncher', () => {
it('doesn\'t fail when killed twice', () => {
log.setLevel('error');
const chromeInstance = new ChromeLauncher();
return chromeInstance.run()
.then(() => {
log.setLevel();
return Promise.all([
chromeInstance.kill(),
chromeInstance.kill()
]);
});
return chromeInstance.run().then(() => {
log.setLevel();
return Promise.all([chromeInstance.kill(), chromeInstance.kill()]);
});
});

it('doesn\'t launch multiple chrome processes', () => {
log.setLevel('error');
const chromeInstance = new ChromeLauncher();
let pid;
return chromeInstance.run()
.then(() => {
pid = chromeInstance.chrome.pid;
return chromeInstance.run();
})
.then(() => {
log.setLevel();
assert.strictEqual(pid, chromeInstance.chrome.pid);
return chromeInstance.kill();
});
.then(() => {
pid = chromeInstance.chrome.pid;
return chromeInstance.run();
})
.then(() => {
log.setLevel();
assert.strictEqual(pid, chromeInstance.chrome.pid);
return chromeInstance.kill();
});
});
});
19 changes: 19 additions & 0 deletions chrome-launcher/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"compilerOptions": {
"module": "commonjs",
"target": "es6",
"noImplicitAny": true,
"inlineSourceMap": true,
"noEmitOnError": false,
"strictNullChecks": true,
"noImplicitReturns": true,
"noUnusedLocals": true,
"noUnusedParameters": true
},
"exclude": [
"node_modules"
],
"include": [
"*.ts"
]
}

0 comments on commit 9c06fc1

Please sign in to comment.