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.

Sign up for GitHub

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

Jump to bottom

add url blocking function to driver #1195

Merged
merged 3 commits into from
Jan 4, 2017
Merged

add url blocking function to driver #1195

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3b41e07
add url blocking function to driver
weiwei-lin Dec 21, 2016
3fd4e9b
update fake driver to match new driver
weiwei-lin Dec 21, 2016
acacb12
simplified Driver.blockUrlPatterns
weiwei-lin Jan 4, 2017
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
8 changes: 8 additions & 0 deletions lighthouse-core/gather/driver.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,14 @@ class Driver {

return collectUsage;
}

blockUrlPatterns(urlPatterns) {
const promiseArr = [];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could simplify to

    const promiseArr = urlPatterns.map(url => this.sendCommand('Network.addBlockedURL', {url}));
    return Promise.all(promiseArr);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Already changed.

for (const url of urlPatterns) {
promiseArr.push(this.sendCommand('Network.addBlockedURL', {url}));
}
return Promise.all(promiseArr);
}
}

/**
Expand Down
3 changes: 2 additions & 1 deletion lighthouse-core/gather/gather-runner.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ class GatherRunner {
.then(_ => driver.enableRuntimeEvents())
.then(_ => driver.evaluateScriptOnLoad('window.__nativePromise = Promise;'))
.then(_ => driver.cleanAndDisableBrowserCaches())
.then(_ => driver.clearDataForOrigin(options.url));
.then(_ => driver.clearDataForOrigin(options.url))
.then(_ => driver.blockUrlPatterns(options.flags.blockedUrlPatterns || []));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was mulling over if blockedUrlPatterns should live right on options rather than options.flags, but it'd probably require a line of plumbing in index.js that I don't love.

So yeah, I'm also fine with it hanging out on options.flags even if its not user-exposed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. As you mentioned, the main reason I did this is because I don't want to add an extra (and optional) parameter in lighthouse() and runLighthouse(), which looks ugly.

}

static disposeDriver(driver) {
Expand Down
3 changes: 3 additions & 0 deletions lighthouse-core/test/gather/fake-driver.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,5 +61,8 @@ module.exports = {
return Promise.resolve({
schemeIsCryptographic: true
});
},
blockUrlPatterns() {
return Promise.resolve();
}
};