Omit WebUI SSL Client Cert selector in Windows and Mac builds.

Omit WebUI SSL Client Cert selector in Windows and Mac builds.

http://crrev.com/107401 introduced a WebUI cert selector that relies upon NSS and/or OpenSSL.  These aren't used on Win/Mac, so linking fails.

BUG=97856
TEST=build on windows, see if it works

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=107599

[grt (UTC plus 2), 2011-10-27 15:47:21]

Hi guys,

The new WebUI selector breaks the build for me even after a full-on clobber. 
The errors are all due to ssl_client_certificate_selector_webui.cc's use of
x509_certificate_model and other things that don't exist on Windows.  I haven't
been able to figure out how the builders on the waterfall are able to build it. 
Here are the unresolved symbols (referenced by
ssl_client_certificate_selector_webui.obj):

x509_certificate_model::GetTokenName
x509_certificate_model::GetIssuerName
x509_certificate_model::GetEmailAddresses
x509_certificate_model::GetKeyUsageString
x509_certificate_model::GetUsageStrings
x509_certificate_model::GetTimes
x509_certificate_model::GetSerialNumberHexified
x509_certificate_model::GetSubjectName
browser::UnlockCertSlotIfNecessary
x509_certificate_model::GetNicknameStringsFromCertList
browser::ShowNativeSSLClientCertificateSelector

Is this intended to work on Windows?  If not, I think it should be omitted. 
Thanks.

http://codereview.chromium.org/8403043/diff/1/chrome/chrome_browser.gypi
File chrome/chrome_browser.gypi (right):

http://codereview.chromium.org/8403043/diff/1/chrome/chrome_browser.gypi#newc...
chrome/chrome_browser.gypi:4533:
'browser/ui/webui/ssl_client_certificate_selector_webui.cc',
is this better off in the "use_nss==0 and use_openssl==0" section above?

[wyck, 2011-10-27 15:55:06]

On 2011/10/27 15:47:21, grt wrote:
> Hi guys,
> 
> The new WebUI selector breaks the build for me even after a full-on clobber. 
> The errors are all due to ssl_client_certificate_selector_webui.cc's use of
> x509_certificate_model and other things that don't exist on Windows.  I
haven't
> been able to figure out how the builders on the waterfall are able to build
it. 
> Here are the unresolved symbols (referenced by
> ssl_client_certificate_selector_webui.obj):
> 
> x509_certificate_model::GetTokenName
> x509_certificate_model::GetIssuerName
> x509_certificate_model::GetEmailAddresses
> x509_certificate_model::GetKeyUsageString
> x509_certificate_model::GetUsageStrings
> x509_certificate_model::GetTimes
> x509_certificate_model::GetSerialNumberHexified
> x509_certificate_model::GetSubjectName
> browser::UnlockCertSlotIfNecessary
> x509_certificate_model::GetNicknameStringsFromCertList
> browser::ShowNativeSSLClientCertificateSelector
> 
> Is this intended to work on Windows?  If not, I think it should be omitted. 
> Thanks.
> 
> http://codereview.chromium.org/8403043/diff/1/chrome/chrome_browser.gypi
> File chrome/chrome_browser.gypi (right):
> 
>
http://codereview.chromium.org/8403043/diff/1/chrome/chrome_browser.gypi#newc...
> chrome/chrome_browser.gypi:4533:
> 'browser/ui/webui/ssl_client_certificate_selector_webui.cc',
> is this better off in the "use_nss==0 and use_openssl==0" section above?

We're currently trying to attack this from another angle.  See
http://codereview.chromium.org/8363048/

[Rick Byers, 2011-10-27 16:04:20]

We should definitely go ahead and submit a fix like this immediately to unblock
people.  We can follow up with the right fix later.

Just to clarify though, this is only a problem in component builds for some
reason, right?  The windows try jobs for this change passed - it would be good
to understand why.

[wyck, 2011-10-27 16:04:30]

On 2011/10/27 15:55:06, wyck wrote:
> On 2011/10/27 15:47:21, grt wrote:
> > Hi guys,
> > 
> > The new WebUI selector breaks the build for me even after a full-on clobber.

> > The errors are all due to ssl_client_certificate_selector_webui.cc's use of
> > x509_certificate_model and other things that don't exist on Windows.  I
> haven't
> > been able to figure out how the builders on the waterfall are able to build
> it. 
> > Here are the unresolved symbols (referenced by
> > ssl_client_certificate_selector_webui.obj):
> > 
> > x509_certificate_model::GetTokenName
> > x509_certificate_model::GetIssuerName
> > x509_certificate_model::GetEmailAddresses
> > x509_certificate_model::GetKeyUsageString
> > x509_certificate_model::GetUsageStrings
> > x509_certificate_model::GetTimes
> > x509_certificate_model::GetSerialNumberHexified
> > x509_certificate_model::GetSubjectName
> > browser::UnlockCertSlotIfNecessary
> > x509_certificate_model::GetNicknameStringsFromCertList
> > browser::ShowNativeSSLClientCertificateSelector
> > 
> > Is this intended to work on Windows?  If not, I think it should be omitted. 
> > Thanks.
> > 
> > http://codereview.chromium.org/8403043/diff/1/chrome/chrome_browser.gypi
> > File chrome/chrome_browser.gypi (right):
> > 
> >
>
http://codereview.chromium.org/8403043/diff/1/chrome/chrome_browser.gypi#newc...
> > chrome/chrome_browser.gypi:4533:
> > 'browser/ui/webui/ssl_client_certificate_selector_webui.cc',
> > is this better off in the "use_nss==0 and use_openssl==0" section above?
> 
> We're currently trying to attack this from another angle.  See
> http://codereview.chromium.org/8363048/
OK, pretty clear the "another angle" is bogus. if there's no NSS and/or OpenSSL
on windows.  So nevermind 8363048.

Your approach is closer to the right thing, but I think it will still break on
mac.  Your exclude only handles OS=win.

[wyck, 2011-10-27 16:16:34]

http://codereview.chromium.org/8403043/diff/1/chrome/chrome_browser.gypi
File chrome/chrome_browser.gypi (right):

http://codereview.chromium.org/8403043/diff/1/chrome/chrome_browser.gypi#newc...
chrome/chrome_browser.gypi:4533:
'browser/ui/webui/ssl_client_certificate_selector_webui.cc',
On 2011/10/27 15:47:21, grt wrote:
> is this better off in the "use_nss==0 and use_openssl==0" section above?

Yes.

[grt (UTC plus 2), 2011-10-27 16:20:12]

PTAL

[wyck, 2011-10-27 16:21:35]

LGTM

I hope that fixes it for mac and windows platforms.  *fingers crossed*

[commit-bot: I haz the power, 2011-10-27 16:21:59]

No LGTM from valid reviewers yet.

[commit-bot: I haz the power, 2011-10-27 16:22:38]

No LGTM from valid reviewers yet.

[wyck, 2011-10-27 16:22:52]

On 2011/10/27 16:21:35, wyck wrote:
> LGTM
> 
> I hope that fixes it for mac and windows platforms.  *fingers crossed*
Might be good to quote bug # 101913 in the notes.

[wyck, 2011-10-27 16:23:13]

On 2011/10/27 16:22:52, wyck wrote:
> On 2011/10/27 16:21:35, wyck wrote:
> > LGTM
> > 
> > I hope that fixes it for mac and windows platforms.  *fingers crossed*
> Might be good to quote bug # 101913 in the notes.
And I am not a committer.  :P  sorry.

[grt (UTC plus 2), 2011-10-27 16:26:11]

On 2011/10/27 16:04:20, Rick Byers wrote:
> We should definitely go ahead and submit a fix like this immediately to
unblock
> people.  We can follow up with the right fix later.

Rick: could you have a look and LGTM this change if it looks good to you?

> Just to clarify though, this is only a problem in component builds for some
> reason, right?  The windows try jobs for this change passed - it would be good
> to understand why.

I wasted quite a bit of time this morning trying to figure that out.  I still
have no clue.

[Rick Byers, 2011-10-27 16:28:49]

LGTM

On 2011/10/27 16:26:11, grt wrote:
> On 2011/10/27 16:04:20, Rick Byers wrote:
> > We should definitely go ahead and submit a fix like this immediately to
> unblock
> > people.  We can follow up with the right fix later.
> 
> Rick: could you have a look and LGTM this change if it looks good to you?

Yes, this is fine as a first step, Chad can follow up with a better fix that
includes Mac.  Sorry I didn't LGTM it right away - I forgot that Chad's LGTM
wouldn't be good enough for the commit bot.

> > Just to clarify though, this is only a problem in component builds for some
> > reason, right?  The windows try jobs for this change passed - it would be
good
> > to understand why.
> 
> I wasted quite a bit of time this morning trying to figure that out.  I still
> have no clue.

Thanks for looking.  Sorry for the pain this has caused.

[commit-bot: I haz the power, 2011-10-27 16:31:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/grt@chromium.org/8403043/1002

[commit-bot: I haz the power, 2011-10-27 17:39:21]

Try job failure for 8403043-1002 (retry) on linux_rel for steps "browser_tests,
ui_tests".
It's a second try, previously, steps "browser_tests, ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[wyck, 2011-10-27 18:57:14]

On 2011/10/27 17:39:21, I haz the power (commit-bot) wrote:
> Try job failure for 8403043-1002 (retry) on linux_rel for steps
"browser_tests,
> ui_tests".
> It's a second try, previously, steps "browser_tests, ui_tests" failed.
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

I am very confused by the output of that linux_rel try.