Fix library paths for preloading NSS on Ubuntu 11.10.

Fix library paths for preloading NSS on Ubuntu 11.10.


BUG=99053, 91962
TEST=Manual


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

[awong, 2011-10-05 18:07:42]

http://codereview.chromium.org/8143012/diff/1/crypto/nss_util.cc
File crypto/nss_util.cc (right):

http://codereview.chromium.org/8143012/diff/1/crypto/nss_util.cc#newcode659
crypto/nss_util.cc:659:
paths.push_back(FilePath("/usr/lib/x86_64-linux-gnu/nss"));
Can we confirm with the ubuntu folks that this nss path isn't in the default
ld.so.conf?

[Lambros, 2011-10-05 21:03:24]

On 2011/10/05 18:07:42, awong wrote:
> http://codereview.chromium.org/8143012/diff/1/crypto/nss_util.cc
> File crypto/nss_util.cc (right):
> 
> http://codereview.chromium.org/8143012/diff/1/crypto/nss_util.cc#newcode659
> crypto/nss_util.cc:659:
> paths.push_back(FilePath("/usr/lib/x86_64-linux-gnu/nss"));
> Can we confirm with the ubuntu folks that this nss path isn't in the default
> ld.so.conf?
I'm pretty sure it isn't.  I don't have my Oneiric system handy right now, but
my Ubuntu 10.04 system definitely has nothing in /etc/ld.so.conf.d/ for NSS.  I
also looked at the Oneiric NSS packages, and they don't install anything to this
directory.

[awong, 2011-10-05 21:13:39]

LGTM

[commit-bot: I haz the power, 2011-10-06 00:32:35]

CQ is trying the patch. Follow status at
https://chromium-status.appspot.com/cq/lambroslambrou@chromium.org/8143012/1

[commit-bot: I haz the power, 2011-10-06 00:32:36]

Presubmit check for 8143012-1 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for: crypto/nss_util.cc

[Lambros, 2011-10-06 00:41:23]

agl: Please could I have OWNERS approval?

[agl, 2011-10-06 00:53:11]

This is something for wtc to approve. Adding paths should be safe, at least, but
there might be something subtle about NSS that I'm missing.

[wtc, 2011-10-06 01:06:44]

Lambros: thanks for the patch.  I really hope this is a
duplicate of bug 91962 that I have had trouble debugging
for a long time (because I don't have an Ubuntu 11.10 computer).

http://codereview.chromium.org/8143012/diff/1/crypto/nss_util.cc
File crypto/nss_util.cc (right):

http://codereview.chromium.org/8143012/diff/1/crypto/nss_util.cc#newcode662
crypto/nss_util.cc:662: #endif
Please test __x86_64__ and __i386__ or our equivalent macros
ARCH_CPU_X86_64 and ARCH_CPU_X86 instead, because Chrome
also supports ARM processors.

Lambros: do you think this is the cause of
http://code.google.com/p/chromium/issues/detail?id=91962 ?
Perhaps issue 99053 is a duplicate of that bug?

[wtc, 2011-10-06 01:13:18]

http://codereview.chromium.org/8143012/diff/1/crypto/nss_util.cc
File crypto/nss_util.cc (right):

http://codereview.chromium.org/8143012/diff/1/crypto/nss_util.cc#newcode659
crypto/nss_util.cc:659:
paths.push_back(FilePath("/usr/lib/x86_64-linux-gnu/nss"));
On 2011/10/05 18:07:42, awong wrote:
> Can we confirm with the ubuntu folks that this nss path isn't in the default
> ld.so.conf?

awong: This is true.  The NSS package in Debian derivatives
has been patched to know about these /usr/lib/*/nss
directories.  It is not in the default .so search path.

[Lambros, 2011-10-06 01:40:47]

On 2011/10/06 01:06:44, wtc wrote:
> Lambros: thanks for the patch.  I really hope this is a
> duplicate of bug 91962 that I have had trouble debugging
> for a long time (because I don't have an Ubuntu 11.10 computer).
> 
> http://codereview.chromium.org/8143012/diff/1/crypto/nss_util.cc
> File crypto/nss_util.cc (right):
> 
> http://codereview.chromium.org/8143012/diff/1/crypto/nss_util.cc#newcode662
> crypto/nss_util.cc:662: #endif
> Please test __x86_64__ and __i386__ or our equivalent macros
> ARCH_CPU_X86_64 and ARCH_CPU_X86 instead, because Chrome
> also supports ARM processors.
Good point!  I will make that change, but I don't know what the correct path
would be for ARM processors, so I might have to leave this as a TODO: if that's
OK?  I guess we need to find an Ubuntu Oneiric ARM system to test :)

> 
> Lambros: do you think this is the cause of
> http://code.google.com/p/chromium/issues/detail?id=91962 ?
> Perhaps issue 99053 is a duplicate of that bug?
At first glance, I'm inclined to think it is the same problem.  It has the same
hallmarks: Ubuntu Oneiric, and "permission denied" opening libsoftokn3.so.

[wtc, 2011-10-06 17:27:24]

Lambros: could you add 91962 to the BUG= field in the
CL's commit message?  Thanks.

http://codereview.chromium.org/8143012/diff/1/crypto/nss_util.cc
File crypto/nss_util.cc (right):

http://codereview.chromium.org/8143012/diff/1/crypto/nss_util.cc#newcode662
crypto/nss_util.cc:662: #endif

It is OK to leave ARM processor support as a TODO.

According to http://packages-rore.debian.org/en/sid/armel/libnss3-1d/filelist,
I beleive the pathname for ARM little-endian processors is
"/usr/lib/arm-linux-gnueabi/nss", so we can also just try
that.

http://codereview.chromium.org/8143012/diff/1/crypto/nss_util.cc#newcode686
crypto/nss_util.cc:686: LOG(WARNING) << "Failed to load NSS libraries.";

Can you find out why we don't see this WARNING message in
the log output?  It would have pointed us in the right
direction sooner.

In any case, let's change this to a LOG(ERROR) message,
because it is a serious error to have failed to load NSS
libraries.  Hopefully that will make the message appear
in the log output.

[Lambros, 2011-10-06 20:32:19]

http://codereview.chromium.org/8143012/diff/1/crypto/nss_util.cc
File crypto/nss_util.cc (right):

http://codereview.chromium.org/8143012/diff/1/crypto/nss_util.cc#newcode662
crypto/nss_util.cc:662: #endif
On 2011/10/06 01:06:44, wtc wrote:
> Please test __x86_64__ and __i386__ or our equivalent macros
> ARCH_CPU_X86_64 and ARCH_CPU_X86 instead, because Chrome
> also supports ARM processors.

Done.

http://codereview.chromium.org/8143012/diff/1/crypto/nss_util.cc#newcode686
crypto/nss_util.cc:686: LOG(WARNING) << "Failed to load NSS libraries.";
On 2011/10/06 17:27:24, wtc wrote:
> 
> Can you find out why we don't see this WARNING message in
> the log output?  It would have pointed us in the right
> direction sooner.
Chrome (Official Release build) only logs ERROR (and higher severity) messages
to stderr.  If I run chrome with --enable-logging, then I do see this warning
printed in the chrome_debug.log file.

> 
> In any case, let's change this to a LOG(ERROR) message,
> because it is a serious error to have failed to load NSS
> libraries.  Hopefully that will make the message appear
> in the log output.

Done.

[wtc, 2011-10-06 21:18:39]

Patch Set 2 LGTM.  Thanks!

[commit-bot: I haz the power, 2011-10-06 23:57:18]

CQ is trying the patch. Follow status at
https://chromium-status.appspot.com/cq/lambroslambrou@chromium.org/8143012/12001

[commit-bot: I haz the power, 2011-10-07 01:34:00]

Change committed as 104421