Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(451)

Issue 661241: Linux: implement Client SSL Certificate selection UI (Closed)

Created:
10 years, 10 months ago by mattm
Modified:
9 years, 7 months ago
Reviewers:
Chris Masone, wtc
CC:
chromium-reviews_googlegroups.com, jam+cc_chromium.org, brettw+cc_chromium.org, darin-cc_chromium.org, ben+cc_chromium.org, Alexander Potapenko, jam, John Grabowski, kuchhal, ncarter (slow), fbarchard, Alpha Left Google, Erik does not do reviews, Timur Iskhodzhanov, apatrick_chromium, stuartmorgan, tim (not reviewing), dpranke+watch_chromium.org, pam+watch_chromium.org, awong, scherkus (not reviewing), idana, Aaron Boodman, Paweł Hajdan Jr.
Visibility:
Public.

Description

Linux: implement Client SSL Certificate selection UI BUG=25241 TEST=Visit site for which you have a client certificate, verify that dialog comes up and you can select your cert (or cancel) Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=40587

Patch Set 1 #

Patch Set 2 : move include since it's not used there (yet?) #

Total comments: 32

Patch Set 3 : review changes #

Total comments: 7

Patch Set 4 : review changes #

Patch Set 5 : rebase #

Total comments: 2

Patch Set 6 : fix views and chromeos builds hopefully #

Unified diffs Side-by-side diffs Delta from patch set Stats (+573 lines, -90 lines) Patch
M chrome/app/generated_resources.grd View 1 2 3 2 chunks +46 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/login/login_manager_view.cc View 1 chunk +0 lines, -7 lines 0 comments Download
M chrome/browser/gtk/certificate_viewer.cc View 1 2 3 2 chunks +9 lines, -31 lines 0 comments Download
M chrome/browser/renderer_host/resource_dispatcher_host.cc View 1 2 2 chunks +0 lines, -9 lines 0 comments Download
M chrome/browser/ssl/ssl_client_auth_handler.h View 1 2 chunks +5 lines, -5 lines 0 comments Download
M chrome/browser/ssl/ssl_client_auth_handler_gtk.cc View 1 2 3 1 chunk +320 lines, -2 lines 0 comments Download
M chrome/chrome_browser.gypi View 1 2 3 4 5 2 chunks +4 lines, -0 lines 0 comments Download
M chrome/common/chrome_switches.h View 1 2 3 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/common/chrome_switches.cc View 1 2 3 1 chunk +0 lines, -10 lines 0 comments Download
M chrome/third_party/mozilla_security_manager/nsNSSCertHelper.h View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M chrome/third_party/mozilla_security_manager/nsNSSCertHelper.cpp View 1 2 3 4 5 3 chunks +46 lines, -22 lines 0 comments Download
M chrome/third_party/mozilla_security_manager/nsNSSCertificate.h View 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/third_party/mozilla_security_manager/nsNSSCertificate.cpp View 1 2 3 2 chunks +13 lines, -0 lines 0 comments Download
A chrome/third_party/mozilla_security_manager/nsUsageArrayHelper.h View 1 2 1 chunk +53 lines, -0 lines 0 comments Download
A chrome/third_party/mozilla_security_manager/nsUsageArrayHelper.cpp View 1 2 1 chunk +73 lines, -0 lines 0 comments Download

Messages

Total messages: 11 (0 generated)
mattm
re:nsUsageArrayHelper.cpp, don't think it's really necessary for that function to go in third_party, but couldn't ...
10 years, 10 months ago (2010-02-27 03:37:52 UTC) #1
wtc
LGTM. Thanks a lot! http://codereview.chromium.org/661241/diff/1001/1002 File chrome/app/generated_resources.grd (right): http://codereview.chromium.org/661241/diff/1001/1002#newcode2502 chrome/app/generated_resources.grd:2502: <message name="IDS_CERT_SELECTOR_DIALOG_TITLE" desc="Title of dialog ...
10 years, 9 months ago (2010-03-02 21:51:45 UTC) #2
mattm
http://codereview.chromium.org/661241/diff/1001/1002 File chrome/app/generated_resources.grd (right): http://codereview.chromium.org/661241/diff/1001/1002#newcode2502 chrome/app/generated_resources.grd:2502: <message name="IDS_CERT_SELECTOR_DIALOG_TITLE" desc="Title of dialog asking using to select ...
10 years, 9 months ago (2010-03-03 00:15:10 UTC) #3
wtc
LGTM. http://codereview.chromium.org/661241/diff/1001/1005 File chrome/browser/ssl/ssl_client_auth_handler.h (right): http://codereview.chromium.org/661241/diff/1001/1005#newcode39 chrome/browser/ssl/ssl_client_auth_handler.h:39: void CertificateSelected(net::X509Certificate* cert); I see. This is not ...
10 years, 9 months ago (2010-03-03 03:16:47 UTC) #4
wtc
http://codereview.chromium.org/661241/diff/35/46 File chrome/third_party/mozilla_security_manager/nsNSSCertificate.cpp (right): http://codereview.chromium.org/661241/diff/35/46#newcode75 chrome/third_party/mozilla_security_manager/nsNSSCertificate.cpp:75: token = l10n_util::GetStringUTF8(IDS_CERT_INTERNAL_TOKEN); I just tested your patch. A ...
10 years, 9 months ago (2010-03-03 03:42:48 UTC) #5
wtc
Matt, Does this gtk-based UI work on Chrome OS also? I heard that Chrome OS ...
10 years, 9 months ago (2010-03-03 18:26:24 UTC) #6
mattm
re:chromeos, I think it will work on chromeos, assuming all the gyp rules are correct ...
10 years, 9 months ago (2010-03-03 21:22:00 UTC) #7
wtc
LGTM, with one change. You may want to convert my explanation below into a comment ...
10 years, 9 months ago (2010-03-03 21:30:16 UTC) #8
mattm
tested with toolkit_views=1 (not quite the same but I don't have a chromeos build environment) ...
10 years, 9 months ago (2010-03-03 23:01:06 UTC) #9
Chris Masone
On 2010/03/03 23:01:06, mattm wrote: > tested with toolkit_views=1 (not quite the same but I ...
10 years, 9 months ago (2010-03-03 23:03:53 UTC) #10
wtc
10 years, 9 months ago (2010-03-04 20:11:37 UTC) #11
http://codereview.chromium.org/661241/diff/2008/3023
File chrome/third_party/mozilla_security_manager/nsNSSCertHelper.cpp (right):

http://codereview.chromium.org/661241/diff/2008/3023#newcode701
chrome/third_party/mozilla_security_manager/nsNSSCertHelper.cpp:701: for (size_t
i = 0; i * 8 < bitfield->len && i < sizeof(bits); ++i) {
On second thought, I like the code you wrote here.  Could you
add it back?  Thanks.

Powered by Google App Engine
This is Rietveld 408576698