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

app: select-client-certificate event callback can accept certificate optionally #8134

Merged
merged 4 commits into from Dec 15, 2016

Conversation

Projects
None yet
2 participants
@deepak1556
Member

deepak1556 commented Dec 2, 2016

Fixes #8088

@kevinsawicki kevinsawicki self-assigned this Dec 6, 2016

@deepak1556

This comment has been minimized.

Show comment
Hide comment
@deepak1556

deepak1556 Dec 7, 2016

Member

Have addressed the comments, thanks!

Member

deepak1556 commented Dec 7, 2016

Have addressed the comments, thanks!

@@ -378,9 +378,21 @@ void OnClientCertificateSelected(
v8::Isolate* isolate,
std::shared_ptr<content::ClientCertificateDelegate> delegate,
mate::Arguments* args) {
if (args->Length() == 2) {

This comment has been minimized.

@kevinsawicki

kevinsawicki Dec 12, 2016

Contributor

I'm not sure I understand this check for 2 here.

Calling callback(cert, null) would hit here and return early and not use the cert but calling callback(cert, null, null) would continue with the cert.

What case is this targeting?

@kevinsawicki

kevinsawicki Dec 12, 2016

Contributor

I'm not sure I understand this check for 2 here.

Calling callback(cert, null) would hit here and return early and not use the cert but calling callback(cert, null, null) would continue with the cert.

What case is this targeting?

This comment has been minimized.

@deepak1556

deepak1556 Dec 15, 2016

Member

We want request to continue with nullptr when the callback is called with no arguments or null. Here callback has already two bound arguments on c++ side, so when callback() is called, the condition will be satisfied.

@deepak1556

deepak1556 Dec 15, 2016

Member

We want request to continue with nullptr when the callback is called with no arguments or null. Here callback has already two bound arguments on c++ side, so when callback() is called, the condition will be satisfied.

This comment has been minimized.

@kevinsawicki

kevinsawicki Dec 15, 2016

Contributor

Oh awesome, totally makes sense, thanks for explaining 👍

@kevinsawicki

kevinsawicki Dec 15, 2016

Contributor

Oh awesome, totally makes sense, thanks for explaining 👍

@kevinsawicki kevinsawicki merged commit 2a8b36c into electron:master Dec 15, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Dec 15, 2016

Contributor

Thanks for this @deepak1556 🚢

Contributor

kevinsawicki commented Dec 15, 2016

Thanks for this @deepak1556 🚢

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment